Fix CC detection #334
Fix CC detection #334
Conversation
|
Thank you and congrats on the Debian 12 release. Looking forward to a new container once it becomes available. Will check this PR later today/tomorrow, but as a whole can you glance at #298 and comment on it. Do you see it working (reliably enough) for Debian, can we nuke |
| @@ -61,7 +61,7 @@ jobs: | |||
| if: matrix.distro.name == 'debian' | |||
| run: | | |||
| apt-get update -q | |||
| apt-get install -qy gcc make linux-headers-amd64 linux-image-amd64 openssl | |||
| apt-get install -qy make linux-headers-amd64 linux-image-amd64 openssl | |||
There was a problem hiding this comment.
Interesting - Arch and Fedora (IIRC) don't pull the compiler stack, when the headers are installed.
Should we do the same change in the Ubuntu case below?
There was a problem hiding this comment.
Already tried, the gcc dependency is required at least for older Ubuntu releases.
There was a problem hiding this comment.
Ack, please mention that in the commit message. For non Debian/Ubuntu experts such variations are non-obvious. Thanks
There was a problem hiding this comment.
jfyi @xnox is the above variation, something Ubuntu team is willing to address? I don't have a strong opinion, just making sure devs in both distros are aware.
There was a problem hiding this comment.
even i don't know or understand if and when and why we have compiler depedency. i think in ubuntu we add "Package: dkms Depends: gcc" because we don't have dependency from headers to the compiler either (similarish to fedora). But we also have different compilers used for different kernels, thus maybe our headers should pull in correct compiler in.....
| export CC="$cc" | ||
| export KERNEL_CC="$cc" | ||
| fi | ||
|
|
||
| if grep -q 'CONFIG_CC_IS_CLANG=y' "${kernel_config}"; then |
There was a problem hiding this comment.
The hunk was removed by @hadogenes with 4729aef. *Reintroducing it seems correct at a glance.
@hadogenes can you confirm this doesn't regress on your end? Which Suse variant/version are you using - SLES, Leap, Tumbleweed? Thanks
There was a problem hiding this comment.
clang detection is probably wrong if versioned clang-xx is to be used, but as long as we don't have a clang compiled kernel in Debian, I'm not going to investigate
anbe42
force-pushed
the
fix-CC-detection
branch
4 times, most recently
from
4e357f1 to
d9c94ce
Compare
where the .config based compiler detection happens, ensures that the compiler is set for PRE_BUILD
Will probaly need to be rebased after the current fixes got in. Will check in depth later.
I'd like to keep .kernelvariables for supporting older packaged kernels, but it should be remodeled and only used as a fallback if we cannot get the information from .config |
Debian (but not Ubuntu) builds its kernels with a versioned compiler binary (gcc-XX) and the linux-headers-* packages depend on that compiler
if for some reason $arch cannot be determined (e.g. due to a broken rpm shim on distributions using a different package manager), dkms will happily build a module, placing state info in /var/lib/dkms/$module/$version/$kernel//, but that subsequently causes errors because the module is not considered to be installed and at the same time cannot be removed any more Before: sid# dkms status sid# ln -s /bin/true /usr/bin/rpm sid# dkms add dkms_test/1.0 Creating symlink /var/lib/dkms/dkms_test/1.0/source -> /usr/src/dkms_test-1.0 sid# dkms status dkms_test/1.0: added sid# dkms build -k 6.1.0-9-amd64 dkms_test/1.0 Sign command: /usr/lib/linux-kbuild-6.1/scripts/sign-file Signing key: /var/lib/dkms/mok.key Public certificate (MOK): /var/lib/dkms/mok.pub Building module: Cleaning build area... make -j16 KERNELRELEASE=6.1.0-9-amd64 -C /lib/modules/6.1.0-9-amd64/build M=/var/lib/dkms/dkms_test/1.0/build... Signing module /var/lib/dkms/dkms_test/1.0/build/dkms_test.ko Cleaning build area... sid# dkms status dkms_test/1.0: added sid# dkms build -k 6.1.0-9-amd64 dkms_test/1.0 Sign command: /usr/lib/linux-kbuild-6.1/scripts/sign-file Signing key: /var/lib/dkms/mok.key Public certificate (MOK): /var/lib/dkms/mok.pub Error! This module/version has already been built on: 6.1.0-9-amd64 Directory /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64/ already exists. Use the dkms remove function before trying to build again. sid# dkms status dkms_test/1.0: added sid# dkms remove -k 6.1.0-9-amd64 dkms_test/1.0 Module dkms_test 1.0 is not installed for kernel 6.1.0-9-amd64 (). Skipping... Module dkms_test 1.0 is not built for kernel 6.1.0-9-amd64 (). Skipping... sid# dkms status dkms_test/1.0: added sid# dkms remove dkms_test/1.0 --all sid# dkms status dkms_test/1.0: added sid# find /var/lib/dkms/dkms_test/1.0/ /var/lib/dkms/dkms_test/1.0/ /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64 /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64/module /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64/module/dkms_test.ko /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64/log /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64/log/make.log /var/lib/dkms/dkms_test/1.0/6.1.0-9-amd64/log/.config /var/lib/dkms/dkms_test/1.0/source After: sid# dkms status sid# ln -s /bin/true /usr/bin/rpm sid# dkms add dkms_test/1.0 Error! Could not determine architecture. sid# echo $? 12 to manually recover from the bug use rm -rf /var/lib/dkms/$module/$version and add/build/install the module(s) again
There was a problem hiding this comment.
Thanks for the updates and feedback regarding the other PR.
This series is good to land, but let's give @hadogenes a few days to a week to reply. I had a quick try at adding Suse CI but it seems fiddly, so could not personally test the relevant change for regressions.
The recent clang detection improvements broke getting a non-clang compiler from the kernel config.
Also move .kernelvariables parsing to prepare_build() where the other CC handling is.
On Debian there is no need for gcc, the header package has the required compiler dependency.
We had an interesting bug report on Debian (https://bugs.debian.org/1036033) where a broken rpm shim caused weird errors in dkms after a module was added and built with an empty $arch. So error out early if $arch cannot be determined, before misplacing stuff in /var/lib/dkms.
PS: Debian 12 (bookworm) was released yesterday, but there is no container image, yet, otherwise I would have added that to the matrix.