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

Add --enable-OPTIONALSPKG options to configure #28095

Closed
mkoeppe opened this issue Jul 1, 2019 · 33 comments
Closed

Add --enable-OPTIONALSPKG options to configure #28095

mkoeppe opened this issue Jul 1, 2019 · 33 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 1, 2019

This ticket adds options such as --enable-lrslib, --enable-topcom for all optional and experimental packages to configure. As per configure --help:

  ...
  --enable-lie={no|if_installed|yes}
                          enable build and use of the experimental package lie
                          (default: "if_installed")
                          package information: ./sage -info lie
  --disable-lie           disable build and uninstall if previously installed
                          by Sage in PREFIX; same as --enable-lie=no
  ...

Subsequent make will then install or uninstall these packages.

This is an interface alternative to sage -i OPTIONALSPKG / make OPTIONALSPKG and make OPTIONALSPKG-clean.

This will simplify installations of sage with a list of optional packages.

(This was previously discussed in #21538, which did not clearly enough distinguish this proposal from the topic of using system packages, #27567.)

Follow-up:

CC: @dimpase @embray @saraedum @jhpalmieri

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: 4ed7b05

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-8.9 milestone Jul 1, 2019
@embray
Copy link
Contributor

embray commented Jul 2, 2019

comment:1

+1 that's a good idea.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 26, 2019

comment:2

It seems tricky to generate AC_ARG_ENABLE macro invocations for all packages: it would have to be done during bootstrap (not sure if we want this), or perhaps using m4 loops (https://www.gnu.org/software/autoconf/manual/autoconf-2.62/html_node/Looping-constructs.html).

@dimpase
Copy link
Member

dimpase commented Jul 26, 2019

comment:3

well, we do things like collecting spkg-configure.m4 files for each package in ./bootstrap, nothing wrong with doing more at this stage...

@embray
Copy link
Contributor

embray commented Aug 13, 2019

comment:4

Yes, you could do it during bootstrap, in principle.

I have tried in the past to get m4 itself to loop over package directories and generate this stuff (as opposed to the current approach of generating bits of m4 in ./bootstrap). And in principle that can work, but other, higher-level tools like automake get terribly confused when you try to do too much weird stuff like this.

@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:5

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

New commits:

cf5607eAdd configure options '--enable-SPKG', --disable-SPKG' for optional/experimental SPKG

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

Commit: cf5607e

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Jan 22, 2020

comment:8

how is the actual installation of the enabled packages triggered?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

comment:9

Oh, the ticket is not ready for review. So far there is only a sufficiently pretty interface.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2020

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

e4675d1Actually implement --enable-SPKG and --disable-SPKG

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2020

Changed commit from cf5607e to e4675d1

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 23, 2020

comment:12

Now it's ready.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 23, 2020

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 30, 2020

comment:14

Anyone interested in reviewing this?

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Jan 30, 2020

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Jan 30, 2020

comment:17

Very nice! It works, but I don't like that ./configure does not output which packages are tagged for installation/deletion.
Can we have a follow-up ticket on this?
Otherwise, positive review.

@dimpase
Copy link
Member

dimpase commented Jan 30, 2020

comment:18

Also, I don't understand the difference between --with-system-gmp=force and --enable-gmp. These appear to conflict each other.

@jhpalmieri
Copy link
Member

comment:19

Replying to @dimpase:

Very nice! It works, but I don't like that ./configure does not output which packages are tagged for installation/deletion.
Can we have a follow-up ticket on this?

Is #28788 the right place?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 30, 2020

comment:20

Replying to @jhpalmieri:

Replying to @dimpase:

Very nice! It works, but I don't like that ./configure does not output which packages are tagged for installation/deletion.
Can we have a follow-up ticket on this?

Is #28788 the right place?

Yes.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 30, 2020

comment:21

Replying to @dimpase:

Also, I don't understand the difference between --with-system-gmp=force and --enable-gmp. These appear to conflict each other.

That's caused by another misclassification, for historical reasons, of package types (like the one for gfortran fixed in #29053).

$ cat build/pkgs/gmp/type 
optional
$ cat build/pkgs/mpir/type 
optional

These should actually be standard - they are mutually exclusive standard packages.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 30, 2020

comment:22

Fix on this ticket or follow up?

@dimpase
Copy link
Member

dimpase commented Jan 30, 2020

comment:23

here, I guess.

It would be good to check that this change in the package types won't cause broken builds, with both gmp and mpir getting installed...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 30, 2020

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

a3e0e47Change type of gmp, mpir to 'standard' (they are mutually exclusive standard packages)
4ed7b05Fix indentation of '--with-mp=' options in configure --help

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 30, 2020

Changed commit from e4675d1 to 4ed7b05

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 30, 2020

comment:25

Done. Works for me

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 30, 2020

comment:27

Thanks for reviewing!

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:28

Interesting, I didn't know this syntax:

+OPTIONAL_CLEANED_PACKAGES_CLEANS = $(OPTIONAL_CLEANED_PACKAGES:%=%-clean)

+1 from me. This is an idea that had crossed my mind before but I wasn't really sure how to go about it. Thanks!

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:29

Replying to @mkoeppe:

Replying to @dimpase:

Also, I don't understand the difference between --with-system-gmp=force and --enable-gmp. These appear to conflict each other.

That's caused by another misclassification, for historical reasons, of package types (like the one for gfortran fixed in #29053).

$ cat build/pkgs/gmp/type 
optional
$ cat build/pkgs/mpir/type 
optional

These should actually be standard - they are mutually exclusive standard packages.

This has always annoyed me too. I tried to fix it a while ago but I think my approach to the problem was too complicated at the time. I think now we have a better solution.

@vbraun
Copy link
Member

vbraun commented Jan 31, 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