Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move configuration part of build/make/install to configure #19043

Closed
jdemeyer opened this issue Aug 16, 2015 · 41 comments
Closed

Move configuration part of build/make/install to configure #19043

jdemeyer opened this issue Aug 16, 2015 · 41 comments

Comments

@jdemeyer
Copy link

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:

  1. Running make is a lot simpler and hence faster.
  2. 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.

Depends on #18885
Depends on #19187
Depends on #19266

Component: build

Author: Jeroen Demeyer

Branch/Commit: 4178b80

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/19043

@jdemeyer jdemeyer added this to the sage-6.9 milestone Aug 16, 2015
@jdemeyer
Copy link
Author

Branch: u/jdemeyer/move_install_to_configure

@jdemeyer
Copy link
Author

Commit: dcf0098

@jdemeyer
Copy link
Author

New commits:

dcf0098Move configuration part of build/make/install to configure

@jdemeyer

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Sep 21, 2015

comment:4

I would be willing to review this.

@jdemeyer
Copy link
Author

Dependencies: #18885

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

f708487Fix for Debian/Ubuntu GCC version numbers
462b605Merge commit 'f7084871d4054445de04338b6b5d5d50fd67a5a6' into HEAD
a79a646Temporarily roll back changes between 6.9.beta2 and 6.9.beta7
d42c44dMerge commit 'a79a646f74e09cbdb97976ad3545b9f02318e743' into t/19043/move_install_to_configure
f3b6d2bRe-apply changes between 6.9.beta2 and 6.9.beta7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2015

Changed commit from dcf0098 to f3b6d2b

@jdemeyer
Copy link
Author

Changed dependencies from #18885 to #18885, #19187

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2015

Changed commit from f3b6d2b to 4178b80

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

52a3cf0Add rules for installing packages with pip
919da67Merge #19187 and #18885 on top of 6.9.beta7
dea2513Temporary roll back changes between 6.9.beta2 and 6.9.beta8
0c60518Merge 6.9.beta8 except build/make/install into t/19043/move_install_to_configure
4178b80Move changes of build/make/install to configure.ac

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

comment:9

Rebased. I know the git history looks funny, but you really should just review the result, not the individual commits.

@tscrim
Copy link
Collaborator

tscrim commented Sep 21, 2015

comment:10

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.

@jdemeyer
Copy link
Author

comment:11

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?

@tscrim
Copy link
Collaborator

tscrim commented Sep 21, 2015

comment:12

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.

@tscrim
Copy link
Collaborator

tscrim commented Sep 21, 2015

Reviewer: Travis Scrimshaw

@jdemeyer
Copy link
Author

comment:13

Replying to @tscrim:

Fair enough, but let's make that spkg dev guide update ticket a blocker.

I created #19267. I don't believe that pure documentation tickets should be blockers. But given that we have not yet reached the "rc" phase, I think there is still time.

@vbraun
Copy link
Member

vbraun commented Sep 22, 2015

Changed branch from u/jdemeyer/move_install_to_configure to 4178b80

@vbraun
Copy link
Member

vbraun commented Sep 22, 2015

Changed commit from 4178b80 to none

@vbraun
Copy link
Member

vbraun commented Sep 22, 2015

comment:15

OSX doesn't build gcc any more with this ticket, but just fails with configure: error: Exiting, since a Fortran compiler was not found.

http://build.sagedev.org/release/builders/%20%20fast%20Volker%20MiniMac%20%28OSX%2010.10%20x86_64%29%20full/builds/95/steps/compile_1/logs/stdio

@jdemeyer
Copy link
Author

comment:18

I don't understand why ./bootstrap -d wasn't run though in your logs. Is there anything funny in your build system affecting configure?

@jdemeyer
Copy link
Author

comment:19

Or anything in your buildbot affecting timestamps as a whole? What happens if you replace distclean by maintainer-clean?

@vbraun
Copy link
Member

vbraun commented Sep 22, 2015

comment:20

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.

@jdemeyer
Copy link
Author

comment:22

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?

@jdemeyer
Copy link
Author

comment:23

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.

@jdemeyer
Copy link
Author

Changed branch from 4178b80 to u/jdemeyer/4178b80c3ff5ce8e1753bffe48f32dc5bcccb728

@jdemeyer
Copy link
Author

Commit: 4eafd9b

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

comment:25

Is this what you need?


New commits:

dcf0098Move configuration part of build/make/install to configure
f708487Fix for Debian/Ubuntu GCC version numbers
52a3cf0Add rules for installing packages with pip
919da67Merge #19187 and #18885 on top of 6.9.beta7
dea2513Temporary roll back changes between 6.9.beta2 and 6.9.beta8
0c60518Merge 6.9.beta8 except build/make/install into t/19043/move_install_to_configure
4178b80Move changes of build/make/install to configure.ac
4eafd9bBump configure version

@vbraun
Copy link
Member

vbraun commented Sep 22, 2015

comment:26

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.

@vbraun
Copy link
Member

vbraun commented Sep 22, 2015

comment:27

Thats not the file:

$ md5 configure-114.tar.gz 
MD5 (configure-114.tar.gz) = 0413a0763d9b33111d11f03f720429a2

@jdemeyer
Copy link
Author

comment:28

I will deal with it on #19266.


New commits:

dcf0098Move configuration part of build/make/install to configure
f708487Fix for Debian/Ubuntu GCC version numbers
52a3cf0Add rules for installing packages with pip
919da67Merge #19187 and #18885 on top of 6.9.beta7
dea2513Temporary roll back changes between 6.9.beta2 and 6.9.beta8
0c60518Merge 6.9.beta8 except build/make/install into t/19043/move_install_to_configure
4178b80Move changes of build/make/install to configure.ac

@jdemeyer
Copy link
Author

Changed commit from 4eafd9b to 4178b80

@jdemeyer
Copy link
Author

@jdemeyer

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Sep 23, 2015

comment:30

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).

@vbraun
Copy link
Member

vbraun commented Sep 23, 2015

Changed dependencies from #18885, #19187 to #18885, #19187, #19266

@jdemeyer
Copy link
Author

comment:32

Volker, can you please try to merge this in the next beta? Otherwise there will be a conflict with the configure version.

@vbraun
Copy link
Member

vbraun commented Sep 23, 2015

Changed branch from u/jdemeyer/move_install_to_configure to 4178b80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants