diff mbox series

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

Message ID 20231219181957.1449958-1-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/3] kbuild: deb-pkg: do not query DEB_HOST_MULTIARCH | expand

Commit Message

Masahiro Yamada Dec. 19, 2023, 6:19 p.m. UTC
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(-)

Comments

Nicolas Schier Dec. 22, 2023, 2:30 p.m. UTC | #1
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
>
Masahiro Yamada Dec. 26, 2023, 5:34 a.m. UTC | #2
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
> >
diff mbox series

Patch

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