Opened 2 years ago

Closed 6 months ago

Last modified 6 months ago

#27373 closed task (duplicate)

some dummy packages must be only re-installable via ./configure --with-...

Reported by: dimpase Owned by: embray
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords:
Cc: embray Merged in:
Authors: Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #27212 Stopgaps:

Status badges

Description (last modified by dimpase)

$SAGE_LOCAL/bin/sage-env-config must get updated after, say, sage -i mpir (or any other dummy package that sets anything in the build environment). Currently this does not happen. This is a bug introduced in #27212 that leads to sage -i mpir breaking stuff.

To deal with this, we introduce a kind of dummy packages, freezable, which differ from dummy so that they (once frozen) cannot be re-installed via sage -i, but only via re-running ./configire --with-... first.

Specifically, for gmp and mpir, which are the only freezable packages so far, ./configure --with-mp=system freezes, them, and ./sage -i gmp or ./sage -i mpir errors out. To unfreeze, one needs to do first ./configure --with-mp=gmp (or =mpir).

The branch implements this. If an attempt is made to run ./configure --with-mp=gmp, (or any other "frozen" package) while Sage is configured with external gmp, one gets the error message

***********************************************
 before running ./sage -i/f gmp please run
   ./configure <dollar sign>(./config.status --config) --with-mp=gmp
***********************************************

Change History (85)

comment:1 Changed 2 years ago by dimpase

We can also merely document that ./sage -i blah must be run after ./configure --with-blah=install, perhaps also insert some warning print somewhere...

comment:2 Changed 2 years ago by dimpase

I looked a bit into how ./sage -i blah may trigger a run of ./configure --with-blah=install - this is not hard, assuming --with-blah is a parameter known to configure. E.g. it can grep for it in the output of ./configure -h, and run, if found.

This will require having each package blah for which such a configure run is needed to implement the corresponding --with-blah=, (or, perhaps, something like --install-blah, to make it even clearer)


An alternative might be to do this via special Makefile targets, something I am much less keen on.

comment:3 Changed 2 years ago by dimpase

On further reflection, it won't fly yet, as it would need to remember the exact parameters used to call ./configure...

An easier hack would be to set a flag in the call of sage -i blah which would be tested by the spkg-install of blah and if it's on, rewrite the corresponding part of src/bin/sage-env-config. A relatively ugly hack, but looks relatively easy to implement.

comment:4 follow-up: Changed 2 years ago by dimpase

  • Description modified (diff)

Scratch all the above - it's simply the matter of having spkg-install to change src/bin/sage-env-config $SAGE_LOCAL/bin/sage-env-config.

Does this look right?

I suppose this fix should be added to #27212, not here...

Last edited 2 years ago by dimpase (previous) (diff)

comment:5 Changed 2 years ago by dimpase

  • Description modified (diff)

comment:6 in reply to: ↑ 4 Changed 2 years ago by fbissey

Replying to dimpase:

Scratch all the above - it's simply the matter of having spkg-install to change src/bin/sage-env-config.

Does this look right?

I suppose this fix should be added to #27212, not here...

If you change src/bin/sage-env-config, you'll have to re-install it. The one used at runtime is SAGE_LOCAL/bin/sage-env-config. The real way to do that is to have individual packages install the environment variable they need and then sage-env to read them. Like Erik suggested before. Such variables not being handled by the package themselves just lead to madness just as you are experiencing now.

comment:7 Changed 2 years ago by dimpase

right, I realised it must be $SAGE_LOCAL/bin/sage-env-config an hour ago, as you can see from the ticket description.

I don't get the second part of the comment - using a system package instead of Sage-supplied one might affect the environment, and the package responsible for this must take care of this, so that its "dependees" know about it. I don't see how sage-env is involved (besides that it uses sage-env-config).

comment:8 Changed 2 years ago by dimpase

  • Authors set to Dima Pasechnik
  • Owner changed from (none) to dimpase

comment:9 Changed 2 years ago by dimpase

  • Branch set to u/dimpase/packages/gmp-config
  • Commit set to 84736587b00c0e6fdcc26fe094a25dff85a74989
  • Status changed from new to needs_review

This should do it; I wrote a short sdh_- function to use on #27212 branch and in the other places (and call it).


New commits:

c4e42e6spkg-configure for GMP
b070088Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
3b6eebdReplace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
b80bf72Reworked this a bit more
0ca3f56fix typo
06798caadded a bit more explanation
b592d77correct logic for SAGE_GMP_PREFIX etc
5057680add the AX_ABSOLUTE_HEADER macro
101537biml in particular is very picky about being given an absolute path to the
8473658sdh_update_var_env_config, call it to update SAGE_GMP_*

comment:10 Changed 2 years ago by dimpase

Please review the last commit, the rest are from #27212.

comment:11 Changed 2 years ago by dimpase

This is of course far from ideal, as it makes the actual state of the configuration different from the one obtained after running ./configure.

I don't know how to reconcile them---it still seems that a better way would be to always call ./configure, but does it remember the way it was called in the 1st place?

comment:12 Changed 2 years ago by embray

I can see that there's a problem here, but I don't think the ticket description describes what the problem is very well, but I need to think about it more.

I think ultimately what we want here is if you're changing your package selections it should be necessary to re-run ./configure. There's no simple way around that and that's really the only correct solution.

Another part of the problem is that ./sage -i never really worked the way you're trying to make it work. It was mostly only ever for installing optional packages that were not installed by default (but were still selected for possible installation in the Makefile generated by configure).

