[PATCH v2] kbuild: deb-pkg: remove the fakeroot builds support

[PATCH v2] kbuild: deb-pkg: remove the fakeroot builds support

[Masahiro Yamada]

In 2017, the dpkg suite introduced the rootless builds support with the
following commits:

  - 2436807c87b0 ("dpkg-deb: Add support for rootless builds")
  - fca1bfe84068 ("dpkg-buildpackage: Add support for rootless builds")

This feature is available in the default dpkg on Debian 10 and Ubuntu
20.04.

Remove the old method.

Additionally, export DEB_RULES_REQUIRES_ROOT=no in case debian/rules is
invoked without dpkg-buildpackage. This change aligns with the Debian
kernel commit 65206e29f378 ("Allow to run d/rules.real without root").
While the upstream kernel currently does not run dh_testroot, it may
be useful in the future.

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

Changes in v2:
  - add DEB_RULES_REQUIRES_ROOT=no to debian/rules

 scripts/Makefile.package     | 4 +---
 scripts/package/builddeb     | 8 +-------
 scripts/package/debian/rules | 2 ++
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/scripts/Makefile.package b/scripts/Makefile.package
index 0c3adc48dfe8..a81dfb1f5181 100644
--- a/scripts/Makefile.package
+++ b/scripts/Makefile.package
@@ -109,8 +109,6 @@ debian-orig: linux.tar$(debian-orig-suffix) debian
 		cp $< ../$(orig-name); \
 	fi
 
-KBUILD_PKG_ROOTCMD ?= 'fakeroot -u'
-
 PHONY += deb-pkg srcdeb-pkg bindeb-pkg
 
 deb-pkg:    private build-type := source,binary
@@ -125,7 +123,7 @@ deb-pkg srcdeb-pkg bindeb-pkg:
 	$(if $(findstring source, $(build-type)), \
 		--unsigned-source --compression=$(KDEB_SOURCE_COMPRESS)) \
 	$(if $(findstring binary, $(build-type)), \
-		-R'$(MAKE) -f debian/rules' -j1 -r$(KBUILD_PKG_ROOTCMD) -a$$(cat debian/arch), \
+		-R'$(MAKE) -f debian/rules' -j1 -a$$(cat debian/arch), \
 		--no-check-builddeps) \
 	$(DPKG_FLAGS))
 
diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index d7dd0d04c70c..2fe51e6919da 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -36,19 +36,13 @@ create_package() {
 	sh -c "cd '$pdir'; find . -type f ! -path './DEBIAN/*' -printf '%P\0' \
 		| xargs -r0 md5sum > DEBIAN/md5sums"
 
-	# Fix ownership and permissions
-	if [ "$DEB_RULES_REQUIRES_ROOT" = "no" ]; then
-		dpkg_deb_opts="--root-owner-group"
-	else
-		chown -R root:root "$pdir"
-	fi
 	# 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 $dpkg_deb_opts ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS} --build "$pdir" ..
+	dpkg-deb --root-owner-group ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS} --build "$pdir" ..
 }
 
 install_linux_image () {
diff --git a/scripts/package/debian/rules b/scripts/package/debian/rules
index 3dafa9496c63..f23d97087948 100755
--- a/scripts/package/debian/rules
+++ b/scripts/package/debian/rules
@@ -5,6 +5,8 @@ include debian/rules.vars
 
 srctree ?= .
 
+export DEB_RULES_REQUIRES_ROOT := no
+
 ifneq (,$(filter-out parallel=1,$(filter parallel=%,$(DEB_BUILD_OPTIONS))))
     NUMJOBS = $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
     MAKEFLAGS += -j$(NUMJOBS)
-- 
2.40.1

Re: [PATCH v2] kbuild: deb-pkg: remove the fakeroot builds support

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 4552 bytes --]

On Wed, 2023-11-29 at 00:38 +0900, Masahiro Yamada wrote:
> In 2017, the dpkg suite introduced the rootless builds support with the
> following commits:
> 
>   - 2436807c87b0 ("dpkg-deb: Add support for rootless builds")
>   - fca1bfe84068 ("dpkg-buildpackage: Add support for rootless builds")
> 
> This feature is available in the default dpkg on Debian 10 and Ubuntu
> 20.04.
> 
> Remove the old method.

This seems reasonable.


> Additionally, export DEB_RULES_REQUIRES_ROOT=no in case debian/rules is
> invoked without dpkg-buildpackage. This change aligns with the Debian
> kernel commit 65206e29f378 ("Allow to run d/rules.real without root").

The Debian linux package has multiple makefiles used recursively
(rather than included).  The referenced commit is kind of a hack to
make rootless builds of a subset of binary packages work when invoking
one of the lower-level makefiles directly.

It works because the package runs dh_builddeb, which checks
DEB_RULES_REQUIRES_ROOT.  But setting DEB_RULES_REQUIRES_ROOT has
absolutely zero effect on dpkg-deb or other low-level tools.

