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 a new kernel header gap_all.h for use by package authors instead of compiled.h #3745

Merged
merged 1 commit into from
Nov 15, 2019

Conversation

fingolfin
Copy link
Member

So far, most package kernel extensions #include "compiled.h" in order
to conveniently get access to (almost) all source headers of GAP at once
(which is nice for us because it means we are free to rename headers or
move stuff around between them without breaking kernel extensions).

But the result is rather counterintuitive: if one is not familiar with
this, seeing #include "compiled.h" in code is not at all clear. Also,
the original of compiled.h was to be included by C code generated by
gac, and it contains stuff we'd rather not encourage others to use.

Thus, add a new header gap_all.h containing all #include statements
formerly found in compiled.h -- this resolves all concerns.


Of course it will take time for kernel extensions to switch to this new header, especially if they care about staying compatible with older GAP versions. But the sooner we add this, the sooner such a switch could happen. Hence it would be nice to backport this (any objections).

So far, most package kernel extensions `#include "compiled.h"` in order
to conveniently get access to (almost) all source headers of GAP at once
(which is nice for us because it means we are free to rename headers or
move stuff around between them without breaking kernel extensions).

But the result is rather counterintuitive: if one is not familiar with
this, seeing `#include "compiled.h"` in code is not at all clear. Also,
the original of `compiled.h` was to be included by C code generated by
`gac`, and it contains stuff we'd rather not encourage others to use.

Thus, add a new header `gap_all.h` containing all `#include` statements
formerly found in `compiled.h` -- this resolves all concerns.
@stevelinton
Copy link
Contributor

No objections. It might be worth looking for cases where packages use the other functionality from compiled.h. Some cases might be functionality that should be moved elsewhere, others might be considered errors to fix in the package.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.513% when pulling 130dd7a on fingolfin:mh/gap_all.h into 012238d on gap-system:master.

@DominikBernhardt
Copy link
Contributor

@fingolfin It seems to me we might want to add this to the release notes so people who write kernel extensions are actually aware of this change?

@frankluebeck
Copy link
Member

The change looks ok to me. I have checked that the Browse and EDIM packages work as usual with including gap_all.h instead of compiled.h.

@fingolfin fingolfin added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Nov 15, 2019
@fingolfin
Copy link
Member Author

@stevelinton I am pretty sure we can easily adapt any package which uses something from compiled.h that is not in gap_all.h; and even if not, there is no immediate plan to remove compiled.h, at least not on my side, so package would still keep working.

@DominikBernhardt agreed, so I added the label

@frankluebeck thanks for the test!

@ChrisJefferson ChrisJefferson merged commit b5b9381 into gap-system:master Nov 15, 2019
@stevelinton
Copy link
Contributor

@fingolfin Agreed. My thinking was just that it might point us to things incompiled.h which ought to be somewhere else because they are generally useful.

@fingolfin fingolfin deleted the mh/gap_all.h branch November 15, 2019 15:16
@fingolfin
Copy link
Member Author

Backported to stable-4.11 in commit 35ffd5f

@fingolfin fingolfin changed the title kernel: add new header gap_all.h Add a new kernel header gap_all.h for use by package authors instead of compiled.h Nov 28, 2019
@fingolfin fingolfin added 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 Dec 5, 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
backport-to-4.11-DONE release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants