Opened 5 years ago
Closed 5 years ago
#19043 closed enhancement (fixed)
Move configuration part of build/make/install to configure
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.9 |
Component: | build | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 4178b80 (Commits) | Commit: | 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728 |
Dependencies: | #18885, #19187, #19266 | Stopgaps: |
Description (last modified by )
Most of what build/make/install
does really belongs in configure
, in particular creating build/make/Makefile
. This ticket moves that part to configure
and adjusts the top-level build system to make this work.
There are two advantages:
- Running
make
is a lot simpler and hence faster. - It allows using
./configure
to configure the list of packages (note that this ticket does not implement that, it just makes it possible to implement that in future tickets)
There is an important change for developers: any change to the list or versions of packages will require a run of ./configure
to take effect. Note that ./configure
will be re-run automatically if ./configure
itself changes, which happens with every Sage version. So there is no need to manually rerun ./configure
if you're just switching between versions.
Change History (33)
comment:1 Changed 5 years ago by
- Branch set to u/jdemeyer/move_install_to_configure
comment:2 Changed 5 years ago by
- Commit set to dcf009850cfb0f52a7e7e48bd005bec147eefb1a
- Status changed from new to needs_review
comment:3 Changed 5 years ago by
- Description modified (diff)
comment:4 Changed 5 years ago by
I would be willing to review this.
comment:5 Changed 5 years ago by
- Dependencies set to #18885
comment:6 Changed 5 years ago by
- Commit changed from dcf009850cfb0f52a7e7e48bd005bec147eefb1a to f3b6d2baae1420d1ecf5ea35aa06d237ca9725d8
Branch pushed to git repo; I updated commit sha1. New commits:
f708487 | Fix for Debian/Ubuntu GCC version numbers
|
462b605 | Merge commit 'f7084871d4054445de04338b6b5d5d50fd67a5a6' into HEAD
|
a79a646 | Temporarily roll back changes between 6.9.beta2 and 6.9.beta7
|
d42c44d | Merge commit 'a79a646f74e09cbdb97976ad3545b9f02318e743' into t/19043/move_install_to_configure
|
f3b6d2b | Re-apply changes between 6.9.beta2 and 6.9.beta7
|
comment:7 Changed 5 years ago by
- Dependencies changed from #18885 to #18885, #19187
comment:8 Changed 5 years ago by
- Commit changed from f3b6d2baae1420d1ecf5ea35aa06d237ca9725d8 to 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
52a3cf0 | Add rules for installing packages with pip
|
919da67 | Merge #19187 and #18885 on top of 6.9.beta7
|
dea2513 | Temporary roll back changes between 6.9.beta2 and 6.9.beta8
|
0c60518 | Merge 6.9.beta8 except build/make/install into t/19043/move_install_to_configure
|
4178b80 | Move changes of build/make/install to configure.ac
|
comment:9 Changed 5 years ago by
- Description modified (diff)
Rebased. I know the git history looks funny, but you really should just review the result, not the individual commits.
comment:10 follow-up: ↓ 11 Changed 5 years ago by
I don't care about having a "perfect" history (and normally review the final diff from develop
).
Overall it looks good from my testing. However, I'm not quite sure about this
There is an important change for developers: any change to the list or versions of packages will require a run of
./configure
to take effect.
To make sure I understand correctly, does this mean that anytime we want to do a version bump of packages (or add one), we have to manually run $SAGE_ROOT/configure
? If so, then I think we should put something about this in the spkg development guide. Also, is configure
updated on beta version changes or just on stable releases?
Also is there a reason why you moved the "if root" test lower in the file? I would think we'd want such build failures to happen as early on as possible.
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 5 years ago by
Replying to tscrim:
To make sure I understand correctly, does this mean that anytime we want to do a version bump of packages (or add one), we have to manually run
$SAGE_ROOT/configure
?
Yes.
If so, then I think we should put something about this in the spkg development guide.
OK, I see your point. I'd rather make that a separate ticket, there might be other changes to the spkg development guide I want to make.
Also, is
configure
updated on beta version changes or just on stable releases?
All releases, beta and stable. So in practice, you only really need to run ./configure
manually if you yourself are working on a package (as author or reviewer). If the package upgrade happens as part of a Sage version bump, no special action is needed.
Also is there a reason why you moved the "if root" test lower in the file?
There is no real particular reason, it felt more logical to put it where it currently is.
I would think we'd want such build failures to happen as early on as possible.
Why?
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 5 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Replying to jdemeyer:
Replying to tscrim:
To make sure I understand correctly, does this mean that anytime we want to do a version bump of packages (or add one), we have to manually run
$SAGE_ROOT/configure
?Yes.
If so, then I think we should put something about this in the spkg development guide.
OK, I see your point. I'd rather make that a separate ticket, there might be other changes to the spkg development guide I want to make.
Fair enough, but let's make that spkg dev guide update ticket a blocker.
Also, is
configure
updated on beta version changes or just on stable releases?All releases, beta and stable. So in practice, you only really need to run
./configure
manually if you yourself are working on a package (as author or reviewer). If the package upgrade happens as part of a Sage version bump, no special action is needed.
Thanks for the explanation.
Also is there a reason why you moved the "if root" test lower in the file?
There is no real particular reason, it felt more logical to put it where it currently is.
I would think we'd want such build failures to happen as early on as possible.
Why?
I feel that some of those configuration steps could take some time, just to have it fail for an unrelated reason. However I don't really care about this; it was more out of curiosity.
comment:13 in reply to: ↑ 12 Changed 5 years ago by
comment:14 Changed 5 years ago by
- Branch changed from u/jdemeyer/move_install_to_configure to 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728
- Resolution set to fixed
- Status changed from positive_review to closed
comment:15 Changed 5 years ago by
- Commit 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728 deleted
- Resolution fixed deleted
- Status changed from closed to new
OSX doesn't build gcc any more with this ticket, but just fails with configure: error: Exiting, since a Fortran compiler was not found.
comment:16 Changed 5 years ago by
- Status changed from new to needs_review
Most likely, you didn't apply this branch properly: it needs an updated upstream/configure-NNN.tar.gz
if the system you test it on doesn't have autotools.
comment:17 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:18 Changed 5 years ago by
I don't understand why ./bootstrap -d
wasn't run though in your logs. Is there anything funny in your build system affecting configure
?
comment:19 Changed 5 years ago by
Or anything in your buildbot affecting timestamps as a whole? What happens if you replace distclean
by maintainer-clean
?
comment:20 Changed 5 years ago by
Its the same machine that you also have an account for. It compiles fine without this ticket.
If this ticket needs an updated confball then it should be committed, too. The change in confball needs to be in some commit, otherwise the buildbot can't test it.
comment:21 Changed 5 years ago by
- Status changed from positive_review to needs_work
comment:22 Changed 5 years ago by
If this ticket needs an updated confball then it should be committed, too.
It's the first time I hear this. I thought that was the job of the release manager when creating the sdist?
comment:23 Changed 5 years ago by
I need more information about the buildbot setup, otherwise I don't know what to do. From your logs, it looks like ./bootstrap
isn't even run, so just updating the confball won't help in that case.
comment:24 Changed 5 years ago by
- Branch changed from 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728 to u/jdemeyer/4178b80c3ff5ce8e1753bffe48f32dc5bcccb728
comment:25 Changed 5 years ago by
- Commit set to 4eafd9b7b0ba7d0a2956bf4e71987e4572b22aa0
- Description modified (diff)
- Status changed from needs_work to positive_review
Is this what you need?
New commits:
dcf0098 | Move configuration part of build/make/install to configure
|
f708487 | Fix for Debian/Ubuntu GCC version numbers
|
52a3cf0 | Add rules for installing packages with pip
|
919da67 | Merge #19187 and #18885 on top of 6.9.beta7
|
dea2513 | Temporary roll back changes between 6.9.beta2 and 6.9.beta8
|
0c60518 | Merge 6.9.beta8 except build/make/install into t/19043/move_install_to_configure
|
4178b80 | Move changes of build/make/install to configure.ac
|
4eafd9b | Bump configure version
|
comment:26 Changed 5 years ago by
The confball is updated during release but ideally we can test a ticket before the release...
The buildbot just pulls the source and runs make -j8 -k start
on the OSX machine.
comment:27 Changed 5 years ago by
- Status changed from positive_review to needs_work
Thats not the file:
$ md5 configure-114.tar.gz MD5 (configure-114.tar.gz) = 0413a0763d9b33111d11f03f720429a2
comment:28 Changed 5 years ago by
- Branch changed from u/jdemeyer/4178b80c3ff5ce8e1753bffe48f32dc5bcccb728 to u/jdemeyer/move_install_to_configure
- Commit changed from 4eafd9b7b0ba7d0a2956bf4e71987e4572b22aa0 to 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728
I will deal with it on #19266.
New commits:
dcf0098 | Move configuration part of build/make/install to configure
|
f708487 | Fix for Debian/Ubuntu GCC version numbers
|
52a3cf0 | Add rules for installing packages with pip
|
919da67 | Merge #19187 and #18885 on top of 6.9.beta7
|
dea2513 | Temporary roll back changes between 6.9.beta2 and 6.9.beta8
|
0c60518 | Merge 6.9.beta8 except build/make/install into t/19043/move_install_to_configure
|
4178b80 | Move changes of build/make/install to configure.ac
|
comment:29 Changed 5 years ago by
- Description modified (diff)
comment:30 Changed 5 years ago by
- Status changed from needs_work to positive_review
Since the confball will be handled on #19266, we're back to positive review (I tested it with the one Jeroen provided after running a checksum fix).
comment:31 Changed 5 years ago by
- Dependencies changed from #18885, #19187 to #18885, #19187, #19266
comment:32 Changed 5 years ago by
Volker, can you please try to merge this in the next beta? Otherwise there will be a conflict with the configure
version.
comment:33 Changed 5 years ago by
- Branch changed from u/jdemeyer/move_install_to_configure to 4178b80c3ff5ce8e1753bffe48f32dc5bcccb728
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Move configuration part of build/make/install to configure