Received-SPF: pass (google.com: domain of linux-kernel+bounces-9785-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Fri, 22 Dec 2023 15:30:00 +0100
From: Nicolas Schier <n.schier@avm.de>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org, Ben Hutchings <ben@decadent.org.uk>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nicolas Schier <nicolas@fjasle.eu>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] kbuild: deb-pkg: do not query DEB_HOST_MULTIARCH
Message-ID: <ZYWdaCiNFsseZWWS@buildd.core.avm.de>
Mail-Followup-To: Masahiro Yamada <masahiroy@kernel.org>,
	linux-kbuild@vger.kernel.org, Ben Hutchings <ben@decadent.org.uk>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	linux-kernel@vger.kernel.org
References: <20231219181957.1449958-1-masahiroy@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20231219181957.1449958-1-masahiroy@kernel.org>
Organization: AVM GmbH

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
>