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

Improve buildsystem by exporting GMP_PREFIX in sysinfo.gap for package build systems #5079

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

fingolfin
Copy link
Member

Resolves #5072

@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: build system labels Oct 4, 2022
@fingolfin fingolfin changed the title Export GMP_PREFIX in sysinfo.gap for package build systems Improve buildsystem by exporting GMP_PREFIX in sysinfo.gap for package build systems Oct 4, 2022
Copy link
Contributor

@RussWoodroofe RussWoodroofe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks generally sensible to me. I like that it documents the circumstance under which GMP_PREFIX will be empty. I'm not exactly sure what should happen under the "make install" situation (it seems possible that there may be some unforeseen consequences). Since "make install" is not yet a going concern, this seems academic for now.

@fingolfin
Copy link
Member Author

fingolfin commented Oct 4, 2022

I'm not exactly sure what should happen under the "make install" situation (it seems possible that there may be some unforeseen consequences).

Not sure what you mean (EDIT: as in: I am not sure how to interpret the above, which certainly is my fault; I'll try my best to interpret it and give answers to my guesses what you may have meant, sorry if I guessed wrong) -- as documented, the version of sysinfo.gap produced by make install always has GMP_PREFIX="" in it. So then packages have to resort to the usual means for locating GMP (i.e., expecting it to be available in the default compilers search paths and/or for the user to use --with-gmp=PREFIX and/or for suitable CPPFLAGS and LDFLAGS to be set up.

Or perhaps you were wondering if there are other values GMP_PREFIX could be set to when doing make install? Well, it for sure must not be set to point to a copy of GMP built by GAP and installed in the GAP build dir (indeed, in that case, it'd probably best if make install resulted in an error, but that's something else). In most cases I'd expect it to be left empty on the GAP side anyway (as e.g. the Debian package surely would build GAP against the GMP installed in /usr anyway, and that's the default prefix, so no need to specify it). That leaves only "weird" situations -- in those cases I prefer to err on the side of caution and not export a potentially "dangerous" path (in the sense that it could later on cause trouble when trying to build packages against that installed GAP).

@fingolfin
Copy link
Member Author

Demo PR for nq to use this: gap-packages/nq#24

@fingolfin
Copy link
Member Author

If nobody minds (@ChrisJefferson ?) I'd like to backport this for 4.12.1 -- while it's not strictly a bugfix, it's a minor thing that'll be useful for packages and users, and the sooner we get it into a release, the sooner it can have a beneficial effect. At the same time, I don't see how it could have any negative impact anywhere (of course that's far from meaning there is no opportunity for that happening... but at least I made sure to check that no package uses a variable named GMP_PREFIX for anything else right now, so at least in that regard we should be good)

@ChrisJefferson
Copy link
Contributor

looks good to me

@fingolfin fingolfin merged commit 90ff36a into gap-system:master Oct 4, 2022
@fingolfin fingolfin deleted the mh/GMP_PREFIX branch October 4, 2022 19:32
@fingolfin
Copy link
Member Author

Backported to stable-4.12 via commit 6a63779

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.12-DONE release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GAP 4.12.0 doesn't seem to export the location of its copy of GMP
3 participants