comment:13 Changed 2 years ago by dimpase

How about disabling 'sage -i' for dummy packages?

comment:14 Changed 2 years ago by embray

For lack of a better answer, at the moment, I'd be fine with that. Just print an error message like "So and so has been selected to be used from your system; re-run ./configure with <such and such flags> to use the Sage SPKG instead".

The only problem is that for most packages we don't have the appropriate "<such and such flags>". That's been on my to-do list for this since the beginning. It would be easy to add though, I think. Have the SAGE_SPKG_CONFIGURE macro also generate some --with-<package>. If --with-<package>=sage then it would just set sage_spkg_install_<package>=yes and skip any system checks for that package.

comment:15 Changed 2 years ago by embray

The only try part to that is there are still some special cases like --with-mp= which could obviously conflict with that, so additional care might be needed to handle such a conflict, alas.

comment:16 Changed 2 years ago by dimpase

It looks it might be most natural to disallow this undesired building via ./sage -i by not having Makefile rules to "really" build the affected dummy packages - instead the corresponding rules would print an error suggested in comment 14.

This would also still allow dummy packages which are OK to build via ./sage -i to retain this possibility.

If I'm to do such a fix I'd much appreciate some pointers to where to look and what to change...

comment:17 Changed 2 years ago by dimpase

The current plan is to setup something like sage_spkg_freeze_foo for each package foo that is dummy and not sage -i-installable, and change m4/sage_spkg_collect.m4 to earmark these packages (e.g. add them to a separate list SAGE_FROZEN_PACKAGES, and not to SAGE_DUMMY_PACKAGES)

Now, for packages in SAGE_FROZEN_PACKAGES one has to generate the correct make targets. I suppose this is to be done in build/make/Makefile.in?

Does this make sense, or do I miss something? What I don't understand at the moment is how e.g. SAGE_DUMMY_PACKAGES become DUMMY_PACKAGES...

Last edited 2 years ago by dimpase (previous) (diff)

comment:18 Changed 2 years ago by git

  • Commit changed from 84736587b00c0e6fdcc26fe094a25dff85a74989 to f218515b64f81b3b7f964a6c46bbe88622797be6

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

0309d8cspkg-configure for GMP
8435b90Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
3907206Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
dcb676dReworked this a bit more
23209dcfix typo
4621d18added a bit more explanation
503655acorrect logic for SAGE_GMP_PREFIX etc
837b38aadd the AX_ABSOLUTE_HEADER macro
154fed0iml in particular is very picky about being given an absolute path to the
f218515set up frozen packages - only reinstallable via ./configure

comment:19 Changed 2 years ago by dimpase

  • Description modified (diff)

error reporting of these failing sage -i needs a bit of work still...

comment:20 Changed 2 years ago by dimpase

  • Description modified (diff)
  • Summary changed from re-installing dummy packages must update sage-env-config to some dummy packages must be only re-installable via ./configure --with-...

comment:21 Changed 2 years ago by dimpase

  • Status changed from needs_review to needs_work

This totally broke building (make, or any sage -i), for some reason...

comment:22 Changed 2 years ago by git

  • Commit changed from f218515b64f81b3b7f964a6c46bbe88622797be6 to 38120ccdce0dd88519ede9a50cede1bbf6cce8a7

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

38120ccset up frozen packages - only reinstallable via ./configure

comment:23 Changed 2 years ago by dimpase

  • Owner changed from dimpase to embray

Erik, could you please look at the new ("frozen") targets I'm trying to add to build/make/Makefile.in (see the latest commits on this branch, the loop that populates them is commented out and marked by TODO). I cannot make them work for a reason I don't understand.

The idea is that FROZEN_PACKAGES, a list that has at most mpir and gmp on, has its NORMAL_PACKAGES targets replaced by $(error...), to provide an intelligent error reporting at sage -i attempts on them.

But it does not work (if uncommented), they always get invoked, at normal make runs too, and the build does not go forward.

I don't see what is wrong there (not the least, perhaps, as I still don't quite understand the dummy targets hack, IMHO it deserves more comments in that Makefile.in...)

comment:24 Changed 2 years ago by dimpase

  • Status changed from needs_work to needs_review

comment:25 Changed 2 years ago by dimpase

It would be also OK with me to leave this "meaningful error message" thing for another ticket, if one cannot fix that makefile quickly...

comment:26 in reply to: ↑ description ; follow-up: Changed 2 years ago by jdemeyer

Replying to dimpase:

$SAGE_LOCAL/bin/sage-env-config must get updated after, say, sage -i mpir

Can you explain why?

comment:27 in reply to: ↑ 26 ; follow-up: Changed 2 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

$SAGE_LOCAL/bin/sage-env-config must get updated after, say, sage -i mpir

Can you explain why?

this is modulo #27212: if gmp/mpir came from the system then various packages need to know where to find include files and the shared libraries for it.

comment:28 in reply to: ↑ 27 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Replying to dimpase:

if gmp/mpir came from the system then various packages need to know where to find include files and the shared libraries for it.

Sure, but the decision of whether or not the package is taken from the system should be made by ./configure. Doing sage -i mpir shouldn't change that. So I still don't understand which problem this ticket is trying to solve.

comment:29 follow-up: Changed 2 years ago by dimpase

The problem is that sage -i mpir will break the build if gmp/mpir came from the system. I thought that before #27212 it was working.

comment:30 follow-up: Changed 2 years ago by dimpase

Without this ticket #27212 results in sage -i mpir being installed from sources no matter what.

