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:

Status badges

Description

try it at home:

$ echo 'print "ok"' > 'test file.sage'
$ sage "test file.sage"

Attachments (4)

trac_4354.patch (15.6 KB) - added by anakha 13 years ago.
sage.2 (1.4 KB) - added by abergeron 12 years ago.
trac_4354_rebase.patch (16.3 KB) - added by abergeron 12 years ago.
Rebase against 3.3.alpha1
trac_4354_rebase_3.3.rc0.patch (16.3 KB) - added by GeorgSWeber 12 years ago.
rebase against 3.3.rc0 (you'll need also to put "sage.2" as "sage" into $SAGE_ROOT )

Download all attachments as: .zip

Change History (19)

comment:1 Changed 13 years ago by anakha

  • Component changed from algebra to misc
  • Milestone set to sage-3.2
  • Owner changed from tbd to anakha

Crap, I need to relax on the submit button

Changed 13 years ago by anakha

comment:2 Changed 13 years ago by anakha

  • 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 mvngu

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 was

  • 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 was

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 abergeron

comment:6 Changed 12 years ago by abergeron

  • 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 mabshoff

  • Milestone changed from sage-3.2.3 to sage-3.4

comment:8 Changed 12 years ago by GeorgSWeber

  • 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 GeorgSWeber

Rescheduled for January 18th

comment:10 Changed 12 years ago by mabshoff

  • 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 GeorgSWeber

  • 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).

Changed 12 years ago by abergeron

Rebase against 3.3.alpha1

comment:12 Changed 12 years ago by abergeron

  • 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 GeorgSWeber

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 GeorgSWeber

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 GeorgSWeber

  • 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 mabshoff

  • 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

Note: See TracTickets for help on using tickets.