diff mbox series

[1/2,Hirsute] UBUNTU: [Packaging] rules -- filter out udebs when not needed

Message ID 20210209122339.15763-2-xnox@ubuntu.com
State New
Headers show
Series Skip udeb builds by default | expand

Commit Message

Dimitri John Ledkov Feb. 9, 2021, 12:23 p.m. UTC
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@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

Comments

Tim Gardner Feb. 9, 2021, 12:41 p.m. UTC | #1
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@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
Dimitri John Ledkov Feb. 9, 2021, 1:29 p.m. UTC | #2
On Tue, 9 Feb 2021 at 12:42, Tim Gardner <tim.gardner@canonical.com> wrote:
>
>
>
> 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@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

separately apw mentions that we could stop filtering these things out.
Or to keep things extra tidy have them in a separate control.stub.di
and include it with the kernel wedge generated ones.

> >
> >   .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 ^^ ?
>

So some of the debian.master/control.d/*.stub files do not end with
'\n\n' hence one cannot append a paragraph of a new package right
after it, which is what kernel-wedge generates.
You will notice that the perl script did pre-print '\n' before every
paragraph it filtered in.
So alternative to this echo would be to fix the control.d/*.stub files
to end with double new line at the end.

> >       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.
>

That's true, i did have it separate when i started out, but was not
sure if that was micromanagement. Will resend splitted out more.
Kamal Mostafa Feb. 9, 2021, 9:49 p.m. UTC | #3
On Tue, Feb 9, 2021 at 5:30 AM Dimitri John Ledkov <xnox@ubuntu.com> wrote:

> On Tue, 9 Feb 2021 at 12:42, Tim Gardner <tim.gardner@canonical.com>
> wrote:
>
> > > +ifeq ($(do_di),true)
> > > +     echo >> debian/control
> >
> > What is the point of this line ^^ ?
> >
>
> So some of the debian.master/control.d/*.stub files do not end with
> '\n\n' hence one cannot append a paragraph of a new package right
> after it, which is what kernel-wedge generates.
> You will notice that the perl script did pre-print '\n' before every
> paragraph it filtered in.
> So alternative to this echo would be to fix the control.d/*.stub files
> to end with double new line at the end.
>
>
Relying on the *.stub files to stay that way seems pretty fragile, so I
think keeping it scripted (as you're doing) is the right approach.  How
about adding a comment documenting the purpose of that blank line echo?

 -Kamal
diff mbox series

Patch

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
 	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);