On Fri, Jun 12, 2009 at 9:02 PM, Allan McRae wrote:

Jürgen Hötzel wrote:

...

Hi,

I doubt this was by intention:

errors in build() functions are only fatal, if "--log" is enabled. I just made a buggy pkg because some "install ..." commands where not handled by "|| return 1".

Our PKGBUILDs are cluttert full of "|| return 1". Failing commands in build functions should always result in an build error.  Signed-off-by: Juergen Hoetzel ---  scripts/makepkg.sh.in |    7 ++++---  1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index f46b7f8..84d4599 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -703,6 +703,7 @@ run_build() {        local ret=0        if [ "$LOGGING" -eq 1 ]; then

 BUILDLOG="${startdir}/${pkgbase}-${pkgver}-${pkgrel}-${CARCH}-build.log" +               BUILDLOG_CMD="tee $BUILDLOG"                if [ -f "$BUILDLOG" ]; then                        local i=1                        while true; do @@ -714,11 +715,11 @@ run_build() {                        done                        mv "$BUILDLOG" "$BUILDLOG.$i"                fi - -               build 2>&1 | tee "$BUILDLOG"; ret=${PIPESTATUS[0]}        else -               build 2>&1 || ret=$? +               BUILDLOG_CMD="cat -"        fi + +       build 2>&1 | ${BUILDLOG_CMD}; ret=${PIPESTATUS[0]}        # reset our shell options        eval "$shellopts"

Seems fine.  We will want to do the "cat -" thing in run_package too to catch packaging errors.   In fact, as the tee mechanism is so different there, we will need to check that it actually catches errors...

Should I wait to apply this then, or take it for now and wait for another patch? -Dan

On Sat, Jun 13, 2009 at 10:01 AM, Allan McRae wrote:

Dan McGee wrote:

...

On Fri, Jun 12, 2009 at 9:02 PM, Allan McRae wrote:

...

Jürgen Hötzel wrote:

...

Hi,

I doubt this was by intention:

errors in build() functions are only fatal, if "--log" is enabled. I just made a buggy pkg because some "install ..." commands where not handled by "|| return 1".

Our PKGBUILDs are cluttert full of "|| return 1". Failing commands in build functions should always result in an build error.  Signed-off-by: Juergen Hoetzel ---  scripts/makepkg.sh.in |    7 ++++---  1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index f46b7f8..84d4599 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -703,6 +703,7 @@ run_build() {       local ret=0       if [ "$LOGGING" -eq 1 ]; then

 BUILDLOG="${startdir}/${pkgbase}-${pkgver}-${pkgrel}-${CARCH}-build.log" +               BUILDLOG_CMD="tee $BUILDLOG"               if [ -f "$BUILDLOG" ]; then                       local i=1                       while true; do @@ -714,11 +715,11 @@ run_build() {                       done                       mv "$BUILDLOG" "$BUILDLOG.$i"               fi - -               build 2>&1 | tee "$BUILDLOG"; ret=${PIPESTATUS[0]}       else -               build 2>&1 || ret=$? +               BUILDLOG_CMD="cat -"       fi + +       build 2>&1 | ${BUILDLOG_CMD}; ret=${PIPESTATUS[0]}       # reset our shell options       eval "$shellopts"

Seems fine.  We will want to do the "cat -" thing in run_package too to catch packaging errors.   In fact, as the tee mechanism is so different there, we will need to check that it actually catches errors...

Should I wait to apply this then, or take it for now and wait for another patch?

I like to keep the run_build and run_package functions as similar as possible (one day I might get around to refactoring them...  especially as I want to add run_check it the future). So I would prefer to wait until this is fix for both functions in as similar way as possible.

Address this before 3.3? We never got a resubmit. -Dan