[PATCH 1/3] kbuild: deb-pkg: do not query DEB_HOST_MULTIARCH

[PATCH 1/3] kbuild: deb-pkg: do not query DEB_HOST_MULTIARCH

[Masahiro Yamada]

Since commit 491b146d4c13 ("kbuild: builddeb: Eliminate debian/arch
use"), the direct execution of debian/rules fails with:

  dpkg-architecture: error: unknown option 'DEB_HOST_MULTIARCH'

I am not sure how important to support such a use case, but at least
the current code:

  dpkg-architecture -a$DEB_HOST_ARCH -qDEB_HOST_MULTIARCH

... looks weird because:

 - For this code to work correctly, DEB_HOST_ARCH must be defined.
   In this case, DEB_HOST_MULTIARCH is likely defined, so there is no
   need to query DEB_HOST_MULTIARCH in the first place. This is likely
   the case where the package build was initiated by dpkg-buildpackage.

 - If DEB_HOST_MULTIARCH is undefined, DEB_HOST_ARCH is likely undefined.
   So, you cannot query DEB_HOST_MULTIARCH in this way. This is mostly
   the case where debian/rules is directly executed.

If we want to run debian/rules directly, we can revert 491b146d4c13 or
add code to remember DEB_HOST_MULTIARCH, but I chose to remove the
useless code for now.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/package/builddeb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 2fe51e6919da..2eb4910f0ef3 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -171,9 +171,8 @@ install_libc_headers () {
 
 	# move asm headers to /usr/include/<libc-machine>/asm to match the structure
 	# used by Debian-based distros (to support multi-arch)
-	host_arch=$(dpkg-architecture -a$DEB_HOST_ARCH -qDEB_HOST_MULTIARCH)
-	mkdir $pdir/usr/include/$host_arch
-	mv $pdir/usr/include/asm $pdir/usr/include/$host_arch/
+	mkdir "$pdir/usr/include/${DEB_HOST_MULTIARCH}"
+	mv "$pdir/usr/include/asm" "$pdir/usr/include/${DEB_HOST_MULTIARCH}"
 }
 
 rm -f debian/files
-- 
2.40.1

[PATCH 2/3] kbuild: deb-pkg: hard-code Build-Depends

[Masahiro Yamada]

The condition to require libelf-dev:native is stale because objtool is
now enabled by CONFIG_OBJTOOL instead of CONFIG_UNWINDER_ORC. Not only
objtool but also resolve_btfids requires libelf-dev:native; therefore,
CONFIG_DEBUG_INFO_BTF should be checked as well.

Similarly, CONFIG_SYSTEM_TRUSTED_KEYRING is not the only case that
requires libssl-dev:native.

Perhaps, the following code would provide better coverage, but it is
hard to maintain (and may still be imperfect).

  if is_enabled CONFIG_OBJTOOL || is_enabled CONFIG_DEBUG_INFO_BTF; then
          build_depends="${build_depends}, libelf-dev:native"
  fi

  if is_enabled CONFIG_SYSTEM_TRUSTED_KEYRING ||
     is_enabled CONFIG_SYSTEM_REVOCATION_LIST ||
     is_enabled CONFIG_MODULE_SIG_FORMAT; then
          build_depends="${build_depends}, libssl-dev:native"
  fi

Let's hard-code the build dependency.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/package/mkdebian | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian
index 91f0e09600b1..93a24712b9a1 100755
--- a/scripts/package/mkdebian
+++ b/scripts/package/mkdebian
@@ -176,8 +176,6 @@ else
 fi
 
 echo $debarch > debian/arch
-extra_build_depends=", $(if_enabled_echo CONFIG_UNWINDER_ORC libelf-dev:native)"
-extra_build_depends="$extra_build_depends, $(if_enabled_echo CONFIG_SYSTEM_TRUSTED_KEYRING libssl-dev:native)"
 
 # Generate a simple changelog template
 cat <<EOF > debian/changelog
@@ -195,7 +193,8 @@ Section: kernel
 Priority: optional
 Maintainer: $maintainer
 Rules-Requires-Root: no
-Build-Depends: bc, debhelper, rsync, kmod, cpio, bison, flex $extra_build_depends
+Build-Depends: debhelper
+Build-Depends-Arch: bc, bison, cpio, flex, kmod, libelf-dev:native, libssl-dev:native, rsync
 Homepage: https://www.kernel.org/
 
 Package: $packagename-$version
-- 
2.40.1

Re: [PATCH 2/3] kbuild: deb-pkg: hard-code Build-Depends

[Nicolas Schier]

On Wed, Dec 20, 2023 at 03:19:56AM +0900, Masahiro Yamada wrote:
> The condition to require libelf-dev:native is stale because objtool is
> now enabled by CONFIG_OBJTOOL instead of CONFIG_UNWINDER_ORC. Not only
> objtool but also resolve_btfids requires libelf-dev:native; therefore,
> CONFIG_DEBUG_INFO_BTF should be checked as well.
> 
> Similarly, CONFIG_SYSTEM_TRUSTED_KEYRING is not the only case that
> requires libssl-dev:native.
> 
> Perhaps, the following code would provide better coverage, but it is
> hard to maintain (and may still be imperfect).
> 
>   if is_enabled CONFIG_OBJTOOL || is_enabled CONFIG_DEBUG_INFO_BTF; then
>           build_depends="${build_depends}, libelf-dev:native"
>   fi
> 
>   if is_enabled CONFIG_SYSTEM_TRUSTED_KEYRING ||
>      is_enabled CONFIG_SYSTEM_REVOCATION_LIST ||
>      is_enabled CONFIG_MODULE_SIG_FORMAT; then
>           build_depends="${build_depends}, libssl-dev:native"
>   fi
> 
> Let's hard-code the build dependency.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

Reviewed-by: Nicolas Schier <n.schier@avm.de>

[PATCH 3/3] kbuild: deb-pkg: use more debhelper commands

[Masahiro Yamada]

Commit 36862e14e316 ("kbuild: deb-pkg: use dh_listpackages to know
enabled packages") started to require the debhelper tool suite.

Use more dh_* commands in create_package():

 - dh_installdocs to install copyright
 - dh_installchangelogs to install changelog
 - dh_compress to compress changelog
 - dh_fixperms to replace the raw chmod command
 - dh_gencontrol to replace the raw dpkg-gencontrol command
 - dh_md5sums to record the md5sum of included files
 - dh_builddeb to replace the raw dpkg-deb command

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/package/builddeb | 23 ++++++++---------------
 scripts/package/mkdebian |  2 +-
 2 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 2eb4910f0ef3..436d55a83ab0 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -26,23 +26,16 @@ if_enabled_echo() {
 
 create_package() {
 	local pname="$1" pdir="$2"
-	local dpkg_deb_opts
 
-	mkdir -m 755 -p "$pdir/DEBIAN"
-	mkdir -p "$pdir/usr/share/doc/$pname"
-	cp debian/copyright "$pdir/usr/share/doc/$pname/"
-	cp debian/changelog "$pdir/usr/share/doc/$pname/changelog.Debian"
-	gzip -n -9 "$pdir/usr/share/doc/$pname/changelog.Debian"
-	sh -c "cd '$pdir'; find . -type f ! -path './DEBIAN/*' -printf '%P\0' \
-		| xargs -r0 md5sum > DEBIAN/md5sums"
+	export DH_OPTIONS="-p${pname} -P${pdir}"
 
-	# a+rX in case we are in a restrictive umask environment like 0077
-	# ug-s in case we build in a setuid/setgid directory
-	chmod -R go-w,a+rX,ug-s "$pdir"
-
-	# Create the package
-	dpkg-gencontrol -p$pname -P"$pdir"
-	dpkg-deb --root-owner-group ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS} --build "$pdir" ..
+	dh_installdocs
+	dh_installchangelogs
+	dh_compress
+	dh_fixperms
+	dh_gencontrol
+	dh_md5sums
+	dh_builddeb -- ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS}
 }
 
 install_linux_image () {
diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian
index 93a24712b9a1..070149c985fe 100755
--- a/scripts/package/mkdebian
+++ b/scripts/package/mkdebian
@@ -193,7 +193,7 @@ Section: kernel
 Priority: optional
 Maintainer: $maintainer
 Rules-Requires-Root: no
-Build-Depends: debhelper
+Build-Depends: debhelper-compat (= 12)
 Build-Depends-Arch: bc, bison, cpio, flex, kmod, libelf-dev:native, libssl-dev:native, rsync
 Homepage: https://www.kernel.org/
 
-- 
2.40.1

Re: [PATCH 3/3] kbuild: deb-pkg: use more debhelper commands

[Nicolas Schier]

On Wed, Dec 20, 2023 at 03:19:57AM +0900, Masahiro Yamada wrote:
> Commit 36862e14e316 ("kbuild: deb-pkg: use dh_listpackages to know
> enabled packages") started to require the debhelper tool suite.
> 
> Use more dh_* commands in create_package():
> 
>  - dh_installdocs to install copyright
>  - dh_installchangelogs to install changelog
>  - dh_compress to compress changelog
>  - dh_fixperms to replace the raw chmod command
>  - dh_gencontrol to replace the raw dpkg-gencontrol command
>  - dh_md5sums to record the md5sum of included files
>  - dh_builddeb to replace the raw dpkg-deb command
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/package/builddeb | 23 ++++++++---------------
>  scripts/package/mkdebian |  2 +-
>  2 files changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 2eb4910f0ef3..436d55a83ab0 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -26,23 +26,16 @@ if_enabled_echo() {
>  
>  create_package() {
>  	local pname="$1" pdir="$2"
> -	local dpkg_deb_opts
>  
> -	mkdir -m 755 -p "$pdir/DEBIAN"
> -	mkdir -p "$pdir/usr/share/doc/$pname"
> -	cp debian/copyright "$pdir/usr/share/doc/$pname/"
> -	cp debian/changelog "$pdir/usr/share/doc/$pname/changelog.Debian"
> -	gzip -n -9 "$pdir/usr/share/doc/$pname/changelog.Debian"
> -	sh -c "cd '$pdir'; find . -type f ! -path './DEBIAN/*' -printf '%P\0' \
> -		| xargs -r0 md5sum > DEBIAN/md5sums"
> +	export DH_OPTIONS="-p${pname} -P${pdir}"
>  
> -	# a+rX in case we are in a restrictive umask environment like 0077
> -	# ug-s in case we build in a setuid/setgid directory
> -	chmod -R go-w,a+rX,ug-s "$pdir"
> -
> -	# Create the package
> -	dpkg-gencontrol -p$pname -P"$pdir"
> -	dpkg-deb --root-owner-group ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS} --build "$pdir" ..
> +	dh_installdocs
> +	dh_installchangelogs
> +	dh_compress
> +	dh_fixperms
> +	dh_gencontrol
> +	dh_md5sums
> +	dh_builddeb -- ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS}
>  }

That looks much cleaner, thanks!

Reviewed-by: Nicolas Schier <n.schier@avm.de>

Re: [PATCH 1/3] kbuild: deb-pkg: do not query DEB_HOST_MULTIARCH

[Nicolas Schier]

On Wed, Dec 20, 2023 at 03:19:55AM +0900, Masahiro Yamada wrote:
> Since commit 491b146d4c13 ("kbuild: builddeb: Eliminate debian/arch
> use"), the direct execution of debian/rules fails with:
> 
>   dpkg-architecture: error: unknown option 'DEB_HOST_MULTIARCH'
> 
> I am not sure how important to support such a use case, but at least
> the current code:
> 
>   dpkg-architecture -a$DEB_HOST_ARCH -qDEB_HOST_MULTIARCH
> 
> ... looks weird because:
> 
>  - For this code to work correctly, DEB_HOST_ARCH must be defined.
>    In this case, DEB_HOST_MULTIARCH is likely defined, so there is no
>    need to query DEB_HOST_MULTIARCH in the first place. This is likely
>    the case where the package build was initiated by dpkg-buildpackage.
> 
>  - If DEB_HOST_MULTIARCH is undefined, DEB_HOST_ARCH is likely undefined.
>    So, you cannot query DEB_HOST_MULTIARCH in this way. This is mostly
>    the case where debian/rules is directly executed.
> 
> If we want to run debian/rules directly, we can revert 491b146d4c13 or
> add code to remember DEB_HOST_MULTIARCH, but I chose to remove the
> useless code for now.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---

thanks.  Fixing the non-functional things is obviously a good thing.

Reviewed-by: Nicolas Schier <n.schier@avm.de>


Iff we'd like to be able to call debian/rules directly, do we really
have to remember DEB_HOST_MULTIARCH, or just DEB_HOST_ARCH?

In the latter case, might this be a sufficient way to allow calling
debian/rules again?

diff --git a/scripts/package/debian/rules b/scripts/package/debian/rules
index 3dafa9496c63..e3e0001e7556 100755
--- a/scripts/package/debian/rules
+++ b/scripts/package/debian/rules
@@ -3,7 +3,9 @@
 
 include debian/rules.vars
 
-srctree ?= .
+DEB_HOST_ARCH ?= $(shell cat debian/arch)
+
+srctree ?= $(or $(wildcard source), .)
 
 ifneq (,$(filter-out parallel=1,$(filter parallel=%,$(DEB_BUILD_OPTIONS))))
     NUMJOBS = $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))


... but the more I think about it, I am not convinced that we want to
use the debian/arch file.  Actually I think it would be nice if we
strive for an architecture independent source package instead of
engraving the architecture even more.


Nevertheless, your patch looks good to me.

Kind regards,
Nicolas



> 
>  scripts/package/builddeb | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 2fe51e6919da..2eb4910f0ef3 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -171,9 +171,8 @@ install_libc_headers () {
>  
>  	# move asm headers to /usr/include/<libc-machine>/asm to match the structure
>  	# used by Debian-based distros (to support multi-arch)
> -	host_arch=$(dpkg-architecture -a$DEB_HOST_ARCH -qDEB_HOST_MULTIARCH)
> -	mkdir $pdir/usr/include/$host_arch
> -	mv $pdir/usr/include/asm $pdir/usr/include/$host_arch/
> +	mkdir "$pdir/usr/include/${DEB_HOST_MULTIARCH}"
> +	mv "$pdir/usr/include/asm" "$pdir/usr/include/${DEB_HOST_MULTIARCH}"
>  }
>  
>  rm -f debian/files
> -- 
> 2.40.1
> 

Re: [PATCH 1/3] kbuild: deb-pkg: do not query DEB_HOST_MULTIARCH

[Masahiro Yamada]

On Fri, Dec 22, 2023 at 11:30 PM Nicolas Schier <n.schier@avm.de> wrote:
>
> On Wed, Dec 20, 2023 at 03:19:55AM +0900, Masahiro Yamada wrote:
> > Since commit 491b146d4c13 ("kbuild: builddeb: Eliminate debian/arch
> > use"), the direct execution of debian/rules fails with:
> >
> >   dpkg-architecture: error: unknown option 'DEB_HOST_MULTIARCH'
> >
> > I am not sure how important to support such a use case, but at least
> > the current code:
> >
> >   dpkg-architecture -a$DEB_HOST_ARCH -qDEB_HOST_MULTIARCH
> >
> > ... looks weird because:
> >
> >  - For this code to work correctly, DEB_HOST_ARCH must be defined.
> >    In this case, DEB_HOST_MULTIARCH is likely defined, so there is no
> >    need to query DEB_HOST_MULTIARCH in the first place. This is likely
> >    the case where the package build was initiated by dpkg-buildpackage.
> >
> >  - If DEB_HOST_MULTIARCH is undefined, DEB_HOST_ARCH is likely undefined.
> >    So, you cannot query DEB_HOST_MULTIARCH in this way. This is mostly
> >    the case where debian/rules is directly executed.
> >
> > If we want to run debian/rules directly, we can revert 491b146d4c13 or
> > add code to remember DEB_HOST_MULTIARCH, but I chose to remove the
> > useless code for now.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
>
> thanks.  Fixing the non-functional things is obviously a good thing.
>
> Reviewed-by: Nicolas Schier <n.schier@avm.de>
>
>
> Iff we'd like to be able to call debian/rules directly, do we really
> have to remember DEB_HOST_MULTIARCH, or just DEB_HOST_ARCH?


Theoretically, if we know DEB_HOST_ARCH,
other DEB_HOST_* can be derived.


scripts/package/deb-build-option needs to know more DEB_* variables.

  DEB_HOST_ARCH
  DEB_BUILD_ARCH
  DEB_HOST_GNU_TYPE









>
> In the latter case, might this be a sufficient way to allow calling
> debian/rules again?


Not perfectly.
scripts/package/deb-build-option does not work as intended.


>
> diff --git a/scripts/package/debian/rules b/scripts/package/debian/rules
> index 3dafa9496c63..e3e0001e7556 100755
> --- a/scripts/package/debian/rules
> +++ b/scripts/package/debian/rules
> @@ -3,7 +3,9 @@
>
>  include debian/rules.vars
>
> -srctree ?= .
> +DEB_HOST_ARCH ?= $(shell cat debian/arch)
> +
> +srctree ?= $(or $(wildcard source), .)
>
>  ifneq (,$(filter-out parallel=1,$(filter parallel=%,$(DEB_BUILD_OPTIONS))))
>      NUMJOBS = $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
>
>
> ... but the more I think about it, I am not convinced that we want to
> use the debian/arch file.  Actually I think it would be nice if we
> strive for an architecture independent source package instead of
> engraving the architecture even more.


The source package _is_ arch-independent,
can be built for a single architecture because
the source package for upstream contains the .config,
which is configured for a particular architecture.







>
> Nevertheless, your patch looks good to me.
>
> Kind regards,
> Nicolas
>
>
>
> >
> >  scripts/package/builddeb | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> > index 2fe51e6919da..2eb4910f0ef3 100755
> > --- a/scripts/package/builddeb
> > +++ b/scripts/package/builddeb
> > @@ -171,9 +171,8 @@ install_libc_headers () {
> >
> >       # move asm headers to /usr/include/<libc-machine>/asm to match the structure
> >       # used by Debian-based distros (to support multi-arch)
> > -     host_arch=$(dpkg-architecture -a$DEB_HOST_ARCH -qDEB_HOST_MULTIARCH)
> > -     mkdir $pdir/usr/include/$host_arch
> > -     mv $pdir/usr/include/asm $pdir/usr/include/$host_arch/
> > +     mkdir "$pdir/usr/include/${DEB_HOST_MULTIARCH}"
> > +     mv "$pdir/usr/include/asm" "$pdir/usr/include/${DEB_HOST_MULTIARCH}"
> >  }
> >
> >  rm -f debian/files
> > --
> > 2.40.1
> >



-- 
Best Regards
Masahiro Yamada