Opened 13 years ago
Closed 12 years ago
#4354 closed defect (fixed)
[with patch; positive review] loading a file with spaces in the filename doesn't work
Reported by: | anakha | Owned by: | abergeron |
---|---|---|---|
Priority: | major | Milestone: | sage-3.3 |
Component: | misc | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
try it at home:
$ echo 'print "ok"' > 'test file.sage' $ sage "test file.sage"
Attachments (4)
Change History (19)
comment:1 Changed 13 years ago by
- Component changed from algebra to misc
- Milestone set to sage-3.2
- Owner changed from tbd to anakha
Changed 13 years ago by
comment:2 Changed 13 years ago by
- Summary changed from loading a file with spaces in the filename doesn't work to [with patch; needs review] loading a file with spaces in the filename doesn't work
With the above patch and replacement sage script the example given should work and print 'ok'.
Be aware that if you copied the sage script somewhere (like /usr/local/bin) for convenience you need to modify that copy too. The line near the end that reads
"$SAGE_ROOT/local/bin/sage-sage" $*
needs to be replaced by
"$SAGE_ROOT/local/bin/sage-sage" "$@"
comment:3 Changed 13 years ago by
For the patch sage, below are some possible fixes to the documentation. Please also refer to #1389.
1.
-echo "You must compile SAGE first using 'make' in the SAGE root directory." >&2 +echo "You must compile Sage first using 'make' in the Sage root directory." >&2
2.
-echo "(If you have already compiled SAGE, you must set the SAGE_ROOT variable in " +echo "(If you have already compiled Sage, you must set the SAGE_ROOT variable in "
3.
-# whenver SAGE exists. +# whenever Sage exits.
comment:4 Changed 12 years ago by
- Summary changed from [with patch; needs review] loading a file with spaces in the filename doesn't work to [with patch; needs work] loading a file with spaces in the filename doesn't work
REFEREE:
Can you please rebase this against 3.2.1.alpha*? I tried applying and got some many failed hunks I can't go further.
sage: hg_scripts.apply('http://trac.sagemath.org/sage_trac/attachment/ticket/4354/trac_4354.patch') Attempting to load remote file: http://trac.sagemath.org/sage_trac/attachment/ticket/4354/trac_4354.patch?format=raw Loading: [..] cd "/Users/wstein/sage/local/bin" && hg status cd "/Users/wstein/sage/local/bin" && hg status cd "/Users/wstein/sage/local/bin" && hg import "/Users/wstein/.sage/temp/teragon_2.local/1537/tmp_0.patch" applying /Users/wstein/.sage/temp/teragon_2.local/1537/tmp_0.patch patching file sage-sage Hunk #5 succeeded at 227 with fuzz 1 (offset 4 lines). Hunk #6 succeeded at 246 with fuzz 1 (offset 4 lines). Hunk #7 FAILED at 332 Hunk #8 FAILED at 376 Hunk #10 succeeded at 474 with fuzz 1 (offset 41 lines). Hunk #12 succeeded at 507 with fuzz 1 (offset 41 lines). Hunk #18 succeeded at 603 with fuzz 1 (offset 41 lines). Hunk #20 succeeded at 647 with fuzz 1 (offset 41 lines). Hunk #27 FAILED at 768 Hunk #28 FAILED at 794 4 out of 28 hunks FAILED -- saving rejects to file sage-sage.rej abort: patch failed to apply
Note -- I did *read* the patch and I think it's very good. Also, I'm very glad for Mvngu's observations about all those typos, especially line -2 of SAGE_ROOT/sage. Can you fix those typos in sage too?
comment:5 Changed 12 years ago by
From patch author:
I'm kind of overloaded with work from school now, so it will have to wait about 2 or 3 weeks. If nobody does it before then, I'll do it.
Changed 12 years ago by
comment:6 Changed 12 years ago by
- Milestone changed from sage-3.4 to sage-3.2.3
- Owner changed from anakha to abergeron
- Summary changed from [with patch; needs work] loading a file with spaces in the filename doesn't work to [with patch; needs review] loading a file with spaces in the filename doesn't work
I'm back in buisness!
trac_4354.2.patch is a rebase against 3.2.2
sage.2 is the sage startup script with the doc changes proposed by mvngu.
comment:7 Changed 12 years ago by
- Milestone changed from sage-3.2.3 to sage-3.4
comment:8 Changed 12 years ago by
- Summary changed from [with patch; needs review] loading a file with spaces in the filename doesn't work to [with patch; needs review, under review by gsw] loading a file with spaces in the filename doesn't work
Target time for the review: January 13th
comment:9 Changed 12 years ago by
Rescheduled for January 18th
comment:10 Changed 12 years ago by
- Summary changed from [with patch; needs review, under review by gsw] loading a file with spaces in the filename doesn't work to [with patch; needs review] loading a file with spaces in the filename doesn't work
comment:11 Changed 12 years ago by
- Summary changed from [with patch; needs review] loading a file with spaces in the filename doesn't work to [with patch; needs work] loading a file with spaces in the filename doesn't work
Unfortunately, this patch no longer applies cleanly against Sage 3.2.3. It is obvious how to rebase it, though (sigh).
comment:12 Changed 12 years ago by
- Status changed from new to assigned
- Summary changed from [with patch; needs work] loading a file with spaces in the filename doesn't work to [with patch; needs review] loading a file with spaces in the filename doesn't work
Changed 12 years ago by
rebase against 3.3.rc0 (you'll need also to put "sage.2" as "sage" into $SAGE_ROOT )
comment:13 Changed 12 years ago by
Well, this time "sage-run" had changed in between 3.3.alpha0 and 3.3.rc0.
I carefully hand-edited the 3.3.alpha0 patch (which did apply to 3.3.rc0 "sage-sage" only, but no longer to "sage-run") to reflect these changes, so the HG changeset header with the credit stayed the same.
The resulting patch now applies cleanly against 3.3.rc0.
comment:14 Changed 12 years ago by
- Milestone changed from sage-3.4.1 to sage-3.3
- Summary changed from [with patch; needs review] loading a file with spaces in the filename doesn't work to [with patch; positive review] loading a file with spaces in the filename doesn't work
Hello Michael,
this should go in 3.3 now. Of the handful files above, you'll need two:
A) The third one "sage.2" to go directly (no repo!!) right under $SAGE_ROOT as "sage" to replace the older script of the same name there. Mind the usual file executable flag issues ;-)
B) The last one "trac_4354_rebase_3.3.rc0.patch" applies as a HG patch to /local/bin (Sage scripts) repo.
Then test (as the original ticket comment says) from the bash command line (say after "your_favourite_path_to/sage -sh"):
$ echo 'print "ok"' > 'test file.sage' $ sage "test file.sage"
(This can hardly be a doctest, it's just outside scope --- and IMHO the issue of testing this kind of environmental stuff should not burden this ticket here, although it is a valid question.)
comment:15 Changed 12 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged sage.2 and trac_4354_rebase_3.3.rc0.patch in Sage 3.3.rc1.
I double checked both patches and I am confident they do the right thing.
Cheers,
Michael
Crap, I need to relax on the submit button