> While the upstream kernel currently does not run dh_testroot, it may
> be useful in the future.

We can do one of:

1. Ignore DEB_RULES_REQUIRES_ROOT, assume that dpkg-deb supports
   --root-owner-group and use it unconditionally (your v1).
2. Check DEB_RULES_REQUIRES_ROOT, do either fakeroot and chown or
   dpkg-deb --root-owner-group (current behaviour), and maybe also do
   the equivalent of dh_testroot.
3. Delegate this to dh_builddeb.  Since we use dh_listpackages now,
   debhelper is already required and this would make things a lot
   simpler.

But the combination of changes in v2 does not make sense to me.

Ben.

> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
> Changes in v2:
>   - add DEB_RULES_REQUIRES_ROOT=no to debian/rules
> 
>  scripts/Makefile.package     | 4 +---
>  scripts/package/builddeb     | 8 +-------
>  scripts/package/debian/rules | 2 ++
>  3 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> index 0c3adc48dfe8..a81dfb1f5181 100644
> --- a/scripts/Makefile.package
> +++ b/scripts/Makefile.package
> @@ -109,8 +109,6 @@ debian-orig: linux.tar$(debian-orig-suffix) debian
>  		cp $< ../$(orig-name); \
>  	fi
>  
> -KBUILD_PKG_ROOTCMD ?= 'fakeroot -u'
> -
>  PHONY += deb-pkg srcdeb-pkg bindeb-pkg
>  
>  deb-pkg:    private build-type := source,binary
> @@ -125,7 +123,7 @@ deb-pkg srcdeb-pkg bindeb-pkg:
>  	$(if $(findstring source, $(build-type)), \
>  		--unsigned-source --compression=$(KDEB_SOURCE_COMPRESS)) \
>  	$(if $(findstring binary, $(build-type)), \
> -		-R'$(MAKE) -f debian/rules' -j1 -r$(KBUILD_PKG_ROOTCMD) -a$$(cat debian/arch), \
> +		-R'$(MAKE) -f debian/rules' -j1 -a$$(cat debian/arch), \
>  		--no-check-builddeps) \
>  	$(DPKG_FLAGS))
>  
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index d7dd0d04c70c..2fe51e6919da 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -36,19 +36,13 @@ create_package() {
>  	sh -c "cd '$pdir'; find . -type f ! -path './DEBIAN/*' -printf '%P\0' \
>  		| xargs -r0 md5sum > DEBIAN/md5sums"
>  
> -	# Fix ownership and permissions
> -	if [ "$DEB_RULES_REQUIRES_ROOT" = "no" ]; then
> -		dpkg_deb_opts="--root-owner-group"
> -	else
> -		chown -R root:root "$pdir"
> -	fi
>  	# 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 $dpkg_deb_opts ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS} --build "$pdir" ..
> +	dpkg-deb --root-owner-group ${KDEB_COMPRESS:+-Z$KDEB_COMPRESS} --build "$pdir" ..
>  }
>  
>  install_linux_image () {
> diff --git a/scripts/package/debian/rules b/scripts/package/debian/rules
> index 3dafa9496c63..f23d97087948 100755
> --- a/scripts/package/debian/rules
> +++ b/scripts/package/debian/rules
> @@ -5,6 +5,8 @@ include debian/rules.vars
>  
>  srctree ?= .
>  
> +export DEB_RULES_REQUIRES_ROOT := no
> +
>  ifneq (,$(filter-out parallel=1,$(filter parallel=%,$(DEB_BUILD_OPTIONS))))
>      NUMJOBS = $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
>      MAKEFLAGS += -j$(NUMJOBS)

-- 
Ben Hutchings
For every complex problem
there is a solution that is simple, neat, and wrong.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] kbuild: deb-pkg: remove the fakeroot builds support

[Masahiro Yamada]

On Wed, Nov 29, 2023 at 1:31 AM Ben Hutchings <ben@decadent.org.uk> wrote:
>
> On Wed, 2023-11-29 at 00:38 +0900, Masahiro Yamada wrote:
> > In 2017, the dpkg suite introduced the rootless builds support with the
> > following commits:
> >
> >   - 2436807c87b0 ("dpkg-deb: Add support for rootless builds")
> >   - fca1bfe84068 ("dpkg-buildpackage: Add support for rootless builds")
> >
> > This feature is available in the default dpkg on Debian 10 and Ubuntu
> > 20.04.
> >
> > Remove the old method.
>
> This seems reasonable.
>
>
> > Additionally, export DEB_RULES_REQUIRES_ROOT=no in case debian/rules is
> > invoked without dpkg-buildpackage. This change aligns with the Debian
> > kernel commit 65206e29f378 ("Allow to run d/rules.real without root").
>
> The Debian linux package has multiple makefiles used recursively
> (rather than included).  The referenced commit is kind of a hack to
> make rootless builds of a subset of binary packages work when invoking
> one of the lower-level makefiles directly.


The upstream kernel does not support individual package build
since it is implemented in scripts/package/builddeb shell script.


