Opened 2 years ago

Closed 21 months ago

#28095 closed enhancement (fixed)

Add --enable-OPTIONALSPKG options to configure

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.1
Component: build: configure Keywords:
Cc: dimpase, embray, saraedum, jhpalmieri 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 mkoeppe)

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 2 years ago by embray

+1 that's a good idea.

comment:2 Changed 2 years ago by mkoeppe

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 2 years ago by dimpase

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 2 years ago by embray

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 22 months ago by embray

  • Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:6 Changed 21 months ago by mkoeppe

  • Branch set to u/mkoeppe/add___enable_optionalspkg_options_to_configure

comment:7 Changed 21 months ago by mkoeppe

  • Commit set to cf5607e42f491164219c464ce78c260157b403b8
  • Description modified (diff)

New commits:

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

comment:8 Changed 21 months ago by dimpase

how is the actual installation of the enabled packages triggered?

comment:9 Changed 21 months ago by mkoeppe

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

comment:10 Changed 21 months ago by mkoeppe

  • Cc saraedum added

comment:11 Changed 21 months ago by git

  • Commit changed from cf5607e42f491164219c464ce78c260157b403b8 to e4675d1fa110259361002d3f629e9975adec1dc9

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

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

comment:12 Changed 21 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)
  • Status changed from new to needs_review

Now it's ready.

comment:13 Changed 21 months ago by mkoeppe

  • Cc jhpalmieri added

comment:14 Changed 21 months ago by mkoeppe

Anyone interested in reviewing this?

comment:15 Changed 21 months ago by mkoeppe

  • Description modified (diff)

comment:16 Changed 21 months ago by dimpase

  • Reviewers set to Dima Pasechnik

comment:17 follow-up: Changed 21 months ago by dimpase

  • Status changed from needs_review to positive_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 follow-up: Changed 21 months ago by dimpase

  • Status changed from positive_review to needs_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 ; follow-up: Changed 21 months ago by 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?

comment:20 in reply to: ↑ 19 Changed 21 months ago by mkoeppe

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 ; follow-up: Changed 21 months ago by 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.

comment:22 Changed 21 months ago by mkoeppe

Fix on this ticket or follow up?

comment:23 Changed 21 months ago by dimpase

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 21 months ago by git

  • Commit changed from e4675d1fa110259361002d3f629e9975adec1dc9 to 4ed7b05cb04a291268607425408c3da0738ad94d

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 21 months ago by mkoeppe

  • Status changed from needs_info to needs_review

Done. Works for me

comment:26 Changed 21 months ago by dimpase

  • Status changed from needs_review to positive_review

comment:27 Changed 21 months ago by mkoeppe

Thanks for reviewing!

comment:28 Changed 21 months ago by embray

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 21 months ago by embray

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 21 months ago by vbraun

  • Branch changed from u/mkoeppe/add___enable_optionalspkg_options_to_configure to 4ed7b05cb04a291268607425408c3da0738ad94d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.