[PATCH 0/3] "root" to "builder" repair series

[PATCH 0/3] "root" to "builder" repair series

[Henning Schild]

These three patches should be applied in the given order. The one from
Jan has been taken off the Mailinglist and has started the discussion
about reverting the other one and not doing chowning anymore.

Henning Schild (2):
  Revert "Change ownership of WORKDIR prior to unpacking"
  buildchroot: do not chown to builder:builder anymore

Jan Kiszka (1):
  buildchroot: Align UID and GID of builder user with caller

 meta/classes/base.bbclass                               | 5 -----
 meta/recipes-devtools/buildchroot/buildchroot.inc       | 4 +++-
 meta/recipes-devtools/buildchroot/files/build.sh        | 2 --
 meta/recipes-devtools/buildchroot/files/configscript.sh | 4 ++--
 4 files changed, 5 insertions(+), 10 deletions(-)

-- 
2.19.1

[PATCH 1/3] buildchroot: Align UID and GID of builder user with caller

[Henning Schild]

From: Jan Kiszka <jan.kiszka@siemens.com>

This fixes EPERM on rebuild and also some clean builds: We have to align
the IDs of the builder user with the user in the host environment.
Otherwise, files and directories can become unaccessible during the
build.

Fixes: be291cd991bd ("buildchroot: build debian packages as "builder" not "root"")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 meta/recipes-devtools/buildchroot/buildchroot.inc       | 4 +++-
 meta/recipes-devtools/buildchroot/files/configscript.sh | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 7dd909e..2c44db9 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -36,7 +36,9 @@ do_build() {
 
     # Configure root filesystem
     sudo install -m 755 ${WORKDIR}/configscript.sh ${BUILDCHROOT_DIR}
-    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
+    USER_ID=$(id -u)
+    GROUP_ID=$(id -g)
+    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh $USER_ID $GROUP_ID
 
     sudo mount --bind ${DL_DIR} ${BUILDCHROOT_DIR}/downloads
 }
diff --git a/meta/recipes-devtools/buildchroot/files/configscript.sh b/meta/recipes-devtools/buildchroot/files/configscript.sh
index 30660e7..7e49385 100644
--- a/meta/recipes-devtools/buildchroot/files/configscript.sh
+++ b/meta/recipes-devtools/buildchroot/files/configscript.sh
@@ -10,6 +10,6 @@ locales locales/locales_to_be_generated multiselect en_US.UTF-8 UTF-8
 locales locales/default_environment_locale select en_US.UTF-8
 END
 
-addgroup --quiet --system builder
-useradd --system --gid builder --no-create-home --home /home/builder --no-user-group --comment "Isar buildchroot build user" builder
+addgroup --quiet --system builder --gid $2
+useradd --system --uid $1 --gid builder --no-create-home --home /home/builder --no-user-group --comment "Isar buildchroot build user" builder
 chown -R builder:builder /home/builder
-- 
2.19.1

Re: [PATCH 1/3] buildchroot: Align UID and GID of builder user with caller

[Jan Kiszka]

On 12.11.18 16:51, Henning Schild wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This fixes EPERM on rebuild and also some clean builds: We have to align
> the IDs of the builder user with the user in the host environment.
> Otherwise, files and directories can become unaccessible during the
> build.
> 
> Fixes: be291cd991bd ("buildchroot: build debian packages as "builder" not "root"")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>   meta/recipes-devtools/buildchroot/buildchroot.inc       | 4 +++-
>   meta/recipes-devtools/buildchroot/files/configscript.sh | 4 ++--
>   2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
> index 7dd909e..2c44db9 100644
> --- a/meta/recipes-devtools/buildchroot/buildchroot.inc
> +++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
> @@ -36,7 +36,9 @@ do_build() {
>   
>       # Configure root filesystem
>       sudo install -m 755 ${WORKDIR}/configscript.sh ${BUILDCHROOT_DIR}
> -    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
> +    USER_ID=$(id -u)
> +    GROUP_ID=$(id -g)
> +    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh $USER_ID $GROUP_ID
>   
>       sudo mount --bind ${DL_DIR} ${BUILDCHROOT_DIR}/downloads
>   }
> diff --git a/meta/recipes-devtools/buildchroot/files/configscript.sh b/meta/recipes-devtools/buildchroot/files/configscript.sh
> index 30660e7..7e49385 100644
> --- a/meta/recipes-devtools/buildchroot/files/configscript.sh
> +++ b/meta/recipes-devtools/buildchroot/files/configscript.sh
> @@ -10,6 +10,6 @@ locales locales/locales_to_be_generated multiselect en_US.UTF-8 UTF-8
>   locales locales/default_environment_locale select en_US.UTF-8
>   END
>   
> -addgroup --quiet --system builder
> -useradd --system --gid builder --no-create-home --home /home/builder --no-user-group --comment "Isar buildchroot build user" builder
> +addgroup --quiet --system builder --gid $2
> +useradd --system --uid $1 --gid builder --no-create-home --home /home/builder --no-user-group --comment "Isar buildchroot build user" builder
>   chown -R builder:builder /home/builder
> 

