NACK: [SRU][L, K, J, F][M, U][PATCH 1/1] UBUNTU: [Packaging] Expose base (parent) abi for derivatives

[Roxana Nicolescu]

On 06/06/2023 10:20, Juerg Haefliger wrote:
> Title is misleading, you're not exposing the ABI, just the ABI number.
>
True. Changed in v2
>> BugLink: https://bugs.launchpad.net/bugs/2017006
>>
>> This will introduce a new variable UTS_UBUNTU_BASE_RELEASE_ABI equal
>> with the parent abi. For backports with multiple inheritance (like
>> jammy:linux-aws-5.19) the version from debian.master (kinetic:linux)
>> wil be used. In case there is no inheritance, the actual kernel abi
>> will be used.
>> This will be exposed in <generated/utsrelease.h> so that (dkms)
>> modules can use this variable for versioning.
>>
>> This is particularly needed in 5.19 kernels, but in anticipation of
>> similar usecases, it is best to introduce this in all releases.
>>
>> The dkms fix that needs this variable is caused by a commit  that
>> broke the interface between the kernel and the lttng-module dkms.
>> For other releases, the kernel upstream version was used, but this
>> version has stayed the same because 5.19 version is not maintained
>> anymore upstream and the commit that broke the interface was fetched
>> from the 5.15 stable release.
>> We do expose the abi version of the kernel, but it alone will not solve
>> this issue because we deliver many derivatives, each with their own
>> version. Hence a base main abi version needs to exposed in the
>> headers.
>>
>> Signed-off-by: Roxana Nicolescu <roxana.nicolescu at canonical.com>
>> ---
>>   debian/rules.d/0-common-vars.mk | 9 +++++++++
>>   debian/rules.d/2-binary-arch.mk | 4 ++++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk
>> index 1d8f8b85772f..11e202598ee7 100644
>> --- a/debian/rules.d/0-common-vars.mk
>> +++ b/debian/rules.d/0-common-vars.mk
>> @@ -88,6 +88,15 @@ ifneq ($(full_build),false)
>>     uploadnum	:= $(uploadnum)-Ubuntu
>>   endif
>>   
>> +# This exposes the base (parent) abi number
>> +# For main kernels, this version matches their abi version
>> +# Because we may deal with multiple inheritance (jammy:linux-aws-5.19)
>> +# we always extract the abi version from debian.master/changelog
>> +DEBIAN_BASE_MASTER=debian.master
>> +src_base_pkg_name=linux
>> +base_revision	:= $(shell sed -n '1s/^$(src_base_pkg_name)\ .*($(release)-\(.*\)).*$$/\1/p' $(DEBIAN_BASE_MASTER)/changelog)
>> +base_abinum		:= $(shell echo $(base_revision) | sed -r -e 's/([^\+~]*)\.[^\.]+(~.*)?(\+.*)?$$/\1/')$(abi_suffix)
>> +
> Yikes.
> 1) Do we really need to define make variables which are only used in a single
>     rule?
> 2) If make variables then they (base_<foo>) need to be added to the output of
>     'printenv' and the definitions should be moved to where the other abinum,
>     prev_abinum are defined to keep it all in one place.
> 3) Defining DEBIAN_BASE_MASTER is pointless.
> 3) Why hard-code/define the source package name? It's not really needed, is
>     it?
> 4) That sed magic seems overly complicated just to get the abi number...
>     Why not use dpkg-parsechangelog to get the version? Look through
>     0-common-vars.mk how similar stuff is done elsewhere.
> 5) $(abi_suffix) was dropped in lunar, please don't reintroduce it.
>
>
> In hindsight, it might be helpful to define make variables and show them in
> printenv for debugging purposes... So I'm not totally opposed to that :-)
>
> ...Juerg
>
Thanks for your feedback. I agree, the rule is a big ugly and there's a 
better alternative.

I submitted a v2 where I use dpkg-parsechangelog and I also removed 
these variables.

>>   # XXX: linux-libc-dev got bumped to -803.N inadvertantly by a ti-omap4 upload
>>   #      shift our version higher for this package only.  Ensure this only
>>   #      occurs for the v2.6.35 kernel so that we do not propogate this into
>> diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
>> index 6eef2039ec7a..100f3d8e9f7e 100644
>> --- a/debian/rules.d/2-binary-arch.mk
>> +++ b/debian/rules.d/2-binary-arch.mk
>> @@ -335,6 +335,10 @@ endif
>>   		"$(hdrdir)/include/generated/compile.h"
>>   	# Add UTS_UBUNTU_RELEASE_ABI since UTS_RELEASE is difficult to parse.
>>   	echo "#define UTS_UBUNTU_RELEASE_ABI $(abinum)" >> $(hdrdir)/include/generated/utsrelease.h
>> +
>> +	# Add UTS_UBUNTU_BASE_RELEASE_ABI, needed for dkms modules where we cannot use the upstream version
>> +	echo "#define UTS_UBUNTU_BASE_RELEASE_ABI $(base_abinum)" >> $(hdrdir)/include/generated/utsrelease.h
>> +
>>   	# powerpc kernel arch seems to need some .o files for external module linking. Add them in.
>>   ifeq ($(build_arch),powerpc)
>>   	mkdir -p $(hdrdir)/arch/powerpc/lib