Use probes instead of version/distribution conditionals#572
Conversation
|
This also fixes some strange conditions/comments, for example: In all cases those checks had the opposite effects on EL9. |
|
Another alternative, with the same model as of now, would be to replace the host-derived Because these come from the target kernel's own headers, they fix two of the three weaknesses above: no But it still encodes which version backported what by hand for each RHEL minior release, and it will eventually suffer the same issues as of today, where it's constantly broken. |
|
I was working on this same issue yesterday. Had issues building against the latest AlmaLinux kernel (6.12.0-211.7.4.el10_2.x86_64) due to a backport added in 6.12.0-211.7.3.el10_2, so I also implemented the feature flag concept just like you, but just for the specific backport that was broken for me (your EVDI_HAVE_FB_FORMAT_INFO flag). I got mine working and was going to submit a PR and then noticed this. I just pulled your fork and tested it on that same kernel and sure enough it built fine. I successfully tested compilation on all of the following kernels:
So just wanted to let you know it appears to be compiling fine on the latest AlmaLinux 10 kernels. Thanks for providing this! |
|
Awesome, thank you for the feedback. Testing all the RHEL/CentOS kernels was next on my todo list. @jakub-prussak-synaptics any chance this can be merged? If yes, I'll do all the other tests and paste the results here. Thanks. |
|
I've tested all kernels and made a small adjustment for
|
|
I've also found a way to extract the root filesystem from the Raspberry PI image, so I could test it as a container, so I can add this to the list:
|
|
For those intersted, attached is the log with all the autodetection tests of the kernels listed above. Markging this MR as ready! |
|
That's a lot of work, thank you. We will take a look at it in the near future |
|
I think this is really great and it's working perfectly on my EL systems! There are some errors when testing on non-EL kernels with For example, when running Just to see if it was something weird going on with the |
|
I found out what was causing the issue and submitted a PR to your branch: |
|
I submitted another PR to your branch (based on scaronni#1) that adds the necessary probes to get the build to succeed on kernels 4.15 to 5.9: |
|
Looks very promising. First please make sure it passes, it was not working for me, perhaps you need fixes from Richard. Also please extend ./ci/build_against_kernel script to also build on centos/redhat kernels. So we can validate it in daily builds. |
|
@richgieg thank you. I merged them both. I see also needs a rebase now. Will check the |
|
Everything has been rebased against The last 3 are not RHEL but AlmaLinux, but for the purposes of the test this does not matter. The RHEL sources require a valid subscription to download. |
|
Considering build dependencies and what not, it would probably be easier to have a ci script that builds iterating over containers, but i'll try to wire in those URLs directly. |
|
@synaptics-lspintzyk where do you execute the CI script? Wiring in the RHEL kernels is quite an effort, you must select OpenSSL versions specific to a certain range, some libraries which are patched only on RHEL, etc. Would you accept a replacement for: That accepts the same parameters (so wherever you run this should not be impacted) but uses containers for all builds instead? This also speeds up dramatically by using prebuilt packages, as all the kernel repositories are huge. |
|
I couldn't build the EL9 kernel on my Fedora system at all. I had to resort to an EL9 container with matching |
|
For DKMS i run the very long suite of tests against all these distributions (I keep them up to date): https://github.com/dkms-project/dkms/blob/main/.github/workflows/tests.yml These are not the actual upstream kernels, but you have anyway ~10 years of kernels at your disposal for testing. |
|
We are running ci internally. See Jenskinsfile and Dockerfile it this repo. |
Something I wanted to do for a while. I think the various conditions based on kernel version mixed with kernels that perform backports (mostly RHEL based kernels) is not very maintainable, and parsing
/etc/os-releaseis not very robust with the plethora of derivatives distributions (Oracle Linux, Alma Linux, etc.). Every new CentOS kernel after a RHEL point release requires adding new conditions.This pull request uses the same approach as NVIDIA's modules, that is run a
conftest.shscript that tests for each API instead of relying on manual range/versions kernel definitions plus EL8/EL9/EL10/CENTOS9/CENTOS10/RPI. It's a bit more effort in the initial part but it's more maintainable in the long run, and should not break when a distribution backports something from newer kernels.Instead of guessing an API's presence from a version number or a distro string, we test for it: a small shell helper compiles a tiny snippet against the actual target kernel headers. If it compiles, the API is present.
conftest.shis invoked from the Kbuild stage, where the kernel's exact compiler ($(CC)) and include/cflags are available. For each feature it compiles a probe snippet with-fsyntax-only -Werror=implicit-function-declaration -Werror=incompatible-pointer-types -DMODULEand, on success, emits#define EVDI_HAVE_xxx 1into a generatedevdi_detect.h(failures emit a self-documenting comment instead).-fsyntax-onlyperforms a full parse + type-check without code generation, so it catches missing headers, removed struct members, changed function signatures, renamed/removed functions, and absent macros.evdi_detect.his force-included into every object viaccflags-y, and every object is made to depend on it. It is regenerated on each build but only rewritten when its contents change, so incremental builds and target-kernel switches both behave correctly.Source guards become "intent-revealing" capability checks, e.g.:
It works identically for standalone
make, DKMS, and in-tree builds, and it lets theMakefiledrop the/etc/os-releaseand/proc/cpuinfoparsing entirely.To support a new API difference, add a
compile_testblock toconftest.shand use the resultingEVDI_HAVE_*macro.