-o ?

Jan

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

Re: [PATCH 1/3] buildchroot: Align UID and GID of builder user with caller

[Henning Schild]

Am Tue, 13 Nov 2018 08:53:03 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 12.11.18 16:51, Henning Schild wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > This fixes EPERM on rebuild and also some clean builds: We have to
> > align the IDs of the builder user with the user in the host
> > environment. Otherwise, files and directories can become
> > unaccessible during the build.
> > 
> > Fixes: be291cd991bd ("buildchroot: build debian packages as
> > "builder" not "root"") Signed-off-by: Jan Kiszka
> > <jan.kiszka@siemens.com> ---
> >   meta/recipes-devtools/buildchroot/buildchroot.inc       | 4 +++-
> >   meta/recipes-devtools/buildchroot/files/configscript.sh | 4 ++--
> >   2 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc
> > b/meta/recipes-devtools/buildchroot/buildchroot.inc index
> > 7dd909e..2c44db9 100644 ---
> > a/meta/recipes-devtools/buildchroot/buildchroot.inc +++
> > b/meta/recipes-devtools/buildchroot/buildchroot.inc @@ -36,7 +36,9
> > @@ do_build() { 
> >       # Configure root filesystem
> >       sudo install -m 755 ${WORKDIR}/configscript.sh
> > ${BUILDCHROOT_DIR}
> > -    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
> > +    USER_ID=$(id -u)
> > +    GROUP_ID=$(id -g)
> > +    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh $USER_ID
> > $GROUP_ID 
> >       sudo mount --bind ${DL_DIR} ${BUILDCHROOT_DIR}/downloads
> >   }
> > diff --git
> > a/meta/recipes-devtools/buildchroot/files/configscript.sh
> > b/meta/recipes-devtools/buildchroot/files/configscript.sh index
> > 30660e7..7e49385 100644 ---
> > a/meta/recipes-devtools/buildchroot/files/configscript.sh +++
> > b/meta/recipes-devtools/buildchroot/files/configscript.sh @@ -10,6
> > +10,6 @@ locales locales/locales_to_be_generated multiselect
> > en_US.UTF-8 UTF-8 locales locales/default_environment_locale select
> > en_US.UTF-8 END -addgroup --quiet --system builder
> > -useradd --system --gid builder --no-create-home
> > --home /home/builder --no-user-group --comment "Isar buildchroot
> > build user" builder +addgroup --quiet --system builder --gid $2
> > +useradd --system --uid $1 --gid builder --no-create-home
> > --home /home/builder --no-user-group --comment "Isar buildchroot
> > build user" builder chown -R builder:builder /home/builder 
> 
> -o ?

Not sure that would be the best idea. And we still have the problem
with the GID. In that version the commands should fail on a clash, and
that situation is actually highly unlikely. So i would rather wait for
that odd situation before applying even more hacks.
If you still want to change that patch, reply your v2 in this thread.

Henning

> Jan
> 

[PATCH v2 1/3] buildchroot: Align UID and GID of builder user with caller

[Jan Kiszka]

This fixes EPERM on rebuild and also some clean builds: We have to align
the IDs of the builder user with the user in the host environment.
Otherwise, files and directories can become unaccessible during the
build.

Fixes: be291cd991bd ("buildchroot: build debian packages as "builder" not "root"")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - permit duplicate UID and GID inside buildchroot

 meta/recipes-devtools/buildchroot/buildchroot.inc       | 4 +++-
 meta/recipes-devtools/buildchroot/files/configscript.sh | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 7dd909e..2c44db9 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -36,7 +36,9 @@ do_build() {
 
     # Configure root filesystem
     sudo install -m 755 ${WORKDIR}/configscript.sh ${BUILDCHROOT_DIR}
-    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
+    USER_ID=$(id -u)
+    GROUP_ID=$(id -g)
+    sudo chroot ${BUILDCHROOT_DIR} /configscript.sh $USER_ID $GROUP_ID
 
     sudo mount --bind ${DL_DIR} ${BUILDCHROOT_DIR}/downloads
 }
diff --git a/meta/recipes-devtools/buildchroot/files/configscript.sh b/meta/recipes-devtools/buildchroot/files/configscript.sh
index 30660e7..25a21ef 100644
--- a/meta/recipes-devtools/buildchroot/files/configscript.sh
+++ b/meta/recipes-devtools/buildchroot/files/configscript.sh
@@ -10,6 +10,6 @@ locales locales/locales_to_be_generated multiselect en_US.UTF-8 UTF-8
 locales locales/default_environment_locale select en_US.UTF-8
 END
 
-addgroup --quiet --system builder
-useradd --system --gid builder --no-create-home --home /home/builder --no-user-group --comment "Isar buildchroot build user" builder
+groupadd --system builder -o --gid $2
+useradd --system -o --uid $1 --gid builder --no-create-home --home /home/builder --no-user-group --comment "Isar buildchroot build user" builder
 chown -R builder:builder /home/builder
-- 
2.16.4

[PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Henning Schild]

We do not build as root anymore and the non-root uid/gid are now in sync
between inside and outside the chroot.

This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
---
 meta/classes/base.bbclass | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index fce1084..d4082de 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
 
 # Unpack package and put it into working directory
 python do_unpack() {
-    import subprocess
-
     src_uri = (d.getVar('SRC_URI', True) or "").split()
     if len(src_uri) == 0:
         return
 
     rootdir = d.getVar('WORKDIR', True)
 
-    uid = str(os.getuid())
-    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir, shell=True)
-
     try:
         fetcher = bb.fetch2.Fetch(src_uri, d)
         fetcher.unpack(rootdir)
-- 
2.19.1

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Jan Kiszka]

