Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#27642 closed defect (fixed)

Re-run configure+make after installing an SPKG with sage -i

Reported by: embray Owned by:
Priority: major Milestone: sage-8.8
Component: build Keywords:
Cc: dimpase Merged in:
Authors: Erik Bray Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: public/build/ticket-27642 (Commits, GitHub, GitLab) Commit: e16873284b5cec5c813bcf38861e0aa42ff2b93e
Dependencies: #27641 Stopgaps:

Status badges

Description

This is an idea for fixing #27373.

As explained in comment#73 on that ticket, after installing a new package it is really necessary to re-run configure after installing a new package, whether it was an optional package that was not previously installed, or a standard package that was not previously installed (due to a configure-time check determining that it was not needed).

This is to ensure that files generated by configure reflect the new system configuration, in terms of what SPKGs are installed.

Further, it is advisable to re-run make all-build in case the sage -i installed packages whose dependents need to be rebuilt.

This also needs to ensure that if an SPKG is installed for some standard package, that we account for that when re-running configure: It must not try again to check the system for that dependency, and just reflect that the SPKG is installed. For example, from #27373, if Sage is first built using GMP/MPIR from the system, then we run sage -i mpir, the system configuration must be updated to reflect that we are now using the mpir SPKG instead of GMP/MPIR from the system. It's also necessary to rebuilt any of its dependent packages.

Change History (23)

comment:1 Changed 3 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/build/ticket-27642
  • Commit set to a29056a29dbd0ec6188457c06d1e797644913c15

This branch mostly fixes the bug in #27373 for me.

As noted in the commit message this does not yet preserve arguments previously passed to configure. I think we can fix that by running ./config.status --config but I need to test that a bit more.


Last 10 new commits:

687cdf0Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
6de961dReworked this a bit more
f76d6d5fix typo
5f90ff7added a bit more explanation
3dee97ccorrect logic for SAGE_GMP_PREFIX etc
0308142add the AX_ABSOLUTE_HEADER macro
eea0685iml in particular is very picky about being given an absolute path to the
6f29e86Add support for unconditional pre- and post-check actions in spkg-configure.m4
a2bab81Refactor the mpir/spkg-configure.m4 (and to a lesser extent the one for gmp) to use the new pre and post arguments to SAGE_SPKG_CONFIGURE
a29056aTrac #27642: Initial attempt at fix: re-run `configure` and `make all-build` after

comment:2 Changed 3 years ago by dimpase

could you rebase it?

comment:3 Changed 3 years ago by dimpase

  • Branch changed from u/embray/build/ticket-27642 to public/build/ticket-27642
  • Commit changed from a29056a29dbd0ec6188457c06d1e797644913c15 to 248c071fa16ce48c0c2635b28e84db151d008da0

New commits:

191269eAdd support for unconditional pre- and post-check actions in spkg-configure.m4
f9c9a91Refactor the mpir/spkg-configure.m4 (and to a lesser extent the one for gmp) to use the new pre and post arguments to SAGE_SPKG_CONFIGURE
248c071Trac #27642: Initial attempt at fix: re-run `configure` and `make all-build` after

comment:4 Changed 3 years ago by dimpase

  • Status changed from new to needs_review

Trying this with ./configure leaves gmp not marked as an not to be installed, while mpir is marked as not to be installed.

Version 0, edited 3 years ago by dimpase (next)

comment:5 Changed 3 years ago by git

  • Commit changed from 248c071fa16ce48c0c2635b28e84db151d008da0 to 4bce1a36e95a6b18ed5ade3e4267df4045aa6bc9

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

191269eAdd support for unconditional pre- and post-check actions in spkg-configure.m4
1c24df1Refactor the mpir/spkg-configure.m4 (and to a lesser extent the one for gmp) to use the new pre and post arguments to SAGE_SPKG_CONFIGURE
dc8fd7fRefactor the mpir/spkg-configure.m4 (and to a lesser extent the one for gmp) to use the new pre and post arguments to SAGE_SPKG_CONFIGURE
4bce1a3Trac #27642: Initial attempt at fix: re-run `configure` and `make all-build` after

comment:6 Changed 3 years ago by dimpase

rebased over updated #27641

comment:7 Changed 3 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to needs_work

now this branch leaves gmp not marked as an not to be installed, while mpir is marked as not to be installed, despite being fixed on #27641...

comment:8 Changed 3 years ago by embray

Your rebasing did not apparently include the fixes I made to #27641 (compare gmp/spkg-configure.m4)

comment:9 Changed 3 years ago by dimpase

gmp/spkg-configure.m4 was not touched on #27641, have a look at git show 1c24df1. `

comment:10 Changed 3 years ago by dimpase

More precisely, you overwrote this change in https://trac.sagemath.org/ticket/27641#comment:15

comment:11 follow-up: Changed 3 years ago by embray

Right. That was intentional. It should not be changed.

Last edited 3 years ago by embray (previous) (diff)

comment:12 in reply to: ↑ 11 Changed 3 years ago by dimpase

Replying to embray:

Right. That was intentional. It should not be changed.

Should I re-create the un-rebased branch and try again, or it's simply removing the changes to gmp/spkg-configure.m4 that would do?

comment:13 Changed 3 years ago by git

  • Commit changed from 4bce1a36e95a6b18ed5ade3e4267df4045aa6bc9 to 18d612df2b6d24200d6910924241c1694cf97267

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

18d612dleave GMP's spkg-configure.m4 as it was

comment:14 Changed 3 years ago by dimpase

  • Status changed from needs_work to positive_review

comment:15 Changed 3 years ago by embray

  • Status changed from positive_review to needs_work

This is a bit of a mess. I just need to rebase on top of the final version of #27641, not add spurious new commits.

comment:16 Changed 3 years ago by git

  • Commit changed from 18d612df2b6d24200d6910924241c1694cf97267 to e16873284b5cec5c813bcf38861e0aa42ff2b93e

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

d9bbdd4Add support for unconditional pre- and post-check actions in spkg-configure.m4
a360518Refactor the mpir/spkg-configure.m4 (and to a lesser extent the one for gmp) to use the new pre and post arguments to SAGE_SPKG_CONFIGURE
e168732Trac #27642: Initial attempt at fix: re-run `configure` and `make all-build` after

comment:17 Changed 3 years ago by embray

Updated to the final version of #27641. Still need to kick around a bit with making sure it can preserve previous configure flags.

comment:18 Changed 3 years ago by vbraun

  • Resolution set to fixed
  • Status changed from needs_work to closed

Too late, a previous version of this branch was released in 8.8.beta5

comment:19 Changed 3 years ago by dimpase

I am afraid it has to be reverted. It's not good as it is now.

comment:20 Changed 3 years ago by dimpase

See #27820

comment:21 follow-up: Changed 3 years ago by embray

I'm confused. How was it merged? It was not set to positive review, and you never replaced the branch with the merge commit as in your usual process?

comment:22 in reply to: ↑ 21 Changed 3 years ago by dimpase

Replying to embray:

I'm confused. How was it merged? It was not set to positive review,

it was, see comment:14

I gather it wasn't merged in the usual way as it didn't come with a (formally) needed configure bump

comment:23 Changed 3 years ago by dimpase

I can confirm that this branch is merged in 8.8.rc0. So this is done and dusted.

Note: See TracTickets for help on using tickets.