[v2] dpkg-base: Remove newly deployed debs from buildchroots

Message ID c1b59179-3be5-75e1-7078-f2f899bdc554@siemens.com
State Accepted, archived
Headers show
Series [v2] dpkg-base: Remove newly deployed debs from buildchroots | expand

Commit Message

Jan Kiszka Aug. 10, 2020, 11:05 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

This ensures clean reinstallation after partial rebuilds.

A typical error pattern so far was that firmware packages pulled by the
buildchroot-target were not updated there on rebuilds, causing the old
firmware being deployed into the image.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - removed bogus -U options which sneaked in from reading the wrong man
   page (zypper's apt-get wrapper: -U, --no-clean-deps)

 meta/classes/dpkg-base.bbclass | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Harald Seiler Aug. 10, 2020, 11:47 p.m. UTC | #1
On Tue, 2020-08-11 at 09:05 +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This ensures clean reinstallation after partial rebuilds.
> 
> A typical error pattern so far was that firmware packages pulled by the
> buildchroot-target were not updated there on rebuilds, causing the old
> firmware being deployed into the image.

Tested-by: Harald Seiler <hws@denx.de>
Reviewed-by: Harald Seiler <hws@denx.de>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Changes in v2:
>  - removed bogus -U options which sneaked in from reading the wrong man
>    page (zypper's apt-get wrapper: -U, --no-clean-deps)
> 
>  meta/classes/dpkg-base.bbclass | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> index 9aa2d546..08880d7d 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -154,20 +154,26 @@ python do_dpkg_build() {
>  
>  addtask dpkg_build before do_build
>  
> -CLEANFUNCS += "repo_clean"
> +CLEANFUNCS += "deb_clean"
>  
> -repo_clean() {
> +deb_clean() {
>      DEBS=$( find ${S}/.. -maxdepth 1 -name "*.deb" || [ ! -d ${S} ] )
>      if [ -n "${DEBS}" ]; then
>          for d in ${DEBS}; do
>              repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
>                  "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${d}"
> +            package=$(basename "${d}")
> +            package_remove="/usr/bin/apt-get remove -y ${package%%_*}"
> +            sudo -E chroot ${BUILDCHROOT_DIR} ${package_remove} || true
> +            if [ "${BUILDCHROOT_DIR}" != "${BUILDCHROOT_TARGET_DIR}" ]; then
> +                    sudo -E chroot ${BUILDCHROOT_TARGET_DIR} ${package_remove} || true
> +            fi
>          done
>      fi
>  }
>  
>  do_deploy_deb() {
> -    repo_clean
> +    deb_clean
>      repo_add_packages "${REPO_ISAR_DIR}"/"${DISTRO}" \
>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" ${S}/../*.deb
>  }
> -- 
> 2.26.2
>
Henning Schild Aug. 17, 2020, 4:50 a.m. UTC | #2
On Tue, 11 Aug 2020 09:05:59 +0200
"[ext] Jan Kiszka" <jan.kiszka@siemens.com> wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This ensures clean reinstallation after partial rebuilds.
> 
> A typical error pattern so far was that firmware packages pulled by
> the buildchroot-target were not updated there on rebuilds, causing
> the old firmware being deployed into the image.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Changes in v2:
>  - removed bogus -U options which sneaked in from reading the wrong
> man page (zypper's apt-get wrapper: -U, --no-clean-deps)
> 
>  meta/classes/dpkg-base.bbclass | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass
> b/meta/classes/dpkg-base.bbclass index 9aa2d546..08880d7d 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -154,20 +154,26 @@ python do_dpkg_build() {
>  
>  addtask dpkg_build before do_build
>  
> -CLEANFUNCS += "repo_clean"
> +CLEANFUNCS += "deb_clean"
>  
> -repo_clean() {
> +deb_clean() {
>      DEBS=$( find ${S}/.. -maxdepth 1 -name "*.deb" || [ ! -d ${S} ] )
>      if [ -n "${DEBS}" ]; then
>          for d in ${DEBS}; do
>              repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
>                  "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}"
> "${d}"
> +            package=$(basename "${d}")
> +            package_remove="/usr/bin/apt-get remove -y
> ${package%%_*}"
> +            sudo -E chroot ${BUILDCHROOT_DIR} ${package_remove} ||
> true

This will cause serious headache when rebuilding essential packages.
You risk removing dpkg/apt and others.

I think we rather need an "apt-get upgrade --allow-downgrades" together
with a "apt-get autoremove" somewhere.

Henning

> +            if [ "${BUILDCHROOT_DIR}" != "${BUILDCHROOT_TARGET_DIR}"
> ]; then
> +                    sudo -E chroot ${BUILDCHROOT_TARGET_DIR}
> ${package_remove} || true
> +            fi
>          done
>      fi
>  }
>  
>  do_deploy_deb() {
> -    repo_clean
> +    deb_clean
>      repo_add_packages "${REPO_ISAR_DIR}"/"${DISTRO}" \
>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}"
> ${S}/../*.deb }
Jan Kiszka Aug. 17, 2020, 7:38 a.m. UTC | #3
On 17.08.20 14:50, Henning Schild wrote:
> On Tue, 11 Aug 2020 09:05:59 +0200
> "[ext] Jan Kiszka" <jan.kiszka@siemens.com> wrote:
> 
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This ensures clean reinstallation after partial rebuilds.
>>
>> A typical error pattern so far was that firmware packages pulled by
>> the buildchroot-target were not updated there on rebuilds, causing
>> the old firmware being deployed into the image.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v2:
>>  - removed bogus -U options which sneaked in from reading the wrong
>> man page (zypper's apt-get wrapper: -U, --no-clean-deps)
>>
>>  meta/classes/dpkg-base.bbclass | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/classes/dpkg-base.bbclass
>> b/meta/classes/dpkg-base.bbclass index 9aa2d546..08880d7d 100644
>> --- a/meta/classes/dpkg-base.bbclass
>> +++ b/meta/classes/dpkg-base.bbclass
>> @@ -154,20 +154,26 @@ python do_dpkg_build() {
>>  
>>  addtask dpkg_build before do_build
>>  
>> -CLEANFUNCS += "repo_clean"
>> +CLEANFUNCS += "deb_clean"
>>  
>> -repo_clean() {
>> +deb_clean() {
>>      DEBS=$( find ${S}/.. -maxdepth 1 -name "*.deb" || [ ! -d ${S} ] )
>>      if [ -n "${DEBS}" ]; then
>>          for d in ${DEBS}; do
>>              repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>                  "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}"
>> "${d}"
>> +            package=$(basename "${d}")
>> +            package_remove="/usr/bin/apt-get remove -y
>> ${package%%_*}"
>> +            sudo -E chroot ${BUILDCHROOT_DIR} ${package_remove} ||
>> true
> 
> This will cause serious headache when rebuilding essential packages.
> You risk removing dpkg/apt and others.

Not sure if that would already work, at least when the result actually
targets the buildchroot and not just the device.

> 
> I think we rather need an "apt-get upgrade --allow-downgrades" together
> with a "apt-get autoremove" somewhere.

I think those only work if the version number changed which is not
guaranteed. We would need some switch that enforces re-installation, and
that possibly only when some trigger tells us that the package has
changed. apt-get reinstall?

If we do not find a quick solution, I think such corner cases could also
be addressed when needed. The current situation is too problematic to
leave it like it us much longer.

Jan
Henning Schild Aug. 17, 2020, 9:21 a.m. UTC | #4
On Mon, 17 Aug 2020 17:38:46 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 17.08.20 14:50, Henning Schild wrote:
> > On Tue, 11 Aug 2020 09:05:59 +0200
> > "[ext] Jan Kiszka" <jan.kiszka@siemens.com> wrote:
> >   
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> This ensures clean reinstallation after partial rebuilds.
> >>
> >> A typical error pattern so far was that firmware packages pulled by
> >> the buildchroot-target were not updated there on rebuilds, causing
> >> the old firmware being deployed into the image.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>
> >> Changes in v2:
> >>  - removed bogus -U options which sneaked in from reading the wrong
> >> man page (zypper's apt-get wrapper: -U, --no-clean-deps)
> >>
> >>  meta/classes/dpkg-base.bbclass | 12 +++++++++---
> >>  1 file changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/meta/classes/dpkg-base.bbclass
> >> b/meta/classes/dpkg-base.bbclass index 9aa2d546..08880d7d 100644
> >> --- a/meta/classes/dpkg-base.bbclass
> >> +++ b/meta/classes/dpkg-base.bbclass
> >> @@ -154,20 +154,26 @@ python do_dpkg_build() {
> >>  
> >>  addtask dpkg_build before do_build
> >>  
> >> -CLEANFUNCS += "repo_clean"
> >> +CLEANFUNCS += "deb_clean"
> >>  
> >> -repo_clean() {
> >> +deb_clean() {
> >>      DEBS=$( find ${S}/.. -maxdepth 1 -name "*.deb" || [ ! -d ${S}
> >> ] ) if [ -n "${DEBS}" ]; then
> >>          for d in ${DEBS}; do
> >>              repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
> >>                  "${REPO_ISAR_DB_DIR}"/"${DISTRO}"
> >> "${DEBDISTRONAME}" "${d}"
> >> +            package=$(basename "${d}")
> >> +            package_remove="/usr/bin/apt-get remove -y
> >> ${package%%_*}"
> >> +            sudo -E chroot ${BUILDCHROOT_DIR} ${package_remove} ||
> >> true  
> > 
> > This will cause serious headache when rebuilding essential packages.
> > You risk removing dpkg/apt and others.  
> 
> Not sure if that would already work, at least when the result actually
> targets the buildchroot and not just the device.

I am also not sure, especially when looking at the buildchroots.

> > 
> > I think we rather need an "apt-get upgrade --allow-downgrades"
> > together with a "apt-get autoremove" somewhere.  
> 
> I think those only work if the version number changed which is not
> guaranteed. We would need some switch that enforces re-installation,
> and that possibly only when some trigger tells us that the package has
> changed. apt-get reinstall?

Not changing the version number is probably bad style. I think one
should always change it when working on a recipe. While working on it
people hopefully know what to clean, and others merging changes will
simply get a proper rebuild with a new version.

> If we do not find a quick solution, I think such corner cases could
> also be addressed when needed. The current situation is too
> problematic to leave it like it us much longer.

Agreed, it could just be a made-up corner-case. Just wanted to point
out the potential danger and stress the fact that changed content
without changing the version is a bad idea.

Henning

> Jan
>
Baurzhan Ismagulov Aug. 17, 2020, 11:27 a.m. UTC | #5
On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
> This ensures clean reinstallation after partial rebuilds.
> 
> A typical error pattern so far was that firmware packages pulled by the
> buildchroot-target were not updated there on rebuilds, causing the old
> firmware being deployed into the image.

Applied to next, thanks for the patch and the review.

Yes, Debian Policy section 3.2 requires that different packages have different
version numbers. Maybe we should introduce a script that would do that for the
developer.

With kind regards,
Baurzhan.
Jan Kiszka Aug. 17, 2020, 9:53 p.m. UTC | #6
On 17.08.20 19:21, Henning Schild wrote:
> On Mon, 17 Aug 2020 17:38:46 +0200
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>> On 17.08.20 14:50, Henning Schild wrote:
>>> On Tue, 11 Aug 2020 09:05:59 +0200
>>> "[ext] Jan Kiszka" <jan.kiszka@siemens.com> wrote:
>>>   
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> This ensures clean reinstallation after partial rebuilds.
>>>>
>>>> A typical error pattern so far was that firmware packages pulled by
>>>> the buildchroot-target were not updated there on rebuilds, causing
>>>> the old firmware being deployed into the image.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>>  - removed bogus -U options which sneaked in from reading the wrong
>>>> man page (zypper's apt-get wrapper: -U, --no-clean-deps)
>>>>
>>>>  meta/classes/dpkg-base.bbclass | 12 +++++++++---
>>>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/meta/classes/dpkg-base.bbclass
>>>> b/meta/classes/dpkg-base.bbclass index 9aa2d546..08880d7d 100644
>>>> --- a/meta/classes/dpkg-base.bbclass
>>>> +++ b/meta/classes/dpkg-base.bbclass
>>>> @@ -154,20 +154,26 @@ python do_dpkg_build() {
>>>>  
>>>>  addtask dpkg_build before do_build
>>>>  
>>>> -CLEANFUNCS += "repo_clean"
>>>> +CLEANFUNCS += "deb_clean"
>>>>  
>>>> -repo_clean() {
>>>> +deb_clean() {
>>>>      DEBS=$( find ${S}/.. -maxdepth 1 -name "*.deb" || [ ! -d ${S}
>>>> ] ) if [ -n "${DEBS}" ]; then
>>>>          for d in ${DEBS}; do
>>>>              repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>>>                  "${REPO_ISAR_DB_DIR}"/"${DISTRO}"
>>>> "${DEBDISTRONAME}" "${d}"
>>>> +            package=$(basename "${d}")
>>>> +            package_remove="/usr/bin/apt-get remove -y
>>>> ${package%%_*}"
>>>> +            sudo -E chroot ${BUILDCHROOT_DIR} ${package_remove} ||
>>>> true  
>>>
>>> This will cause serious headache when rebuilding essential packages.
>>> You risk removing dpkg/apt and others.  
>>
>> Not sure if that would already work, at least when the result actually
>> targets the buildchroot and not just the device.
> 
> I am also not sure, especially when looking at the buildchroots.
> 
>>>
>>> I think we rather need an "apt-get upgrade --allow-downgrades"
>>> together with a "apt-get autoremove" somewhere.  
>>
>> I think those only work if the version number changed which is not
>> guaranteed. We would need some switch that enforces re-installation,
>> and that possibly only when some trigger tells us that the package has
>> changed. apt-get reinstall?
> 
> Not changing the version number is probably bad style. I think one
> should always change it when working on a recipe. While working on it
> people hopefully know what to clean, and others merging changes will
> simply get a proper rebuild with a new version.

We've discussed this too often: This is not how people work, and it also
makes zero sense for a local development cycle to continuously bump
version numbers.

Jan

> 
>> If we do not find a quick solution, I think such corner cases could
>> also be addressed when needed. The current situation is too
>> problematic to leave it like it us much longer.
> 
> Agreed, it could just be a made-up corner-case. Just wanted to point
> out the potential danger and stress the fact that changed content
> without changing the version is a bad idea.
> 
> Henning
> 
>> Jan
>>
>
Jan Kiszka Aug. 17, 2020, 9:57 p.m. UTC | #7
On 17.08.20 21:27, Baurzhan Ismagulov wrote:
> On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
>> This ensures clean reinstallation after partial rebuilds.
>>
>> A typical error pattern so far was that firmware packages pulled by the
>> buildchroot-target were not updated there on rebuilds, causing the old
>> firmware being deployed into the image.
> 
> Applied to next, thanks for the patch and the review.
> 
> Yes, Debian Policy section 3.2 requires that different packages have different
> version numbers. Maybe we should introduce a script that would do that for the
> developer.

This won't help in the cases you play with a recipe, change small
details, rebuild, test. It's reasonable to update the version when
publishing. However, it's a needless burden to do that for a local
development cycle. Here, Isar shall support that the just built instance
of a package is also used locally, without an explicit "clean".

Jan
Baurzhan Ismagulov Aug. 23, 2020, 8:06 a.m. UTC | #8
Hello Jan,

On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
> This ensures clean reinstallation after partial rebuilds.
> 
> A typical error pattern so far was that firmware packages pulled by the
> buildchroot-target were not updated there on rebuilds, causing the old
> firmware being deployed into the image.

With this applied, we encounter random build failures downstream, e.g.:

1. ERROR: mc:pixi-cdl100-buster:desktop-shortcuts-1.0-r0 do_install_builddeps: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.do_install_builddeps.19582' failed with exit code 1:
   mk-build-deps: You must have equivs installed to use this program.
   WARNING: exit code 1 from a shell command.

2. Exception: bb.process.ExecutionError: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.dpkg_runbuild.16998' failed with exit code 2:
   dpkg-buildpackage: info: source package desktop-shortcuts
   dpkg-buildpackage: info: source version 1.0
   dpkg-buildpackage: info: source distribution UNRELEASED
   dpkg-buildpackage: info: source changed by isar-users <isar-users@googlegroups.com>
   dpkg-architecture: warning: specified GNU system type arm-linux-gnueabihf does not match CC system type x86_64-linux-gnu, try setting a correct CC environment variable
    dpkg-source -I --before-build .
   dpkg-buildpackage: info: host architecture armhf
    fakeroot debian/rules clean
   dh clean
   make: dh: Command not found

Seems we have a race, possibly related to removing packages. What I don't
understand is how it could affect equivs or debhelper -- we don't do anything
with them. Ideas?

With kind regards,
Baurzhan.
Baurzhan Ismagulov Aug. 23, 2020, 8:30 a.m. UTC | #9
On Tue, Aug 18, 2020 at 07:57:26AM +0200, Jan Kiszka wrote:
> > Yes, Debian Policy section 3.2 requires that different packages have different
> > version numbers. Maybe we should introduce a script that would do that for the
> > developer.
> 
> This won't help in the cases you play with a recipe, change small
> details, rebuild, test. It's reasonable to update the version when
> publishing. However, it's a needless burden to do that for a local
> development cycle. Here, Isar shall support that the just built instance
> of a package is also used locally, without an explicit "clean".

If this is how the upstream tooling works (which is a requirement for them
since there is no other way), things will get complicated and keep breaking. An
alternative could be to automate version bumping by requiring a local commit.
In the long term, I don't see a way around that.

With kind regards,
Baurzhan.
Jan Kiszka Aug. 23, 2020, 11:16 p.m. UTC | #10
On 23.08.20 18:06, Baurzhan Ismagulov wrote:
> Hello Jan,
> 
> On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
>> This ensures clean reinstallation after partial rebuilds.
>>
>> A typical error pattern so far was that firmware packages pulled by the
>> buildchroot-target were not updated there on rebuilds, causing the old
>> firmware being deployed into the image.
> 
> With this applied, we encounter random build failures downstream, e.g.:
> 
> 1. ERROR: mc:pixi-cdl100-buster:desktop-shortcuts-1.0-r0 do_install_builddeps: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.do_install_builddeps.19582' failed with exit code 1:
>    mk-build-deps: You must have equivs installed to use this program.
>    WARNING: exit code 1 from a shell command.
> 
> 2. Exception: bb.process.ExecutionError: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.dpkg_runbuild.16998' failed with exit code 2:
>    dpkg-buildpackage: info: source package desktop-shortcuts
>    dpkg-buildpackage: info: source version 1.0
>    dpkg-buildpackage: info: source distribution UNRELEASED
>    dpkg-buildpackage: info: source changed by isar-users <isar-users@googlegroups.com>
>    dpkg-architecture: warning: specified GNU system type arm-linux-gnueabihf does not match CC system type x86_64-linux-gnu, try setting a correct CC environment variable
>     dpkg-source -I --before-build .
>    dpkg-buildpackage: info: host architecture armhf
>     fakeroot debian/rules clean
>    dh clean
>    make: dh: Command not found
> 
> Seems we have a race, possibly related to removing packages. What I don't
> understand is how it could affect equivs or debhelper -- we don't do anything
> with them. Ideas?

Which packages are you (re-)building? Debian packages? Or just custom
dependencies of that desktop-shortcuts package?

In theory, do_deploy_deb should be locked against do_install_builddeps.
Anything that the latter needs should be available after the former.
Maybe you are missing a build dependency in desktop-shortcuts? Try build
that only over a clean buildchroot, that will tell you.

Jan
Jan Kiszka Aug. 23, 2020, 11:17 p.m. UTC | #11
On 23.08.20 18:30, Baurzhan Ismagulov wrote:
> On Tue, Aug 18, 2020 at 07:57:26AM +0200, Jan Kiszka wrote:
>>> Yes, Debian Policy section 3.2 requires that different packages have different
>>> version numbers. Maybe we should introduce a script that would do that for the
>>> developer.
>>
>> This won't help in the cases you play with a recipe, change small
>> details, rebuild, test. It's reasonable to update the version when
>> publishing. However, it's a needless burden to do that for a local
>> development cycle. Here, Isar shall support that the just built instance
>> of a package is also used locally, without an explicit "clean".
> 
> If this is how the upstream tooling works (which is a requirement for them
> since there is no other way), things will get complicated and keep breaking. An

Can you name concrete other issues?

> alternative could be to automate version bumping by requiring a local commit.
> In the long term, I don't see a way around that.

Commits for rebuilds are user-unfriendly. We will need to solve that in
a better way versioning should actually require a bump.

Jan
Jan Kiszka Sept. 21, 2020, 11:17 a.m. UTC | #12
On 24.08.20 09:16, [ext] Jan Kiszka wrote:
> On 23.08.20 18:06, Baurzhan Ismagulov wrote:
>> Hello Jan,
>>
>> On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
>>> This ensures clean reinstallation after partial rebuilds.
>>>
>>> A typical error pattern so far was that firmware packages pulled by the
>>> buildchroot-target were not updated there on rebuilds, causing the old
>>> firmware being deployed into the image.
>>
>> With this applied, we encounter random build failures downstream, e.g.:
>>
>> 1. ERROR: mc:pixi-cdl100-buster:desktop-shortcuts-1.0-r0 do_install_builddeps: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.do_install_builddeps.19582' failed with exit code 1:
>>     mk-build-deps: You must have equivs installed to use this program.
>>     WARNING: exit code 1 from a shell command.
>>
>> 2. Exception: bb.process.ExecutionError: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.dpkg_runbuild.16998' failed with exit code 2:
>>     dpkg-buildpackage: info: source package desktop-shortcuts
>>     dpkg-buildpackage: info: source version 1.0
>>     dpkg-buildpackage: info: source distribution UNRELEASED
>>     dpkg-buildpackage: info: source changed by isar-users <isar-users@googlegroups.com>
>>     dpkg-architecture: warning: specified GNU system type arm-linux-gnueabihf does not match CC system type x86_64-linux-gnu, try setting a correct CC environment variable
>>      dpkg-source -I --before-build .
>>     dpkg-buildpackage: info: host architecture armhf
>>      fakeroot debian/rules clean
>>     dh clean
>>     make: dh: Command not found
>>
>> Seems we have a race, possibly related to removing packages. What I don't
>> understand is how it could affect equivs or debhelper -- we don't do anything
>> with them. Ideas?
> 
> Which packages are you (re-)building? Debian packages? Or just custom
> dependencies of that desktop-shortcuts package?
> 
> In theory, do_deploy_deb should be locked against do_install_builddeps.
> Anything that the latter needs should be available after the former.
> Maybe you are missing a build dependency in desktop-shortcuts? Try build
> that only over a clean buildchroot, that will tell you.
> 

Meanwhile, I understood the issue is it happened on out CI system as 
well during nightly runs (that shouldn't have happen, but that a local 
issue). The logs you cited were incomplete:

[...]
NOTE: recipe cowsay-git-r0: task do_dpkg_build: Succeeded
NOTE: Running task 1617 of 1683 (mc:qemumipsel-buster:/builds/ebsy/debian/isar/meta-isar/recipes-app/cowsay/cowsay_git.bb:do_deploy_deb)
NOTE: recipe cowsay-git-r0: task do_deploy_deb: Started
[...]
NOTE: Running noexec task 1637 of 1683 (mc:rpi-stretch:/builds/ebsy/debian/isar/meta-isar/recipes-core/images/isar-image-base.bb:do_build)
[...]
ERROR: mc:qemuarm-buster:isar-image-base-1.0-r0 do_rootfs_install: Error executing a python function in exec_python_func() autogenerated:
[...]
E: Failed to fetch file:/isar-apt/pool/main/c/cowsay/cowsay_3.03+dfsg2-4_all.deb  File not found - /isar-apt/pool/main/c/cowsay/cowsay_3.03+dfsg2-4_all.deb (2: No such file or directory)
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
WARNING: exit code 100 from a shell command.


ERROR: Logfile of failure stored in: /builds/ebsy/debian/isar/build/tmp/work/debian-buster-armhf/isar-image-base-qemuarm-ext4-img/1.0-r0/temp/log.do_rootfs_install.32804
NOTE: recipe isar-image-base-1.0-r0: task do_rootfs_install: Failed
ERROR: Task (mc:qemuarm-buster:/builds/ebsy/debian/isar/meta-isar/recipes-core/images/isar-image-base.bb:do_rootfs_install) failed with exit code '1'
NOTE: recipe cowsay-git-r0: task do_deploy_deb: Succeeded

The problem is that we have a false sharing between all archs building 
cowsay for buster. That is because this demo package is arch "all", but 
the tasks assume it is arch-specific, thus are building and 
(re-)deploying it happily in parallel. This is similar to the issue I 
had with [1]. It is not caused by this patch, it is revealed by it, 
finally.

What can we do?
 - select a different rebuild-example, one that does not generate "all"
   packages (i.e. stick our head into the sand)
 - break up isar-apt on a per-arch basis (as considered for [1])
 - find a magic spell that allows to express the commonality between
   all mc:<arch>-<distro>:<package> recipes that actually produce the
   same output

I suspect the latter will not happen any time soon, so I'm inclined to 
fix this via the second option.

Jan

[1] https://groups.google.com/d/msgid/isar-users/e4ca6d44-8b99-24ba-4fcd-c36cedac1ff7%40siemens.com
Vijai Kumar K Oct. 28, 2020, 3:39 a.m. UTC | #13
Hello Jan,

-----Original Message-----
From: isar-users@googlegroups.com [mailto:isar-users@googlegroups.com] On Behalf Of Jan Kiszka
Sent: 22 September 2020 00:48
To: isar-users <isar-users@googlegroups.com>; Baurzhan Ismagulov <ibr@ilbers.de>
Subject: Re: [PATCH v2] dpkg-base: Remove newly deployed debs from buildchroots

On 24.08.20 09:16, [ext] Jan Kiszka wrote:
> On 23.08.20 18:06, Baurzhan Ismagulov wrote:
>> Hello Jan,
>>
>> On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
>>> This ensures clean reinstallation after partial rebuilds.
>>>
>>> A typical error pattern so far was that firmware packages pulled by 
>>> the buildchroot-target were not updated there on rebuilds, causing 
>>> the old firmware being deployed into the image.
>>
>> With this applied, we encounter random build failures downstream, e.g.:
>>
>> 1. ERROR: mc:pixi-cdl100-buster:desktop-shortcuts-1.0-r0 do_install_builddeps: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.do_install_builddeps.19582' failed with exit code 1:
>>     mk-build-deps: You must have equivs installed to use this program.
>>     WARNING: exit code 1 from a shell command.
>>
>> 2. Exception: bb.process.ExecutionError: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.dpkg_runbuild.16998' failed with exit code 2:
>>     dpkg-buildpackage: info: source package desktop-shortcuts
>>     dpkg-buildpackage: info: source version 1.0
>>     dpkg-buildpackage: info: source distribution UNRELEASED
>>     dpkg-buildpackage: info: source changed by isar-users <isar-users@googlegroups.com>
>>     dpkg-architecture: warning: specified GNU system type arm-linux-gnueabihf does not match CC system type x86_64-linux-gnu, try setting a correct CC environment variable
>>      dpkg-source -I --before-build .
>>     dpkg-buildpackage: info: host architecture armhf
>>      fakeroot debian/rules clean
>>     dh clean
>>     make: dh: Command not found
>>
>> Seems we have a race, possibly related to removing packages. What I 
>> don't understand is how it could affect equivs or debhelper -- we 
>> don't do anything with them. Ideas?

I have a  similar condition now. We build openssl with a patch downstream. Looks like with this patch now, in our setup openssl is removed from buildchrootdir, which inturn removes
devscripts(contains mk-build-deps). This causes other recipe builds to fail. Please see the log below.


ERROR: expand-on-first-boot-1.1-r0 do_install_builddeps: Execution of '/work/build/tmp/work/ebsy-buster-amd64/expand-on-first-boot/1.1-r0/temp/run.do_install_builddeps.10653' failed with exit code 1:
/isar/deps.sh: line 39: mk-build-deps: command not found
WARNING: exit code 1 from a shell command.

ERROR: Logfile of failure stored in: /work/build/tmp/work/ebsy-buster-amd64/expand-on-first-boot/1.1-r0/temp/log.do_install_builddeps.10653
Log data follows:
| DEBUG: Executing shell function do_install_builddeps
| /isar/deps.sh: line 39: mk-build-deps: command not found
| WARNING: exit code 1 from a shell command.
| ERROR: Execution of '/work/build/tmp/work/ebsy-buster-amd64/expand-on-first-boot/1.1-r0/temp/run.do_install_builddeps.10653' failed with exit code 1:
| /isar/deps.sh: line 39: mk-build-deps: command not found
| WARNING: exit code 1 from a shell command.
|
ERROR: Task (/work/isar/meta/recipes-support/expand-on-first-boot/expand-on-first-boot_1.1.bb:do_install_builddeps) failed with exit code '1'
ERROR: customizations-debug-0.1-ebsy-r0 do_install_builddeps: Execution of '/work/build/tmp/work/ebsy-buster-amd64/customizations-debug/0.1-ebsy-r0/temp/run.do_install_builddeps.10655' failed with exit code 1:
/isar/deps.sh: line 39: mk-build-deps: command not found
WARNING: exit code 1 from a shell command.

Thanks,
Vijai Kumar K

> 
> Which packages are you (re-)building? Debian packages? Or just custom 
> dependencies of that desktop-shortcuts package?
> 
> In theory, do_deploy_deb should be locked against do_install_builddeps.
> Anything that the latter needs should be available after the former.
> Maybe you are missing a build dependency in desktop-shortcuts? Try 
> build that only over a clean buildchroot, that will tell you.
> 

Meanwhile, I understood the issue is it happened on out CI system as well during nightly runs (that shouldn't have happen, but that a local issue). The logs you cited were incomplete:

[...]
NOTE: recipe cowsay-git-r0: task do_dpkg_build: Succeeded
NOTE: Running task 1617 of 1683 (mc:qemumipsel-buster:/builds/ebsy/debian/isar/meta-isar/recipes-app/cowsay/cowsay_git.bb:do_deploy_deb)
NOTE: recipe cowsay-git-r0: task do_deploy_deb: Started [...]
NOTE: Running noexec task 1637 of 1683 (mc:rpi-stretch:/builds/ebsy/debian/isar/meta-isar/recipes-core/images/isar-image-base.bb:do_build)
[...]
ERROR: mc:qemuarm-buster:isar-image-base-1.0-r0 do_rootfs_install: Error executing a python function in exec_python_func() autogenerated:
[...]
E: Failed to fetch file:/isar-apt/pool/main/c/cowsay/cowsay_3.03+dfsg2-4_all.deb  File not found - /isar-apt/pool/main/c/cowsay/cowsay_3.03+dfsg2-4_all.deb (2: No such file or directory)
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
WARNING: exit code 100 from a shell command.


ERROR: Logfile of failure stored in: /builds/ebsy/debian/isar/build/tmp/work/debian-buster-armhf/isar-image-base-qemuarm-ext4-img/1.0-r0/temp/log.do_rootfs_install.32804
NOTE: recipe isar-image-base-1.0-r0: task do_rootfs_install: Failed
ERROR: Task (mc:qemuarm-buster:/builds/ebsy/debian/isar/meta-isar/recipes-core/images/isar-image-base.bb:do_rootfs_install) failed with exit code '1'
NOTE: recipe cowsay-git-r0: task do_deploy_deb: Succeeded

The problem is that we have a false sharing between all archs building cowsay for buster. That is because this demo package is arch "all", but the tasks assume it is arch-specific, thus are building and (re-)deploying it happily in parallel. This is similar to the issue I had with [1]. It is not caused by this patch, it is revealed by it, finally.

What can we do?
 - select a different rebuild-example, one that does not generate "all"
   packages (i.e. stick our head into the sand)
 - break up isar-apt on a per-arch basis (as considered for [1])
 - find a magic spell that allows to express the commonality between
   all mc:<arch>-<distro>:<package> recipes that actually produce the
   same output

I suspect the latter will not happen any time soon, so I'm inclined to fix this via the second option.

Jan

[1] https://groups.google.com/d/msgid/isar-users/e4ca6d44-8b99-24ba-4fcd-c36cedac1ff7%40siemens.com

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

--
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/c3998f33-521e-49ed-41c5-95f1f935d5df%40siemens.com.
Jan Kiszka Oct. 28, 2020, 3:52 a.m. UTC | #14
On 28.10.20 14:39, Kanagarajan, Vijaikumar wrote:
> Hello Jan,
> 
> -----Original Message-----
> From: isar-users@googlegroups.com [mailto:isar-users@googlegroups.com] On Behalf Of Jan Kiszka
> Sent: 22 September 2020 00:48
> To: isar-users <isar-users@googlegroups.com>; Baurzhan Ismagulov <ibr@ilbers.de>
> Subject: Re: [PATCH v2] dpkg-base: Remove newly deployed debs from buildchroots
> 
> On 24.08.20 09:16, [ext] Jan Kiszka wrote:
>> On 23.08.20 18:06, Baurzhan Ismagulov wrote:
>>> Hello Jan,
>>>
>>> On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
>>>> This ensures clean reinstallation after partial rebuilds.
>>>>
>>>> A typical error pattern so far was that firmware packages pulled by 
>>>> the buildchroot-target were not updated there on rebuilds, causing 
>>>> the old firmware being deployed into the image.
>>>
>>> With this applied, we encounter random build failures downstream, e.g.:
>>>
>>> 1. ERROR: mc:pixi-cdl100-buster:desktop-shortcuts-1.0-r0 do_install_builddeps: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.do_install_builddeps.19582' failed with exit code 1:
>>>     mk-build-deps: You must have equivs installed to use this program.
>>>     WARNING: exit code 1 from a shell command.
>>>
>>> 2. Exception: bb.process.ExecutionError: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.dpkg_runbuild.16998' failed with exit code 2:
>>>     dpkg-buildpackage: info: source package desktop-shortcuts
>>>     dpkg-buildpackage: info: source version 1.0
>>>     dpkg-buildpackage: info: source distribution UNRELEASED
>>>     dpkg-buildpackage: info: source changed by isar-users <isar-users@googlegroups.com>
>>>     dpkg-architecture: warning: specified GNU system type arm-linux-gnueabihf does not match CC system type x86_64-linux-gnu, try setting a correct CC environment variable
>>>      dpkg-source -I --before-build .
>>>     dpkg-buildpackage: info: host architecture armhf
>>>      fakeroot debian/rules clean
>>>     dh clean
>>>     make: dh: Command not found
>>>
>>> Seems we have a race, possibly related to removing packages. What I 
>>> don't understand is how it could affect equivs or debhelper -- we 
>>> don't do anything with them. Ideas?
> 
> I have a  similar condition now. We build openssl with a patch downstream. Looks like with this patch now, in our setup openssl is removed from buildchrootdir, which inturn removes
> devscripts(contains mk-build-deps). This causes other recipe builds to fail. Please see the log below.
> 

I've proposed
https://groups.google.com/d/msgid/isar-users/76603767-1d25-c347-f7b1-84d4c106e771%40siemens.com
to address that scenario. We are using that in one internal layer already.

Jan
Vijai Kumar K Oct. 28, 2020, 4:20 a.m. UTC | #15
-----Original Message-----
From: Jan Kiszka [mailto:jan.kiszka@siemens.com] 
Sent: 28 October 2020 19:22
To: Kanagarajan, Vijaikumar <Vijaikumar_Kanagarajan@mentor.com>; isar-users <isar-users@googlegroups.com>; Baurzhan Ismagulov <ibr@ilbers.de>
Subject: Re: [PATCH v2] dpkg-base: Remove newly deployed debs from buildchroots

On 28.10.20 14:39, Kanagarajan, Vijaikumar wrote:
> Hello Jan,
> 
> -----Original Message-----
> From: isar-users@googlegroups.com [mailto:isar-users@googlegroups.com] 
> On Behalf Of Jan Kiszka
> Sent: 22 September 2020 00:48
> To: isar-users <isar-users@googlegroups.com>; Baurzhan Ismagulov 
> <ibr@ilbers.de>
> Subject: Re: [PATCH v2] dpkg-base: Remove newly deployed debs from 
> buildchroots
> 
> On 24.08.20 09:16, [ext] Jan Kiszka wrote:
>> On 23.08.20 18:06, Baurzhan Ismagulov wrote:
>>> Hello Jan,
>>>
>>> On Tue, Aug 11, 2020 at 09:05:59AM +0200, Jan Kiszka wrote:
>>>> This ensures clean reinstallation after partial rebuilds.
>>>>
>>>> A typical error pattern so far was that firmware packages pulled by 
>>>> the buildchroot-target were not updated there on rebuilds, causing 
>>>> the old firmware being deployed into the image.
>>>
>>> With this applied, we encounter random build failures downstream, e.g.:
>>>
>>> 1. ERROR: mc:pixi-cdl100-buster:desktop-shortcuts-1.0-r0 do_install_builddeps: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.do_install_builddeps.19582' failed with exit code 1:
>>>     mk-build-deps: You must have equivs installed to use this program.
>>>     WARNING: exit code 1 from a shell command.
>>>
>>> 2. Exception: bb.process.ExecutionError: Execution of 'build/tmp/work/debian-buster-armhf/desktop-shortcuts/1.0-r0/temp/run.dpkg_runbuild.16998' failed with exit code 2:
>>>     dpkg-buildpackage: info: source package desktop-shortcuts
>>>     dpkg-buildpackage: info: source version 1.0
>>>     dpkg-buildpackage: info: source distribution UNRELEASED
>>>     dpkg-buildpackage: info: source changed by isar-users <isar-users@googlegroups.com>
>>>     dpkg-architecture: warning: specified GNU system type arm-linux-gnueabihf does not match CC system type x86_64-linux-gnu, try setting a correct CC environment variable
>>>      dpkg-source -I --before-build .
>>>     dpkg-buildpackage: info: host architecture armhf
>>>      fakeroot debian/rules clean
>>>     dh clean
>>>     make: dh: Command not found
>>>
>>> Seems we have a race, possibly related to removing packages. What I 
>>> don't understand is how it could affect equivs or debhelper -- we 
>>> don't do anything with them. Ideas?
> 
> I have a  similar condition now. We build openssl with a patch 
> downstream. Looks like with this patch now, in our setup openssl is removed from buildchrootdir, which inturn removes devscripts(contains mk-build-deps). This causes other recipe builds to fail. Please see the log below.
> 

I've proposed
https://groups.google.com/d/msgid/isar-users/76603767-1d25-c347-f7b1-84d4c106e771%40siemens.com
to address that scenario. We are using that in one internal layer already.


Great. I will try that out.

Thanks,
Vijai Kumar K

Jan

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Patch

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 9aa2d546..08880d7d 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -154,20 +154,26 @@  python do_dpkg_build() {
 
 addtask dpkg_build before do_build
 
-CLEANFUNCS += "repo_clean"
+CLEANFUNCS += "deb_clean"
 
-repo_clean() {
+deb_clean() {
     DEBS=$( find ${S}/.. -maxdepth 1 -name "*.deb" || [ ! -d ${S} ] )
     if [ -n "${DEBS}" ]; then
         for d in ${DEBS}; do
             repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
                 "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${d}"
+            package=$(basename "${d}")
+            package_remove="/usr/bin/apt-get remove -y ${package%%_*}"
+            sudo -E chroot ${BUILDCHROOT_DIR} ${package_remove} || true
+            if [ "${BUILDCHROOT_DIR}" != "${BUILDCHROOT_TARGET_DIR}" ]; then
+                    sudo -E chroot ${BUILDCHROOT_TARGET_DIR} ${package_remove} || true
+            fi
         done
     fi
 }
 
 do_deploy_deb() {
-    repo_clean
+    deb_clean
     repo_add_packages "${REPO_ISAR_DIR}"/"${DISTRO}" \
         "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" ${S}/../*.deb
 }