On 12.11.18 16:51, Henning Schild wrote:
> We do not build as root anymore and the non-root uid/gid are now in sync
> between inside and outside the chroot.
> 
> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
> ---
>   meta/classes/base.bbclass | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
> index fce1084..d4082de 100644
> --- a/meta/classes/base.bbclass
> +++ b/meta/classes/base.bbclass
> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>   
>   # Unpack package and put it into working directory
>   python do_unpack() {
> -    import subprocess
> -
>       src_uri = (d.getVar('SRC_URI', True) or "").split()
>       if len(src_uri) == 0:
>           return
>   
>       rootdir = d.getVar('WORKDIR', True)
>   
> -    uid = str(os.getuid())
> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir, shell=True)
> -
>       try:
>           fetcher = bb.fetch2.Fetch(src_uri, d)
>           fetcher.unpack(rootdir)
> 

This possibly causes this regression:

ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed: do_install (log file is located at /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
ERROR: Logfile of failure stored in: /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263
Log data follows:
| DEBUG: Executing shell function do_install
| install: cannot remove '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service': Permission denied
| WARNING: exit code 1 from a shell command.
| ERROR: Function failed: do_install (log file is located at /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)

Jan

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

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Jan Kiszka]

On 14.11.18 14:10, Jan Kiszka wrote:
> On 12.11.18 16:51, Henning Schild wrote:
>> We do not build as root anymore and the non-root uid/gid are now in sync
>> between inside and outside the chroot.
>>
>> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
>> ---
>>    meta/classes/base.bbclass | 5 -----
>>    1 file changed, 5 deletions(-)
>>
>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>> index fce1084..d4082de 100644
>> --- a/meta/classes/base.bbclass
>> +++ b/meta/classes/base.bbclass
>> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>    
>>    # Unpack package and put it into working directory
>>    python do_unpack() {
>> -    import subprocess
>> -
>>        src_uri = (d.getVar('SRC_URI', True) or "").split()
>>        if len(src_uri) == 0:
>>            return
>>    
>>        rootdir = d.getVar('WORKDIR', True)
>>    
>> -    uid = str(os.getuid())
>> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir, shell=True)
>> -
>>        try:
>>            fetcher = bb.fetch2.Fetch(src_uri, d)
>>            fetcher.unpack(rootdir)
>>
> 
> This possibly causes this regression:
> 
> ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed: do_install (log file is located at /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
> ERROR: Logfile of failure stored in: /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263
> Log data follows:
> | DEBUG: Executing shell function do_install
> | install: cannot remove '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service': Permission denied
> | WARNING: exit code 1 from a shell command.
> | ERROR: Function failed: do_install (log file is located at /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
> 

OK, that wasn't a "clean" re-build (rather a rebuild after the Isar update). 
Retesting with the same Isar version for build 1 and 2.

Jan

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

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Jan Kiszka]

On 14.11.18 14:23, Jan Kiszka wrote:
> On 14.11.18 14:10, Jan Kiszka wrote:
>> On 12.11.18 16:51, Henning Schild wrote:
>>> We do not build as root anymore and the non-root uid/gid are now in sync
>>> between inside and outside the chroot.
>>>
>>> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
>>> ---
>>>    meta/classes/base.bbclass | 5 -----
>>>    1 file changed, 5 deletions(-)
>>>
>>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>>> index fce1084..d4082de 100644
>>> --- a/meta/classes/base.bbclass
>>> +++ b/meta/classes/base.bbclass
>>> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>>    # Unpack package and put it into working directory
>>>    python do_unpack() {
>>> -    import subprocess
>>> -
>>>        src_uri = (d.getVar('SRC_URI', True) or "").split()
>>>        if len(src_uri) == 0:
>>>            return
>>>        rootdir = d.getVar('WORKDIR', True)
>>> -    uid = str(os.getuid())
>>> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir, shell=True)
>>> -
>>>        try:
>>>            fetcher = bb.fetch2.Fetch(src_uri, d)
>>>            fetcher.unpack(rootdir)
>>>
>>
>> This possibly causes this regression:
>>
>> ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed: do_install 
>> (log file is located at 
>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263) 
>>
>> ERROR: Logfile of failure stored in: 
>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263 
>>
>> Log data follows:
>> | DEBUG: Executing shell function do_install
>> | install: cannot remove 
>> '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service': 
>> Permission denied
>> | WARNING: exit code 1 from a shell command.
>> | ERROR: Function failed: do_install (log file is located at 
>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263) 
>>
>>
> 
> OK, that wasn't a "clean" re-build (rather a rebuild after the Isar update). 
> Retesting with the same Isar version for build 1 and 2.
> 

It persists, at least for dpkg-raw, and that is likely because of the chroot we 
do for that package.

How does normal dpkg get away without chroot now and still produce the right 
ownership when installing a package? Can we adopt that?

Jan

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

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Henning Schild]

