#21479 closed enhancement (fixed)
./configure prefix=SAGE_LOCAL
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  build  Keywords:  
Cc:  jdemeyer, embray, fbissey, leif, dimpase  Merged in:  
Authors:  Matthias Koeppe, Jeroen Demeyer  Reviewers:  Jeroen Demeyer, Erik Bray, Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  e5926b1 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
I propose to support choosing a location for the SAGE_LOCAL tree, using
./configure prefix=SAGE_LOCAL
(the default would be, as before, the local
subdirectory of SAGE_ROOT  see patch on the ticket).
I am fully aware that Sage's build system does not have a separation between 'make' and 'make install'; see #21495 for that.
Nevertheless, SAGE_LOCAL should be considered the same as the prefix
in the sense of the autotools.
This ticket is a step towards #21566.
Release manager configure tarball: http://sage.ugent.be/www/jdemeyer/sage/configure214.tar.gz
Change History (129)
comment:1 Changed 6 years ago by
 Branch set to u/mkoeppe/__configure___prefix_sage_local
comment:2 Changed 6 years ago by
 Commit set to fa16faeb0677d6dec1b9c07f7580b3fe5046bb88
 Description modified (diff)
comment:3 Changed 6 years ago by
comment:4 Changed 6 years ago by
 Description modified (diff)
comment:5 Changed 6 years ago by
 Description modified (diff)
comment:6 Changed 6 years ago by
(i completely agree with this)
a main obstacle is: the build system writes to $prefix during the build...
i suggest to do the transition onepackageatatime, and after disable$package
is in place.
with disableall
, it will be easiest to avoid writing to $prefix.
comment:7 Changed 6 years ago by
On this *short term* ticket, we do allow "make" to write into $prefix; and "make install" will be a noop.
More ambitious plans are to be discussed on the *long term* ticket #21495, not on this ticket. Thanks.
comment:8 Changed 6 years ago by
i agree that there are intermediate steps to take. but i don't yet fully understand this approach.
i think the behaviour of just "make" should not change, regardless of prefix
. it works perfectly well and *as intended* right now. clearly it should do something different under the hood, but that's not on this ticket.
we do allow "make" to write into $prefix
why would you need/want that? can you please give an example?
my feeling is, that entangling prefix and SAGE_LOCAL further complicates the transition considerably. there will be no way to tell whether "this part still uses SAGE_LOCAL" vs. "this has already been cleaned". newcomers tend to grep a variable from the code, see how it is used and use it the same way. for a looong time to come.
comment:9 Changed 6 years ago by
 Milestone changed from sage7.4 to sage7.5
comment:10 Changed 6 years ago by
 Dependencies set to #21501
 Description modified (diff)
