Opened 3 years ago

Closed 3 years ago

#28095 closed enhancement (fixed)

Add --enable-OPTIONALSPKG options to configure

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.1
Component: build: configure Keywords:
Cc: Dima Pasechnik, Erik Bray, Julian Rüth, John Palmieri Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 4ed7b05 (Commits, GitHub, GitLab) Commit: 4ed7b05cb04a291268607425408c3da0738ad94d
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

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:

  • #29113: Reimplement sage -i SPKG as configure --enable-SPKG && make build

Change History (30)

comment:1 Changed 3 years ago by Erik Bray

+1 that's a good idea.

comment:2 Changed 3 years ago by Matthias Köppe

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

comment:3 Changed 3 years ago by Dima Pasechnik

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

comment:4 Changed 3 years ago by Erik Bray

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.

comment:5 Changed 3 years ago by Erik Bray

Milestone: sage-8.9sage-9.1

Ticket retargeted after milestone closed

comment:6 Changed 3 years ago by Matthias Köppe

Branch: u/mkoeppe/add___enable_optionalspkg_options_to_configure

comment:7 Changed 3 years ago by Matthias Köppe

Commit: cf5607e42f491164219c464ce78c260157b403b8
Description: modified (diff)

New commits:

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

comment:8 Changed 3 years ago by Dima Pasechnik

how is the actual installation of the enabled packages triggered?

comment:9 Changed 3 years ago by Matthias Köppe

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

comment:10 Changed 3 years ago by Matthias Köppe

Cc: Julian Rüth added

comment:11 Changed 3 years ago by git

Commit: cf5607e42f491164219c464ce78c260157b403b8e4675d1fa110259361002d3f629e9975adec1dc9

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

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

comment:12 Changed 3 years ago by Matthias Köppe

Authors: Matthias Koeppe
Description: modified (diff)
Status: newneeds_review

Now it's ready.

comment:13 Changed 3 years ago by Matthias Köppe

Cc: John Palmieri added

comment:14 Changed 3 years ago by Matthias Köppe

Anyone interested in reviewing this?

comment:15 Changed 3 years ago by Matthias Köppe

Description: modified (diff)

comment:16 Changed 3 years ago by Dima Pasechnik

Reviewers: Dima Pasechnik

comment:17 Changed 3 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

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.

comment:18 Changed 3 years ago by Dima Pasechnik

Status: positive_reviewneeds_info

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

comment:19 in reply to:  17 ; Changed 3 years ago by John Palmieri

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?

comment:20 in reply to:  19 Changed 3 years ago by Matthias Köppe

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.

comment:21 in reply to:  18 ; Changed 3 years ago by Matthias Köppe

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.

comment:22 Changed 3 years ago by Matthias Köppe

Fix on this ticket or follow up?

comment:23 Changed 3 years ago by Dima Pasechnik

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

comment:24 Changed 3 years ago by git

Commit: e4675d1fa110259361002d3f629e9975adec1dc94ed7b05cb04a291268607425408c3da0738ad94d

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

comment:25 Changed 3 years ago by Matthias Köppe

Status: needs_infoneeds_review

Done. Works for me

comment:26 Changed 3 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

comment:27 Changed 3 years ago by Matthias Köppe

Thanks for reviewing!

comment:28 Changed 3 years ago by Erik Bray

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!

comment:29 in reply to:  21 Changed 3 years ago by Erik Bray

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.

comment:30 Changed 3 years ago by Volker Braun

Branch: u/mkoeppe/add___enable_optionalspkg_options_to_configure4ed7b05cb04a291268607425408c3da0738ad94d
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.