Is the direct execution of debian/rules still worth supporting
in the upstream kernel?


If the answer is no, "export DEB_RULES_REQUIRES_ROOT=no"
is meaningless.


> It works because the package runs dh_builddeb, which checks
> DEB_RULES_REQUIRES_ROOT.  But setting DEB_RULES_REQUIRES_ROOT has
> absolutely zero effect on dpkg-deb or other low-level tools.

Please let me clarify your statement.

Do you mean this?  ("is needed" ?)

"It is needed because the package runs dh_builddeb, which checks
 DEB_RULES_REQUIRES_ROOT."





> > While the upstream kernel currently does not run dh_testroot, it may
> > be useful in the future.
>
> We can do one of:
>
> 1. Ignore DEB_RULES_REQUIRES_ROOT, assume that dpkg-deb supports
>    --root-owner-group and use it unconditionally (your v1).
> 2. Check DEB_RULES_REQUIRES_ROOT, do either fakeroot and chown or
>    dpkg-deb --root-owner-group (current behaviour), and maybe also do
>    the equivalent of dh_testroot.
> 3. Delegate this to dh_builddeb.  Since we use dh_listpackages now,
>    debhelper is already required and this would make things a lot
>    simpler.
>
> But the combination of changes in v2 does not make sense to me.



I like 1 or 3.



If I go with 3.,
does splitting it into two patches make sense?


1/2:  remove fakeroot  (just like v1)
2/2:  dh_* conversion + "export DEB_RULES_REQUIRES_ROOT=no"


--
Best Regards
Masahiro Yamada

Re: [PATCH v2] kbuild: deb-pkg: remove the fakeroot builds support

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]

On Wed, 2023-11-29 at 03:56 +0900, Masahiro Yamada wrote:
> On Wed, Nov 29, 2023 at 1:31 AM Ben Hutchings <ben@decadent.org.uk> wrote:
> > 
> > On Wed, 2023-11-29 at 00:38 +0900, Masahiro Yamada wrote:
> > > In 2017, the dpkg suite introduced the rootless builds support with the
> > > following commits:
> > > 
> > >   - 2436807c87b0 ("dpkg-deb: Add support for rootless builds")
> > >   - fca1bfe84068 ("dpkg-buildpackage: Add support for rootless builds")
> > > 
> > > This feature is available in the default dpkg on Debian 10 and Ubuntu
> > > 20.04.
> > > 
> > > Remove the old method.
> > 
> > This seems reasonable.
> > 
> > 
> > > Additionally, export DEB_RULES_REQUIRES_ROOT=no in case debian/rules is
> > > invoked without dpkg-buildpackage. This change aligns with the Debian
> > > kernel commit 65206e29f378 ("Allow to run d/rules.real without root").
> > 
> > The Debian linux package has multiple makefiles used recursively
> > (rather than included).  The referenced commit is kind of a hack to
> > make rootless builds of a subset of binary packages work when invoking
> > one of the lower-level makefiles directly.
> 
> 
> The upstream kernel does not support individual package build
> since it is implemented in scripts/package/builddeb shell script.
> 
> 
> Is the direct execution of debian/rules still worth supporting
> in the upstream kernel?

I don't have an opinion on that.

> If the answer is no, "export DEB_RULES_REQUIRES_ROOT=no"
> is meaningless.
> 
> 
> > It works because the package runs dh_builddeb, which checks
> > DEB_RULES_REQUIRES_ROOT.  But setting DEB_RULES_REQUIRES_ROOT has
> > absolutely zero effect on dpkg-deb or other low-level tools.
> 
> Please let me clarify your statement.
> 
> Do you mean this?  ("is needed" ?)
> 
> "It is needed because the package runs dh_builddeb, which checks
>  DEB_RULES_REQUIRES_ROOT."

Yes.

> > > While the upstream kernel currently does not run dh_testroot, it may
> > > be useful in the future.
> > 
> > We can do one of:
> > 
> > 1. Ignore DEB_RULES_REQUIRES_ROOT, assume that dpkg-deb supports
> >    --root-owner-group and use it unconditionally (your v1).
> > 2. Check DEB_RULES_REQUIRES_ROOT, do either fakeroot and chown or
> >    dpkg-deb --root-owner-group (current behaviour), and maybe also do
> >    the equivalent of dh_testroot.
> > 3. Delegate this to dh_builddeb.  Since we use dh_listpackages now,
> >    debhelper is already required and this would make things a lot
> >    simpler.
> > 
> > But the combination of changes in v2 does not make sense to me.
> 
> 
> 
> I like 1 or 3.
> 
> 
> 
> If I go with 3.,
> does splitting it into two patches make sense?
> 
> 
> 1/2:  remove fakeroot  (just like v1)
> 2/2:  dh_* conversion + "export DEB_RULES_REQUIRES_ROOT=no"

Yes, that makes sense to me.

Ben.

-- 
Ben Hutchings
It is easier to write an incorrect program
than to understand a correct one.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]