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

Reimplement sage -i SPKG for optional/experimental packages as configure --enable-SPKG && make build #29113

Closed
mkoeppe opened this issue Jan 30, 2020 · 17 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 30, 2020

#28095 adds configure --enable-SPKG.

As a follow-up, using this new interface we reimplement sage -i for optional/experimental packages, falling back to direct invocation of make for all other packages.

CC: @dimpase @embray @jhpalmieri

Component: build

Author: Matthias Koeppe

Branch/Commit: 3417f43

Reviewer: John Palmieri

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

@mkoeppe mkoeppe added this to the sage-9.1 milestone Jan 30, 2020
@mkoeppe

This comment has been minimized.

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:2

Ah, I misread this at first--I thought you were proposing to replace sage -i as the way to install optional packages.

If I understand correctly though, you are just proposing a change to how it works internally. If so I'm +1.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 18, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 18, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 18, 2020

New commits:

3417f43src/bin/sage: Handle 'sage -i SPKG' for optional/experimental packages by configure --enable-SPKG

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 18, 2020

Commit: 3417f43

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Reimplement sage -i SPKG as configure --enable-SPKG && make build Reimplement sage -i SPKG for optional/experimental packages as configure --enable-SPKG && make build Mar 18, 2020
@jhpalmieri
Copy link
Member

comment:6

I took a fresh tarball and merged this branch. When I ran ./sage -i meataxe, it ran configure three times in a row. Is this expected?

@jhpalmieri
Copy link
Member

comment:7

By the way, after running ./sage -i meataxe or ./configure --enable-meataxe, config.log still says

configure:30333: result: meataxe-1.0.p0: does not support check for system package; optional, use "./configure --enable-meataxe" to install

It would be nice if this actually noted that the user has requested its installation.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 23, 2020

comment:8

Replying to @jhpalmieri:

I took a fresh tarball and merged this branch. When I ran ./sage -i meataxe, it ran configure three times in a row. Is this expected?

That's clearly too much.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 23, 2020

comment:9

Replying to @jhpalmieri:

By the way, after running ./sage -i meataxe or ./configure --enable-meataxe, config.log still says

configure:30333: result: meataxe-1.0.p0: does not support check for system package; optional, use "./configure --enable-meataxe" to install

It would be nice if this actually noted that the user has requested its installation.

That's #29363 "At the end of configure, indicate which optional/experimental packages are configured to be installed"

@jhpalmieri
Copy link
Member

comment:10

Replying to @mkoeppe:

Replying to @jhpalmieri:

I took a fresh tarball and merged this branch. When I ran ./sage -i meataxe, it ran configure three times in a row. Is this expected?

That's clearly too much.

With a fully built Sage, running ./sage -i -p_group_cohomology just ran configure once.

Let me clarify about the other case. I took a fresh tarball for 9.1.beta8, got this branch and ran git rebase develop. That's when configure ran three times. The first time:

% ./sage -i meataxe                           
make -j6 build/make/Makefile --stop
make[1]: warning: -jN forced in submake: disabling jobserver mode.
rm -f config.log
mkdir -p logs/pkgs
ln -s logs/pkgs/config.log config.log
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... config/install-sh -c -d
checking for gawk... no
...

The second time:

real	0m0.206s
user	0m0.156s
sys	0m0.038s
Sage build/upgrade complete!

running ./configure   '--enable-meataxe'
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... config/install-sh -c -d
checking for gawk... no

The third time:

checking for the package system in use... homebrew
configure: No equivalent system packages for homebrew are known to Sage
make -j6 build/make/Makefile --stop
make[1]: warning: -jN forced in submake: disabling jobserver mode.
rm -f config.log
mkdir -p logs/pkgs
ln -s logs/pkgs/config.log config.log
running CONFIG_SHELL=/bin/sh /bin/sh ./configure --enable-meataxe --no-create --no-recursion
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... config/install-sh -c -d
checking for gawk... no

If I start with a 9.1.beta7 tarball and use this branch, configure only runs twice.

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

comment:11

these multiple runs of configure, in some cases up to 3 times, also happen without this branch. so this is not a regression.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:12

Replying to @dimpase:

these multiple runs of configure, in some cases up to 3 times, also happen without this branch. so this is not a regression.

I agree. But independent of this ticket, we should probably look at some point how to clean this up a bit.

@jhpalmieri
Copy link
Member

comment:13

Well, I'm happy with this. Positive review from me. Dima (or @embray, if you're active these days), if you want to look at it more, feel free to set back to needs review.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@vbraun
Copy link
Member

vbraun commented Mar 29, 2020

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

5 participants