Am Wed, 14 Nov 2018 14:58:57 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 14.11.18 14:23, Jan Kiszka wrote:
> > On 14.11.18 14:10, Jan Kiszka wrote:  
> >> On 12.11.18 16:51, Henning Schild wrote:  
> >>> We do not build as root anymore and the non-root uid/gid are now
> >>> in sync between inside and outside the chroot.
> >>>
> >>> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
> >>> ---
> >>>    meta/classes/base.bbclass | 5 -----
> >>>    1 file changed, 5 deletions(-)
> >>>
> >>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
> >>> index fce1084..d4082de 100644
> >>> --- a/meta/classes/base.bbclass
> >>> +++ b/meta/classes/base.bbclass
> >>> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] =
> >>> "${DISTRO}-${DISTRO_ARCH}" # Unpack package and put it into
> >>> working directory python do_unpack() {
> >>> -    import subprocess
> >>> -
> >>>        src_uri = (d.getVar('SRC_URI', True) or "").split()
> >>>        if len(src_uri) == 0:
> >>>            return
> >>>        rootdir = d.getVar('WORKDIR', True)
> >>> -    uid = str(os.getuid())
> >>> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir,
> >>> shell=True) -
> >>>        try:
> >>>            fetcher = bb.fetch2.Fetch(src_uri, d)
> >>>            fetcher.unpack(rootdir)
> >>>  
> >>
> >> This possibly causes this regression:
> >>
> >> ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed:
> >> do_install (log file is located at 
> >> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263) 
> >>
> >> ERROR: Logfile of failure stored in: 
> >> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263 
> >>
> >> Log data follows:
> >> | DEBUG: Executing shell function do_install
> >> | install: cannot remove 
> >> '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service': 
> >> Permission denied
> >> | WARNING: exit code 1 from a shell command.
> >> | ERROR: Function failed: do_install (log file is located at 
> >> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263) 
> >>
> >>  
> > 
> > OK, that wasn't a "clean" re-build (rather a rebuild after the Isar
> > update). Retesting with the same Isar version for build 1 and 2.
> >   

