#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: |
Description (last modified by )
$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 3 years ago by
comment:2 Changed 3 years ago by
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 3 years ago by
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: ↓ 6 Changed 3 years ago by
- 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...
comment:5 Changed 3 years ago by
- Description modified (diff)
comment:6 in reply to: ↑ 4 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
- Owner changed from (none) to dimpase
comment:9 Changed 3 years ago by
- 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:
c4e42e6 | spkg-configure for GMP
|
b070088 | Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
|
3b6eebd | Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
|
b80bf72 | Reworked this a bit more
|
0ca3f56 | fix typo
|
06798ca | added a bit more explanation
|
b592d77 | correct logic for SAGE_GMP_PREFIX etc
|
5057680 | add the AX_ABSOLUTE_HEADER macro
|
101537b | iml in particular is very picky about being given an absolute path to the
|
8473658 | sdh_update_var_env_config, call it to update SAGE_GMP_*
|
comment:10 Changed 3 years ago by
Please review the last commit, the rest are from #27212.
comment:11 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
How about disabling 'sage -i' for dummy packages?
comment:14 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
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
...
comment:18 Changed 3 years ago by
- Commit changed from 84736587b00c0e6fdcc26fe094a25dff85a74989 to f218515b64f81b3b7f964a6c46bbe88622797be6
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0309d8c | spkg-configure for GMP
|
8435b90 | Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
|
3907206 | Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
|
dcb676d | Reworked this a bit more
|
23209dc | fix typo
|
4621d18 | added a bit more explanation
|
503655a | correct logic for SAGE_GMP_PREFIX etc
|
837b38a | add the AX_ABSOLUTE_HEADER macro
|
154fed0 | iml in particular is very picky about being given an absolute path to the
|
f218515 | set up frozen packages - only reinstallable via ./configure
|
comment:19 Changed 3 years ago by
- Description modified (diff)
error reporting of these failing sage -i
needs a bit of work still...
comment:20 Changed 3 years ago by
- 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 3 years ago by
- Status changed from needs_review to needs_work
This totally broke building (make
, or any sage -i
), for some reason...
comment:22 Changed 3 years ago by
- Commit changed from f218515b64f81b3b7f964a6c46bbe88622797be6 to 38120ccdce0dd88519ede9a50cede1bbf6cce8a7
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
38120cc | set up frozen packages - only reinstallable via ./configure
|
comment:23 Changed 3 years ago by
- 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 3 years ago by
- Status changed from needs_work to needs_review
comment:25 Changed 3 years ago by
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: ↓ 27 Changed 3 years ago by
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: ↓ 28 Changed 3 years ago by
comment:28 in reply to: ↑ 27 Changed 3 years ago by
- 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: ↓ 31 Changed 3 years ago by
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: ↓ 32 Changed 3 years ago by
Without this ticket #27212 results in sage -i mpir being installed from sources no matter what.
comment:31 in reply to: ↑ 29 ; follow-up: ↓ 33 Changed 3 years ago by
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 3 years ago by
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: ↓ 35 Changed 3 years ago by
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 3 years ago by
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: ↓ 36 Changed 3 years ago by
Thanks a lot for those clarifications. I couldn't understand much from the ticket description.
So it boils down to basically two issues:
- 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.
- 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 thatmpir
is already installed.
comment:36 in reply to: ↑ 35 ; follow-up: ↓ 37 Changed 3 years ago by
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:
- 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.
- 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 thatmpir
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: ↓ 41 Changed 3 years ago by
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 Makefile
s 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 3 years ago by
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 3 years ago by
- Commit changed from 38120ccdce0dd88519ede9a50cede1bbf6cce8a7 to 9f02d15aab404c0c5473c6c3f24da0210375a345
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
b070088 | Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
|
3b6eebd | Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
|
b80bf72 | Reworked this a bit more
|
0ca3f56 | fix typo
|
06798ca | added a bit more explanation
|
b592d77 | correct logic for SAGE_GMP_PREFIX etc
|
5057680 | add the AX_ABSOLUTE_HEADER macro
|
101537b | iml in particular is very picky about being given an absolute path to the
|
862ca6a | Merge remote-tracking branch 'trac/develop' into HEAD
|
9f02d15 | set up frozen packages - only reinstallable via ./configure
|
comment:40 Changed 3 years ago by
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 3 years ago by
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
Makefile
s should influence the build of mpir. Whether or not mpir is a dummy package shouldn't influence whatsage -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 3 years ago by
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 3 years ago by
- Commit changed from 9f02d15aab404c0c5473c6c3f24da0210375a345 to f86e16d723cf7c20e721aa458ce1e0b7bc1ced64
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
8435b90 | Move all the --with-mp logic into the spkg-configure.m4 for MPIR so that it
|
3907206 | Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
|
dcb676d | Reworked this a bit more
|
23209dc | fix typo
|
4621d18 | added a bit more explanation
|
503655a | correct logic for SAGE_GMP_PREFIX etc
|
837b38a | add the AX_ABSOLUTE_HEADER macro
|
154fed0 | iml in particular is very picky about being given an absolute path to the
|
38120cc | set up frozen packages - only reinstallable via ./configure
|
f86e16d | don't mix NORMAL and BUILT, and $(info...) with @echo
|
comment:44 Changed 3 years ago by
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 3 years ago by
- Commit changed from f86e16d723cf7c20e721aa458ce1e0b7bc1ced64 to df478e1741c82431c8c5407119194d9012fe5786
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
0ca3f56 | fix typo
|
06798ca | added a bit more explanation
|
b592d77 | correct logic for SAGE_GMP_PREFIX etc
|
5057680 | add the AX_ABSOLUTE_HEADER macro
|
101537b | iml in particular is very picky about being given an absolute path to the
|
862ca6a | Merge remote-tracking branch 'trac/develop' into HEAD
|
7e2b100 | Remove broken SAGE_BUILD_TOOLCHAIN support
|
03dc987 | Merged trac #27215 in
|
b472f0f | Merge public/packages/gmp_m4, bump to Sage 8.6.beta7
|
df478e1 | error message with special-casing MP_LIBRARY
|
comment:46 Changed 3 years ago by
- 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: ↓ 48 Changed 3 years ago by
- 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.
comment:48 in reply to: ↑ 47 Changed 3 years ago by
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: ↓ 53 Changed 3 years ago by
OK, but I don't like your solution because:
- It does break
sage -i mpir
- 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: ↓ 52 Changed 3 years ago by
Why did you merge the branch of #27212 in this ticket? Was that intentional?
comment:51 follow-up: ↓ 54 Changed 3 years ago by
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 3 years ago by
comment:53 in reply to: ↑ 49 ; follow-up: ↓ 55 Changed 3 years ago by
Replying to jdemeyer:
OK, but I don't like your solution because:
- 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.
- 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 3 years ago by
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 3 years ago by
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: ↓ 59 Changed 3 years ago by
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 3 years ago by
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 36 36 export SAGE_PYTHON3=yes 37 37 fi 38 38 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. 42 if [ -r "$SAGE_LOCAL/include/gmp.h" ]; then 43 export SAGE_GMP_PREFIX="$SAGE_LOCAL" 44 export SAGE_GMP_INCLUDE="$SAGE_LOCAL/include" 45 else # 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@" 48 fi 42 49 if [ -n "$SAGE_GMP_PREFIX" ]; then 43 50 # Many packages that depend on GMP accept a --with-gmp=<prefix> flag to 44 51 # 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 3 years ago by
We were writing comments at the same time...
comment:59 in reply to: ↑ 56 ; follow-up: ↓ 61 Changed 3 years ago by
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: ↓ 62 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
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: ↓ 66 Changed 3 years ago by
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 expectconfigure
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 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
- Milestone changed from sage-8.7 to sage-8.8
Moving all my in-progress tickets to 8.8 milestone.
comment:70 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
I think I've got something almost working. It's got another prerequisite ticket but I'll post it tomorrow.
comment:76 Changed 3 years ago by
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 3 years ago by
- 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 3 years ago by
- Milestone changed from sage-8.9 to sage-duplicate/invalid/wontfix
- Status changed from needs_info to positive_review
comment:79 Changed 3 years ago by
Why the change of status? Is this fixed / a non-issue now?
comment:80 Changed 3 years ago by
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 3 years ago by
- 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 2 years ago by
I think this will just be a special case of #29113 (Reimplement sage -i SPKG
as configure --enable-SPKG && make build
).
comment:83 Changed 2 years ago by
- 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 20 months ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
comment:85 Changed 20 months ago by
- Branch u/dimpase/packages/gmp-config deleted
- Commit df478e1741c82431c8c5407119194d9012fe5786 deleted
We can also merely document that
./sage -i blah
must be run after./configure --with-blah=install
, perhaps also insert some warning print somewhere...