comment:31 in reply to: ↑ 29 ; follow-up: Changed 2 years ago by jdemeyer

Replying to dimpase:

The problem is that sage -i mpir will break the build if gmp/mpir came from the system.

But why? I see nothing in sage-env-config that would depend on the installation of mpir.

comment:32 in reply to: ↑ 30 Changed 2 years ago by jdemeyer

Replying to dimpase:

sage -i mpir being installed from sources no matter what.

But that's a feature. It's the same for other optionally-from-the-system packages.

The command sage -i mpir means "build the Sage package mpir". I think it's good to have an override for system packages: if I really want the Sage mpir package, I would be able to install it with sage -i mpir.

comment:33 in reply to: ↑ 31 ; follow-up: Changed 2 years ago by embray

Replying to jdemeyer:

Replying to dimpase:

The problem is that sage -i mpir will break the build if gmp/mpir came from the system.

But why? I see nothing in sage-env-config that would depend on the installation of mpir.

There isn't anything currently but there would be after #27212. That's what Dima is concerned about.

The issue is that if you're using MPIR (or any package, really, but let's keep using it as an example), and then you want to do sage -i mpir and install the Sage SPKG (which may even be a different version), then it may be necessary to re-configure any other installed packages that depend on mpir; likely rebuild them as well.

This is a change in the configuration to the system, hence my point earlier that it's thus necessary to re-run Sage's top-level ./configure script. Unfortunately what we're missing at the moment (but would be easy to add, and I will do it if Dima hasn't already) is an easy way to tell Sage's ./configure explicitly that we want to use the SPKG for a given dependency.

comment:34 Changed 2 years ago by embray

For example if you run sage -i mpir then it should run

./configure $(./config.status --config) --with-mpir=spkg

where ./config.status --config just prints the flags of the last ./configure run.

Or something like that. What we currently lack is the --with-mpir=spkg part, or some equivalent that means "don't check for a system mpir, just build and install the mpir SPKG".

No further mucking around with makefiles or anything is required--./configure would re-build build/make/Makefile with all the correct selections.

comment:35 in reply to: ↑ 33 ; follow-up: Changed 2 years ago by jdemeyer

Thanks a lot for those clarifications. I couldn't understand much from the ticket description.

So it boils down to basically two issues:

  1. We need a way to force running ./configure after installing a particular package. We could re-use the file "$SAGE_LOCAL/lib/sage-force-relocate.txt" (possibly with a different name) as a trigger for that.
  1. We need to ensure that ./configure treats already-installed packages as not coming from the system. I don't think that we need a --with-spkg flag though. ./configure should auto-detect that mpir is already installed.

comment:36 in reply to: ↑ 35 ; follow-up: Changed 2 years ago by embray

Replying to jdemeyer:

Thanks a lot for those clarifications. I couldn't understand much from the ticket description.

So it boils down to basically two issues:

  1. We need a way to force running ./configure after installing a particular package. We could re-use the file "$SAGE_LOCAL/lib/sage-force-relocate.txt" (possibly with a different name) as a trigger for that.
  1. We need to ensure that ./configure treats already-installed packages as not coming from the system. I don't think that we need a --with-spkg flag though. ./configure should auto-detect that mpir is already installed.

I think you might still have things backwards: What I'm proposing is that configure should be re-run before installing the SPKG. That makes it clear, from the Makefile on down, that our build is configured to use the mpir SPKG. Thus after its installation we should also re-run make build to re-build any of its dependencies.

Otherwise you end up in this confusing state where mpir is in the list of "DUMMY PACKAGES" and thus not really managed by Sage's makefile, and yet is actually installed by Sage.

comment:37 in reply to: ↑ 36 ; follow-up: Changed 2 years ago by jdemeyer

Replying to embray:

I think you might still have things backwards: What I'm proposing is that configure should be re-run before installing the SPKG. That makes it clear, from the Makefile on down, that our build is configured to use the mpir SPKG.

I don't really see how the contents of the Sage-the-distribution Makefiles should influence the build of mpir. Whether or not mpir is a dummy package shouldn't influence what sage -i mpir does: it just runs the build script of mpir.

Otherwise you end up in this confusing state where mpir is in the list of "DUMMY PACKAGES" and thus not really managed by Sage's makefile, and yet is actually installed by Sage.

Of course. But I would suggest to run ./configure after building MPIR instead of before. I think that's easier to implement. We actually used to do this for GCC (to implement SAGE_BUILD_TOOLCHAIN but that got broken).

comment:38 Changed 2 years ago by embray

Okay, I think I agree with you--without having actually tried it at least--that you could do things in either order. To me it makes more sense to re-configure first but if you think it would be easy to do afterwards, and successfully detect that we've installed the Sage SPKG, I'm okay with that too.

comment:39 Changed 2 years ago by git

  • Commit changed from 38120ccdce0dd88519ede9a50cede1bbf6cce8a7 to 9f02d15aab404c0c5473c6c3f24da0210375a345

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

b070088Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
3b6eebdReplace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
b80bf72Reworked this a bit more
0ca3f56fix typo
06798caadded a bit more explanation
b592d77correct logic for SAGE_GMP_PREFIX etc
5057680add the AX_ABSOLUTE_HEADER macro
101537biml in particular is very picky about being given an absolute path to the
862ca6aMerge remote-tracking branch 'trac/develop' into HEAD
9f02d15set up frozen packages - only reinstallable via ./configure

comment:40 Changed 2 years ago by dimpase

