[PATCH] buildchroot: chown the whole WORKDIR to builder:builder

[PATCH] buildchroot: chown the whole WORKDIR to builder:builder

[Henning Schild]

The initial idea was to chown as little as possible, but there is a
"chown -R" in do_unpack that we need to work against when rebuilding a
package.

Fixes: be291cd991bd ("buildchroot: build debian packages as "builder" not "root"")
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/recipes-devtools/buildchroot/files/build.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/meta/recipes-devtools/buildchroot/files/build.sh b/meta/recipes-devtools/buildchroot/files/build.sh
index 66b1a77..112aac3 100644
--- a/meta/recipes-devtools/buildchroot/files/build.sh
+++ b/meta/recipes-devtools/buildchroot/files/build.sh
@@ -15,6 +15,5 @@ for i in configure aclocal.m4 Makefile.am Makefile.in; do
 done
 
 # Build the package as user "builder"
-chown -R builder:builder $1 # the sources
-chown builder:builder $1/.. # the output
+chown -R builder:builder $1/.. # the whole WORKDIR
 su builder -c "cd $1; dpkg-buildpackage -a$target_arch -d --source-option=-I"
-- 
2.19.1

Re: [PATCH] buildchroot: chown the whole WORKDIR to builder:builder

[Henning Schild]

This is truly wrong, because now we chown temp/ and other bitbake owned
files.
The real fix probably is reverting 624b7c484bf5, will send another
patch.

Henning

Am Mon, 12 Nov 2018 12:53:15 +0100
schrieb Henning Schild <henning.schild@siemens.com>:

> The initial idea was to chown as little as possible, but there is a
> "chown -R" in do_unpack that we need to work against when rebuilding a
> package.
> 
> Fixes: be291cd991bd ("buildchroot: build debian packages as "builder"
> not "root"") Signed-off-by: Henning Schild
> <henning.schild@siemens.com> ---
>  meta/recipes-devtools/buildchroot/files/build.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/meta/recipes-devtools/buildchroot/files/build.sh
> b/meta/recipes-devtools/buildchroot/files/build.sh index
> 66b1a77..112aac3 100644 ---
> a/meta/recipes-devtools/buildchroot/files/build.sh +++
> b/meta/recipes-devtools/buildchroot/files/build.sh @@ -15,6 +15,5 @@
> for i in configure aclocal.m4 Makefile.am Makefile.in; do done
>  
>  # Build the package as user "builder"
> -chown -R builder:builder $1 # the sources
> -chown builder:builder $1/.. # the output
> +chown -R builder:builder $1/.. # the whole WORKDIR
>  su builder -c "cd $1; dpkg-buildpackage -a$target_arch -d
> --source-option=-I"

Re: [PATCH] buildchroot: chown the whole WORKDIR to builder:builder

[Jan Kiszka]

On 12.11.18 14:46, [ext] Henning Schild wrote:
> This is truly wrong, because now we chown temp/ and other bitbake owned
> files.
> The real fix probably is reverting 624b7c484bf5, will send another
> patch.

Please finally accept: We need alignment of the ownership rights (and that 
implies user IDs) between the bitbake and the chroot build environment. Then and 
only then we can revert that commit.

Jan

> 
> Henning
> 
> Am Mon, 12 Nov 2018 12:53:15 +0100
> schrieb Henning Schild <henning.schild@siemens.com>:
> 
>> The initial idea was to chown as little as possible, but there is a
>> "chown -R" in do_unpack that we need to work against when rebuilding a
>> package.
>>
>> Fixes: be291cd991bd ("buildchroot: build debian packages as "builder"
>> not "root"") Signed-off-by: Henning Schild
>> <henning.schild@siemens.com> ---
>>   meta/recipes-devtools/buildchroot/files/build.sh | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/meta/recipes-devtools/buildchroot/files/build.sh
>> b/meta/recipes-devtools/buildchroot/files/build.sh index
>> 66b1a77..112aac3 100644 ---
>> a/meta/recipes-devtools/buildchroot/files/build.sh +++
>> b/meta/recipes-devtools/buildchroot/files/build.sh @@ -15,6 +15,5 @@
>> for i in configure aclocal.m4 Makefile.am Makefile.in; do done
>>   
>>   # Build the package as user "builder"
>> -chown -R builder:builder $1 # the sources
>> -chown builder:builder $1/.. # the output
>> +chown -R builder:builder $1/.. # the whole WORKDIR
>>   su builder -c "cd $1; dpkg-buildpackage -a$target_arch -d
>> --source-option=-I"
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] buildchroot: chown the whole WORKDIR to builder:builder

[Henning Schild]

Am Mon, 12 Nov 2018 14:55:30 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 12.11.18 14:46, [ext] Henning Schild wrote:
> > This is truly wrong, because now we chown temp/ and other bitbake
> > owned files.
> > The real fix probably is reverting 624b7c484bf5, will send another
> > patch.  
> 
> Please finally accept: We need alignment of the ownership rights (and
> that implies user IDs) between the bitbake and the chroot build
> environment. Then and only then we can revert that commit.

I can confirm that just reverting this one will also not be correct.
And yes i would now also conclude that aligning the ids is probably
what we should do.

The reason is that we access several folders inside WORKDIR as non-root
from both inside and outside chroot. The alternative to alignment would
be chowning back and forth as we enter/leave ... not even thinking
about parallel jobs.

I will send a series where i will include your alignment patch, to
propose a merging order of patches.

Henning


> Jan
> 
> > 
> > Henning
> > 
> > Am Mon, 12 Nov 2018 12:53:15 +0100
> > schrieb Henning Schild <henning.schild@siemens.com>:
> >   
> >> The initial idea was to chown as little as possible, but there is a
> >> "chown -R" in do_unpack that we need to work against when
> >> rebuilding a package.
> >>
> >> Fixes: be291cd991bd ("buildchroot: build debian packages as
> >> "builder" not "root"") Signed-off-by: Henning Schild
> >> <henning.schild@siemens.com> ---
> >>   meta/recipes-devtools/buildchroot/files/build.sh | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/meta/recipes-devtools/buildchroot/files/build.sh
> >> b/meta/recipes-devtools/buildchroot/files/build.sh index
> >> 66b1a77..112aac3 100644 ---
> >> a/meta/recipes-devtools/buildchroot/files/build.sh +++
> >> b/meta/recipes-devtools/buildchroot/files/build.sh @@ -15,6 +15,5
> >> @@ for i in configure aclocal.m4 Makefile.am Makefile.in; do done
> >>   
> >>   # Build the package as user "builder"
> >> -chown -R builder:builder $1 # the sources
> >> -chown builder:builder $1/.. # the output
> >> +chown -R builder:builder $1/.. # the whole WORKDIR
> >>   su builder -c "cd $1; dpkg-buildpackage -a$target_arch -d
> >> --source-option=-I"  
> >   
>