Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dimpase opened this issue Feb 27, 2019 · 96 comments
Closed

Comments

@dimpase
Copy link
Member

dimpase commented Feb 27, 2019

$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
***********************************************

Depends on #27212

CC: @embray

Component: build

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/27373

@dimpase dimpase added this to the sage-8.7 milestone Feb 27, 2019
@dimpase
Copy link
Member Author

dimpase commented Feb 27, 2019

comment:1

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

@dimpase
Copy link
Member Author

dimpase commented Feb 28, 2019

comment:2

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.

@dimpase
Copy link
Member Author

dimpase commented Feb 28, 2019

comment:3

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.

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

comment:4

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

@dimpase

This comment has been minimized.

@dimpase

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Mar 1, 2019

comment:6

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.

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

comment:7

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

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

Author: Dima Pasechnik

@dimpase dimpase self-assigned this Mar 1, 2019
@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

Commit: 8473658

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

Branch: u/dimpase/packages/gmp-config

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

comment:9

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_*

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

comment:10

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

@dimpase
Copy link
Member Author

dimpase commented Mar 1, 2019

comment:11

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?

@embray
Copy link
Contributor

embray commented Mar 5, 2019

comment:12

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

@dimpase
Copy link
Member Author

dimpase commented Mar 5, 2019

comment:13

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

@embray
Copy link
Contributor

embray commented Mar 5, 2019

comment:14

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 to use the Sage SPKG instead".

The only problem is that for most packages we don't have the appropriate "". 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.

@embray
Copy link
Contributor

embray commented Mar 5, 2019

comment:15

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.

@dimpase
Copy link
Member Author

dimpase commented Mar 8, 2019

comment:16

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

@dimpase
Copy link
Member Author

dimpase commented Mar 8, 2019

comment:17

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2019

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2019

Changed commit from 8473658 to f218515

@dimpase
Copy link
Member Author

dimpase commented Mar 9, 2019

comment:19

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

@dimpase

This comment has been minimized.

@embray
Copy link
Contributor

embray commented Apr 11, 2019

comment:76

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.

@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:77

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

@embray embray modified the milestones: sage-8.8, sage-8.9 Jun 14, 2019
@dimpase dimpase removed this from the sage-8.9 milestone Sep 7, 2019
@embray
Copy link
Contributor

embray commented Sep 11, 2019

comment:79

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

@dimpase
Copy link
Member Author

dimpase commented Sep 11, 2019

comment:80

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

@embray
Copy link
Contributor

embray commented Sep 18, 2019

comment:81

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.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 7, 2020

comment:82

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

@dimpase
Copy link
Member Author

dimpase commented Feb 8, 2020

comment:83

further work on this to continue on #29113

@dimpase
Copy link
Member Author

dimpase commented Feb 8, 2020

Reviewer: Dima Pasechnik

@slel
Copy link
Member

slel commented Oct 11, 2020

Changed commit from df478e1 to none

@slel
Copy link
Member

slel commented Oct 11, 2020

Changed author from Dima Pasechnik to none

@slel
Copy link
Member

slel commented Oct 11, 2020

Changed branch from u/dimpase/packages/gmp-config to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants