| Message ID | 20230227144953.935987-2-dimitri.ledkov@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [lunar:linux,lunar:linux-unstable,v2,1/3] UBUNTU: [Packaging] Support skipped dkms modules | expand |
On Mon, Feb 27, 2023 at 02:49:51PM +0000, Dimitri John Ledkov wrote: > In recent releases we have fixed up most dkms modules to correctly > detect, and skip building dkms modules when compiling against > incompatible kernels. For example, trying to build a WiFi driver > against a kernel without 80221 config enabled, results in dkms > returning error code 9 skipped build, due to incompatibility. Our DKMS > autopkgtests already honor such cases, and mark test results as > passing for skipped {kernel, dkms-module} combinations. > > Improve kernel's dkms-build script to distinguish failed versus > skipped builds, by propagating exact status code. For standalone > skipped dkms modules, skip generating an empty deb packages. > > Also do not fail over missing make.log for the skipped dkms builds, > which do not have one. > > This change will enable us to use a static list of dkms-versions > across all kernels, without need to do anything special for cases were > a vendored dkms module is declared incompatible in dkms.conf for a > given kernel. > > For example, v4l2loopback incompatible with cloud kernels that do not > enable CONFIG_VIDEO, or backports-iwlwifi incompatible with kvm > kernel. > > This patch is tested against working, skipepd, and FTBFS dkms modules, > which correctly built the full kernel, skipped some dkms modules, or > failed the overall build. > > Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> > --- > debian/rules.d/2-binary-arch.mk | 3 ++- > debian/scripts/dkms-build | 5 ++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 4cebe66dfd..4d393e890e 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -71,7 +71,7 @@ define build_dkms_sign = > ) > endef > define build_dkms = > - ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) > + rc=0; ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) || rc=$$?; if [ "$$rc" = "9" ]; then echo do_$(4)_$*=false >> $(builddir)/skipped-dkms.mk; rc=0; fi; if [ "$$rc" != "0" ]; then exit $$rc; fi What does rc==9 mean? It seems to be the "skip case, right? Maybe a little comment here explaining the meaning of this special value would be helpful. Everything else looks good to me. -Andrea
On Tue, 7 Mar 2023 at 09:46, Andrea Righi <andrea.righi@canonical.com> wrote: > > On Mon, Feb 27, 2023 at 02:49:51PM +0000, Dimitri John Ledkov wrote: > > In recent releases we have fixed up most dkms modules to correctly > > detect, and skip building dkms modules when compiling against > > incompatible kernels. For example, trying to build a WiFi driver > > against a kernel without 80221 config enabled, results in dkms > > returning error code 9 skipped build, due to incompatibility. Our DKMS > > autopkgtests already honor such cases, and mark test results as > > passing for skipped {kernel, dkms-module} combinations. > > > > Improve kernel's dkms-build script to distinguish failed versus > > skipped builds, by propagating exact status code. For standalone > > skipped dkms modules, skip generating an empty deb packages. > > > > Also do not fail over missing make.log for the skipped dkms builds, > > which do not have one. > > > > This change will enable us to use a static list of dkms-versions > > across all kernels, without need to do anything special for cases were > > a vendored dkms module is declared incompatible in dkms.conf for a > > given kernel. > > > > For example, v4l2loopback incompatible with cloud kernels that do not > > enable CONFIG_VIDEO, or backports-iwlwifi incompatible with kvm > > kernel. > > > > This patch is tested against working, skipepd, and FTBFS dkms modules, > > which correctly built the full kernel, skipped some dkms modules, or > > failed the overall build. > > > > Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> > > --- > > debian/rules.d/2-binary-arch.mk | 3 ++- > > debian/scripts/dkms-build | 5 ++--- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > index 4cebe66dfd..4d393e890e 100644 > > --- a/debian/rules.d/2-binary-arch.mk > > +++ b/debian/rules.d/2-binary-arch.mk > > @@ -71,7 +71,7 @@ define build_dkms_sign = > > ) > > endef > > define build_dkms = > > - ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) > > + rc=0; ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) || rc=$$?; if [ "$$rc" = "9" ]; then echo do_$(4)_$*=false >> $(builddir)/skipped-dkms.mk; rc=0; fi; if [ "$$rc" != "0" ]; then exit $$rc; fi > > What does rc==9 mean? It seems to be the "skip case, right? Maybe a > little comment here explaining the meaning of this special value would > be helpful. > In our case it is: # Error out if build_exclude is set [[ $build_exclude ]] && die 9 \ $"The $base_dir/dkms.conf for module $module includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config."\ $"This indicates that it should not be built." However, exit code 9 can have other meanings too, w.r.t. missing dkms.conf and permissions when making precompiled tarballs. I think this is ok for now, but i will contact upstream about making error codes unique / fixed. > Everything else looks good to me. > > -Andrea
On Thu, Mar 09, 2023 at 02:13:58PM +0000, Dimitri John Ledkov wrote: ... > > > define build_dkms = > > > - ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) > > > + rc=0; ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) || rc=$$?; if [ "$$rc" = "9" ]; then echo do_$(4)_$*=false >> $(builddir)/skipped-dkms.mk; rc=0; fi; if [ "$$rc" != "0" ]; then exit $$rc; fi > > > > What does rc==9 mean? It seems to be the "skip case, right? Maybe a > > little comment here explaining the meaning of this special value would > > be helpful. > > > > In our case it is: > > # Error out if build_exclude is set > [[ $build_exclude ]] && die 9 \ > $"The $base_dir/dkms.conf for module $module includes a > BUILD_EXCLUSIVE directive which does not match this > kernel/arch/config."\ > $"This indicates that it should not be built." > > However, exit code 9 can have other meanings too, w.r.t. missing > dkms.conf and permissions when making precompiled tarballs. I think > this is ok for now, but i will contact upstream about making error > codes unique / fixed. OK, looks good for now, thanks for the clarification. Applied to lunar/linux. -Andrea
diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk index 4cebe66dfd..4d393e890e 100644 --- a/debian/rules.d/2-binary-arch.mk +++ b/debian/rules.d/2-binary-arch.mk @@ -71,7 +71,7 @@ define build_dkms_sign = ) endef define build_dkms = - ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) + rc=0; ARCH=$(build_arch) CROSS_COMPILE=$(CROSS_COMPILE) $(SHELL) $(DROOT)/scripts/dkms-build $(dkms_dir) $(abi_release)-$* '$(call build_dkms_sign,$(builddir)/build-$*)' $(1) $(2) $(3) $(4) $(5) || rc=$$?; if [ "$$rc" = "9" ]; then echo do_$(4)_$*=false >> $(builddir)/skipped-dkms.mk; rc=0; fi; if [ "$$rc" != "0" ]; then exit $$rc; fi endef define install_control = @@ -603,6 +603,7 @@ endif $(call dh_all,linux-libc-dev) endif +-include $(builddir)/skipped-dkms.mk binary-%: pkgimg = $(bin_pkg_name)-$* binary-%: pkgimg_mods = $(mods_pkg_name)-$* binary-%: pkgimg_ex = $(mods_extra_pkg_name)-$* diff --git a/debian/scripts/dkms-build b/debian/scripts/dkms-build index 67566a47df..6690322476 100755 --- a/debian/scripts/dkms-build +++ b/debian/scripts/dkms-build @@ -166,14 +166,13 @@ $fakeroot /usr/sbin/dkms build --no-prepare-kernel --no-clean-kernel \ --sourcetree "$dkms_dir/source" \ --dkmstree "$dkms_dir/build" \ --kernelsourcedir "$dkms_dir/headers/linux-headers-$abi_flavour" \ - "$dkms_conf" || rc=1 + "$dkms_conf" || rc=$? # Find the log and add it to our own. for log in "$dkms_dir/build/$dkms_package/$dkms_version/$abi_flavour"/*/"log/make.log" "$dkms_dir/build/$dkms_package/$dkms_version/build/make.log" do - [ -f "$log" ] && break + [ -f "$log" ] && sed -e "s@$dkms_dir@<<DKMSDIR>>@g" <"$log" done -sed -e "s@$dkms_dir@<<DKMSDIR>>@g" <"$log" # If this build failed then exit here. [ "$rc" != 0 ] && exit "$rc"
In recent releases we have fixed up most dkms modules to correctly detect, and skip building dkms modules when compiling against incompatible kernels. For example, trying to build a WiFi driver against a kernel without 80221 config enabled, results in dkms returning error code 9 skipped build, due to incompatibility. Our DKMS autopkgtests already honor such cases, and mark test results as passing for skipped {kernel, dkms-module} combinations. Improve kernel's dkms-build script to distinguish failed versus skipped builds, by propagating exact status code. For standalone skipped dkms modules, skip generating an empty deb packages. Also do not fail over missing make.log for the skipped dkms builds, which do not have one. This change will enable us to use a static list of dkms-versions across all kernels, without need to do anything special for cases were a vendored dkms module is declared incompatible in dkms.conf for a given kernel. For example, v4l2loopback incompatible with cloud kernels that do not enable CONFIG_VIDEO, or backports-iwlwifi incompatible with kvm kernel. This patch is tested against working, skipepd, and FTBFS dkms modules, which correctly built the full kernel, skipped some dkms modules, or failed the overall build. Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com> --- debian/rules.d/2-binary-arch.mk | 3 ++- debian/scripts/dkms-build | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-)