Opened 10 years ago
Closed 10 years ago
#12347 closed defect (fixed)
make install broken in Sage 4.8
Reported by: | zimmerma | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | build | Keywords: | |
Cc: | Merged in: | sage-5.0.beta14 | |
Authors: | Punarbasu Purkayastha | Reviewers: | Jeroen Demeyer, Paul Zimmermann |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
when I compile Sage 4.8 from source in say /tmp/sage-4.8
, then
I do make install DESTDIR=/usr/local/sage-4.8
, and I run the binary /usr/local/sage-4.8/bin/sage
I get:
[zimmerma@coing ~]$ /usr/local/sage-4.8/bin/sage ************************************************************************ You must compile Sage first using 'make' in the Sage root directory. (If you have already compiled Sage, you must set the SAGE_ROOT variable in the file '/usr/local/sage-4.8/bin/sage'). ************************************************************************
This way of compiling and installing Sage worked before, up to Sage 4.7.2.
A fix is to add the line
SAGE_ROOT="/usr/local/sage-4.8/sage
in the script /usr/local/sage-4.8/bin/sage
. This was done
automatically by make install
up to 4.7.2, and was broken in Sage 4.8.
Paul
Hard coding of SAGE_ROOT is removed from make install
now. We simply do a symbolic link.
- Apply trac_12347-remove-sage-hardcode.patch to
SAGE_ROOT/local/bin
- Apply trac_12347.patch to
SAGE_ROOT
Attachments (2)
Change History (37)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Hmm.. using @ is not a good idea there. The correct patch should be:
-
Makefile
old new 149 149 mkdir -p $(DESTDIR)/sage 150 150 mkdir -p $(DESTDIR)/bin/ 151 151 cp -rpv * $(DESTDIR)/sage/ 152 python local/bin/sage-hardcode_sage_root $(DESTDIR)/sage/sage "$(DESTDIR)"/sage152 sed -i -e "s|^#SAGE_ROOT=/path/.*$|SAGE_ROOT=\"$(DESTDIR)/sage\"|;q" "$(DESTDIR)/sage/sage" 153 153 cp $(DESTDIR)/sage/sage $(DESTDIR)/bin/ 154 154 cd $(DESTDIR)/bin/; ./sage -c 155 155
comment:3 Changed 10 years ago by
I tried your patch and got:
sed -i -e "s|^#SAGE_ROOT=/path/.*SAGE_ROOT=\"/usr/local/sage-4.8a/sage\"|;q" "/usr/local/sage-4.8a/sage/sage" sed: -e expression #1, char 62: unterminated `s' command make: *** [install] Error 1
Did you really test your patch?
[root@coing sage-4.8]# diff Makefile.orig Makefile 152c152 < python local/bin/sage-hardcode_sage_root $(DESTDIR)/sage/sage "$(DESTDIR)"/sage --- > sed -i -e "s|^#SAGE_ROOT=/path/.*$|SAGE_ROOT=\"$(DESTDIR)/sage\"|;q" "$(DESTDIR)/sage/sage"
Paul
comment:4 Changed 10 years ago by
Sigh. Sorry for the trouble. I don't have sage-4.8 built yet. So, I can't test this as "make install". But I did test this sed expression before putting it here (that's how I caught the @
problem).
I see from
sed -i -e "s|^#SAGE_ROOT=/path/.*SAGE_ROOT=\"/usr/local/sage-4.8a/sage\"|;q"
that the $|
was eaten away by the make process (it is not stripped away when run on the terminal). This is why you are getting that sed error. Can you try it without the $
? Then the patch will look like this:
-
Makefile
old new 149 149 mkdir -p $(DESTDIR)/sage 150 150 mkdir -p $(DESTDIR)/bin/ 151 151 cp -rpv * $(DESTDIR)/sage/ 152 python local/bin/sage-hardcode_sage_root $(DESTDIR)/sage/sage "$(DESTDIR)"/sage152 sed -i -e "s|^#SAGE_ROOT=/path/.*|SAGE_ROOT=\"$(DESTDIR)/sage\"|;q" "$(DESTDIR)/sage/sage" 153 153 cp $(DESTDIR)/sage/sage $(DESTDIR)/bin/ 154 154 cd $(DESTDIR)/bin/; ./sage -c 155 155
comment:5 Changed 10 years ago by
it still does not work. With the change of comment 4, the $(DESTDIR)/bin/sage
file contains a single line:
tarte% cat /tmp/sage-4.8/bin/sage #!/usr/bin/env bash
Paul
comment:6 Changed 10 years ago by
Ok. I will leave it up to someone else to come up with a fix to the python file local/bin/sage-hardcode_sage_root
. I will check this ticket again after I have downloaded and built sage-4.8.
Thanks for your patience and feedback.
comment:7 Changed 10 years ago by
- Status changed from new to needs_review
Ok. Here is a fix for the hard coding of the path. The simplest fix is to not do any hard coding at all!
Please apply trac_12347.patch to SAGE_ROOT
(the directory where the Makefile
is present). If the review is positive then we should also remove SAGE_ROOT/local/bin/sage-hardcode_sage_root
from the tree since it is no longer needed.
I also ran into some problems with cp
, in particular with this line in the Makefile
:
cp -rpv * $(DESTDIR)/sage/
On the system that I tried this, cp actually errored out with error status 2. The last few lines of the make output are as follows:
$ make DESTDIR="/tmp/sage-install" install .... tons of copying stuff .... `spkg/logs/matplotlib-1.0.1.p0.log' -> `/tmp/sage-install/sage/spkg/logs/matplotlib-1.0.1.p0.log' `spkg/logs/sqlite-3.7.5.p0.log' -> `/tmp/sage-install/sage/spkg/logs/sqlite-3.7.5.p0.log' `start.log' -> `/tmp/sage-install/sage/start.log' `tmp' -> `/tmp/sage-install/sage/tmp' make: *** [install] Error 1
I can not fathom why it errors out. As a workaround, I changed the cp
command to include -u
and the cp finishes fine after invoking the make command once again:
$ make DESTDIR="/tmp/sage-install" install echo "Experimental use only!" Experimental use only! if [ "/tmp/sage-install" = "" ]; then \ echo "Set DESTDIR"; \ exit 1; \ fi mkdir -p /tmp/sage-install mkdir -p /tmp/sage-install/sage mkdir -p /tmp/sage-install/bin/ cp -rpuv * /tmp/sage-install/sage/ `Makefile' -> `/tmp/sage-install/sage/Makefile' cd /tmp/sage-install/bin; ln -sfn ../sage/sage ./sage; ./sage -c The Sage installation tree may have moved (from /home/punarbasu/Installations/sage-4.8 to /tmp/sage-install/sage). Changing various hardcoded paths... (Please wait at most a few minutes.) DO NOT INTERRUPT THIS. Done resetting paths.
comment:8 Changed 10 years ago by
- Status changed from needs_review to needs_work
You shouldn't use the options -u
, -v
, or -R
for cp. Re -r
, the man page for cp on OS X says
Historic versions of the cp utility had a -r option. This implementation supports that option; however, its use is strongly discouraged, as it does not correctly copy special files, symbolic links, or fifo's.
The OS X version of cp doesn't recognize the -u
option at all. The Solaris version of cp doesn't support -u
or -v
.
Make sure to only use Posix-standard options; I think this web page is accurate.
Along the same lines, on Solaris (and perhaps other platforms), ln -f
doesn't behave the way it does on linux; see the comment on line 71 of the file SAGE_ROOT/local/bin/sage-build.
comment:9 follow-up: ↓ 10 Changed 10 years ago by
@jhpalmieri: I added -u to make sure that the second time make install
is run, cp does not go around copying the files all over again. The options -r
, -v
were all present from before. ln -f
was needed in case the user ran make install
yet again, otherwise ln would give error. If the first invocation of make install doesn't fail, then there is no need to use -f with ln.
If cp
is unreliable then what is the alternative (it is actually quite reliable on linux with the -a
option, that is, it copies symlinks properly)? Alternatively, can we assume that rsync
is available on all platforms and use rsync instead?
comment:10 in reply to: ↑ 9 Changed 10 years ago by
Replying to ppurka:
@jhpalmieri: I added -u to make sure that the second time
make install
is run, cp does not go around copying the files all over again.
Solaris is a supported platform for Sage on which the -u
option is not recognized, so it doesn't matter too much why you added it: it's not going to work.
The options
-r
,-v
were all present from before.
But while you're fixing other things, you might as well fix this. Just use -R
or -pR
instead.
ln -f
was needed in case the user ranmake install
yet again, otherwise ln would give error. If the first invocation of make install doesn't fail, then there is no need to use -f with ln.
Same comment as before: it's not going to work this way, so you should do it a different way. (For example, first run 'rm -f ...' on the link, then do 'ln -s ...'.)
If
cp
is unreliable then what is the alternative (it is actually quite reliable on linux with the-a
option, that is, it copies symlinks properly)? Alternatively, can we assume thatrsync
is available on all platforms and use rsync instead?
I think that 'cp -pR' is reliable on all platforms. Note also that the 'make install' option says "Experimental use only!", so I don't think it's been well-maintained or supported, or even used much, which is why, for example, the -v
option for cp wasn't removed earlier. But if you're working on this, you should do it right, so that it works on all supported platforms. Using POSIX-standard commands should be completely reliable; every platform's version of 'cp' should support the POSIX-standard options for cp.
comment:11 Changed 10 years ago by
- Description modified (diff)
Updated the patches.
Now I see why cp failed:
...stallations/sage-4.8> make DESTDIR="/tmp/sage-xxxx" install |& tee -i -a ~/tmp/log-new.txt echo "Experimental use only!" Experimental use only! if [ "/tmp/sage-xxxx" = "" ]; then \ echo "Set DESTDIR"; \ exit 1; \ fi mkdir -p /tmp/sage-xxxx mkdir -p /tmp/sage-xxxx/sage mkdir -p /tmp/sage-xxxx/bin/ cp -Rp * /tmp/sage-xxxx/sage/ cp: reading `local/lib/libpari.a': Input/output error make: *** [install] Error 1
Rebuilding pari makes this bug go away
~/Installations/sage-4.8> ./sage -f spkg/standard/pari-2.5.0.p3.spkg >& ~/tmp/pari.txt ~/Installations/sage-4.8> cp local/lib/libpari.a /tmp ~/Installations/sage-4.8>
comment:12 Changed 10 years ago by
is the patch ready for review? No work issues were added in comment 8.
Paul
comment:13 Changed 10 years ago by
- Status changed from needs_work to needs_review
Sorry, forgot to set it to needs_review.
comment:14 Changed 10 years ago by
- Status changed from needs_review to needs_work
Replace
cd $(DESTDIR)/bin; ln -s ../sage/sage ./sage; ./sage -c
by
-rm -f $(DESTDIR)/bin/sage ln -s ../sage/sage $(DESTDIR)/bin/sage $(DESTDIR)/bin/sage -c # Need to run sage-location
comment:15 Changed 10 years ago by
updated.
comment:16 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:17 Changed 10 years ago by
Just one more thing: it would be better to first delete $(DESTDIR)/sage/
for two reasons:
- if there already was a previous version, there might be some kind of conflict (although I cannot immediately think of a concrete problem that could occur).
- some program in the old version might still be running (considering
sage-cleaner
, this is less unlikely than you might think at first).
comment:18 Changed 10 years ago by
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to needs_work
comment:19 Changed 10 years ago by
Updated the patch. I guess we could write some logic to detect if a sage process is running, but it would not be robust enough and would bloat the install part.
I updated the patch to not proceed with the installation if the $(DESTDIR)/sage
is a file or symlink. It will (hopefully) prevent accidents. :)
comment:20 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:21 follow-up: ↓ 23 Changed 10 years ago by
"[ -e
" isn't portable. "[ -f
" is portable.
Why do you talk about symlinks in your comment? I don't see that in your code.
I would simply remove the whole elif
block. mkdir -p "$(DESTDIR)"/sage
should fail anyway if "$(DESTDIR)"/sage
is a non-directory.
comment:22 Changed 10 years ago by
Mention in a comment that the "[ -d
" test is meant to exclude the case that "$(DESTDIR)"/sage is an existing file, not a directory.
comment:23 in reply to: ↑ 21 Changed 10 years ago by
Replying to jdemeyer:
Why do you talk about symlinks in your comment? I don't see that in your code.
Well, -e
checks for both. I didn't know it wasn't portable. You are correct that mkdir will fail. I hadn't thought of that.
comment:24 Changed 10 years ago by
Updated.
comment:25 follow-up: ↓ 26 Changed 10 years ago by
Actually -e
should be portable, going by this page.
comment:26 in reply to: ↑ 25 Changed 10 years ago by
Replying to ppurka:
Actually
-e
should be portable, going by this page.
Maybe it "should be", but it isn't.
comment:27 Changed 10 years ago by
For shell script portability discussions, I often refer to the GNU autoconf manual: http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.68/html_node/Portable-Shell.html#Portable-Shell.
Quoting from there:
But occasionally you may find it necessary to check whether some arbitrary file exists. To do so, use ‘test -f’ or ‘test -r’. Do not use ‘test -x’, because 4.3BSD does not have it. Do not use ‘test -e’ either, because Solaris /bin/sh lacks it. To test for symbolic links on systems that have them, use ‘test -h’ rather than ‘test -L’; either form conforms to Posix 1003.1-2001, but older shells like Solaris 8 /bin/sh support only -h.
comment:28 Changed 10 years ago by
Thanks!
comment:29 follow-up: ↓ 30 Changed 10 years ago by
the patches apply successfully to sage-5.0.beta13 and make install
works.
My only comment after looking at the patch: shouldn't we check also if $(DESTDIR)/bin
already exists?
Paul
comment:30 in reply to: ↑ 29 Changed 10 years ago by
Replying to zimmerma:
shouldn't we check also if $(DESTDIR)/bin already exists?
Why? What would you propose?
comment:31 follow-up: ↓ 32 Changed 10 years ago by
if we assume that $(DESTDIR)/sage can already exist, the same can happen for $(DESTDIR)/bin with a previous installation of Sage. What will then happen? I propose to remove $(DESTDIR)/bin if it exists and is a directory.
Paul
comment:32 in reply to: ↑ 31 Changed 10 years ago by
Replying to zimmerma:
if we assume that $(DESTDIR)/sage can already exist, the same can happen for $(DESTDIR)/bin with a previous installation of Sage. What will then happen?
Nothing bad will happen. The file or symbolic link $(DESTDIR)/bin/sage
will be deleted and recreated as symbolic link to $(DESTDIR)/sage/sage
.
comment:33 follow-up: ↓ 34 Changed 10 years ago by
- Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Paul Zimmermann
- Status changed from needs_review to positive_review
ok, then I give a positive review.
Paul
comment:34 in reply to: ↑ 33 Changed 10 years ago by
comment:35 Changed 10 years ago by
- Merged in set to sage-5.0.beta14
- Resolution set to fixed
- Status changed from positive_review to closed
The sage directory is relocatable and so simply moving the directory from
/tmp/sage-4.8
to/usr/local/sage-4.8
will enable you to run sage again. After moving the folder you should also run sage just once (/usr/local/sage-4.8/sage
) so that it resolves all the paths.As for make install being broken, it seems to happen because there is no
SAGE_ROOT="....."
in the file anymore and so that hard coding doesn't get done any more. A simple fix should be this patch to the Makefile in your/tmp/sage-4.8/Makefile
:Makefile
python local/bin/sage-hardcode_sage_root $(DESTDIR)/sage/sage "$(DESTDIR)"/sage