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

Move GAP kernel version to configure script #2873

Merged
merged 1 commit into from
Sep 28, 2018

Conversation

ChrisJefferson
Copy link
Contributor

This lifts the GAP kernel version to the configure script, so it can be used during configure and creating the Makefile. The kernel version is used in two new places:

  1. We put it in sysinfo.gap, so it's easier for packages to get it

  2. We put it in the build directory (so mine is now x86_64-pc-linux-gnu-default64-abi4, note the abi4)

  3. is useful when using thee same package directory with multiple versions of GAP (which will be particularly useful in a world where more people use the new package manager...)

  4. is useful for languages (in my case Rust) where we want to create packages and need to know the kernel version, but don't want to have to dig through C header files to find it.

There are two broad things I'm interested in:

A) Does putting something like the major kernel version in the gaparch seem like a good idea, to make it easier for GAPs to co-exist?

B) Does this seem like a sensible way to do it?

@ChrisJefferson ChrisJefferson added topic: build system release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Sep 26, 2018
.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Based on what I see so far, I really think cnf/GAP-KERNEL_MAJOR_VERSION and its MINOR variant should go. Other than that, I am happy with this PR (though I worry slightly about possible confusion from the -abi string in the ARCH -- but it should be fine to revise this later on, should we feel a need, i.e., no need to hold up this PR)

AC_SUBST([gap_kernel_minor_version])

AC_DEFINE_UNQUOTED([GAP_KERNEL_MAJOR_VERSION], $gap_kernel_major_version, [The major version of the kernel ABI])
AC_DEFINE_UNQUOTED([GAP_KERNEL_MINOR_VERSION], $gap_kernel_minor_version, [The minor version of the kernel ABI])
Copy link
Member

Choose a reason for hiding this comment

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

If we do this, we might as well start using this to version the libgap shared library, by passing.Not at all saying this should be done in this PR (it's a bit tricky to get semantically right, though not much if one takes the time to read the explanations on what the arguments mean), just wanted to mention it -- we could file an issue for that idea, then somebody (possibly me) can one day look into it.

configure.ac Outdated Show resolved Hide resolved
Makefile.rules Outdated Show resolved Hide resolved
Makefile.rules Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

Oh, and overall, I think this PR is a very good idea, thank you!

This allow the version to be used in the ABI and allow it to be
more easily accessed by packages without parsing headers
@fingolfin fingolfin merged commit bd03ba1 into gap-system:master Sep 28, 2018
@ChrisJefferson ChrisJefferson deleted the move-kernel-version branch October 4, 2018 18:51
@wucas wucas added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned 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 20, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants