On Sun, Dec 04, 2011 at 05:41:22PM +0100, Lukas Fleischer wrote:

On Sun, Dec 04, 2011 at 11:13:46AM -0500, Dave Reisner wrote:

...

This fixes a compound command such as the one below from continuing even if the first fails.

extra-x86_64-build && extra-i686-build

The problem is that the EXIT trap is fired if we let the script run to completion. Instead, explicitly call cleanup with the return of the makechrootpkg call to properly exit with error.

Signed-off-by: Dave Reisner dreisner@archlinux.org

archbuild.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

Signed-off-by: Lukas Fleischer archlinux@cryptocrack.de

...

diff --git a/archbuild.in b/archbuild.in index 502654c..733a79a 100644 --- a/archbuild.in +++ b/archbuild.in @@ -84,3 +84,4 @@ fi

msg "Building in chroot for [${repo}] (${arch})..." setarch "${arch}" makechrootpkg -c -n -r "${chroots}/${repo}-${arch}"

+cleanup $?

1.7.8

I'm having second thoughts about this, actually... the debug output shows that we call the cleanup routine in makechrootpkg, and then fall out and call the cleanup routine in archbuild. We should just exec the last command here and let it replace the shell so that it exits properly. I'll send a new patch.

d

On Sun, Dec 04, 2011 at 12:09:16PM -0500, Dave Reisner wrote:

This fixes a compound command such as the one below from continuing even if the first fails.

extra-x86_64-build && extra-i686-build

The problem is that 'cleanup 0' is triggered in archbuild even after an unsucessful call to makechrootpkg. Since both archbuild and makechrootpkg share the exact same cleanup function (from lib/common), we simply force the shell to exit with the true return value by exec'ing the call to makechrootpkg.

Signed-off-by: Dave Reisner dreisner@archlinux.org

v2 of the earlier patch.

archbuild.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

The first version looked better to me :)

cleanup() might do different things, depending on the script that invokes it (it checks for local variables and enters different branches, depending on their values). There might be a script like find-libdeps that creates a huge temporary directory and sets a flag that is checked in cleanup(). If that script executes another script at the very end and we do not ensure we call cleanup() in both the script and its child, the temporary directory might not be removed properly.

If we want cleanup() to be called only once, we'd have to fix a dozen of other places as well. Having each script call cleanup() is the right way to do it.

diff --git a/archbuild.in b/archbuild.in index 502654c..4e16d1f 100644 --- a/archbuild.in +++ b/archbuild.in @@ -83,4 +83,4 @@ else fi

msg "Building in chroot for [${repo}] (${arch})..." -setarch "${arch}" makechrootpkg -c -n -r "${chroots}/${repo}-${arch}"

+exec setarch "${arch}" makechrootpkg -c -n -r "${chroots}/${repo}-${arch}"

1.7.8