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

Add support for make install (still somewhat experimental, but ready to be tested by downstream packagers) #4492

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented May 17, 2021

I wrote this a year ago and never finished. I figured I should post it here, to (a) remind me about it and (b) see if anybody has feedback, or even (c) is willing to help.

Help could range from trying it out and reporting issues as they are encountered (I am sure there are some, perhaps even many, as I didn't finish this), over suggesting things to actually implementing improvements.

Resolves #197

Some TODOs I do recall are these (and the patch also contains some):

  • add a CI test for this feature
    • i.e., make install GAP somewhere, then try to run it, and also try to build some select GAP packages against it
    • also test make install DESTDIR=/tmp/or/whatever to check that DESTDIR support works (see the automake manual for more on DESTDIR)
    • writing a CI test probably should be the first thing to do when continuing this PR, as it'd surely reveal issues, which then could be fixed in tandem with finishing the CI test
  • deal with config.h
    • see the FAQ entry in README.buildsys.md for some more details, and feel free to ask me for more if you are interested
      • actually I made a lot of progress on removing need for config.h from most headers; but there are some hold outs: actual configuration choices (see next point); the kernel version (see point after the next); and SYS_IS_64_BIT
      • so we still may want to generate a gap-config.h for things like HPCGAP or USE_GASMAN or USE_JULIA_GC. Then again, ideally we may want to share the same headers between regular GAP, HPC-GAP, Julia GAP, ... which would suggest that it'd be better to add these to GAP_CPPFLAGS (so it'd end up in sysinfo.gap, which would exist separately for each build variant of GAP; and would also be automatically used by gac this way
      • but GAP_KERNEL_MAJOR_VERSION and GAP_KERNEL_MINOR_VERSION should still be common, so perhaps that could be put into a specific header after all...
      • the definition of SYS_IS_64_BIT still relies on SIZEOF_VOID_P. But I worked hard on not using it in headers, and only one use (in intobj.h is left). But of course packages use (cvec, datastructures, NormalizInterface). I guess this is another thing we could (should?) define in GAP_CPPFLAGS.... Or perhaps we can use a trick as in https://stackoverflow.com/a/32717129/928031
    • but for starters, we could just install it "as is", that should be OK for many use cases and surely beats not having make install at all
  • deal with gac
  • deal with the case were we are using bundled GMP/Boehm/libatomic/... -- presumably just error then? Or maybe install them? Or ...?
  • document make install (including how it interacts with the configure --prefix and DESTDIR -- can be short and point to the automake manual, as this is mostly for packagers who generally are familiar with this)
  • ... more... I'll add it as I think of it

@fingolfin fingolfin added topic: build system release notes: added PRs introducing changes that have since been mentioned in the release notes labels May 17, 2021
@wilfwilson wilfwilson added release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes and removed release notes: added PRs introducing changes that have since been mentioned in the release notes labels Jun 10, 2021
@fingolfin fingolfin force-pushed the mh/install branch 2 times, most recently from 9e88023 to 4df8fbb Compare October 18, 2021 09:53
@fingolfin fingolfin force-pushed the mh/install branch 3 times, most recently from 7aeb5bd to 4f542f5 Compare October 25, 2021 21:50
@fingolfin
Copy link
Member Author

I've made major progress towards getting rid of the need to installing config.h in PRs #4678 and #4674. With these, this PR now can finally get rid of all includes of config.h in headers!

This mainly leaves gac and of course CI tests.

dev/audit-config-h.sh Outdated Show resolved Hide resolved
@fingolfin fingolfin force-pushed the mh/install branch 4 times, most recently from 92a4aca to bfb0986 Compare November 4, 2021 14:21
@fingolfin fingolfin force-pushed the mh/install branch 2 times, most recently from 7c8ba2d to 11e1794 Compare April 22, 2022 11:00
@fingolfin fingolfin mentioned this pull request Jul 9, 2022
3 tasks
@fingolfin fingolfin force-pushed the mh/install branch 2 times, most recently from 1270aca to a7b9c8a Compare July 11, 2022 20:43
@fingolfin fingolfin changed the title buildsys: add make install (WIP) buildsys: activate make install, add CI test for it Jul 11, 2022
@fingolfin fingolfin added the release notes: highlight PRs introducing changes that should be highlighted at the top of the release notes label Jul 11, 2022
@fingolfin fingolfin force-pushed the mh/install branch 7 times, most recently from b6794b1 to a005eba Compare July 14, 2022 10:29
This only covers installing GAP itself, so the library, kernel, libgap;
but not packages.
@fingolfin fingolfin marked this pull request as ready for review July 14, 2022 12:05
@fingolfin fingolfin merged commit 3a88c31 into gap-system:master Jul 14, 2022
@fingolfin fingolfin deleted the mh/install branch July 14, 2022 13:27
@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 17, 2022
@fingolfin fingolfin changed the title buildsys: activate make install, add CI test for it Add support for make install (still somewhat experimental, but ready to be tested by downstream packagers) Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: highlight PRs introducing changes that should be highlighted at the top of the release notes 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.

Makefile lacks 'install' target
3 participants