[PATCH resend 0/2] fakeroot dwimery for scripts/package/*

[PATCH resend 0/2] fakeroot dwimery for scripts/package/*

[Jonathan Nieder]

Without these patches, to build an upgraded kernel on a Debian machine,
one has to use commands I keep on forgetting:

	make oldconfig
	make
	fakeroot -u make deb-pkg

With these patches applied, as long as fakeroot is installed, a single
command will do:

	make oldconfig deb-pkg

If fakeroot is not installed, there should be no change in behavior.
I have been using this to build the last few kernels, finding it very
convenient.

I look forward to your thoughts, even if they are just "I have no time
to look at this now".  I would like to get an idea of whether people
think these patches are a good idea, so I can fix them if they're
broken and chase them out of my local tree.

Jonathan Nieder (2):
  scripts/package: add ROOTCMD variable
  scripts/package: use fakeroot if available

 scripts/package/Makefile |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

[PATCH resend 1/2] scripts/package: add ROOTCMD variable

[Jonathan Nieder]

Allow "make foo-pkg" to acquire root or fake root privileges before
running commands that need it.  This is especially important for
tar-pkg, since otherwise if a user forgets to run make as root, the
installed kernel will have that user as owner and can easily be
overwritten by a broken program running as that user later.

The new ROOTCMD variable, if defined, is used as a command to run
other commands with possibly fake elevated privileges.  If it is not
defined, the behavior is as before, and hopefully make is running as
root already.

In other words, as a shortcut, instead of running

$ make oldconfig &&
> make &&
> fakeroot -u make rpm-pkg

now one can use a single command:

$ make oldconfig rpm-pkg ROOTCMD="fakeroot -u"

Idea from Ryan Anderson
<http://thread.gmane.org/gmane.comp.version-control.git/14770/focus=14802>.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Changes from v1: rebased on top of current torvalds/master.

 scripts/package/Makefile |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index f67cc88..14b65a8 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -53,7 +53,8 @@ rpm-pkg rpm: $(objtree)/kernel.spec FORCE
 	set -e; \
 	mv -f $(objtree)/.tmp_version $(objtree)/.version
 
-	$(RPM) $(RPMOPTS) --target $(UTS_MACHINE) -ta ../$(KERNELPATH).tar.gz
+	$(ROOTCMD) $(RPM) $(RPMOPTS) --target $(UTS_MACHINE) \
+		-ta ../$(KERNELPATH).tar.gz
 	rm ../$(KERNELPATH).tar.gz
 
 clean-files := $(objtree)/kernel.spec
@@ -70,8 +71,8 @@ binrpm-pkg: $(objtree)/binkernel.spec FORCE
 	set -e; \
 	mv -f $(objtree)/.tmp_version $(objtree)/.version
 
-	$(RPM) $(RPMOPTS) --define "_builddir $(objtree)" --target \
-		$(UTS_MACHINE) -bb $<
+	$(ROOTCMD) $(RPM) $(RPMOPTS) --define "_builddir $(objtree)" \
+		--target $(UTS_MACHINE) -bb $<
 
 clean-files += $(objtree)/binkernel.spec
 
@@ -79,7 +80,7 @@ clean-files += $(objtree)/binkernel.spec
 # ---------------------------------------------------------------------------
 deb-pkg: FORCE
 	$(MAKE) KBUILD_SRC=
-	$(CONFIG_SHELL) $(srctree)/scripts/package/builddeb
+	$(ROOTCMD) $(CONFIG_SHELL) $(srctree)/scripts/package/builddeb
 
 clean-dirs += $(objtree)/debian/
 
@@ -88,7 +89,7 @@ clean-dirs += $(objtree)/debian/
 # ---------------------------------------------------------------------------
 tar%pkg: FORCE
 	$(MAKE) KBUILD_SRC=
-	$(CONFIG_SHELL) $(srctree)/scripts/package/buildtar $@
+	$(ROOTCMD) $(CONFIG_SHELL) $(srctree)/scripts/package/buildtar $@
 
 clean-dirs += $(objtree)/tar-install/
 
@@ -96,10 +97,10 @@ clean-dirs += $(objtree)/tar-install/
 # Help text displayed when executing 'make help'
 # ---------------------------------------------------------------------------
 help: FORCE
+	@echo ' Set ROOTCMD={sudo|fakeroot -u|super|...} and make as non-root:'
 	@echo '  rpm-pkg         - Build both source and binary RPM kernel packages'
 	@echo '  binrpm-pkg      - Build only the binary kernel package'
 	@echo '  deb-pkg         - Build the kernel as an deb package'
 	@echo '  tar-pkg         - Build the kernel as an uncompressed tarball'
 	@echo '  targz-pkg       - Build the kernel as a gzip compressed tarball'
 	@echo '  tarbz2-pkg      - Build the kernel as a bzip2 compressed tarball'
-
-- 
1.6.5.rc1.199.g596ec

[PATCH resend 2/2] scripts/package: use fakeroot if available

[Jonathan Nieder]

Unless already running as root, use fakeroot to ensure the files
in the generated binary packages are owned by root.  Without this
change, you have to set ROOTCMD or become root yourself to run
"make foo-pkg".

With this patch applied, you can run "make oldconfig rpm-pkg" as
an ordinary user to build a binary package for an updated kernel
tree and it should just work.

fakeroot is a bit too zealous by default in pretending files are
owned by root.  Unless directed otherwise, its wrapped stat() and
lstat() set st_uid and st_gid to 0 for all files.  This slows
down the Linux build with CONFIG_LOCALVERSION_AUTO a lot, since
git notices that the owners have changed and has to reread the
entire kernel tree to learn that the content has not changed.
Since "make modules_install" and "scripts/package/foo" run within
the same fakeroot session, we are free to avoid this by telling
fakeroot to use the actual owner and group for preexisting files,
by passing it the -u option.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 scripts/package/Makefile |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index 14b65a8..14fdeda 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -1,6 +1,15 @@
 # Makefile for the different targets used to generate full packages of a kernel
 # It uses the generic clean infrastructure of kbuild
 
+# How to acquire (fake) root privileges
+ifndef ROOTCMD
+ifneq ($(shell id -u),0)
+ifeq ($(shell which fakeroot >/dev/null 2>&1 && echo found),found)
+ROOTCMD := fakeroot -u
+endif
+endif
+endif
+
 # RPM target
 # ---------------------------------------------------------------------------
 # The rpm target generates two rpm files:
-- 
1.6.5.rc1.199.g596ec

Re: [PATCH resend 0/2] fakeroot dwimery for scripts/package/*

[Sam Ravnborg]

On Wed, Oct 14, 2009 at 01:46:23AM -0500, Jonathan Nieder wrote:
> Without these patches, to build an upgraded kernel on a Debian machine,
> one has to use commands I keep on forgetting:
> 
> 	make oldconfig
> 	make
> 	fakeroot -u make deb-pkg
> 
> With these patches applied, as long as fakeroot is installed, a single
> command will do:
> 
> 	make oldconfig deb-pkg
> 
> If fakeroot is not installed, there should be no change in behavior.
> I have been using this to build the last few kernels, finding it very
> convenient.
> 
> I look forward to your thoughts, even if they are just "I have no time
> to look at this now".  I would like to get an idea of whether people
> think these patches are a good idea, so I can fix them if they're
> broken and chase them out of my local tree.

I have not yet started to look at -next material where
this obviously belongs.
I skimmed the path last time you sent it (and have it queued
in my inbox) and my first reaction was:

    Is ROOTCMD an established way to specify this?

I asked google and not too many hits.
So I think we should come up with something more in the
line of the other stuff we have.

That could be the longer:

   KBUILD_PKG_ROOTCMD

Which with the KBUILD_ prefix could be a globel environment
variable for some.

The rationale for the _PKG is that this is restricted to
the *pkg targets (at least for now).

	Sam

Re: [PATCH resend 0/2] fakeroot dwimery for scripts/package/*

[Frans Pop]

On Wednesday 14 October 2009, Sam Ravnborg wrote:
>     Is ROOTCMD an established way to specify this?
>
> I asked google and not too many hits.
> So I think we should come up with something more in the
> line of the other stuff we have.
>
> That could be the longer:
>
>    KBUILD_PKG_ROOTCMD
>
> Which with the KBUILD_ prefix could be a globel environment
> variable for some.

Agreed with this change. It is also more in line with how setting a root 
command is usually done in Debian. A grep of man pages on my system shows 
the following uses:
DEBUILD_ROOTCMD, BDPEP_ROOTCMD, UUPDATE_ROOTCMD, BUILDD_ROOTCMD, 
PBUILDERROOTCMD.

I have not tested it, but the patches look good otherwise, so with Sam's 
suggested change:
Acked-by: Frans Pop <elendil@planet.nl>

Cheers,
FJP

Re: [PATCH resend 0/2] fakeroot dwimery for scripts/package/*

[maximilian attems]

On Wed, Oct 14, 2009 at 11:59:02AM +0200, Frans Pop wrote:
> On Wednesday 14 October 2009, Sam Ravnborg wrote:
> >     Is ROOTCMD an established way to specify this?
> >
> > I asked google and not too many hits.
> > So I think we should come up with something more in the
> > line of the other stuff we have.
> >
> > That could be the longer:
> >
> >    KBUILD_PKG_ROOTCMD
> >
> > Which with the KBUILD_ prefix could be a globel environment
> > variable for some.
> 
> Agreed with this change. It is also more in line with how setting a root 
> command is usually done in Debian. A grep of man pages on my system shows 
> the following uses:
> DEBUILD_ROOTCMD, BDPEP_ROOTCMD, UUPDATE_ROOTCMD, BUILDD_ROOTCMD, 
> PBUILDERROOTCMD.
> 
> I have not tested it, but the patches look good otherwise, so with Sam's 
> suggested change:
> Acked-by: Frans Pop <elendil@planet.nl>
Acked-by: maximilian attems <max@stro.at>

I also agree that this should land in -next as feature patch.
btw thanks for kicking that item of my TODO :)

[PATCH v2 0/2] fakeroot dwimery for scripts/package/*

[Jonathan Nieder]

maximilian attems wrote:
> Frans Pop wrote:
>> Sam Ravnborg wrote:

>>> That could be the longer:
>>>
>>>    KBUILD_PKG_ROOTCMD
>>>
>>> Which with the KBUILD_ prefix could be a globel environment
>>> variable for some.
>>
>> I have not tested it, but the patches look good otherwise, so with Sam's 
>> suggested change:
>> Acked-by: Frans Pop <elendil@planet.nl>
>
> Acked-by: maximilian attems <max@stro.at>
>
> I also agree that this should land in -next as feature patch.
> btw thanks for kicking that item of my TODO :)

Thanks, all.  Here’s an updated version, tested with a new build last
night.  This is indeed intended for -next.

Jonathan Nieder (2):
  scripts/package: add KBUILD_PKG_ROOTCMD variable
  scripts/package: use fakeroot if available

 scripts/package/Makefile |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

[PATCH v2 1/2] scripts/package: add KBUILD_PKG_ROOTCMD variable

[Jonathan Nieder]

Allow "make foo-pkg" to acquire root or fake root privileges before
running commands that need them.  This is especially important for
tar-pkg, since if buildtar is run as an unprivileged user, the
installed kernel will have that user as owner and can easily be
overwritten by a broken program running as that user later.

The new KBUILD_PKG_ROOTCMD variable, if defined, is used as a command
to run other commands with possibly fake elevated privileges.  Its use
is restricted to *pkg targets, hence the name.  If it is not defined,
the behavior is as before, and hopefully make is running as root
already.

In other words, as a shortcut, instead of running

	make oldconfig &&
	make &&
	fakeroot -u make rpm-pkg

now one can use a single command:

	make oldconfig rpm-pkg KBUILD_PKG_ROOTCMD="fakeroot -u"

Idea from Ryan Anderson
<http://thread.gmane.org/gmane.comp.version-control.git/14770/focus=14802>.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Changes from v1:
 - use KBUILD_PKG_ROOTCMD instead of ROOTCMD
 - shorten advice in "make help" so it still fits on one terminal line

 scripts/package/Makefile |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index f67cc88..6997f38 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -53,7 +53,8 @@ rpm-pkg rpm: $(objtree)/kernel.spec FORCE
 	set -e; \
 	mv -f $(objtree)/.tmp_version $(objtree)/.version
 
-	$(RPM) $(RPMOPTS) --target $(UTS_MACHINE) -ta ../$(KERNELPATH).tar.gz
+	$(KBUILD_PKG_ROOTCMD) $(RPM) $(RPMOPTS) --target $(UTS_MACHINE) \
+		-ta ../$(KERNELPATH).tar.gz
 	rm ../$(KERNELPATH).tar.gz
 
 clean-files := $(objtree)/kernel.spec
@@ -70,8 +71,9 @@ binrpm-pkg: $(objtree)/binkernel.spec FORCE
 	set -e; \
 	mv -f $(objtree)/.tmp_version $(objtree)/.version
 
-	$(RPM) $(RPMOPTS) --define "_builddir $(objtree)" --target \
-		$(UTS_MACHINE) -bb $<
+	$(KBUILD_PKG_ROOTCMD) $(RPM) $(RPMOPTS) \
+		--define "_builddir $(objtree)" \
+		--target $(UTS_MACHINE) -bb $<
 
 clean-files += $(objtree)/binkernel.spec
 
@@ -79,7 +81,8 @@ clean-files += $(objtree)/binkernel.spec
 # ---------------------------------------------------------------------------
 deb-pkg: FORCE
 	$(MAKE) KBUILD_SRC=
-	$(CONFIG_SHELL) $(srctree)/scripts/package/builddeb
+	$(KBUILD_PKG_ROOTCMD) $(CONFIG_SHELL) \
+		$(srctree)/scripts/package/builddeb
 
 clean-dirs += $(objtree)/debian/
 
@@ -88,7 +91,8 @@ clean-dirs += $(objtree)/debian/
 # ---------------------------------------------------------------------------
 tar%pkg: FORCE
 	$(MAKE) KBUILD_SRC=
-	$(CONFIG_SHELL) $(srctree)/scripts/package/buildtar $@
+	$(KBUILD_PKG_ROOTCMD) $(CONFIG_SHELL) \
+		$(srctree)/scripts/package/buildtar $@
 
 clean-dirs += $(objtree)/tar-install/
 
@@ -96,10 +100,10 @@ clean-dirs += $(objtree)/tar-install/
 # Help text displayed when executing 'make help'
 # ---------------------------------------------------------------------------
 help: FORCE
+	@echo ' Set KBUILD_PKG_ROOTCMD={sudo|fakeroot -u|...} and make as non-root:'
 	@echo '  rpm-pkg         - Build both source and binary RPM kernel packages'
 	@echo '  binrpm-pkg      - Build only the binary kernel package'
 	@echo '  deb-pkg         - Build the kernel as an deb package'
 	@echo '  tar-pkg         - Build the kernel as an uncompressed tarball'
 	@echo '  targz-pkg       - Build the kernel as a gzip compressed tarball'
 	@echo '  tarbz2-pkg      - Build the kernel as a bzip2 compressed tarball'
-
-- 
1.6.5.rc1.199.g596ec

[PATCH v2 2/2] scripts/package: use fakeroot if available

[Jonathan Nieder]

Unless already running as root, use fakeroot to ensure the files in
the generated binary packages are owned by root.  Without this change,
you have to set KBUILD_PKG_ROOTCMD or become root yourself to run
"make foo-pkg".

With this patch applied, you can run "make oldconfig rpm-pkg" as an
ordinary user to build a binary package for an updated kernel tree and
it should just work.

fakeroot is a bit too zealous by default in pretending files are owned
by root.  Unless directed otherwise, its wrapped stat() and lstat()
set st_uid and st_gid to 0 for all files.  This slows down the Linux
build with CONFIG_LOCALVERSION_AUTO a lot, since git notices that the
owners have changed and has to reread the entire kernel tree to learn
that the content has not changed.  Since "make modules_install" and
"scripts/package/foo" run within the same fakeroot session, we are
free to avoid this by telling fakeroot to use the actual owner and
group for preexisting files, by passing it the -u option.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Changes from v1:
 - rename ROOTCMD variable to KBUILD_PKG_ROOTCMD

 scripts/package/Makefile |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index 6997f38..2aa7e8c 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -1,6 +1,15 @@
 # Makefile for the different targets used to generate full packages of a kernel
 # It uses the generic clean infrastructure of kbuild
 
+# How to acquire (fake) root privileges
+ifndef KBUILD_PKG_ROOTCMD
+ifneq ($(shell id -u),0)
+ifeq ($(shell which fakeroot >/dev/null 2>&1 && echo found),found)
+KBUILD_PKG_ROOTCMD := fakeroot -u
+endif
+endif
+endif
+
 # RPM target
 # ---------------------------------------------------------------------------
 # The rpm target generates two rpm files:
-- 
1.6.5.rc1.199.g596ec

Re: [PATCH v2 0/2] fakeroot dwimery for scripts/package/*

[maximilian attems]

On Thu, 15 Oct 2009, Jonathan Nieder wrote:

> maximilian attems wrote:
> > Frans Pop wrote:
> >> Sam Ravnborg wrote:
> 
> >>> That could be the longer:
> >>>
> >>>    KBUILD_PKG_ROOTCMD
> >>>
> >>> Which with the KBUILD_ prefix could be a globel environment
> >>> variable for some.
> >>
> >> I have not tested it, but the patches look good otherwise, so with Sam's 
> >> suggested change:
> >> Acked-by: Frans Pop <elendil@planet.nl>
> >
> > Acked-by: maximilian attems <max@stro.at>
> >

Sam could you please push out those two patches with aboves 2 acks added,
seems author forgot to write them directly in to the -next branch?

thanks

Re: [PATCH v2 1/2] scripts/package: add KBUILD_PKG_ROOTCMD variable

[Michal Marek]

Dne 15.10.2009 15:35, Jonathan Nieder napsal(a):
> diff --git a/scripts/package/Makefile b/scripts/package/Makefile
> index f67cc88..6997f38 100644
> --- a/scripts/package/Makefile
> +++ b/scripts/package/Makefile
> @@ -53,7 +53,8 @@ rpm-pkg rpm: $(objtree)/kernel.spec FORCE
>  	set -e; \
>  	mv -f $(objtree)/.tmp_version $(objtree)/.version
>  
> -	$(RPM) $(RPMOPTS) --target $(UTS_MACHINE) -ta ../$(KERNELPATH).tar.gz
> +	$(KBUILD_PKG_ROOTCMD) $(RPM) $(RPMOPTS) --target $(UTS_MACHINE) \
> +		-ta ../$(KERNELPATH).tar.gz
>  	rm ../$(KERNELPATH).tar.gz
>  
>  clean-files := $(objtree)/kernel.spec
> @@ -70,8 +71,9 @@ binrpm-pkg: $(objtree)/binkernel.spec FORCE
>  	set -e; \
>  	mv -f $(objtree)/.tmp_version $(objtree)/.version
>  
> -	$(RPM) $(RPMOPTS) --define "_builddir $(objtree)" --target \
> -		$(UTS_MACHINE) -bb $<
> +	$(KBUILD_PKG_ROOTCMD) $(RPM) $(RPMOPTS) \
> +		--define "_builddir $(objtree)" \
> +		--target $(UTS_MACHINE) -bb $<

Why is this needed for the rpm targets? The generates spec file doesn't
require root privileges and correctly sets root as the owner of the
packaged files.


>  clean-files += $(objtree)/binkernel.spec
>  
> @@ -79,7 +81,8 @@ clean-files += $(objtree)/binkernel.spec
>  # ---------------------------------------------------------------------------
>  deb-pkg: FORCE
>  	$(MAKE) KBUILD_SRC=
> -	$(CONFIG_SHELL) $(srctree)/scripts/package/builddeb
> +	$(KBUILD_PKG_ROOTCMD) $(CONFIG_SHELL) \
> +		$(srctree)/scripts/package/builddeb
>  
>  clean-dirs += $(objtree)/debian/
>  
> @@ -88,7 +91,8 @@ clean-dirs += $(objtree)/debian/
>  # ---------------------------------------------------------------------------
>  tar%pkg: FORCE
>  	$(MAKE) KBUILD_SRC=
> -	$(CONFIG_SHELL) $(srctree)/scripts/package/buildtar $@
> +	$(KBUILD_PKG_ROOTCMD) $(CONFIG_SHELL) \
> +		$(srctree)/scripts/package/buildtar $@

GNU tar has --owner and --group options to override the owner and group
of the files in the archive, something like

opts=
if tar --owner=root --group=root --help >/dev/null 2>&1; then
    opts="--owner=root --group=root"
fi
tar cf - . $opts ...

would make tar-pkg do the right thing automatically without specifying
fakeroot. I don't know about deb-pkg, if it needs fakeroot, let's add
the variable, but it's not needed for *rpm-pkg and after a simple change
to buildtar it would not be needed for tar-pkg.

Michal

>  
>  clean-dirs += $(objtree)/tar-install/
>  
> @@ -96,10 +100,10 @@ clean-dirs += $(objtree)/tar-install/
>  # Help text displayed when executing 'make help'
>  # ---------------------------------------------------------------------------
>  help: FORCE
> +	@echo ' Set KBUILD_PKG_ROOTCMD={sudo|fakeroot -u|...} and make as non-root:'
>  	@echo '  rpm-pkg         - Build both source and binary RPM kernel packages'
>  	@echo '  binrpm-pkg      - Build only the binary kernel package'
>  	@echo '  deb-pkg         - Build the kernel as an deb package'
>  	@echo '  tar-pkg         - Build the kernel as an uncompressed tarball'
>  	@echo '  targz-pkg       - Build the kernel as a gzip compressed tarball'
>  	@echo '  tarbz2-pkg      - Build the kernel as a bzip2 compressed tarball'
> -

Re: [PATCH v2 1/2] scripts/package: add KBUILD_PKG_ROOTCMD variable

[Jonathan Nieder]

Hi,

Michal Marek wrote:
> Dne 15.10.2009 15:35, Jonathan Nieder napsal(a):

>> diff --git a/scripts/package/Makefile b/scripts/package/Makefile
>> index f67cc88..6997f38 100644
>> --- a/scripts/package/Makefile
>> +++ b/scripts/package/Makefile
>> @@ -53,7 +53,8 @@ rpm-pkg rpm: $(objtree)/kernel.spec FORCE
>>  	set -e; \
>>  	mv -f $(objtree)/.tmp_version $(objtree)/.version
>>  
>> -	$(RPM) $(RPMOPTS) --target $(UTS_MACHINE) -ta ../$(KERNELPATH).tar.gz
>> +	$(KBUILD_PKG_ROOTCMD) $(RPM) $(RPMOPTS) --target $(UTS_MACHINE) \
>> +		-ta ../$(KERNELPATH).tar.gz
>>  	rm ../$(KERNELPATH).tar.gz
>>  
>>  clean-files := $(objtree)/kernel.spec
>> @@ -70,8 +71,9 @@ binrpm-pkg: $(objtree)/binkernel.spec FORCE
>>  	set -e; \
>>  	mv -f $(objtree)/.tmp_version $(objtree)/.version
>>  
>> -	$(RPM) $(RPMOPTS) --define "_builddir $(objtree)" --target \
>> -		$(UTS_MACHINE) -bb $<
>> +	$(KBUILD_PKG_ROOTCMD) $(RPM) $(RPMOPTS) \
>> +		--define "_builddir $(objtree)" \
>> +		--target $(UTS_MACHINE) -bb $<
> 
> Why is this needed for the rpm targets? The generates spec file doesn't
> require root privileges and correctly sets root as the owner of the
> packaged files.

It was my ignorance of how rpm works.

> GNU tar has --owner and --group options to override the owner and group
> of the files in the archive, something like
> 
> opts=
> if tar --owner=root --group=root --help >/dev/null 2>&1; then
>     opts="--owner=root --group=root"
> fi
> tar cf - . $opts ...
> 
> would make tar-pkg do the right thing automatically without specifying
> fakeroot.

Sounds appealing.  Probably it is safe to assume GNU tar is available
for running tar-pkg.  I’ll look into this, though I wouldn’t mind if a
patch appears before I finish.

> I don't know about deb-pkg, if it needs fakeroot, let's add
> the variable, but it's not needed for *rpm-pkg and after a simple change
> to buildtar it would not be needed for tar-pkg.

Yes, deb-pkg needs it.

>> @@ -96,10 +100,10 @@ clean-dirs += $(objtree)/tar-install/
>>  # Help text displayed when executing 'make help'
>>  # ---------------------------------------------------------------------------
>>  help: FORCE
>> +	@echo ' Set KBUILD_PKG_ROOTCMD={sudo|fakeroot -u|...} and make as non-root:'
>>  	@echo '  rpm-pkg         - Build both source and binary RPM kernel packages'
>>  	@echo '  binrpm-pkg      - Build only the binary kernel package'
>>  	@echo '  deb-pkg         - Build the kernel as an deb package'
>>  	@echo '  tar-pkg         - Build the kernel as an uncompressed tarball'
>>  	@echo '  targz-pkg       - Build the kernel as a gzip compressed tarball'
>>  	@echo '  tarbz2-pkg      - Build the kernel as a bzip2 compressed tarball'

If this variable is to only be used for some targets, this help text
starts to look out of place.  Maybe make deb-pkg should error out with
a suggestion to set it if `id -u` is not 0, instead.

Thanks for the thoughtful review.  Updated patch coming soon.

Re: [PATCH v2 1/2] scripts/package: add KBUILD_PKG_ROOTCMD variable

[Michal Marek]

On 24.11.2009 13:03, Jonathan Nieder wrote:
> Hi,
> 
> Michal Marek wrote:
>> GNU tar has --owner and --group options to override the owner and group
>> of the files in the archive, something like
>>
>> opts=
>> if tar --owner=root --group=root --help >/dev/null 2>&1; then
>>     opts="--owner=root --group=root"
>> fi
>> tar cf - . $opts ...
>>
>> would make tar-pkg do the right thing automatically without specifying
>> fakeroot.
> 
> Sounds appealing.  Probably it is safe to assume GNU tar is available
> for running tar-pkg. I’ll look into this, though I wouldn’t mind if a
> patch appears before I finish.

I would still fallback to plain tar, you never know what exotic
system users build on and GNU tar is not mentioned in
Documentation/Changes. But you're right, this would be another patch.


>>> @@ -96,10 +100,10 @@ clean-dirs += $(objtree)/tar-install/
>>>  # Help text displayed when executing 'make help'
>>>  # ---------------------------------------------------------------------------
>>>  help: FORCE
>>> +	@echo ' Set KBUILD_PKG_ROOTCMD={sudo|fakeroot -u|...} and make as non-root:'
>>>  	@echo '  rpm-pkg         - Build both source and binary RPM kernel packages'
>>>  	@echo '  binrpm-pkg      - Build only the binary kernel package'
>>>  	@echo '  deb-pkg         - Build the kernel as an deb package'
>>>  	@echo '  tar-pkg         - Build the kernel as an uncompressed tarball'
>>>  	@echo '  targz-pkg       - Build the kernel as a gzip compressed tarball'
>>>  	@echo '  tarbz2-pkg      - Build the kernel as a bzip2 compressed tarball'
> 
> If this variable is to only be used for some targets, this help text
> starts to look out of place.  Maybe make deb-pkg should error out with
> a suggestion to set it if `id -u` is not 0, instead.

Good idea.


> Thanks for the thoughtful review.  Updated patch coming soon.

Thanks!

Michal

[PATCH v3 0/3] fakeroot dwimery for deb-pkg target

[Jonathan Nieder]

Hi,

As Michal noticed, the tar*-pkg and *rpm-pkg targets can work fine
without being able to write files owned by root.  Only deb-pkg needs
to learn to acquire faked root privileges.  Here is an updated series
reflecting this.

This version no longer documents the KBUILD_PKG_ROOTCMD variable in
"make help" output.  If make already has root privileges or fakeroot
is installed, then the target will just work without any need for the
user to worry about this.  If not, instead of waiting from an error
from chmod, it seemed better to give an error right away with advice
to set KBUILD_PKG_ROOTCMD appropriately.

Since the scope of the variable’s applicability somewhat, I have not
carried over any acks.  But I am hoping you will still like the
patches. :)

Jonathan Nieder (2):
  scripts/package: add KBUILD_PKG_ROOTCMD variable
  scripts/package: deb-pkg: use fakeroot if available

Michal Marek (1):
  scripts/package: tar-pkg: use tar --owner=root

 scripts/package/Makefile |   20 +++++++++++++++++++-
 scripts/package/buildtar |    6 +++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

[PATCH 1/3] scripts/package: tar-pkg: use tar --owner=root

[Jonathan Nieder]

From: Michal Marek <mmarek@suse.cz>

Use the --owner= and --group= options to make sure the entries in
the built tar file are owned by root.  Without this change, a
careless sysadmin using the tar-pkg target can easily end up
installing a kernel that is writable by the unprivileged user
account used to build the kernel.

Test that these options are understood before using them so that
non-GNU versions of tar can still be used if the operator is
appropriately cautious.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 scripts/package/buildtar |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/scripts/package/buildtar b/scripts/package/buildtar
index b1fd48d..51b2aa0 100644
--- a/scripts/package/buildtar
+++ b/scripts/package/buildtar
@@ -101,7 +101,11 @@ esac
 #
 (
 	cd "${tmpdir}"
-	tar cf - . | ${compress} > "${tarball}${file_ext}"
+	opts=
+	if tar --owner=root --group=root --help >/dev/null 2>&1; then
+		opts="--owner=root --group=root"
+	fi
+	tar cf - . $opts | ${compress} > "${tarball}${file_ext}"
 )
 
 echo "Tarball successfully created in ${tarball}${file_ext}"
-- 
1.6.5.3

[PATCH v3 2/3] scripts/package: add KBUILD_PKG_ROOTCMD variable

[Jonathan Nieder]

Let the deb-pkg target acquire (fake) root privileges before
running commands that need them.  Without such privileges,
deb-pkg errors out because chown fails.

The new KBUILD_PKG_ROOTCMD variable, if defined, is used as a
command to run other commands with possibly fake elevated
privileges.  Since this is not needed for the tar-pkg and rpm-pkg
targets, it is only used by deb-pkg.  If it is not defined, the
behavior is as before, and the user will have to rerun make as
root.

In other words, as a shortcut, instead of running 'make oldconfig &&
make && fakeroot -u make deb-pkg', one can use the single command
'make oldconfig deb-pkg KBUILD_PKG_ROOTCMD="fakeroot -u"'.

Suggested-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This patch does not use the variable in the rpm-pkg or tar-pkg targets
any more, since they already cope fine as non-root.

 scripts/package/Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index f67cc88..5c0b43a 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -79,7 +79,8 @@ clean-files += $(objtree)/binkernel.spec
 # ---------------------------------------------------------------------------
 deb-pkg: FORCE
 	$(MAKE) KBUILD_SRC=
-	$(CONFIG_SHELL) $(srctree)/scripts/package/builddeb
+	$(KBUILD_PKG_ROOTCMD) $(CONFIG_SHELL) \
+		$(srctree)/scripts/package/builddeb
 
 clean-dirs += $(objtree)/debian/
 
-- 
1.6.5.3

[PATCH v3 3/3] scripts/package: deb-pkg: use fakeroot if available

[Jonathan Nieder]

Running "make deb-pkg" requires setting KBUILD_PKG_ROOTCMD or
becoming root oneself or it errors out.  Unless already running
as root or KBUILD_PKG_ROOTCMD is already set, use fakeroot as a
good default.

With this patch applied, you can run "make oldconfig rpm-pkg" as
an ordinary user to build a binary package for an updated kernel
tree and it should just work.

fakeroot is too zealous by default in treating files as owned by
root.  Its wrapped stat() sets st_uid and st_gid to 0 for all
files, which causes Git to go on a wild goose chase if
CONFIG_LOCALVERSION_AUTO is set, checking if any file's content
has changed along with its stat information.  Avoid this by
telling fakeroot to use the actual owner and group for
preexisting files, by passing it the -u option.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
The appropriate value for KBUILD_PKG_ROOTCMD is determined in the
rules for deb-pkg instead of the outer Makefile to avoid having to
fork for 'which' and 'id' for unrelated targets.

Before this change, the command for the deb-pkg target was not
abbreviated, so that normal output included something like "bash
/tmp/buildd/scripts/package/builddeb".  It is a shame to get rid of
this kind of transparency, but when I tried lying by setting
quiet_cmd_builddeb to that string, it was confusing to see 'make'
claiming to run that command through bash and not through fakeroot.
So I punted by abbreviating to BUILDDEB.  The curious can always use
"V=1".

 scripts/package/Makefile |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index 5c0b43a..62fcc3a 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -77,10 +77,27 @@ clean-files += $(objtree)/binkernel.spec
 
 # Deb target
 # ---------------------------------------------------------------------------
+quiet_cmd_builddeb = BUILDDEB
+      cmd_builddeb = set -e; \
+	test `id -u` = 0 || \
+	test -n "$(KBUILD_PKG_ROOTCMD)" || { \
+		which fakeroot >/dev/null 2>&1 && \
+		KBUILD_PKG_ROOTCMD="fakeroot -u"; \
+	} || { \
+		echo; \
+		echo "builddeb must be run as root (or using fakeroot)."; \
+		echo "KBUILD_PKG_ROOTCMD is unset and fakeroot not found."; \
+		echo "Try setting KBUILD_PKG_ROOTCMD to a command to acquire"; \
+		echo "root privileges (e.g., 'fakeroot -u' or 'sudo')."; \
+		false; \
+	} && \
+	\
+	$$KBUILD_PKG_ROOTCMD $(CONFIG_SHELL) \
+		$(srctree)/scripts/package/builddeb
+
 deb-pkg: FORCE
 	$(MAKE) KBUILD_SRC=
-	$(KBUILD_PKG_ROOTCMD) $(CONFIG_SHELL) \
-		$(srctree)/scripts/package/builddeb
+	$(call cmd,builddeb)
 
 clean-dirs += $(objtree)/debian/
 
-- 
1.6.5.3

Re: [PATCH v3 3/3] scripts/package: deb-pkg: use fakeroot if available

[Michal Marek]

Jonathan Nieder wrote:
> Running "make deb-pkg" requires setting KBUILD_PKG_ROOTCMD or
> becoming root oneself or it errors out.  Unless already running
> as root or KBUILD_PKG_ROOTCMD is already set, use fakeroot as a
> good default.
> 
> With this patch applied, you can run "make oldconfig rpm-pkg" as
                                                       deb-pkg :)

Otherwise Acked-by: Michal Marek <mmarek@suse.cz> for this and the
previous patch.

Michal