I made a new ticket (#21501) to allow $SAGE_LOCAL
to be changed.
comment:11 Changed 6 years ago by
i wrote
entangling prefix and SAGE_LOCAL further complicates the transition considerably.
@mkoeppe i've thought about it. it seems, the alternatives are much worse (#21501) or much more ambitious.
please go ahead with prefix==SAGE_LOCAL.
please consider to add a remark in some documentation (better place: configure help
?) on this temporary sort of prefix, such as
"currently, make install
is a noop, that will change (hopefully). you should not expect a fully functional installation in $prefix before make install
has finished."
comment:12 Changed 6 years ago by
Thanks Felix for the discussion.
If you want to help, could you work on rebasing #15105.
comment:13 Changed 6 years ago by
 Commit changed from fa16faeb0677d6dec1b9c07f7580b3fe5046bb88 to 924b30bbe2cd1daf28f32ea73293b8f0107ab001
Branch pushed to git repo; I updated commit sha1. New commits:
64e697d  Allow a custom $SAGE_LOCAL directory

dc81cf6  Merge remotetracking branch 'trac/u/jdemeyer/ticket/21501' into t/21479/__configure___prefix_sage_local

ac12edc  Merge tag '7.4.beta5' into t/21479/__configure___prefix_sage_local

924b30b  Handle prefix

comment:14 Changed 6 years ago by
 Commit changed from 924b30bbe2cd1daf28f32ea73293b8f0107ab001 to 6f8eaac5088a39b83859f89fa3ea5ff9e437ad74
Branch pushed to git repo; I updated commit sha1. New commits:
6f8eaac  Fixup

comment:15 followups: ↓ 17 ↓ 25 Changed 6 years ago by
 Cc jdemeyer added
 Status changed from new to needs_review
Here's a first version of what I have in mind.
Some concerns:
 bootstrap. It's somewhat unclear what to do here. By the way, is it supposed to use the distribution's Python or ours?
 sagelocation.
comment:16 followup: ↓ 19 Changed 6 years ago by
I don't like the duplication in build/make/install
and src/bin/sageenv
.
Here is a suggestion: add a new file, say src/bin/sageconfig
(or whatever you want to call it) to deal with configurable things like SAGE_LOCAL
. You can also define the other SAGE_*
directories (except SAGE_ROOT
) there.
Then you can source this file in both src/bin/sageenv
and build/make/install
, which can then be independent of configure
.
And I don't see the point of changing $SAGE_ROOT/sage
because that is additional duplication.
comment:17 in reply to: ↑ 15 Changed 6 years ago by
Replying to mkoeppe:
Some concerns:
 bootstrap. It's somewhat unclear what to do here. By the way, is it supposed to use the distribution's Python or ours?
 sagelocation.
In both cases, I don't get what the problem is.
comment:18 Changed 6 years ago by
 Commit changed from 6f8eaac5088a39b83859f89fa3ea5ff9e437ad74 to 66fc193a9e0e1bf9b0aa2a204ccc0bd13609bbde
comment:19 in reply to: ↑ 16 Changed 6 years ago by
Replying to jdemeyer:
Here is a suggestion: add a new file, say
src/bin/sageconfig
(or whatever you want to call it) to deal with configurable things likeSAGE_LOCAL
. You can also define the otherSAGE_*
directories (exceptSAGE_ROOT
) there.
Thanks for the suggestion. The ticket is indeed simpler that way. Please take a look.
comment:20 Changed 6 years ago by
comment:21 Changed 6 years ago by
 Commit changed from 66fc193a9e0e1bf9b0aa2a204ccc0bd13609bbde to e0dc9a374286e38c82516c313d598bd7df5806c3
Branch pushed to git repo; I updated commit sha1. New commits:
e0dc9a3  Merge tag '7.4.beta6' into t/21479/__configure___prefix_sage_local

comment:22 Changed 6 years ago by
 Commit changed from e0dc9a374286e38c82516c313d598bd7df5806c3 to 82152547946c7776c2d5c28ef44d7da86c22eaf0
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
751bd0f  Reword TODO item

3a8cc0e  Fix typo in comment

0dd9c50  Respect environment variable MAKE

17f90d8  beautification

e5f9065  More comments

7791cd9  Remove buildbase code

74169e7  Pass SAGE_SRC to generate_py_source.mk

0394333  Add new file to MANIFEST.in

fdedb02  Install SAGE_LOCAL/bin/sage instead of SAGE_ROOT/sage as Jupyter kernel

8215254  Merge branch 't/21480/keep_src__clean_by_using___build_base_when_building_sagelib' into t/21479/__configure___prefix_sage_local

comment:23 Changed 6 years ago by
 Cc embray fbissey leif added
comment:24 Changed 6 years ago by
 Description modified (diff)
comment:25 in reply to: ↑ 15 Changed 6 years ago by
Replying to mkoeppe:
Here's a first version of what I have in mind.
Some concerns:
 bootstrap. It's somewhat unclear what to do here. By the way, is it supposed to use the distribution's Python or ours?
Just to answer this question: the bootstrap
package is intended to use the system Python (after all, it's needed well before Sage's Python is ever built).
comment:26 Changed 6 years ago by
 Commit changed from 82152547946c7776c2d5c28ef44d7da86c22eaf0 to 06fe631ba0cd657c145f048280ea0c80fce7acd1
Branch pushed to git repo; I updated commit sha1. New commits:
06fe631  Merge tag '7.4.rc1' into t/21479/__configure___prefix_sage_local

comment:27 Changed 6 years ago by
Branch is now on top of 7.4.rc1.
Jeroen  ready for review.
comment:28 Changed 6 years ago by
 Commit changed from 06fe631ba0cd657c145f048280ea0c80fce7acd1 to 1adc18ce0174f9399f53f817bfe1b06b1b250a9b
Branch pushed to git repo; I updated commit sha1. New commits:
1adc18c  Merge tag '7.4' into t/21479/__configure___prefix_sage_local

comment:29 Changed 6 years ago by
Now on top of 7.4.
comment:30 Changed 6 years ago by
 Commit changed from 1adc18ce0174f9399f53f817bfe1b06b1b250a9b to d82cfbc4bd54dfccda1fa3564bc8d39bd4fba216
Branch pushed to git repo; I updated commit sha1. New commits:
d82cfbc  Merge tag '7.5.beta0' into t/21479/__configure___prefix_sage_local

comment:31 Changed 6 years ago by
 Cc dimpase added
comment:32 followup: ↓ 35 Changed 6 years ago by
What does this comment mean:
## The first test does not actually work (see above).
("see above" can be really confusing because you don't know where to look.
Comments like these don't belong in the sources but in a Trac ticket:
# TODO: Making directories in $prefix # should be done by 'make', not configure.
# TODO: Should the install hierarchy (local) really be deleted by make distclean?
#SAGE_ENV_DIR=`dirname $0` ### Does not work. $0 is /bin/bash
## Note this is a bashism, not sure if this is allowed in this file.
comment:33 followup: ↓ 36 Changed 6 years ago by
Then a more serious remark: I doubt that you are using BASH_SOURCE
correctly. From the bash
man page:
BASH_SOURCE An array variable whose members are the source filenames where the corresponding shell function names in the FUNCNAME array variable are defined. The shell function ${FUNCNAME[$i]} is defined in the file ${BASH_SOURCE[$i]} and called from ${BASH_SOURCE[$i+1]}.
At the very least, I need more explanation of why your code does the right thing.
comment:34 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:35 in reply to: ↑ 32 ; followup: ↓ 37 Changed 6 years ago by
Replying to jdemeyer:
What does this comment mean:
## The first test does not actually work (see above).("see above" can be really confusing because you don't know where to look.
It's referring to dirname $0
 which does not give the location of sageenv but rather /bin/bash. This is because sageenv is sourced (.) rather than executed.
Note this is existing code in sageenv
which simply never worked.
Comments like these don't belong in the sources but in a Trac ticket:
I know, these comments are for the reviewer. I'll be happy remove them *after* we had a discussion about them... So do you have a comment on these?
# TODO: Making directories in $prefix # should be done by 'make', not configure.# TODO: Should the install hierarchy (local) really be deleted by make distclean?#SAGE_ENV_DIR=`dirname $0` ### Does not work. $0 is /bin/bash## Note this is a bashism, not sure if this is allowed in this file.
comment:36 in reply to: ↑ 33 Changed 6 years ago by
Replying to jdemeyer:
Then a more serious remark: I doubt that you are using
BASH_SOURCE
correctly. From thebash
man page:BASH_SOURCE An array variable whose members are the source filenames where the corresponding shell function names in the FUNCNAME array variable are defined. The shell function ${FUNCNAME[$i]} is defined in the file ${BASH_SOURCE[$i]} and called from ${BASH_SOURCE[$i+1]}.At the very least, I need more explanation of why your code does the right thing.
BASH_SOURCE[0] gives the source of the "currently running function". See https://www.gnu.org/software/bash/manual/html_node/BashVariables.html (you need to read the entries for FUNCNAME and BASH_SOURCE).
comment:37 in reply to: ↑ 35 Changed 6 years ago by
Replying to mkoeppe:
Replying to jdemeyer:
What does this comment mean:
## The first test does not actually work (see above).("see above" can be really confusing because you don't know where to look.
It's referring to
dirname $0
 which does not give the location of sageenv but rather /bin/bash. This is because sageenv is sourced (.) rather than executed.
Don't tell me but tell the person reading sageenv
(i.e. clarify the comment in the file).
I know, these comments are for the reviewer. I'll be happy remove them *after* we had a discussion about them... So do you have a comment on these?
Comments for the reviewer can go on Trac.
comment:38 Changed 6 years ago by
 Commit changed from d82cfbc4bd54dfccda1fa3564bc8d39bd4fba216 to c3477688c36c27cd240ca074d683f93fa1f6cb22
comment:39 Changed 6 years ago by
 Status changed from needs_work to needs_review
OK, done; please take a look
comment:40 Changed 6 years ago by
 Commit changed from c3477688c36c27cd240ca074d683f93fa1f6cb22 to 4275121e18f8e7e46dd016bd0ccd96fcbc94ce54
Branch pushed to git repo; I updated commit sha1. New commits:
4275121  Installation manual: Document configure prefix

comment:41 Changed 6 years ago by
 Commit changed from 4275121e18f8e7e46dd016bd0ccd96fcbc94ce54 to ea994cc736493d3ab78dc5ed659cf49af2a00a5c
Branch pushed to git repo; I updated commit sha1. New commits:
ea994cc  Merge tag '7.5.beta1' into t/21479/__configure___prefix_sage_local

comment:42 Changed 6 years ago by
ping?
comment:43 Changed 6 years ago by
 Dependencies #21501 deleted
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to needs_work
Running ./configure
without arguments:
[...] config.status: creating build/make/Makefileauto config.status: creating src/Makefile config.status: creating src/bin/sageenvconfig config.status: executing depfiles commands config.status: executing mkdirs commands config.status: creating directory /usr/local/src/sageconfig/logs/pkgs config.status: creating directory NONE config.status: creating directory NONE/bin config.status: creating directory NONE/etc config.status: creating directory NONE/include config.status: creating directory NONE/lib config.status: creating directory NONE/share config.status: creating directory NONE/var/lib/sage/installed config.status: creating symbolic link lib64 > lib
comment:44 Changed 6 years ago by
This is just because you write SAGE_LOCAL
instead of "$SAGE_LOCAL"
.
When you fix this, I suggest to squash the commits to one commit and rebase to 7.5.beta1
.
comment:45 Changed 6 years ago by
The file sageenvconfig
needs to be ignored by .gitignore
and deleted when running make distclean
.
comment:46 followup: ↓ 57 Changed 6 years ago by
This line in sage
needs quoting:
. $SAGE_ROOT/src/bin/sageenvconfig
And since you're running bash
anyway, you might as well use source
instead of .
which is more visible.
comment:47 Changed 6 years ago by
And do you really need this:
if [ $? ne 0 ]; then echo >&2 "Error: $SAGE_ROOT/src/bin/sageenvconfig could not be read" fi
The shell will already write an error message, no need for an additional message.
comment:48 Changed 6 years ago by
I also think that determining $SAGE_LOCAL
should be done after
# If this is a freshlyunpacked binary tarball then run the installer # Note: relocateonce.py deletes itself upon successful completion if [ x "$SAGE_ROOT/relocateonce.py" ]; then "$SAGE_ROOT/relocateonce.py" fi
comment:49 followup: ↓ 56 Changed 6 years ago by
Can you explain under which circumstances it is needed to read sageenvconfig
from sageenv
? It seems to me that sageenvconfig
would already be read, either by the toplevel sage
script or by build/make/install
.
comment:50 Changed 6 years ago by
It is good style to start all error messages with Error:
and write them to stderr. build/make/install
doesn't do that.
comment:51 Changed 6 years ago by
 Commit changed from ea994cc736493d3ab78dc5ed659cf49af2a00a5c to 4f832c2b1d814a3fd2dc8f2ccd2ab77bfde74451
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4f832c2  Add configure option prefix=SAGE_LOCAL

comment:52 Changed 6 years ago by
Squashed and rebased on 7.5.beta1
comment:53 Changed 6 years ago by
 Commit changed from 4f832c2b1d814a3fd2dc8f2ccd2ab77bfde74451 to 5fcda8326cf504dfd1b32300bcc3493e3f398e86
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6308c3b  Add configure option prefix=SAGE_LOCAL

ff48d5a  Fixup configure without prefix

67baa7a  Source sageenvconfig after calling relocateonce.py

5fcda83  toplevel sage script: Remove redundant sageenvconfig error message

comment:54 Changed 6 years ago by
 Commit changed from 5fcda8326cf504dfd1b32300bcc3493e3f398e86 to e52554739a9a321de2180d5bf39b45f205fd6efc
comment:55 Changed 6 years ago by
 Commit changed from e52554739a9a321de2180d5bf39b45f205fd6efc to 600639a680efa970e2fc4c0047d51504b5053e77
Branch pushed to git repo; I updated commit sha1. New commits:
600639a  sageenv: Quoting fix

comment:56 in reply to: ↑ 49 ; followups: ↓ 59 ↓ 68 Changed 6 years ago by
Replying to jdemeyer:
Can you explain under which circumstances it is needed to read
sageenvconfig
fromsageenv
? It seems to me thatsageenvconfig
would already be read, either by the toplevelsage
script or bybuild/make/install
.
$SAGE_LOCAL/bin/sage
becomes directly executable; one does not have to go through that toplevel script.
comment:57 in reply to: ↑ 46 Changed 6 years ago by
Replying to jdemeyer:
since you're running
bash
anyway, you might as well usesource
instead of.
which is more visible.
I'm not sure, I like .
more than source
.
comment:58 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:59 in reply to: ↑ 56 ; followup: ↓ 63 Changed 6 years ago by
Replying to mkoeppe:
$SAGE_LOCAL/bin/sage
becomes directly executable; one does not have to go through that toplevel script.
Then let me turn the question around: why do you need to change $SAGE_ROOT/sage
?
comment:60 followup: ↓ 62 Changed 6 years ago by
I'm still not entirely convinced that BASH_SOURCE[0]
really does what we want. Why not use $SAGE_ROOT/src/bin/sageenvconfig
?
Or maybe we should just source both sageenvconfig
and sageenv
from src/bin/sage
?
comment:61 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:62 in reply to: ↑ 60 ; followup: ↓ 67 Changed 6 years ago by
Replying to jdemeyer:
Why not use
$SAGE_ROOT/src/bin/sageenvconfig
?
Things installed in SAGE_LOCAL definitely should not depend on the source directory.
Or maybe we should just source both
sageenvconfig
andsageenv
fromsrc/bin/sage
?
That would be possible. There might be a few more places though that need to source the extra file.
comment:63 in reply to: ↑ 59 ; followup: ↓ 64 Changed 6 years ago by
Replying to jdemeyer:
Replying to mkoeppe:
$SAGE_LOCAL/bin/sage
becomes directly executable; one does not have to go through that toplevel script.Then let me turn the question around: why do you need to change
$SAGE_ROOT/sage
?
$SAGE_ROOT/sage
sources $SAGE_ROOT/src/bin/sageenvconfig
so that it works when nothing has been installed so far.
comment:64 in reply to: ↑ 63 ; followup: ↓ 66 Changed 6 years ago by
Replying to mkoeppe:
$SAGE_ROOT/sage
sources$SAGE_ROOT/src/bin/sageenvconfig
so that it works when nothing has been installed so far.
Sorry, I don't follow. What is the "it" in the sentence above? In other words, what would not work?
comment:65 Changed 5 years ago by
 Commit changed from 600639a680efa970e2fc4c0047d51504b5053e77 to a3467bf751cbfbdff0d85c2dfa895f78d32310b4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f6c4513  Add configure option prefix=SAGE_LOCAL

7d44f0b  Fixup configure without prefix

387ab94  Source sageenvconfig after calling relocateonce.py

fc66139  toplevel sage script: Remove redundant sageenvconfig error message

fe19d3f  src/bin/sageenvconfig: Prefix error message with 'Error:'

269ad80  make miscclean: Delete src/bin/sageenvconfig

727123a  Add sageenvconfig to .gitignore

be4c390  sageenv: Quoting fix

a3467bf  toplevel sage script: No need to source sageenvconfig

comment:66 in reply to: ↑ 64 Changed 5 years ago by
Replying to jdemeyer:
Replying to mkoeppe:
$SAGE_ROOT/sage
sources$SAGE_ROOT/src/bin/sageenvconfig
so that it works when nothing has been installed so far.Sorry, I don't follow. What is the "it" in the sentence above? In other words, what would not work?
I retract my comment.
I have removed this code from $SAGE_ROOT/sage
.
(Also rebased on latest beta.)
comment:67 in reply to: ↑ 62 Changed 5 years ago by
Replying to mkoeppe:
Or maybe we should just source both
sageenvconfig
andsageenv
fromsrc/bin/sage
?That would be possible. There might be a few more places though that need to source the extra file.
I think I wouldn't like to change this on this ticket. There are indeed a few places that source sageenv: build/bin/sagespkg
, build/make/deps
, src/bin/sageupdatesrc
. Probably not so nice if we have to change all of them.
comment:68 in reply to: ↑ 56 Changed 5 years ago by
Replying to mkoeppe:
$SAGE_LOCAL/bin/sage
becomes directly executable; one does not have to go through that toplevel script.
I realized that this is not entirely true. sageenv
contains some code for discovering SAGE_ROOT when it is not set, but that code fails if $SAGE_LOCAL is not a direct subdirectory of $SAGE_ROOT.
I suppose I could add the definition of SAGE_ROOT to sageenvconfig.
But that could go on a followup ticket as well.
comment:69 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:70 Changed 5 years ago by
*ping*
comment:71 Changed 5 years ago by
 Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Erik Bray
 Status changed from needs_review to positive_review
comment:72 Changed 5 years ago by
 Milestone changed from sage7.5 to sage7.6
comment:73 followup: ↓ 76 Changed 5 years ago by
Doesn't work if you dont have autotools
19make j2 build/make/Makefile 20make[1]: Entering directory `/home/buildbot/slave/sage_git/build' 21make[1]: warning: jN forced in submake: disabling jobserver mode. 22./bootstrap d 23make[2]: Entering directory `/home/buildbot/slave/sage_git/build' 24rm rf config configure build/make/Makefileauto.in 25make[2]: Leaving directory `/home/buildbot/slave/sage_git/build' 26src/bin/sageenv: line 206: /home/buildbot/slave/sage_git/build/src/bin/sageenvconfig: No such file or directory 27sageenv: Error: /home/buildbot/slave/sage_git/build/src/bin/sageenvconfig could not be read. 28./bootstrap: line 31: aclocal: command not found 29Bootstrap failed, downloading required files instead. 30./bootstrap: line 61: sagedownloadfile: command not found 31Error: downloading configure200.tar.gz failed 32make[1]: *** [configure] Error 1 33make[1]: Target `build/make/Makefile' not remade because of errors. 34make[1]: Leaving directory `/home/buildbot/slave/sage_git/build' 35make: *** [start] Error 2 36program finished with exit code 2
comment:74 Changed 5 years ago by
 Status changed from positive_review to needs_work
comment:75 Changed 5 years ago by
 Commit changed from a3467bf751cbfbdff0d85c2dfa895f78d32310b4 to 52826b18b1474c5f92c9c42f4cc75c6a301c1e87
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
928f67d  Add configure option prefix=SAGE_LOCAL

66a8066  Fixup configure without prefix

b5ecc81  Source sageenvconfig after calling relocateonce.py

c1c36ff  toplevel sage script: Remove redundant sageenvconfig error message

ae6263b  src/bin/sageenvconfig: Prefix error message with 'Error:'

27189be  make miscclean: Delete src/bin/sageenvconfig

eb922b8  Add sageenvconfig to .gitignore

4cdaa43  sageenv: Quoting fix

71f88cc  toplevel sage script: No need to source sageenvconfig

52826b1  bootstrap: Don't use sageenv, set PATH directly

comment:76 in reply to: ↑ 73 Changed 5 years ago by
 Status changed from needs_work to needs_review
Replying to vbraun:
Doesn't work if you dont have autotools
Thanks for catching this. Fixed. Needs review.
comment:77 Changed 5 years ago by
Erik, if you could take another look...
comment:78 Changed 5 years ago by
I'm not really clear about what changed. I don't understand what the issue with autotools was or how this was fixed. I see that you did do something to address that but I don't understand the original issue in the first placewould be better if Volker commented since he pointed it out.
comment:79 Changed 5 years ago by
Jeroen, could you take another look?
comment:80 Changed 5 years ago by
This ticket needs review.
comment:81 Changed 5 years ago by
*pong*
comment:82 followup: ↓ 83 Changed 5 years ago by
Why does this require changes to bootstrap
anyway?
comment:83 in reply to: ↑ 82 Changed 5 years ago by
Replying to jdemeyer:
Why does this require changes to
bootstrap
anyway?
I gather it's to resolve some chicken vs egg problem with autotools hosted by Sage itself.
comment:84 followup: ↓ 100 Changed 5 years ago by
That's right. As the comment in setup_bootstrap_env
explains, bootstrap needs to work both prebuild and postbuild.
comment:85 Changed 5 years ago by
 Branch changed from u/mkoeppe/__configure___prefix_sage_local to u/jdemeyer/__configure___prefix_sage_local
comment:86 Changed 5 years ago by
 Commit changed from 52826b18b1474c5f92c9c42f4cc75c6a301c1e87 to acd688dd6e42f932f0c100cf8acac28e35e3c029
Rebased to 7.6.rc0
New commits:
e55184e  Add configure option prefix=SAGE_LOCAL

004221c  Fixup configure without prefix

ba4dc01  Source sageenvconfig after calling relocateonce.py

734c2de  toplevel sage script: Remove redundant sageenvconfig error message

5bb47bb  src/bin/sageenvconfig: Prefix error message with 'Error:'

aa84b6f  make miscclean: Delete src/bin/sageenvconfig

c7a8508  Add sageenvconfig to .gitignore

75b1174  sageenv: Quoting fix

1a6bfb4  toplevel sage script: No need to source sageenvconfig

acd688d  bootstrap: Don't use sageenv, set PATH directly

comment:87 Changed 5 years ago by
Two details:
 Why remove this comment:
# Assume current directory is SAGE_ROOT
 Use 4 spaces indentation:
SAGE_LOCAL=local
comment:88 followup: ↓ 91 Changed 5 years ago by
Do you expect sageenvconfig
to be installed in $SAGE_LOCAL/bin
? Currently, that's a bit random, depending on whether or not src/bin/sageenvconfig
exists when running ./configure
.
Personally, I think that it should not be installed for an obvious reason: in order to find sageenvconfig
, you need to know SAGE_LOCAL
, for which you need to source sageenvconfig
, for which you need to find sageenvconfig
...
Given that it should not be installed, a better place might be build/bin/sageenvconfig
instead of src/bin/sageenvconfig
.
And then you might as well drop the whole complicated stuff to figure out the directory of sageenv
and just use $SAGE_ROOT/build/bin/sageenvconfig
.
If you agree, I could implement this.
comment:89 Changed 5 years ago by
 Status changed from needs_review to needs_info
comment:90 Changed 5 years ago by
 Branch changed from u/jdemeyer/__configure___prefix_sage_local to u/mkoeppe/__configure___prefix_sage_local
comment:91 in reply to: ↑ 88 ; followup: ↓ 92 Changed 5 years ago by
 Commit changed from acd688dd6e42f932f0c100cf8acac28e35e3c029 to e653bba03ed4642a7d86e9d75e550587fde5241c
Replying to jdemeyer:
Do you expect
sageenvconfig
to be installed in$SAGE_LOCAL/bin
? Currently, that's a bit random, depending on whether or notsrc/bin/sageenvconfig
exists when running./configure
.
Yes, it should be installed.
Personally, I think that it should not be installed for an obvious reason: in order to find
sageenvconfig
, you need to knowSAGE_LOCAL
, for which you need to sourcesageenvconfig
, for which you need to findsageenvconfig
...
sageenvconfig
is intended as a place for more variables defined by configure
, to be used by $SAGE_LOCAL/bin/sage. ($SAGE_LOCAL is just the first one that uses this file.) This is why it should be installed.
The idea is that $SAGE_LOCAL/bin/sage
would be able to work without having to go through the script $SAGE_ROOT/sage
(and without depending on $SAGE_ROOT
being set). As I realized in comment 68 above, it does not quite work yet  because sageenv
still tries to discover SAGE_ROOT.
A complete solution would be to add a line that sets SAGE_ROOT
to sagebinconfig
as well.
If this sounds agreeable, I could do this.
Given that it should not be installed, a better place might be
build/bin/sageenvconfig
instead ofsrc/bin/sageenvconfig
.
See #21707 for a discussion regarding a distinction between buildtime and runtime config variables; but I think SAGE_LOCAL (and SAGE_ROOT) should be defined in src/bin/sageenvconfig (and installed in SAGE_LOCAL/bin/sageenvconfig).
And then you might as well drop the whole complicated stuff to figure out the directory of
sageenv
and just use$SAGE_ROOT/build/bin/sageenvconfig
.If you agree, I could implement this.
In the end, anything that moves this ticket forward is fine with me. But note there is more work planned that depends on this  in particular #21469 (Enable VPATH builds).
New commits:
927b2c8  Restore lost comment

e653bba  Indentation fix

comment:92 in reply to: ↑ 91 ; followup: ↓ 93 Changed 5 years ago by
Replying to mkoeppe:
The idea is that
$SAGE_LOCAL/bin/sage
would be able to work without having to go through the script$SAGE_ROOT/sage
(and without depending on$SAGE_ROOT
being set).
Personally, I think it's reasonable to ask for either SAGE_ROOT
or SAGE_LOCAL
to be defined.
comment:93 in reply to: ↑ 92 ; followup: ↓ 94 Changed 5 years ago by
Replying to jdemeyer:
Replying to mkoeppe:
The idea is that
$SAGE_LOCAL/bin/sage
would be able to work without having to go through the script$SAGE_ROOT/sage
(and without depending on$SAGE_ROOT
being set).Personally, I think it's reasonable to ask for either
SAGE_ROOT
orSAGE_LOCAL
to be defined.
I disagree; a main goal of this ticket is so that one can install into a prefix hierarchy that can just be used normally (in particular without depending on some environment variables).
comment:94 in reply to: ↑ 93 Changed 5 years ago by
Replying to mkoeppe:
I disagree; a main goal of this ticket is so that one can install into a prefix hierarchy that can just be used normally (in particular without depending on some environment variables).
Let me clarify: $SAGE_LOCAL/bin/sage
should not require SAGE_ROOT
or SAGE_LOCAL
to be set because sageenv
can determine it. I'm mainly talking about the logic to determine sageenvconfig
: I think there we already know either SAGE_ROOT
or SAGE_LOCAL
so we can simplify that.
comment:95 Changed 5 years ago by
Thanks for the clarification; I agree that the installed $SAGE_LOCAL/bin/sage
would be able to determine SAGE_LOCAL
. But it would have to read $SAGE_ROOT
from $SAGE_LOCAL/bin/sageenvconfig
.
And the noninstalled $SAGE_ROOT/sage
would be able to determine SAGE_ROOT, but would have to read SAGE_LOCAL from somewhere (currently $SAGE_ROOT/src/bin/sageenvconfig
).
If you'd like to implement something like this, please go ahead.
comment:96 Changed 5 years ago by
 Branch changed from u/mkoeppe/__configure___prefix_sage_local to u/jdemeyer/__configure___prefix_sage_local
comment:97 Changed 5 years ago by
 Commit changed from e653bba03ed4642a7d86e9d75e550587fde5241c to 4554dfaae60f455f41923dad9979b1c9f7179699
 Milestone changed from sage7.6 to sage8.0
 Status changed from needs_info to needs_work
New commits:
4554dfa  Install sageenvconfig, not sageenvconfig.in

comment:98 Changed 5 years ago by
 Commit changed from 4554dfaae60f455f41923dad9979b1c9f7179699 to fe5a7ad50073d4aeeb1b33feec6e8be88dc2bfec
Branch pushed to git repo; I updated commit sha1. New commits:
fe5a7ad  Use SAGE_SCRIPTS_DIR to source sageenvconfig

comment:99 followup: ↓ 103 Changed 5 years ago by
What do you think of these changes?
I still have to look at the bootstrap script.
comment:100 in reply to: ↑ 84 Changed 5 years ago by
Replying to mkoeppe:
That's right. As the comment in
setup_bootstrap_env
explains, bootstrap needs to work both prebuild and postbuild.
For bootstrap, why not just try to source sageenv
and ignore any errors? I feel like the current solution is overengineered: in the case where you want to use autotools from $SAGE_LOCAL
, the file sageenvconfig
will exist. So the current code in bootstrap
is a complicated workaround for a nonexisting problen.
comment:101 Changed 5 years ago by
 Commit changed from fe5a7ad50073d4aeeb1b33feec6e8be88dc2bfec to cadd18e3dc5cec3408f0a968565d32b5f52c1207
Branch pushed to git repo; I updated commit sha1. New commits:
cadd18e  Source sageenv but silence errors

comment:102 Changed 5 years ago by
I agree that this is a simpler solution for the purposes of this ticket.
(setup_bootstrap_env
was unnecessarily ambitious  it set up a cleaner build environment without all the settings that sageenv
does)
comment:103 in reply to: ↑ 99 Changed 5 years ago by
Replying to jdemeyer:
What do you think of these changes?
Fine with me from looking at it (didn't check that it works).
comment:104 Changed 5 years ago by
 Commit changed from cadd18e3dc5cec3408f0a968565d32b5f52c1207 to 4fe509c906fb6c0686e0a0c024ac7276571cb79a
comment:105 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:106 followup: ↓ 108 Changed 5 years ago by
I've had a look and it looks good to me. It might be nice in the future to properly support other standard configure
options like bindir
and libdir
, but that's a separate issue.
comment:107 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:108 in reply to: ↑ 106 Changed 5 years ago by
Replying to embray:
It might be nice in the future to properly support other standard
configure
options likebindir
andlibdir
, but that's a separate issue.
I agree. Could you open a ticket for it? Note that it would require more invasive patching (or autotoolization) of many spkgs that are not autotoolsbased.
comment:109 Changed 5 years ago by
 Reviewers changed from Jeroen Demeyer, Erik Bray to Jeroen Demeyer, Erik Bray, Matthias Koeppe
Jeroen, I've built a version with your changes, and it all looks good. I've added my name to the list of reviewers.
comment:110 Changed 5 years ago by
 Status changed from positive_review to needs_work
Doesnt work if autotools aren't installed:
make j2 k start in dir /home/buildbot/slave/sage_git/build (timeout 86400 secs) watching logfiles {'symmetrica': 'logs/buildbot/symmetrica.log', 'sage': 'logs/buildbot/sage.log', 'gd': 'logs/buildbot/gd.log', 'networkx': 'logs/buildbot/networkx.log', 'libgap': 'logs/buildbot/libgap.log', 'csage': 'logs/buildbot/csage.log', 'cython': 'logs/buildbot/cython.log', 'iconv': 'logs/buildbot/iconv.log', 'iml': 'logs/buildbot/iml.log', 'flintqs': 'logs/buildbot/flintqs.log', 'sympy': 'logs/buildbot/sympy.log', 'freetype': 'logs/buildbot/freetype.log', 'mpc': 'logs/buildbot/mpc.log', 'g2red': 'logs/buildbot/genus2reduction.log', 'scipy': 'logs/buildbot/scipy.log', 'flint': 'logs/buildbot/flint.log', 'polytopes': 'logs/buildbot/polytopes_db.log', 'givaro': 'logs/buildbot/givaro.log', 'python3': 'logs/buildbot/python3.log', 'python2': 'logs/buildbot/python2.log', 'patch': 'logs/buildbot/patch.log', 'mpmath': 'logs/buildbot/mpmath.log', 'cephes': 'logs/buildbot/cephes.log', 'pynac': 'logs/buildbot/pynac.log', 'pkgconf': 'logs/buildbot/pkgconf.log', 'sagenb': 'logs/buildbot/sagenb.log', 'palp': 'logs/buildbot/palp.log', 'seadata': 'logs/buildbot/pari_seadata_small.log', 'sympow': 'logs/buildbot/sympow.log', 'docutils': 'logs/buildbot/docutils.log', 'ell_curves': 'logs/buildbot/elliptic_curves.log', 'eclib': 'logs/buildbot/eclib.log', 'libpng': 'logs/buildbot/libpng.log', 'boost': 'logs/buildbot/boost_cropped.log', 'cliquer': 'logs/buildbot/cliquer.log', 'ecm': 'logs/buildbot/ecm.log', 'ecl': 'logs/buildbot/ecl.log', 'cvxopt': 'logs/buildbot/cvxopt.log', 'matplotlib': 'logs/buildbot/matplotlib.log', 'galdata': 'logs/buildbot/pari_galdata.log', 'ntl': 'logs/buildbot/ntl.log', 'lcalc': 'logs/buildbot/lcalc.log', 'ipython': 'logs/buildbot/ipython.log', 'sagetex': 'logs/buildbot/sagetex.log', 'libfplll': 'logs/buildbot/libfplll.log', 'pillow': 'logs/buildbot/pillow.log', 'conway': 'logs/buildbot/conway_polynomials.log', 'polybori': 'logs/buildbot/polybori.log', 'tachyon': 'logs/buildbot/tachyon.log', 'ppl': 'logs/buildbot/ppl.log', 'linbox': 'logs/buildbot/linbox.log', 'pygments': 'logs/buildbot/pygments.log', 'm4rie': 'logs/buildbot/m4rie.log', 'git': 'logs/buildbot/git.log', 'pexpect': 'logs/buildbot/pexpect.log', 'rproject': 'logs/buildbot/r.log', 'm4ri': 'logs/buildbot/m4ri.log', 'pkgconfig': 'logs/buildbot/pkgconfig.log', 'cddlib': 'logs/buildbot/cddlib.log', 'zeromq': 'logs/buildbot/zeromq.log', 'numpy': 'logs/buildbot/numpy.log', 'pyzmq': 'logs/buildbot/pyzmq.log', 'sqlalchemy': 'logs/buildbot/sqlalchemy.log', 'ncurses': 'logs/buildbot/ncurses.log', 'gdmodule': 'logs/buildbot/gdmodule.log', 'pari': 'logs/buildbot/pari.log', 'gap': 'logs/buildbot/gap.log', 'mpfi': 'logs/buildbot/mpfi.log', 'gf2x': 'logs/buildbot/gf2x.log', 'zlib': 'logs/buildbot/zlib.log', 'setuptools': 'logs/buildbot/setuptools.log', 'sqlite': 'logs/buildbot/sqlite.log', 'mpir': 'logs/buildbot/mpir.log', 'bzip2': 'logs/buildbot/bzip2.log', 'six': 'logs/buildbot/six.log', 'rubiks': 'logs/buildbot/rubiks.log', 'scons': 'logs/buildbot/scons.log', 'openblas': 'logs/buildbot/openblas.log', 'zn_poly': 'logs/buildbot/zn_poly.log', 'singular': 'logs/buildbot/singular.log', 'tornado': 'logs/buildbot/tornado.log', 'boehm_gc': 'logs/buildbot/boehm_gc.log', 'gfan': 'logs/buildbot/gfan.log', 'gcc': 'logs/buildbot/gcc.log', 'readline': 'logs/buildbot/readline.log', 'glpk': 'logs/buildbot/glpk.log', 'ratpoints': 'logs/buildbot/ratpoints.log', 'sphinx': 'logs/buildbot/sphinx.log', 'pycrypto': 'logs/buildbot/pycrypto.log', 'jmol': 'logs/buildbot/jmol.log', 'graphs': 'logs/buildbot/graphs.log', 'jinja2': 'logs/buildbot/jinja2.log', 'mpfr': 'logs/buildbot/mpfr.log', 'gsl': 'logs/buildbot/gsl.log', 'maxima': 'logs/buildbot/maxima.log'} argv: ['make', 'j2', 'k', 'start'] environment: DOT_SAGE=/home/buildbot/slave/sage_git/dot_sage HOME=/var/lib/buildbot LANG=en_US.UTF8 LOGNAME=buildbot MAIL=/var/mail/buildbot MAKE=make j2 PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games PWD=/home/buildbot/slave/sage_git/build SAGE_CRASH_LOGS=/home/buildbot/slave/sage_git/crashlogs SAGE_PARALLEL_SPKG_BUILD=yes SAGE_SERVER=http://sagepad.org/ SHELL=/bin/sh USER=buildbot using PTY: False make j2 build/make/Makefile make[1]: Entering directory `/home/buildbot/slave/sage_git/build' make[1]: warning: jN forced in submake: disabling jobserver mode. ./bootstrap d make[2]: Entering directory `/home/buildbot/slave/sage_git/build' rm rf config configure build/make/Makefileauto.in make[2]: Leaving directory `/home/buildbot/slave/sage_git/build' ./bootstrap: line 30: aclocal: command not found Bootstrap failed, downloading required files instead. ./bootstrap: line 59: sagedownloadfile: command not found Error: downloading configure212.tar.gz failed make[1]: *** [configure] Error 1 make[1]: Target `build/make/Makefile' not remade because of errors. make[1]: Leaving directory `/home/buildbot/slave/sage_git/build' make: *** [start] Error 2 program finished with exit code 2
comment:111 followup: ↓ 114 Changed 5 years ago by
Jeroen, since you did not like my solution to this problem in https://git.sagemath.org/sage.git/commit?id=acd688dd6e42f932f0c100cf8acac28e35e3c029, would you like to fix it?
comment:112 Changed 5 years ago by
 Commit changed from 4fe509c906fb6c0686e0a0c024ac7276571cb79a to 3b1ee6863a5e2e3b0f7604c9e9a1f000685e02c1
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
221bd47  sageenv: Quoting fix

ec0a435  toplevel sage script: No need to source sageenvconfig

7a43067  bootstrap: Don't use sageenv, set PATH directly

b4fae11  Restore lost comment

4ec741c  Indentation fix

01a5706  Install sageenvconfig, not sageenvconfig.in

1effacd  Use SAGE_SCRIPTS_DIR to source sageenvconfig

13205ae  Source sageenv but silence errors

d9f83d2  Let "make install" build Sage

3b1ee68  Revert unneeded changes

comment:113 Changed 5 years ago by
Rebased to 7.6, nothing changed yet.
comment:114 in reply to: ↑ 111 Changed 5 years ago by
Replying to mkoeppe:
Jeroen, since you did not like my solution to this problem in https://git.sagemath.org/sage.git/commit?id=acd688dd6e42f932f0c100cf8acac28e35e3c029, would you like to fix it?
The problem is that this comment is wrong:
# Try to get autotools from our own package into PATH (Trac #21214) # This will fail for initial bootstrap, when sageenvconfig has # not been created yet.
It's not just about autotools, it's also about getting sagedownloadfile
in the $PATH
.
comment:115 Changed 5 years ago by
 Commit changed from 3b1ee6863a5e2e3b0f7604c9e9a1f000685e02c1 to 890d4178cfd9a7c6184cade473668a9e5bf74be1
Branch pushed to git repo; I updated commit sha1. New commits:
890d417  Hardcode path to sagedownloadfile

comment:116 Changed 5 years ago by
 Status changed from needs_work to needs_review
This should work...
comment:117 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:118 Changed 5 years ago by
 Status changed from positive_review to needs_work
config.status: creating directory /Users/buildslavesage/slave/sage_git/build/local/share config.status: creating directory /Users/buildslavesage/slave/sage_git/build/local/var/lib/sage/installed build/bin/sagelogger \ "cd build/make && ./install 'start'" logs/install.log ./install: line 12: /Users/buildslavesage/slave/sage_git/build/src/bin/sageenvconfig: No such file or directory Error: Failed to read sageenvconfig. Did you run configure?
comment:119 followup: ↓ 120 Changed 5 years ago by
 Status changed from needs_work to positive_review
No, this error disappears if you run autoreconf
 and regenerate the configure package for the release using your scripts.
comment:120 in reply to: ↑ 119 Changed 5 years ago by
 Status changed from positive_review to needs_work
Replying to mkoeppe:
regenerate the configure package for the release using your scripts.
If only the release manager would do this when testing a ticket...
comment:121 Changed 5 years ago by
 Commit changed from 890d4178cfd9a7c6184cade473668a9e5bf74be1 to 2a069b194037f7dd406055f0072a93c525f768ea
Branch pushed to git repo; I updated commit sha1. New commits:
2a069b1  Update configure tarball

comment:122 Changed 5 years ago by
 Description modified (diff)
 Status changed from needs_work to positive_review
comment:124 Changed 5 years ago by
... with the configure tarball version
comment:125 Changed 5 years ago by
 Commit changed from 2a069b194037f7dd406055f0072a93c525f768ea to e5926b1513e224e392cc10bd82c7ec7244a16c44
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
f624c95  bootstrap: Don't use sageenv, set PATH directly

2dccfb0  Restore lost comment

2aafa6f  Indentation fix

4eeb130  Install sageenvconfig, not sageenvconfig.in

8965a08  Use SAGE_SCRIPTS_DIR to source sageenvconfig

7b4dfbf  Source sageenv but silence errors

44110ce  Let "make install" build Sage

7d2595c  Revert unneeded changes

1e98d9c  Hardcode path to sagedownloadfile

e5926b1  Update configure tarball

comment:126 Changed 5 years ago by
 Description modified (diff)
 Status changed from needs_work to positive_review
Hopefully this is the final thing needed for this ticket...
comment:127 Changed 5 years ago by
 Branch changed from u/jdemeyer/__configure___prefix_sage_local to e5926b1513e224e392cc10bd82c7ec7244a16c44
 Resolution set to fixed
 Status changed from positive_review to closed
comment:128 Changed 5 years ago by
 Commit e5926b1513e224e392cc10bd82c7ec7244a16c44 deleted
As a consequence of this ticket, with a fresh tarball or after running make distclean
, ./sage i PKG
fails, as does ./sage sh
. The error message says
Error: You must set either the SAGE_LOCAL or SAGE_SCRIPTS_DIR environment variable to run this Error setting environment variables by sourcing '/Users/palmieri/Desktop/TEMP/sage8.0.beta1/src/bin/sageenv'; possibly contact sagedevel (see http://groups.google.com/group/sagedevel).
Should these commands work out of the box? Running ./configure
fixes this, I think, so should the error message be changed, at least?
comment:129 Changed 5 years ago by
I think its reasonable to ask that one runs at least configure once before trying to launch any scripts in the source tarball. So +1 for changing the error message...
New commits:
Use AC_PREFIX_DEFAULT to default prefix to SAGE_ROOT/local