| Message ID | 20231128153858.84932-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kbuild: deb-pkg: remove the fakeroot builds support | expand |
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)
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
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.
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)
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(-)