Some comments Re: [PATCH 1/2][Hirsute] UBUNTU: [Packaging] rules -- filter out udebs when not needed

[Tim Gardner]

On 2/9/21 5:23 AM, Dimitri John Ledkov wrote:
> Filter out all udebs, when not needed. This should allow changing the
> default to building without udebs, and yet build with udebs in hwe
> backports via disable_d_i variable.
> 
> Signed-off-by: Dimitri John Ledkov <xnox at ubuntu.com>
> ---
>   debian.master/control.stub.in            |  1 +
>   debian.master/rules.d/riscv64.mk         |  1 +
>   debian/rules                             | 17 +++++++++++++---
>   debian/rules.d/5-udebs.mk                |  2 +-
>   debian/scripts/misc/kernel-wedge-arch.pl | 26 ------------------------
>   5 files changed, 17 insertions(+), 30 deletions(-)
>   delete mode 100755 debian/scripts/misc/kernel-wedge-arch.pl
> 
> diff --git a/debian.master/control.stub.in b/debian.master/control.stub.in
> index 9c1e956092bf..cae6f6783105 100644
> --- a/debian.master/control.stub.in
> +++ b/debian.master/control.stub.in
> @@ -8,6 +8,7 @@ Build-Depends:
>    dh-systemd,
>    cpio,
>    kernel-wedge,
> + dctrl-tools,
>    kmod <!stage1>,
>    makedumpfile [amd64] <!stage1>,
>    libcap-dev <!stage1>,
> diff --git a/debian.master/rules.d/riscv64.mk b/debian.master/rules.d/riscv64.mk
> index 66c75adf329e..b63f36a4078a 100644
> --- a/debian.master/rules.d/riscv64.mk
> +++ b/debian.master/rules.d/riscv64.mk
> @@ -13,6 +13,7 @@ no_dumpfile	= true
>   
>   do_flavour_image_package = false
>   do_tools	= false
> +do_di	= false
>   do_tools_common	= false
>   do_extras_package = false
>   do_source_package = false
> diff --git a/debian/rules b/debian/rules
> index 4f64f55b8d8f..a2323c184837 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -39,11 +39,12 @@ do_tools_common?=true
>   do_tools_host?=false
>   do_tools_perf_jvmti?=false
>   do_enforce_all?=false
> +do_di?=true
>   
>   # Don't build tools or udebs in a cross compile environment.
>   ifneq ($(DEB_HOST_ARCH),$(DEB_BUILD_ARCH))
>   	do_tools=false
> -	disable_d_i=true
> +	do_di=false
>   	do_zfs=false
>   	do_dkms_nvidia=false
>   	do_dkms_nvidia_server=false
> @@ -77,7 +78,7 @@ endif
>   #  - disable dkms builds as the versions used may have been deleted
>   ifneq ($(filter autopkgtest,$(DEB_BUILD_PROFILES)),)
>   	flavours := $(firstword $(flavours))
> -	disable_d_i=true
> +	do_di=false
>   	do_zfs=false
>   	do_dkms_nvidia=false
>   	do_dkms_nvidia_server=false
> @@ -215,13 +216,23 @@ $(DEBIAN)/control.stub: 				\
>   		-e 's/=SERIES=/$(series)/g'                                     \
>   		>> $(DEBIAN)/control.stub;                                      \
>   	done
> +ifeq ($(do_di),false)
> +	# Building without d-i, excluding udebs from $(DEBIAN)/control.stub
> +	grep-dctrl -v -FPackage-Type,XC-Package-Type udeb $(DEBIAN)/control.stub > $(DEBIAN)/control.stub.no-d-i
> +	mv $(DEBIAN)/control.stub.no-d-i $(DEBIAN)/control.stub
> +	# Drop kernel-wedge build-dependency
> +	sed -i '/kernel-wedge/d' $(DEBIAN)/control.stub
> +endif
>   
>   .PHONY: debian/control
>   debian/control: $(DEBIAN)/control.stub
>   	echo "# placebo control.stub for kernel-wedge flow change" >debian/control.stub
>   	cp $(DEBIAN)/control.stub debian/control
> +ifeq ($(do_di),true)
> +	echo >> debian/control

What is the point of this line ^^ ?

>   	export KW_DEFCONFIG_DIR=$(DEBIAN)/d-i && \
>   	export KW_CONFIG_DIR=$(DEBIAN)/d-i && \
>   	LANG=C kernel-wedge gen-control $(release)-$(abinum) | \
> -		perl -f $(DROOT)/scripts/misc/kernel-wedge-arch.pl $(arch) \
> +		grep-dctrl -FArchitecture $(arch) \
>   		>>$(CURDIR)/debian/control
> +endif
> diff --git a/debian/rules.d/5-udebs.mk b/debian/rules.d/5-udebs.mk
> index e642fe69b4f7..7c400b91dae7 100644
> --- a/debian/rules.d/5-udebs.mk
> +++ b/debian/rules.d/5-udebs.mk
> @@ -1,7 +1,7 @@
>   # Do udebs if not disabled in the arch-specific makefile
>   binary-udebs: binary-debs
>   	@echo Debug: $@
> -ifeq ($(disable_d_i),)
> +ifeq ($(do_di),true)
>   	@$(MAKE) --no-print-directory -f $(DROOT)/rules DEBIAN=$(DEBIAN) \
>   		do-binary-udebs
>   endif
> diff --git a/debian/scripts/misc/kernel-wedge-arch.pl b/debian/scripts/misc/kernel-wedge-arch.pl
> deleted file mode 100755
> index 4b4fefe67c7b..000000000000
> --- a/debian/scripts/misc/kernel-wedge-arch.pl
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -#!/usr/bin/perl
> -#
> -# kernel-wedge-arch.pl -- select only specifiers for the supplied arch.
> -#
> -use strict;
> -
> -require Dpkg::Control;
> -require Dpkg::Deps;
> -
> -my $fh = \*STDIN;
> -
> -my @entries;
> -
> -my $wanted = $ARGV[0];
> -
> -my $entry;
> -while (!eof($fh)) {
> -	$entry = Dpkg::Control->new();
> -	$entry->parse($fh, '???');
> -
> -	if ($entry->{'Architecture'} eq $wanted) {
> -		print("\n" . $entry);
> -	}
> -}
> -
> -close($fh);
> 

Its a bit of a nit, but I think this should be 2 patches; 1) Remove 
kernel-wedge-arch.pl and replace its use with grep-dctrl; 2) Implement 
the do_di logic.

The first patch would stand on its own merit because it removes some 
perl (yay!) and could be applied to older kernels.

rtg
-----------
Tim Gardner
Canonical, Inc