Rebased. I believe that the way of comment:34 (with ./configure run before or after the build - to me it's more natural before) is implementable via rules in build/make/Makefile.in, but I'd rather have Erik on it.

(Or I can do it, but then I first need more info on the inner working of that .dummy targets...)

comment:41 in reply to: ↑ 37 Changed 2 years ago by dimpase

Replying to jdemeyer:

Replying to embray:

I think you might still have things backwards: What I'm proposing is that configure should be re-run before installing the SPKG. That makes it clear, from the Makefile on down, that our build is configured to use the mpir SPKG.

I don't really see how the contents of the Sage-the-distribution Makefiles should influence the build of mpir. Whether or not mpir is a dummy package shouldn't influence what sage -i mpir does: it just runs the build script of mpir.

Otherwise you end up in this confusing state where mpir is in the list of "DUMMY PACKAGES" and thus not really managed by Sage's makefile, and yet is actually installed by Sage.

Of course. But I would suggest to run ./configure after building MPIR instead of before.

I don't understand - unless after running ./configure you do make once again. But then what's the point of running ./configure after make, if you can run in before make, and run make just once?

I think that's easier to implement. We actually used to do this for GCC (to implement SAGE_BUILD_TOOLCHAIN but that got broken).

comment:42 Changed 2 years ago by embray

It might help to look at exactly what sage -i does (from src/bin/sage):

    for PKG in $PACKAGES; do
        echo
        # First check that $PKG is actually a Makefile target
        # See https://trac.sagemath.org/ticket/25078
        if ! echo "$ALL_TARGETS" | grep "^${PKG}$" >/dev/null; then
            echo >&2 "Error: package '$PKG' not found"
            echo >&2 "Note: if it is an old-style package, use -p instead of -i to install it"
            exit 1
        fi
        $MAKE SAGE_SPKG="sage-spkg $INSTALL_OPTIONS" "$PKG"
    done

There are some bits here that obscure things, but ultimately the meat of the command that it's running to install the package is

make <pkgname>

or to use a real example

make mpir

In this case the Makefile target that's just the package name itself is a "phony target": It does not correspond to a file that is expected to exist (e.g. same as all or clean). The thing about phony targets is that when you run them their rules are just followed no matter what (there's no output file to check whether it's already up-to-date).

However, phony targets can still have prerequisites, and when running a phony target it will still follow the usual rules of ensuring that all the prerequisites exist and are up-to-date.

In build/make/Makefile the recipes for <pkgname> phony targets are simple and boil down to (in the case of mpir):

mpir: local/var/lib/sage/installed/mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0

i.e. the "stamp file" that is normally created when an SPKG is installed, indicating that that SPKG is installed and up-to-date.

However, if the mpir SPKG was not installed (or same for any other package that was not installed by default, such as git), then this file does not exist. That's because the "stamp file" for mpir as far as the rest of the Makefile is concerned is local/var/lib/sage/installed/.dummy--just an empty file that is created once (if it doesn't exist) and is otherwise always "up-to-date".

Meanwhile, the make recipe for building the real stamp file at local/var/lib/sage/installed/mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0 boils down to (when you strip away some of the noise like sage-logger)

sage-spkg mpir

which actually installs the Sage SPKG, and which also create the "stamp file" at local/var/lib/sage/installed/mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0 indicating that now the actual mpir SPKG is installed.

This is why, as Jeroen pointed out, it's not strictly necessary to re-run ./configure first, because even if mpir is a "dummy package" in the Makefile, running make mpir will forcibly install the SPKG anyways.

We still would need to re-run ./configure afterwards and ensure that when build/make/Makefile is re-generated it no longer includes mpir in its list of DUMMY_PACKAGES. If it does, then that's a bug that we need to correct.

It might be easier to understand the whole thing if DUMMY_PACKAGES were called something else like DEPENDENCIES_THAT_ARE_NOT_NEEDED_OR_ARE_OTHERWISE_SATISFIED_BY_THE_SYSTEM.

comment:43 Changed 2 years ago by git

  • Commit changed from 9f02d15aab404c0c5473c6c3f24da0210375a345 to f86e16d723cf7c20e721aa458ce1e0b7bc1ced64

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

8435b90Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
3907206Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
dcb676dReworked this a bit more
23209dcfix typo
4621d18added a bit more explanation
503655acorrect logic for SAGE_GMP_PREFIX etc
837b38aadd the AX_ABSOLUTE_HEADER macro
154fed0iml in particular is very picky about being given an absolute path to the
38120ccset up frozen packages - only reinstallable via ./configure
f86e16ddon't mix NORMAL and BUILT, and $(info...) with @echo

comment:44 Changed 2 years ago by dimpase

Thanks, I've got it working. Now an attempt to do ./sage -i gmp if gmp/mpir was configured to come from the system ends with

***********************************************
 gmp must be installed by re-running 
./configure 
--with-X=gmp first, where X=mp for gmp gmp or mpir, and X=gmp otherwise.
***********************************************

One minor nitpick I still need to fix (how?) is to correctly do

@echo ./configure '$(./config.status --config)'

to show the whole string...

comment:45 Changed 2 years ago by git

  • Commit changed from f86e16d723cf7c20e721aa458ce1e0b7bc1ced64 to df478e1741c82431c8c5407119194d9012fe5786

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

