Opened 11 years ago
Closed 11 years ago
#9644 closed defect (fixed)
Add error messages and update documentation for spaces in $SAGE_ROOT
Reported by: | mpatel | Owned by: | jason |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6 |
Component: | misc | Keywords: | |
Cc: | kcrisman, leif, ddrake, schilly, mvngu | Merged in: | sage-4.6.alpha2 |
Authors: | John Palmieri, Leif Leonhardy | Reviewers: | John Palmieri, David Kirkby |
Report Upstream: | N/A | Work issues: | Update README.txt, Wiki page(s); put info on download page(s) |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Reported by Dan on sage-support:
This is an observation about the pre-compiled binaries for Mac OS. If the (unpacked) sage disk image directory is saved in a folder that has a space character in its name, for example: $HOME/Applications/my folder Then sage will not load properly when the "sage" executable is clicked. The terminal session begins, but the application doesn't load successfully. Changing the parent directory name to "my_folder" resolved this issue. While using space characters in directory names probably isn't all that common in UNIX or Linux installations, it is more common in Mac OS installations. Perhaps the installation instructions could be updated to mention this issue?
This actually appears to be a more general problem:
$ hostname sage.math.washington.edu $ pwd /mnt/usb1/scratch/mpatel/tmp/my sage $ ./sage Error setting environment variables by running /mnt/usb1/scratch/mpatel/tmp/my sage/local/bin/sage-env; possibly contact sage-devel (see http://groups.google.com/group/sage-devel). cat: /bin/sage-banner: No such file or directory mkdir: cannot create directory `': No such file or directory cp: cannot create directory `/ipython': Permission denied Traceback (most recent call last): File "./sage-cleaner", line 25, in <module> DOT_SAGE = os.environ['DOT_SAGE'] File "/mnt/usb1/scratch/mpatel/tmp/my sage/local/lib/python2.6/UserDict.py", line 22, in __getitem__ raise KeyError(key) KeyError: 'DOT_SAGE' ********************************************************************** Welcome to IPython. I will try to create a personal configuration directory where you can customize many aspects of IPython's functionality in: /ipython Initializing from configuration /mnt/usb1/scratch/mpatel/tmp/my sage/local/lib/python2.6/site-packages/IPython/UserConfig WARNING: There was a problem with the installation: [Errno 13] Permission denied: '/ipython' Try to correct it or contact the developers if you think it's a bug. IPython will proceed with builtin defaults. Please press <RETURN> to start IPython.
Similar tickets: #5261 (OS X app bundles), #8303 (command-line arguments).
Documentation to correct / clarify:
$SAGE_ROOT/README.txt
(This file gets into the download directories, too.)
- Sage Installation Guide:
- Wiki pages:
- http://wiki.sagemath.org/DownloadAndInstallationGuide (The download pages directly link to this.)
- http://wiki.sagemath.org/DownloadGuide
Perhaps also put a note onto the download pages for binaries, e.g. http://boxen.math.washington.edu/sage/linux/64bit/index.html
Attachments (3)
Change History (43)
comment:1 Changed 11 years ago by
- Description modified (diff)
comment:2 Changed 11 years ago by
- Cc leif added
comment:3 Changed 11 years ago by
<flame>
R T F M ! ;-)
(Milestone Sage 6.0)
</flame>
It is well documented that $SAGE_ROOT
(and therefore $SAGE_LOCAL
must not contain spaces; I only wondered a few times if this is checked/catched anywhere, since almost all scripts (especially spkg-install
scripts) blindly rely on this. So changing this (allowing spaces) is a big task, which is IMHO not worth doing now. It is in general a bad idea to put spaces in path names (filenames are a little different/less problematic).
-Leif
comment:4 Changed 11 years ago by
Ok, it could perhaps be documented in more prominent places, but it's (still) twice in $SAGE_ROOT/README.txt
, the second time a bit to mild:
... MORE DETAILED INSTRUCTIONS TO BUILD FROM SOURCE ----------------------------------------------- ... 3. Extract the Sage source tarball and cd into a directory with no spaces in it. [...] ... RELOCATION ---------- You *should* be able to move the sage-x.y.z directory anywhere you want. If you copy the sage script or make a symbolic link to it, you should modify the script to reflect this (as instructed at the top of the script). It is best if the path to Sage does not have any spaces in it. ...
And afair it is (or at least has been at some time) better documented in the Installation Guide.
So I would open two tickets (one could be this with changed title):
- Make sure
$SAGE_ROOT
does not contain spaces, e.g. insage-env
(blocker).
- Improve documentation (CAPSLOCK? Add/move remarks to top of files.)
comment:5 Changed 11 years ago by
Ouch, I just realized the scripting "logic" is completely insane:
While sage-env
is intended to be sourced (rather than called), and sage-sage
does this,
sage-sage
redirects stdout and stderr while sourcingsage-env
to/dev/null
, thoughsage-env
eventually outputs (valuable) error messagessage-sage
tests its "exit status", though it is sourced, not calledsage-env
actually exits with 1 on error
So even when an error is detected in sage-env
, the user will never see the error message, but Sage will simply "silently" exit.
Argh...
comment:6 Changed 11 years ago by
With the attached patch I get:
leif@portland:~/Sage /sage-4.5.2.alpha0-j12$ ./sage Error: The path to the Sage directory ($SAGE_ROOT) MUST NOT contain spaces. It is currently "/home/leif/Sage /sage-4.5.2.alpha0-j12". You must correct this by moving Sage (or renaming some directories) first. Exiting now... leif@portland:~/Sage /sage-4.5.2.alpha0-j12$
(Note the spaces in the path, I simply renamed a component of the path to my Sage directories.)
comment:7 Changed 11 years ago by
The error message could of course be a bit more friendly, for example:
"Please correct this by moving Sage (or renaming some directories) first."
comment:8 Changed 11 years ago by
An alternative, perhaps simpler, but "less efficient" test for spaces would be:
[ `echo "X${SAGE_ROOT}X" | wc -w` -ne 1 ]
(This one catches leading and trailing blanks, too, like the shell function in the patch when called properly. But we usually have leading and trailing slashs/path separators in $SAGE_ROOT
anyway.)
comment:9 Changed 11 years ago by
I know there was one bit of code in sage which removed one space, but not multiple ones. i.e.
sed 's/ //'
rather than
sed 's/ //g'
I changed the one occurrence I spotted, but there might be others lucking.
Of course, the best way to solve this is to not have any hacks like this, and just get Sage to build with spaces in paths. If things are quoted properly, this should be possible.
Perhaps there should be an environment variable that can be used to bypass that hack which removes spaces. Then we could slowly find the packages that have problems and correct them one by one. Once they were all corrected, the hack could be removed and Sage would build on paths with spaces in them.
Dave
Changed 11 years ago by
Gives error message on spaces in $SAGE_ROOT. (Draft, but functional. Slightly more friendly.) Apply to scripts repo.
comment:10 follow-up: ↓ 11 Changed 11 years ago by
We could require people who update/provide new spkgs to at least check if the upstream supports spaces in path names (I really doubt all will), and to harden the corresponding Sage scripts in that way. But I expect this to be a long lasting process, rather than a goal for any release in the nearer future.
And I have absolutely no idea which "inner" parts of Sage (i.e. Python code) might break on spaces in file or path names.
Of course users that only download and install binaries should somehow get informed, too, e.g. directly on the download page.
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 11 years ago by
Of course users that only download and install binaries should somehow get informed, too, e.g. directly on the download page.
And THAT was the point of the original post - that README.txt should probably also mention this issue for downloads, since he downloaded a *binary*.
Anyway, I'm glad to see this wasn't a platform-specific issue. Probably this ticket should get closed by fixing the manual and adding the error messages, as it were, and then one could open a sage-wishlist milestone ticket for supporting spaces in the path. (The error message may also want to say that certain Sage components would fail otherwise? Or would that be too long?)
comment:12 in reply to: ↑ 11 Changed 11 years ago by
- Cc ddrake schilly added
- Status changed from new to needs_review
- Work issues set to Update README.txt, put info on download page(s)
Replying to kcrisman:
Of course users that only download and install binaries should somehow get informed, too, e.g. directly on the download page.
And THAT was the point of the original post - that README.txt should probably also mention this issue for downloads, since he downloaded a *binary*.
Although the README.txt (and other scattered pieces of information) are rarely changed/current, I really thought this was a well-known issue, since I came across that in the documentation many times. (I think it once was better placed, s.t. nobody could miss it, though people tend to read READMEs and installation guides - if at all - after they run into problems... ;-) )
[...] Probably this ticket should get closed by fixing the manual and adding the error messages, as it were, and then one could open a sage-wishlist milestone ticket for supporting spaces in the path.
Yes, that's what I was thinking of, too.
The error message may also want to say that certain Sage components would fail otherwise? Or would that be too long?
As long as it fits onto a 80x25 screen... Feel free to add a "reviewer patch" or make some suggestion and I'll update the patch. I'm quite sure then this could get into 4.5.2.
Someone volunteering for a revised README?
Harald, ideas for the web page?
comment:13 Changed 11 years ago by
Oh, perhaps the Makefile or spkg/install
should get some early check, too.
comment:14 Changed 11 years ago by
Well, it's possible to add anything somewhere on the webpage, but I think it doesn't look good, or is professional, to list bugs on a download page (especially because it is never read just like the readme).
I don't know the exact problem, but I'm for adding a check at the "sage" script itself -- ahh, i've just seen that above -- and to the early scripts when starting to build sage.
comment:15 Changed 11 years ago by
The Installation Guide definitely needs to be updated, too:
[...] Make sure there are no spaces in the directory name under which you build. Running from a directory with spaces in its name is supported but discouraged. Building is not possible, since several of the components do not build if there are spaces in the path. [...]
(This snippet is actually from 'Steps to Install from Source'.)
comment:16 Changed 11 years ago by
- Summary changed from Make Sage run even when $SAGE_ROOT contains spaces to Add error messages and update documentation for spaces in $SAGE_ROOT
comment:17 Changed 11 years ago by
An aside: For this special situation:
- Build Sage with no spaces in
SAGE_ROOT
. - Rename
SAGE_ROOT
to contain space(s).
the patches here should get Sage to start and get sage -b
, sage -clone
, sage -docbuild
, and sage -t(p)
commands to work. Except for some SYMPOW and GAP-related tests, the long doctests pass. It's likely that fixing these components requires spkg updates.
I'm sure there are other problems to fix, and these patches certainly won't allow Sage to build with spaces in SAGE_ROOT
. But the exercise suggests that to fix most (all?) Python code, we should focus on system calls, e.g., os.{popen*,system}
, subprocess.{call,Popen}
, etc. Of course, we can open other tickets for these changes.
comment:18 Changed 11 years ago by
- Description modified (diff)
- Work issues changed from Update README.txt, put info on download page(s) to Update README.txt, Wiki page, Installation Guide; put info on download page(s)
comment:19 Changed 11 years ago by
- Cc mvngu added
comment:20 follow-up: ↓ 21 Changed 11 years ago by
- Milestone set to sage-4.6
comment:21 in reply to: ↑ 20 Changed 11 years ago by
Thanks, Leif. I'd like to focus, for now, on releasing 4.5.3 and then on the PARI upgrade for 4.6.alpha0. I'll try to work on this ticket after 4.5.3 is out.
comment:22 Changed 11 years ago by
- Description modified (diff)
- Work issues changed from Update README.txt, Wiki page, Installation Guide; put info on download page(s) to Update README.txt, Wiki page(s), Installation Guide; put info on download page(s)
Btw: If you want to have fun, try compiling Sage with -O2
contained in $SAGE_ROOT
.
(It's libgcrypt
that fails IIRC.)
comment:23 Changed 11 years ago by
This looks good to me; positive review for "trac_9644-first_aid_for_spaces_in_SAGE_ROOT-scripts_repo.patch". Trying to get Sage to work with spaces in the path can go on another ticket. Meanwhile, I'm attaching a patch for the installation guide.
comment:24 Changed 11 years ago by
- Reviewers set to John Palmieri, David Kirkby
- Status changed from needs_review to positive_review
John's happy with Leif's patch. I'm happy with John's changes to the installation guide. So positive review.
comment:25 Changed 11 years ago by
Thanks, everyone, for working on this.
comment:26 Changed 11 years ago by
So we are all happy. :)
Minor correction: Swap from and to in "Another approach is to create a symbolic link ...". (This has been in before. IIRC there are somewhere more instances of that, but never mind.)
comment:27 Changed 11 years ago by
How about this?
comment:28 Changed 11 years ago by
Thanks, very good.
comment:29 Changed 11 years ago by
- Status changed from positive_review to needs_work
The invocation of sage-env from the top-level makefile is causing errors. On sage.math:
. local/bin/sage-env && sage-starts && ./sage -tp 0 -sagenb -long devel/sage/doc/common devel/sage/doc/en devel/sage/doc/fr devel/sage/sage 2>&1 | tee -a ptestlong.log local/bin/sage-env: 70: Syntax error: Bad function name make: *** [ptestlong] Error 2
On my mac:
. local/bin/sage-env && sage-starts && ./sage -tp 0 -sagenb -long devel/sage/doc/common devel/sage/doc/en devel/sage/doc/fr devel/sage/sage 2>&1 | tee -a ptestlong.log local/bin/sage-env: line 77: `contains-spaces': not a valid identifier make: *** [ptestlong] Error 2
On t2.math:
. local/bin/sage-env && sage-starts && ./sage -tp 0 -sagenb -long devel/sage/doc/common devel/sage/doc/en devel/sage/doc/fr devel/sage/sage 2>&1 | tee -a ptestlong.log /bin/sh: contains-spaces: is not an identifier make: *** [ptestlong] Error 1
In all cases, changing "contains-spaces" to "contains_spaces" seems to fix the problem. I'm attaching a small patch to do this. (On Solaris systems, I also see a new warning, presumably because we're not redirecting output to /dev/null: after typing "make ptestlong", I see
Build finished. The built documents can be found in /scratch/palmieri/sage-4.5.3.rc0/devel/sage/doc/output/html/fr/a_tour_of_sage . local/bin/sage-env && sage-starts && ./sage -tp 0 -sagenb -long devel/sage/doc/common devel/sage/doc/en devel/sage/doc/fr devel/sage/sage 2>&1 | tee -a ptestlong.log /bin/sh: source: not found Testing that Sage starts... Yes, Sage starts. Testing that Sage starts... Yes, Sage starts. Global iterations: 1 File iterations: 1
Note the line /bin/sh: source: not found
. Is this important? If so, we should deal with it on another ticket...)
comment:30 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:31 Changed 11 years ago by
Thanks, I wanted to make exactly the same change weeks ago... (replacing the dash in the name by an underscore).
Also, source
should be replaced by .
(period). (At #9960 or here.)
(Note that this error message has always been there; you just had to look into /dev/null
... ;-) Redirecting stderr
to Nirvana is rarely a good idea.)
Although the Makefile shouldn't break the assumption that we're using bash
; so either sage-env
should be removed there, or the line should be bash -c ...
, or /usr/bin/env bash -c ...
. There are various ways to fix that.
I wonder if sage-starts
shouldn't use bash
(instead of sh
) and itself source sage-env
; then we could drop . local/bin/sage-env &&
from TESTPRELIMS
. (This would IMHO be cleaner.)
comment:32 Changed 11 years ago by
- Status changed from needs_review to positive_review
The changes look fine.
The /bin/sh: source:
is quite a common error. We should not solve it by redirecting stderr to /dev/null, but removing the word source
and replacing it with a dot. I suspect there are many instances of this lucking around. The problem is the command does not exist in /bin/sh
but does in bash
See below.
Here my default shell is bash
, so source
exists as a built in shell command.
kirkby@t2:64 ~$ source -bash: source: filename argument required source: usage: source filename [arguments]
Now change my shell to /bin/sh
kirkby@t2:64 ~$ sh $ source source: not found $
I'd prefer to use the more portable .
(dot) which will achieve the same, but is not a bashism. These should certainly be addressed on another ticket. The fact they are not producing any known errors, makes me wonder how important it is to source the files in the first place!
comment:33 Changed 11 years ago by
Sourcing sage-env
in the Makefile is only needed to set up SAGE_ROOT
for sage-starts
, which calls a Python script (sage-location
).
The other Sage scripts source sage-env
themselves.
As said before, best would be to change sage-starts
(and the Makefile) on another ticket; replacing source
by .
could be done at #9960 I think.
An even nicer solution would just "source" sage-env
from the Python script (by invoking a shell that does this and echoes the necessary variables), also removing . sage-env &&
from the Makefile.
You can hardly touch a piece of Sage without experiencing other things to fix... ;-)
comment:34 Changed 11 years ago by
Could someone please update the commit strings of the "installation" and "ref" patches to be a bit more descriptive and restore the positive review?
comment:35 Changed 11 years ago by
- Status changed from positive_review to needs_work
Changed 11 years ago by
comment:36 Changed 11 years ago by
- Status changed from needs_work to needs_review
Here are new commit strings. Can we restore the positive review now?
comment:38 Changed 11 years ago by
- Work issues changed from Update README.txt, Wiki page(s), Installation Guide; put info on download page(s) to Update README.txt, Wiki page(s); put info on download page(s)
comment:39 Changed 11 years ago by
I've updated some Wiki pages a while ago.
Harald said we shouldn't "announce bugs" on the download pages; I think a simple comment on where to currently not install Sage shouldn't be interpreted as such.
comment:40 Changed 11 years ago by
- Merged in set to sage-4.6.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
After applying
sage-sage
$SAGE_ROOT/local/bin/sage-env1>/dev/null 2>/dev/nullI get