Arghh ... on a side-note. I have a few Isar-CI partial rebuild patches
in a queue, they target both dpkg and dpkg-raw.

> It persists, at least for dpkg-raw, and that is likely because of the
> chroot we do for that package.

Yes.

> How does normal dpkg get away without chroot now and still produce
> the right ownership when installing a package? Can we adopt that?

Probably fakeroot, i will look into using that for creating the raw
package.

Henning

> Jan
> 

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Henning Schild]

Am Wed, 14 Nov 2018 15:11:39 +0100
schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:

> Am Wed, 14 Nov 2018 14:58:57 +0100
> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> 
> > On 14.11.18 14:23, Jan Kiszka wrote:  
> > > On 14.11.18 14:10, Jan Kiszka wrote:    
> > >> On 12.11.18 16:51, Henning Schild wrote:    
> > >>> We do not build as root anymore and the non-root uid/gid are now
> > >>> in sync between inside and outside the chroot.
> > >>>
> > >>> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
> > >>> ---
> > >>>    meta/classes/base.bbclass | 5 -----
> > >>>    1 file changed, 5 deletions(-)
> > >>>
> > >>> diff --git a/meta/classes/base.bbclass
> > >>> b/meta/classes/base.bbclass index fce1084..d4082de 100644
> > >>> --- a/meta/classes/base.bbclass
> > >>> +++ b/meta/classes/base.bbclass
> > >>> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] =
> > >>> "${DISTRO}-${DISTRO_ARCH}" # Unpack package and put it into
> > >>> working directory python do_unpack() {
> > >>> -    import subprocess
> > >>> -
> > >>>        src_uri = (d.getVar('SRC_URI', True) or "").split()
> > >>>        if len(src_uri) == 0:
> > >>>            return
> > >>>        rootdir = d.getVar('WORKDIR', True)
> > >>> -    uid = str(os.getuid())
> > >>> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir,
> > >>> shell=True) -
> > >>>        try:
> > >>>            fetcher = bb.fetch2.Fetch(src_uri, d)
> > >>>            fetcher.unpack(rootdir)
> > >>>    
> > >>
> > >> This possibly causes this regression:
> > >>
> > >> ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed:
> > >> do_install (log file is located at 
> > >> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263) 
> > >>
> > >> ERROR: Logfile of failure stored in: 
> > >> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263 
> > >>
> > >> Log data follows:
> > >> | DEBUG: Executing shell function do_install
> > >> | install: cannot remove 
> > >> '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service': 
> > >> Permission denied
> > >> | WARNING: exit code 1 from a shell command.
> > >> | ERROR: Function failed: do_install (log file is located at 
> > >> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263) 
> > >>
> > >>    
> > > 
> > > OK, that wasn't a "clean" re-build (rather a rebuild after the
> > > Isar update). Retesting with the same Isar version for build 1
> > > and 2. 
> 
> Arghh ... on a side-note. I have a few Isar-CI partial rebuild patches
> in a queue, they target both dpkg and dpkg-raw.
> 
> > It persists, at least for dpkg-raw, and that is likely because of
> > the chroot we do for that package.  
> 
> Yes.

That actually revealed that we kept collecting changes in ${D} and
never cleaned it before install. I just sent a patch fixing that.

Next step will probably be to make the raw class use standard
debian/rules and just take the long dpkg-buildpkg route. Should not be
too hard to just write a debian/ folder that just does "make install".
Will look into that ...

Henning

> > How does normal dpkg get away without chroot now and still produce
> > the right ownership when installing a package? Can we adopt that?  
> 
> Probably fakeroot, i will look into using that for creating the raw
> package.
> 
> Henning
> 
> > Jan
> >   
> 

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Jan Kiszka]