0ca3f56fix typo
06798caadded a bit more explanation
b592d77correct logic for SAGE_GMP_PREFIX etc
5057680add the AX_ABSOLUTE_HEADER macro
101537biml in particular is very picky about being given an absolute path to the
862ca6aMerge remote-tracking branch 'trac/develop' into HEAD
7e2b100Remove broken SAGE_BUILD_TOOLCHAIN support
03dc987Merged trac #27215 in
b472f0fMerge public/packages/gmp_m4, bump to Sage 8.6.beta7
df478e1error message with special-casing MP_LIBRARY

comment:46 Changed 2 years ago by dimpase

  • Dependencies set to #27212
  • Description modified (diff)
  • Status changed from needs_info to needs_review

OK, I think I am happy with this branch. Please review. I think it's a reasonable compromise to have a clear error here instead of implementing lots of --with-...= things just to cover for a very unlikely case (I hope no-one uses sage -i in scripts...).


(one little thing is that I don't know how to print $ instead of <dollar sign> in the error message, please advise if you think this is important enough)

comment:47 follow-up: Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Dima, could you please explain again (preferably in the ticket description) what you're trying to accomplish here, in other words: which problem does this solve? I don't see any good reason why ./sage -i mpir should be disallowed.

Last edited 2 years ago by jdemeyer (previous) (diff)

comment:48 in reply to: ↑ 47 Changed 2 years ago by dimpase

Replying to jdemeyer:

Dima, could you please explain again (preferably in the ticket description) what you're trying to accomplish here? I don't see any good reason why ./sage -i mpir should be disallowed.

I think I have explained this once already. The reason for this ticket is that the branch of #27212 introduces a breakage into ./sage -i mpir/gmp, ASSUMING Sage is configured to use an external $MP_LIBRARY. So I am fixing this here. And it's not really "disallowed", it's "do this and this, then ./sage -i mpir/gmp".

If Sage is configured with its own gmp/mpir then ./sage -i gmp/mpir will continue to work as before, also with this branch.

comment:49 follow-up: Changed 2 years ago by jdemeyer

OK, but I don't like your solution because:

  1. It does break sage -i mpir
  1. It's more complicated than just rerunning ./configure. In particular, it forces adding a --with-package flag for every package for which you want to implement this trick (for GMP/MPIR we already had a --with-mp flag but I assume that you want to generalize #27212 to other packages).

comment:50 follow-up: Changed 2 years ago by jdemeyer

Why did you merge the branch of #27212 in this ticket? Was that intentional?

comment:51 follow-up: Changed 2 years ago by jdemeyer

Actually, you probably don't need to reconfigure at all. In sage-env-config, you could just check whether Sage's GMP/MPIR is installed:

if [ -r "$SAGE_LOCAL/include/gmp.h" ]; then
    # Use Sage GMP/MPIR
else
    # Use system GMP/MPIR
fi

comment:52 in reply to: ↑ 50 Changed 2 years ago by dimpase

Replying to jdemeyer:

Why did you merge the branch of #27212 in this ticket? Was that intentional?

Of course, there is no point in this ticket without #27212.

comment:53 in reply to: ↑ 49 ; follow-up: Changed 2 years ago by dimpase

Replying to jdemeyer:

OK, but I don't like your solution because:

  1. It does break sage -i mpir

your proposal, running ./configure after sage -i mpir, does not really fly, as sage -i mpir triggers a rebuild of its dependencies without proper changes in the environment, so the dependencies will get rebuild against the external "mpir/gmp".

Running ./configure before sage -i mpir, as Erik proposed, is safer, but needs more of these --with-foo= than we currently have.

  1. It's more complicated than just rerunning ./configure. In particular, it forces adding a --with-package flag for every package for which you want to implement this trick (for GMP/MPIR we already had a --with-mp flag but I assume that you want to generalize #27212 to other packages).

Yes, but there are not too many packages that modify (or will need to modify) sage-env-config.

These are packages (say, gmp) that for historical or other reasons spawned a cottage industry of custom-installated headers and libraries, so that packages that use then do --with-gmp=, or/and --with-gmp-headers=...

A list may be compiled by looking at the output of grep \\-with\\- $SAGE_ROOT/build/pkgs/*/spkg-install. I get blas/lapack/openblas, cdd(lib), flint, glpk, gmp, mpfr, mpir, isl, ntl, pari, polylib, readline, zlib.

However this can be further pruned, as e.g. --with-zlib=$SAGE_LOCAL is harmless, in the sense that if extrenal zlib is used, this option does not break anything. In this sense, despite already allowing several external libraries replacements for the Sage-vendored ones (see #27330, we already can have external bzip2, libffi, lzma, zlib, gf2x, no problems it seems) gmp/mpir are the 1st that really need this special treatment.

comment:54 in reply to: ↑ 51 Changed 2 years ago by dimpase

Replying to jdemeyer:

Actually, you probably don't need to reconfigure at all. In sage-env-config, you could just check whether Sage's GMP/MPIR is installed:

if [ -r "$SAGE_LOCAL/include/gmp.h" ]; then
    # Use Sage GMP/MPIR
else
    # Use system GMP/MPIR
fi

If you look at the branch of #27212, at the end of sage-env-config.in you will see

# This is usually blank if the system GMP is used, or $SAGE_LOCAL otherwise
export SAGE_GMP_PREFIX="@SAGE_GMP_PREFIX@"
export SAGE_GMP_INCLUDE="@SAGE_GMP_INCLUDE@"
if [ -n "$SAGE_GMP_PREFIX" ]; then
    # Many packages that depend on GMP accept a --with-gmp=<prefix> flag to
    # their ./configure scripts.  When using the system's GMP this is not
    # generally necessary, but when using the GMP package installed in
    # SAGE_LOCAL it is useful to pass it.  We define this variable to
    # pass to these packages' ./configure scripts.  When using the system
    # GMP its value is just blank (for many of these packages passing
    # --with-gmp without an argument is actually a bug)
    export SAGE_CONFIGURE_GMP="--with-gmp=$SAGE_GMP_PREFIX"
fi

In general, these two variables SAGE_GMP_PREFIX, SAGE_GMP_INCLUDE, can be non-empty. Indeed, their values can be overridden by your check. OK, I have to think about it.

comment:55 in reply to: ↑ 53 Changed 2 years ago by jdemeyer

Replying to dimpase:

sage -i mpir triggers a rebuild of its dependencies without proper changes in the environment, so the dependencies will get rebuild against the external "mpir/gmp".

I'm confused what you mean here: are you talking about dependencies of mpir or packages depending on mpir? sage -i mpir installs dependencies of mpir (which is only yasm) but for the dependencies, it doesn't matter how mpir is configured.

comment:56 follow-up: Changed 2 years ago by jdemeyer

In more detail, probably sage-env-config could be modified as follows:

if [ -r "$SAGE_LOCAL/include/gmp.h" ]; then
    # Use Sage GMP/MPIR
    export SAGE_GMP_PREFIX="$SAGE_LOCAL"
    export SAGE_GMP_INCLUDE="$SAGE_LOCAL"
else
    # Use system GMP/MPIR
    export SAGE_GMP_PREFIX="@SAGE_GMP_PREFIX@"
    export SAGE_GMP_INCLUDE="@SAGE_GMP_INCLUDE@"
fi
if [ -n "$SAGE_GMP_PREFIX" ]; then
    # Many packages that depend on GMP accept a --with-gmp=<prefix> flag to
    # their ./configure scripts.  When using the system's GMP this is not
    # generally necessary, but when using the GMP package installed in
    # SAGE_LOCAL it is useful to pass it.  We define this variable to
    # pass to these packages' ./configure scripts.  When using the system
    # GMP its value is just blank (for many of these packages passing
    # --with-gmp without an argument is actually a bug)
    export SAGE_CONFIGURE_GMP="--with-gmp=$SAGE_GMP_PREFIX"
fi

comment:57 Changed 2 years ago by dimpase

I'm testing adding this hack to sage-env-config.in on the branch of #27212:

  • src/bin/sage-env-config.in

    diff --git a/src/bin/sage-env-config.in b/src/bin/sage-env-config.in
    index aa41ba2241..5be5f9b022 100644
    a b if [ "$SAGE_PYTHON_VERSION" = 3 ]; then 
    3636    export SAGE_PYTHON3=yes
    3737fi
    3838
    39 # This is usually blank if the system GMP is used, or $SAGE_LOCAL otherwise
    40 export SAGE_GMP_PREFIX="@SAGE_GMP_PREFIX@"
    41 export SAGE_GMP_INCLUDE="@SAGE_GMP_INCLUDE@"
     39# This is usually blank if the system GMP is used, or $SAGE_LOCAL otherwise.
     40# Note that here we override the settings obtained by ./configure run,
     41# to allow ./sage -i gmp/mpir to be used, see a discussion on #27373.
     42if [ -r "$SAGE_LOCAL/include/gmp.h" ]; then
     43    export SAGE_GMP_PREFIX="$SAGE_LOCAL"
     44    export SAGE_GMP_INCLUDE="$SAGE_LOCAL/include"
     45else # This is usually blank if the system GMP is used
     46    export SAGE_GMP_PREFIX="@SAGE_GMP_PREFIX@"
     47    export SAGE_GMP_INCLUDE="@SAGE_GMP_INCLUDE@"
     48fi
    4249if [ -n "$SAGE_GMP_PREFIX" ]; then
    4350    # Many packages that depend on GMP accept a --with-gmp=<prefix> flag to
    4451    # their ./configure scripts.  When using the system's GMP this is not

While I expect this to work, this means living with an inconsistency between the configuration as given by ./config.status --config and the real one, which takes precedence once this patch above is applied. A run of ./configure, either with the proper parameters, or allowing it look into $SAGE_LOCAL, is needed to fix this inconsistency.

I am uneasy about letting ./configure look into $SAGE_LOCAL, as there could be stale or broken packages there in need of a rebuild, which can trick ./configure.

Although the latter seems attractive going forward, as it would allow pointing not only at $SAGE_LOCAL, but at other non-standard locations with goodies ready to be used, too (useful for . This will require adjusting a number of spkg-configure.m4 files.

comment:58 Changed 2 years ago by dimpase

We were writing comments at the same time...

comment:59 in reply to: ↑ 56 ; follow-up: Changed 2 years ago by embray

Replying to jdemeyer:

In more detail, probably sage-env-config could be modified as follows:

That isn't necessary or helpful, and only makes things even more confusing. I agree with you that I'm -1 on this patch as-is, but for different reasons. My feeling is that rather than error out and tell the user what to do we just do the right thing for them; no big deal.

However, what Dima is trying to say is that we already worked out a good scheme for configuring other packages that depend on GMP/MPIR and this uses some variables that are written to sage-env-config at configure-time, so it is necessary to re-run configure either before or after installing a gmp/mpir SPKG. I just think we should do that automatically.

I also think that the discussion in this ticket is suffering from too many cooks in the kitchen who aren't understanding the fuller picture. It might help if you understood what's going on in #27212, and if Dima understood everything I wrote in comment:42.

comment:60 follow-up: Changed 2 years ago by jdemeyer

Would it make sense to close this ticket and discuss everything at #27212? Especially given that these tickets are so tied together, to the point that both tickets contain changes from both tickets.

comment:61 in reply to: ↑ 59 Changed 2 years ago by embray

Replying to embray:

Replying to jdemeyer:

In more detail, probably sage-env-config could be modified as follows:

That isn't necessary or helpful, and only makes things even more confusing. I agree with you that I'm -1 on this patch as-is, but for different reasons.

To expand on this more, I don't think any of the SAGE_FROZEN_PACKAGES stuff is necessary in the slightest. In fact I don't think solving this problem requires any modifications to the makefile. All that's necessary is to have sage -i re-run configure when appropriate. This could be handled either by the implementation of sage -i itself, or in spkg-install (I think the latter might be more appropriate, because it already contains all the machinery to check whether or not we're installing some SPKG for the first time).

comment:62 in reply to: ↑ 60 Changed 2 years ago by embray

Replying to jdemeyer:

Would it make sense to close this ticket and discuss everything at #27212? Especially given that these tickets are so tied together, to the point that both tickets contain changes from both tickets.

I wouldn't necessarily close this ticket as the problem it's addressing is a real problem to be solved, even if I don't agree with the proposed solution. If you have specific comments on #27212 obviously I would make them there. Though that ticket has already been in pretty good shape for a while now I think, and just needs some final review. Certainly your thoughts would be appreciated.

comment:63 Changed 2 years ago by dimpase

The issue of this ticket is not limited to #27212, other tickets in the pipeline suffer from it too.

I am fine with an autoconf-based solution to this one, as opposed to the one on the branch. We can e.g. include in spkg-configure.m4 checks that make targets mentioned in comment 42 are already there, if a new flag to be set up is on. Should I try this?

comment:64 Changed 2 years ago by dimpase

OK, so this is what I'd like to do

  • set up a new ./configure parameter --installed-packages= that can be empty, or point to $SAGE_LOCAL, or to another Sage build tree (maybe outside of $SAGE_ROOT tree).
  • this parameter is checked by SAGE_SPKG_CONFIGURE, which needs to be modified. If it's empty, skip to the current behaviour.
  • otherwise, assume the parameter equals DIR. Check that DIR/local/var/lib/sage/installed/SPKG_NAME-SPKG_VERSION exists, if not, skip to the current behaviour.
  • otherwise, ignore all the --with-SPKG_NAME= and similar (such as --with-mp=) parameters, and use SPKG_NAME from DIR.
  • individual spkg-configure.m4 typically won't need to do anything new here (but perhaps not always).

The call to ./sage -i/f SPKG_NAME will first install SPKG_NAME as it currently does, and then call ./configure $(./config.status --config) --installed-packages=$SAGE_LOCAL.

The whole thing would have the same effect as ./configure $(./config.status --config) --with-SPKG_NAME=install (assuming --with-SPKG_NAME=install is implemented - it need not be the case) followed by sage -i/f SPKG_NAME.

What worries me here a bit are the timestamps that will be backwards, as normally make is run after ./configure, not before?


What's the opinion of the other kitchen chefs on this grand design?

comment:65 follow-up: Changed 2 years ago by fbissey

I guess it will take some time to digest properly. Nevertheless, first impressions:

  • I don't like --installed-packages= either in name or purpose. From its name I would expect to pass a list of packages rather than a directory. That naming can be fixed.
  • Once you get to the fact that it is a directory, I notice you don't mention the possibility to point it to /usr or /usr/local. You imply a separate prefix where you install a bunch of packages. That's part of the second limitation, you pass only one directory.
  • That points to the fact that it is not quite the way autotools work, you can pass hints, various detection macro allow you to pass installation prefix - for individual packages. However if you have a system set with something like lmod your paths and default flags are supposed to be set properly so that you can detect anything you have loaded in your environment without any hints. That's the way I expect configure to work, if it is in my environment, wherever it is, it will be properly detected and set (unless you have done something very wrong and I have a few horror stories to tell about those).

I am not so worried about time stamps and such. The current system already has make run configure in the first place.

comment:66 in reply to: ↑ 65 Changed 2 years ago by dimpase

Replying to fbissey:

I guess it will take some time to digest properly.

TLDR; one needs to reconcile the results of manual running ./sage -i with the results of ./configure. At the same time, it would provide a handy way to use Sage packages build out of the tree in a "trusted" location, without going through macros checking them (such macros might not work for non-standard locations).

Nevertheless, first impressions:

  • I don't like --installed-packages= either in name or purpose. From its name I would expect to pass a list of packages rather than a directory. That naming can be fixed.

sure, how about --installed-packages_prefix=?

  • Once you get to the fact that it is a directory, I notice you don't mention the possibility to point it to /usr or /usr/local. You imply a separate prefix where you install a bunch of packages. That's part of the second limitation, you pass only one directory.

The standard directories are already taken care of by the spkg-configure.m4 macros, but yes, having more then one "trusted" $SAGE_LOCAL might be useful.

comment:67 Changed 2 years ago by embray

This all sounds way overcomplicated to me in order to solve, what, as I've explained, is not a complicated issue. Now you're throwing into the mix the idea of having multiple arbitrary package installation prefixes. While I'm not opposed to that necessarily in principle, it's a huge can of worms at not all needed in order to solve this problem.

I haven't really had time to work on this the last couple weeks as I've had higher priorities, but perhaps I can take a crack at solving this issue soon. I'm certain it doesn't need to get that complicated or difficult.

comment:68 Changed 2 years ago by dimpase

I'd be very glad you you could fix this, especially as this most probably will need changes in your m4 macros.

comment:69 Changed 2 years ago by embray

  • Milestone changed from sage-8.7 to sage-8.8

Moving all my in-progress tickets to 8.8 milestone.

comment:70 Changed 2 years ago by dimpase

Instead of printing the error ("what to do", basically) and bailing out, as the current branch does here, one can exec that "what to do". (This amounts to a bit more work than launching ./configure from make, but should be doable I guess).

Anyhow, I really would like this to move forward, either this or some other way, for currently this ticket basically stalls #27330.

comment:71 Changed 2 years ago by embray

I plan to work on fixing this next now that #27567 is done (which I may still not even need but I wanted to have it first just in case).

That said, I may be forgetting something again, but I don't really see how this is a blocker to progressing on #27330. Yes, it is a bug. But it's a bug that already exists, and will continue to exist whether or not additional packages get added to configure.

I think it's also not all that severe since it only happens if you do sage -i <some-standard-spkg> which is not all that common of a thing to do, and maybe only of particular interest to Sage developers, or users who are having problems using one of their systems' versions of some package.

So yes it should be fixed ASAP, but I don't personally feel held up by it either.

comment:72 Changed 2 years ago by dimpase

well, it's error-prone to have a bunch of unmerged tickets, it makes reviewing harder, etc.

I am totally with you on it not being severe, and I think a solution provided by the current branch is good enough, but Jeroen has another opinion. And it's a bug introduced in #27212, so does not really "exists" in Sage yet.

comment:73 Changed 2 years ago by embray

There is a different but related bug with sage -i that is independent of any of the spkg-configure stuff.

There is a list in build/make/Makefile (that is generated at configure-time) called OPTIONAL_INSTALLED_PACKAGES. This lists packages of type=optional that happen to be installed at the time configure is run (which it checks, slightly sloppily, by checking of local/var/lib/sage/installed/<pkgname>-* exists; I don't have a better way in mind at the moment). In a default install built from this ticket it has:

# All optional installed packages (triggers the auto-update)
OPTIONAL_INSTALLED_PACKAGES = \
    python2 \

(I'm not fond of the fact that python2 is considered an "optional" package but that's a different issue.)

This list is used primarily in the all-sage target, which ensures that all packges are up-to-date when running make--this is usually run as part of the default make target:

all-sage: \
                sagelib \
                $(STANDARD_PACKAGE_INSTS) \
                $(OPTIONAL_INSTALLED_PACKAGE_INSTS) \
                $(EXTCODE) \
                $(SCRIPTS)

This ensures that if some package is updated, all of its dependent packages are also updated, including optional packages that are not normally part of the sagelib dependencies.

However, when running sage -i on some optional package (for example, I ran sage -i fricas) this list is not immediately updated. I can run make build all day after sage -i and it still won't update. This is because, of course, ./configure needs to be re-run. And after configure it should make sense to re-run make build as well, since some packages' dependencies might have been updated.

comment:74 Changed 2 years ago by embray

The other problem is that re-running configure after installing, say, mpir, doesn't move it from DUMMY_PACKAGES to BUILT_PACKAGES.

This should be done in configure by checking of the spkg is installed (again, if nothing else, by looking for some local/var/lib/sage/installed/mpir-*) and forcing sage_spkg_install_mpir=yes, skipping all other checks.

comment:75 Changed 2 years ago by embray

I think I've got something almost working. It's got another prerequisite ticket but I'll post it tomorrow.

comment:76 Changed 2 years ago by embray

See #27641 for the next bit of prerequisite work needed to fix this. I will shortly be adding another ticket with an actual attempt at a fix.

comment:77 Changed 22 months ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

Tickets still needing working or clarification should be moved to the next release milestone at the soonest (please feel free to revert if you think the ticket is close to being resolved).

comment:78 Changed 19 months ago by dimpase

  • Milestone changed from sage-8.9 to sage-duplicate/invalid/wontfix
  • Status changed from needs_info to positive_review

comment:79 Changed 19 months ago by embray

Why the change of status? Is this fixed / a non-issue now?

comment:80 Changed 19 months ago by dimpase

Probably this should become a task, I don't now. So far I haven't heard any reports that this is a real issue...

comment:81 Changed 19 months ago by embray

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-pending
  • Status changed from positive_review to needs_info
  • Type changed from defect to task

I think it my still be an issue but I should investigate. I have heard some stray reports of people building sage with --with-python=3 and then having everything get messed up as soon as they install an optional package or something.

comment:82 Changed 14 months ago by mkoeppe

I think this will just be a special case of #29113 (Reimplement sage -i SPKG as configure --enable-SPKG && make build).

comment:83 Changed 14 months ago by dimpase

  • Milestone changed from sage-pending to sage-duplicate/invalid/wontfix
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_info to positive_review

further work on this to continue on #29113

comment:84 Changed 6 months ago by slelievre

  • Resolution set to duplicate
  • Status changed from positive_review to closed

comment:85 Changed 6 months ago by slelievre

  • Authors Dima Pasechnik deleted
  • Branch u/dimpase/packages/gmp-config deleted
  • Commit df478e1741c82431c8c5407119194d9012fe5786 deleted
Note: See TracTickets for help on using tickets.