#21479 closed enhancement (fixed)
./configure --prefix=SAGE_LOCAL
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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/configure-214.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 one-package-at-a-time, and after --disable-$package
is in place.
with --disable-all
, 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 no-op.
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 sage-7.4 to sage-7.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 no-op, 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 remote-tracking 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 follow-ups: ↓ 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?
- sage-location.
comment:16 follow-up: ↓ 19 Changed 6 years ago by
I don't like the duplication in build/make/install
and src/bin/sage-env
.
Here is a suggestion: add a new file, say src/bin/sage-config
(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/sage-env
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?
- sage-location.
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/sage-config
(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 follow-up: ↓ 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 follow-up: ↓ 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 ; follow-up: ↓ 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 sage-env but rather /bin/bash. This is because sage-env is sourced (.) rather than executed.
Note this is existing code in sage-env
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/Bash-Variables.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 sage-env but rather /bin/bash. This is because sage-env is sourced (.) rather than executed.
Don't tell me but tell the person reading sage-env
(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/Makefile-auto config.status: creating src/Makefile config.status: creating src/bin/sage-env-config config.status: executing depfiles commands config.status: executing mkdirs commands config.status: creating directory /usr/local/src/sage-config/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 sage-env-config
needs to be ignored by .gitignore
and deleted when running make distclean
.
comment:46 follow-up: ↓ 57 Changed 6 years ago by
This line in sage
needs quoting:
. $SAGE_ROOT/src/bin/sage-env-config
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/sage-env-config 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 freshly-unpacked binary tarball then run the installer # Note: relocate-once.py deletes itself upon successful completion if [ -x "$SAGE_ROOT/relocate-once.py" ]; then "$SAGE_ROOT/relocate-once.py" fi
comment:49 follow-up: ↓ 56 Changed 6 years ago by
Can you explain under which circumstances it is needed to read sage-env-config
from sage-env
? It seems to me that sage-env-config
would already be read, either by the top-level 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 sage-env-config after calling relocate-once.py
|
5fcda83 | top-level sage script: Remove redundant sage-env-config 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 | sage-env: Quoting fix
|
comment:56 in reply to: ↑ 49 ; follow-ups: ↓ 59 ↓ 68 Changed 6 years ago by
Replying to jdemeyer:
Can you explain under which circumstances it is needed to read
sage-env-config
fromsage-env
? It seems to me thatsage-env-config
would already be read, either by the top-levelsage
script or bybuild/make/install
.
$SAGE_LOCAL/bin/sage
becomes directly executable; one does not have to go through that top-level 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 ; follow-up: ↓ 63 Changed 6 years ago by
Replying to mkoeppe:
$SAGE_LOCAL/bin/sage
becomes directly executable; one does not have to go through that top-level script.
Then let me turn the question around: why do you need to change $SAGE_ROOT/sage
?
comment:60 follow-up: ↓ 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/sage-env-config
?
Or maybe we should just source both sage-env-config
and sage-env
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 ; follow-up: ↓ 67 Changed 6 years ago by
Replying to jdemeyer:
Why not use
$SAGE_ROOT/src/bin/sage-env-config
?
Things installed in SAGE_LOCAL definitely should not depend on the source directory.
Or maybe we should just source both
sage-env-config
andsage-env
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 ; follow-up: ↓ 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 top-level 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/sage-env-config
so that it works when nothing has been installed so far.
comment:64 in reply to: ↑ 63 ; follow-up: ↓ 66 Changed 6 years ago by
Replying to mkoeppe:
$SAGE_ROOT/sage
sources$SAGE_ROOT/src/bin/sage-env-config
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 sage-env-config after calling relocate-once.py
|
fc66139 | top-level sage script: Remove redundant sage-env-config error message
|
fe19d3f | src/bin/sage-env-config: Prefix error message with 'Error:'
|
269ad80 | make misc-clean: Delete src/bin/sage-env-config
|
727123a | Add sage-env-config to .gitignore
|
be4c390 | sage-env: Quoting fix
|
a3467bf | top-level sage script: No need to source sage-env-config
|
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/sage-env-config
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
sage-env-config
andsage-env
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 sage-env: build/bin/sage-spkg
, build/make/deps
, src/bin/sage-update-src
. 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 top-level script.
I realized that this is not entirely true. sage-env
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 sage-env-config.
But that could go on a follow-up 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 sage-7.5 to sage-7.6
comment:73 follow-up: ↓ 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/Makefile-auto.in 25make[2]: Leaving directory `/home/buildbot/slave/sage_git/build' 26src/bin/sage-env: line 206: /home/buildbot/slave/sage_git/build/src/bin/sage-env-config: No such file or directory 27sage-env: Error: /home/buildbot/slave/sage_git/build/src/bin/sage-env-config could not be read. 28./bootstrap: line 31: aclocal: command not found 29Bootstrap failed, downloading required files instead. 30./bootstrap: line 61: sage-download-file: command not found 31Error: downloading configure-200.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 sage-env-config after calling relocate-once.py
|
c1c36ff | top-level sage script: Remove redundant sage-env-config error message
|
ae6263b | src/bin/sage-env-config: Prefix error message with 'Error:'
|
27189be | make misc-clean: Delete src/bin/sage-env-config
|
eb922b8 | Add sage-env-config to .gitignore
|
4cdaa43 | sage-env: Quoting fix
|
71f88cc | top-level sage script: No need to source sage-env-config
|
52826b1 | bootstrap: Don't use sage-env, 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 place--would 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 follow-up: ↓ 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 follow-up: ↓ 100 Changed 5 years ago by
That's right. As the comment in setup_bootstrap_env
explains, bootstrap needs to work both pre-build and post-build.
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 sage-env-config after calling relocate-once.py
|
734c2de | top-level sage script: Remove redundant sage-env-config error message
|
5bb47bb | src/bin/sage-env-config: Prefix error message with 'Error:'
|
aa84b6f | make misc-clean: Delete src/bin/sage-env-config
|
c7a8508 | Add sage-env-config to .gitignore
|
75b1174 | sage-env: Quoting fix
|
1a6bfb4 | top-level sage script: No need to source sage-env-config
|
acd688d | bootstrap: Don't use sage-env, 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 follow-up: ↓ 91 Changed 5 years ago by
Do you expect sage-env-config
to be installed in $SAGE_LOCAL/bin
? Currently, that's a bit random, depending on whether or not src/bin/sage-env-config
exists when running ./configure
.
Personally, I think that it should not be installed for an obvious reason: in order to find sage-env-config
, you need to know SAGE_LOCAL
, for which you need to source sage-env-config
, for which you need to find sage-env-config
...
Given that it should not be installed, a better place might be build/bin/sage-env-config
instead of src/bin/sage-env-config
.
And then you might as well drop the whole complicated stuff to figure out the directory of sage-env
and just use $SAGE_ROOT/build/bin/sage-env-config
.
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 ; follow-up: ↓ 92 Changed 5 years ago by
- Commit changed from acd688dd6e42f932f0c100cf8acac28e35e3c029 to e653bba03ed4642a7d86e9d75e550587fde5241c
Replying to jdemeyer:
Do you expect
sage-env-config
to be installed in$SAGE_LOCAL/bin
? Currently, that's a bit random, depending on whether or notsrc/bin/sage-env-config
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
sage-env-config
, you need to knowSAGE_LOCAL
, for which you need to sourcesage-env-config
, for which you need to findsage-env-config
...
sage-env-config
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 sage-env
still tries to discover SAGE_ROOT.
A complete solution would be to add a line that sets SAGE_ROOT
to sage-bin-config
as well.
If this sounds agreeable, I could do this.
Given that it should not be installed, a better place might be
build/bin/sage-env-config
instead ofsrc/bin/sage-env-config
.
See #21707 for a discussion regarding a distinction between build-time and run-time config variables; but I think SAGE_LOCAL (and SAGE_ROOT) should be defined in src/bin/sage-env-config (and installed in SAGE_LOCAL/bin/sage-env-config).
And then you might as well drop the whole complicated stuff to figure out the directory of
sage-env
and just use$SAGE_ROOT/build/bin/sage-env-config
.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 ; follow-up: ↓ 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 ; follow-up: ↓ 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 sage-env
can determine it. I'm mainly talking about the logic to determine sage-env-config
: 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/sage-env-config
.
And the non-installed $SAGE_ROOT/sage
would be able to determine SAGE_ROOT, but would have to read SAGE_LOCAL from somewhere (currently $SAGE_ROOT/src/bin/sage-env-config
).
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 sage-7.6 to sage-8.0
- Status changed from needs_info to needs_work
New commits:
4554dfa | Install sage-env-config, not sage-env-config.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 sage-env-config
|
comment:99 follow-up: ↓ 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 pre-build and post-build.
For bootstrap, why not just try to source sage-env
and ignore any errors? I feel like the current solution is over-engineered: in the case where you want to use autotools from $SAGE_LOCAL
, the file sage-env-config
will exist. So the current code in bootstrap
is a complicated work-around for a non-existing 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 sage-env 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 sage-env
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 follow-up: ↓ 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 like--bindir
and--libdir
, 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 autotools-based.
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', 'r-project': '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.UTF-8 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/Makefile-auto.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: sage-download-file: command not found Error: downloading configure-212.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 follow-up: ↓ 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 | sage-env: Quoting fix
|
ec0a435 | top-level sage script: No need to source sage-env-config
|
7a43067 | bootstrap: Don't use sage-env, set PATH directly
|
b4fae11 | Restore lost comment
|
4ec741c | Indentation fix
|
01a5706 | Install sage-env-config, not sage-env-config.in
|
1effacd | Use SAGE_SCRIPTS_DIR to source sage-env-config
|
13205ae | Source sage-env 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 sage-env-config has # not been created yet.
It's not just about autotools, it's also about getting sage-download-file
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 sage-download-file
|
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/buildslave-sage/slave/sage_git/build/local/share config.status: creating directory /Users/buildslave-sage/slave/sage_git/build/local/var/lib/sage/installed build/bin/sage-logger \ "cd build/make && ./install 'start'" logs/install.log ./install: line 12: /Users/buildslave-sage/slave/sage_git/build/src/bin/sage-env-config: No such file or directory Error: Failed to read sage-env-config. Did you run configure?
comment:119 follow-up: ↓ 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 sage-env, set PATH directly
|
2dccfb0 | Restore lost comment
|
2aafa6f | Indentation fix
|
4eeb130 | Install sage-env-config, not sage-env-config.in
|
8965a08 | Use SAGE_SCRIPTS_DIR to source sage-env-config
|
7b4dfbf | Source sage-env but silence errors
|
44110ce | Let "make install" build Sage
|
7d2595c | Revert unneeded changes
|
1e98d9c | Hardcode path to sage-download-file
|
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/sage-8.0.beta1/src/bin/sage-env'; possibly contact sage-devel (see http://groups.google.com/group/sage-devel).
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