On 14.11.18 16:43, Henning Schild wrote:
> Am Wed, 14 Nov 2018 15:11:39 +0100
> schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:
> 
>> Am Wed, 14 Nov 2018 14:58:57 +0100
>> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>>
>>> On 14.11.18 14:23, Jan Kiszka wrote:
>>>> On 14.11.18 14:10, Jan Kiszka wrote:
>>>>> On 12.11.18 16:51, Henning Schild wrote:
>>>>>> We do not build as root anymore and the non-root uid/gid are now
>>>>>> in sync between inside and outside the chroot.
>>>>>>
>>>>>> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
>>>>>> ---
>>>>>>     meta/classes/base.bbclass | 5 -----
>>>>>>     1 file changed, 5 deletions(-)
>>>>>>
>>>>>> diff --git a/meta/classes/base.bbclass
>>>>>> b/meta/classes/base.bbclass index fce1084..d4082de 100644
>>>>>> --- a/meta/classes/base.bbclass
>>>>>> +++ b/meta/classes/base.bbclass
>>>>>> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] =
>>>>>> "${DISTRO}-${DISTRO_ARCH}" # Unpack package and put it into
>>>>>> working directory python do_unpack() {
>>>>>> -    import subprocess
>>>>>> -
>>>>>>         src_uri = (d.getVar('SRC_URI', True) or "").split()
>>>>>>         if len(src_uri) == 0:
>>>>>>             return
>>>>>>         rootdir = d.getVar('WORKDIR', True)
>>>>>> -    uid = str(os.getuid())
>>>>>> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir,
>>>>>> shell=True) -
>>>>>>         try:
>>>>>>             fetcher = bb.fetch2.Fetch(src_uri, d)
>>>>>>             fetcher.unpack(rootdir)
>>>>>>     
>>>>>
>>>>> This possibly causes this regression:
>>>>>
>>>>> ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed:
>>>>> do_install (log file is located at
>>>>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
>>>>>
>>>>> ERROR: Logfile of failure stored in:
>>>>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263
>>>>>
>>>>> Log data follows:
>>>>> | DEBUG: Executing shell function do_install
>>>>> | install: cannot remove
>>>>> '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service':
>>>>> Permission denied
>>>>> | WARNING: exit code 1 from a shell command.
>>>>> | ERROR: Function failed: do_install (log file is located at
>>>>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
>>>>>
>>>>>     
>>>>
>>>> OK, that wasn't a "clean" re-build (rather a rebuild after the
>>>> Isar update). Retesting with the same Isar version for build 1
>>>> and 2.
>>
>> Arghh ... on a side-note. I have a few Isar-CI partial rebuild patches
>> in a queue, they target both dpkg and dpkg-raw.
>>
>>> It persists, at least for dpkg-raw, and that is likely because of
>>> the chroot we do for that package.
>>
>> Yes.
> 
> That actually revealed that we kept collecting changes in ${D} and
> never cleaned it before install. I just sent a patch fixing that.
> 
> Next step will probably be to make the raw class use standard
> debian/rules and just take the long dpkg-buildpkg route. Should not be
> too hard to just write a debian/ folder that just does "make install".
> Will look into that ...
> 

Thanks, looking forward to test everything.

Jan

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

Re: [PATCH 2/3] Revert "Change ownership of WORKDIR prior to unpacking"

[Henning Schild]

Am Wed, 14 Nov 2018 16:45:36 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 14.11.18 16:43, Henning Schild wrote:
> > Am Wed, 14 Nov 2018 15:11:39 +0100
> > schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:
> >   
> >> Am Wed, 14 Nov 2018 14:58:57 +0100
> >> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> >>  
> >>> On 14.11.18 14:23, Jan Kiszka wrote:  
> >>>> On 14.11.18 14:10, Jan Kiszka wrote:  
> >>>>> On 12.11.18 16:51, Henning Schild wrote:  
> >>>>>> We do not build as root anymore and the non-root uid/gid are
> >>>>>> now in sync between inside and outside the chroot.
> >>>>>>
> >>>>>> This reverts commit 624b7c484bf59940ac2a4114018f7d56892dc05e.
> >>>>>> ---
> >>>>>>     meta/classes/base.bbclass | 5 -----
> >>>>>>     1 file changed, 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/meta/classes/base.bbclass
> >>>>>> b/meta/classes/base.bbclass index fce1084..d4082de 100644
> >>>>>> --- a/meta/classes/base.bbclass
> >>>>>> +++ b/meta/classes/base.bbclass
> >>>>>> @@ -120,17 +120,12 @@ do_unpack[stamp-extra-info] =
> >>>>>> "${DISTRO}-${DISTRO_ARCH}" # Unpack package and put it into
> >>>>>> working directory python do_unpack() {
> >>>>>> -    import subprocess
> >>>>>> -
> >>>>>>         src_uri = (d.getVar('SRC_URI', True) or "").split()
> >>>>>>         if len(src_uri) == 0:
> >>>>>>             return
> >>>>>>         rootdir = d.getVar('WORKDIR', True)
> >>>>>> -    uid = str(os.getuid())
> >>>>>> -    subprocess.call('sudo chown -R ' + uid + ' ' + rootdir,
> >>>>>> shell=True) -
> >>>>>>         try:
> >>>>>>             fetcher = bb.fetch2.Fetch(src_uri, d)
> >>>>>>             fetcher.unpack(rootdir)
> >>>>>>       
> >>>>>
> >>>>> This possibly causes this regression:
> >>>>>
> >>>>> ERROR: expand-on-first-boot-1.0-r0 do_install: Function failed:
> >>>>> do_install (log file is located at
> >>>>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
> >>>>>
> >>>>> ERROR: Logfile of failure stored in:
> >>>>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263
> >>>>>
> >>>>> Log data follows:
> >>>>> | DEBUG: Executing shell function do_install
> >>>>> | install: cannot remove
> >>>>> '/work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/image//lib/systemd/system/expand-on-first-boot.service':
> >>>>> Permission denied
> >>>>> | WARNING: exit code 1 from a shell command.
> >>>>> | ERROR: Function failed: do_install (log file is located at
> >>>>> /work/build/tmp/work/ebsy-arm64/expand-on-first-boot-1.0-r0/temp/log.do_install.263)
> >>>>>
> >>>>>       
> >>>>
> >>>> OK, that wasn't a "clean" re-build (rather a rebuild after the
> >>>> Isar update). Retesting with the same Isar version for build 1
> >>>> and 2.  
> >>
> >> Arghh ... on a side-note. I have a few Isar-CI partial rebuild
> >> patches in a queue, they target both dpkg and dpkg-raw.
> >>  
> >>> It persists, at least for dpkg-raw, and that is likely because of
> >>> the chroot we do for that package.  
> >>
> >> Yes.  
> > 
> > That actually revealed that we kept collecting changes in ${D} and
> > never cleaned it before install. I just sent a patch fixing that.
> > 
> > Next step will probably be to make the raw class use standard
> > debian/rules and just take the long dpkg-buildpkg route. Should not
> > be too hard to just write a debian/ folder that just does "make
> > install". Will look into that ...
> >   
> 
> Thanks, looking forward to test everything.

I have a working prototype of the idea, where dpkg-raw is basically
dpkg with a debianization step in front of it. Still looking into the
details because we now have a lot of debhelpers enforcing quality
standards.
And the debianization looks like it could make it into a general lib
that could be reused to debianize random sources.

Henning

> Jan
> 

[PATCH 3/3] buildchroot: do not chown to builder:builder anymore

[Henning Schild]

builder:builder is aligned with the user running Isar outside the
chroot, all files should be owned by this user naturally.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/recipes-devtools/buildchroot/files/build.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/meta/recipes-devtools/buildchroot/files/build.sh b/meta/recipes-devtools/buildchroot/files/build.sh
index 66b1a77..f977b16 100644
--- a/meta/recipes-devtools/buildchroot/files/build.sh
+++ b/meta/recipes-devtools/buildchroot/files/build.sh
@@ -15,6 +15,4 @@ 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
 su builder -c "cd $1; dpkg-buildpackage -a$target_arch -d --source-option=-I"
-- 
2.19.1

Re: [PATCH 0/3] "root" to "builder" repair series

[Maxim Yu. Osipov]

On 11/12/18 6:51 PM, Henning Schild wrote:
> These three patches should be applied in the given order. The one from
> Jan has been taken off the Mailinglist and has started the discussion
> about reverting the other one and not doing chowning anymore.

Applied to the 'next'.

Note: v2 of patch 1/3 "buildchroot: Align UID and GID of builder user 
with caller" was applied.

Thanks,
Maxim.

> Henning Schild (2):
>    Revert "Change ownership of WORKDIR prior to unpacking"
>    buildchroot: do not chown to builder:builder anymore
> 
> Jan Kiszka (1):
>    buildchroot: Align UID and GID of builder user with caller
> 
>   meta/classes/base.bbclass                               | 5 -----
>   meta/recipes-devtools/buildchroot/buildchroot.inc       | 4 +++-
>   meta/recipes-devtools/buildchroot/files/build.sh        | 2 --
>   meta/recipes-devtools/buildchroot/files/configscript.sh | 4 ++--
>   4 files changed, 5 insertions(+), 10 deletions(-)
> 


-- 
Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mosipov@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov