FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 06-21-2010, 05:36 AM
"Allan McRae"
 
Default bash-3 only error trap activation

Hi,

When running makepkg in bash-3.2 I am getting a couple of error trap
activations that we do not get in bash-4.1.


Here is the problem area:

check_deps() {
(( $# > 0 )) || return <- HERE

local ret=0
pmout=$(run_pacman -T "$@") <- HERE
ret=$?


The first one gets set off anytime "depends" or "makedepends" are empty
and can be fixed by using "|| return 0", but the second is doing my head
in... Of course, turning off the error trap around those commands makes
it work, and that may be an OK approach given we are dealing with all
error states below that.


If you want a bash-3.2 package to test grab one from
http://allanmcrae.com/packages/bash3-3.2.051-1.src.tar.gz . Test on a
PKGBUILD with unsatisfied dependencies.


Allan
 
Old 06-21-2010, 05:42 PM
Andres P
 
Default bash-3 only error trap activation

On Mon, Jun 21, 2010 at 1:06 AM, Allan McRae <allan@archlinux.org> wrote:
> * * * *pmout=$(run_pacman -T "$@") * * * <- HERE
> * * * *ret=$?
>
>
> The first one gets set off anytime "depends" or "makedepends" are empty and
> can be fixed by using "|| return 0", but the second is doing my head in...
> *Of course, turning off the error trap around those commands makes it work,
> and that may be an OK approach given we are dealing with all error states
> below that.
>


Try:

ret=$?
pmout=$(run_pacman -T "$@") || ret=$?

Using || after the asigment will prevent setting the err trap.

Funny how you noticed the first, because I was about to submit a patch
that did not return false if there were no arguments.

In reality though, it should be this:
[[ $@ ]] || return 0

Because (( $# )) will not count emtpy arguments.

If check_deps is passed quoted arguments that expand to nothing, it
will malfunction.

Andres P
 
Old 06-21-2010, 05:44 PM
Andres P
 
Default bash-3 only error trap activation

On Mon, Jun 21, 2010 at 1:12 PM, Andres P <aepd87@gmail.com> wrote:
> Try:
>
> ret=$?

Sorry, ret=0

Andres P
 
Old 06-22-2010, 12:03 AM
Allan McRae
 
Default bash-3 only error trap activation

On 22/06/10 03:42, Andres P wrote:

On Mon, Jun 21, 2010 at 1:06 AM, Allan McRae<allan@archlinux.org> wrote:

pmout=$(run_pacman -T "$@")<- HERE
ret=$?


The first one gets set off anytime "depends" or "makedepends" are empty and
can be fixed by using "|| return 0", but the second is doing my head in...
Of course, turning off the error trap around those commands makes it work,
and that may be an OK approach given we are dealing with all error states
below that.




Try:

ret=$?
pmout=$(run_pacman -T "$@") || ret=$?

Using || after the asigment will prevent setting the err trap.


Of course I tried that... It does not work:

==> Making package: pacman-contrib 3.4.0-1 (Tue Jun 22 08:46:58 EST 2010)
==> Checking Runtime Dependencies...
==> 127 ($ret value from within run_pacman)
==> ERROR: An unknown error has occurred. Exiting...
==> 1 ($ret value after "|| ret=$?")
==> ERROR: 'pacman' returned a fatal error (1): pacman>4
(wrong return value so wrong error message)

I looks like in bash-3.2 the following happens. run_pacman returns 127
which sets off the error trap. Then the assignment fails which sets of
another error trap. The use of "|| ret=$?" prevents the assignment
failure error but now there is the wrong return value.



Funny how you noticed the first, because I was about to submit a patch
that did not return false if there were no arguments.

In reality though, it should be this:
[[ $@ ]] || return 0

Because (( $# )) will not count emtpy arguments.

If check_deps is passed quoted arguments that expand to nothing, it
will malfunction.


I think that if check_deps is passed empty arguments, then there is not
dependencies to check and it should just return.


Allan
 
Old 06-22-2010, 12:37 AM
Andres P
 
Default bash-3 only error trap activation

On Mon, Jun 21, 2010 at 7:33 PM, Allan McRae <allan@archlinux.org> wrote:
> Of course I tried that... *It does not work:
>
> ==> Making package: pacman-contrib 3.4.0-1 (Tue Jun 22 08:46:58 EST 2010)
> ==> Checking Runtime Dependencies...
> ==> 127 * * * * * * * * * * *($ret value from within run_pacman)
> ==> ERROR: An unknown error has occurred. Exiting...
> ==> 1 * * * * * * * * * * * *($ret value after "|| ret=$?")
> ==> ERROR: 'pacman' returned a fatal error (1): pacman>4
> * * * * * * * * * * * * * * (wrong return value so wrong error message)
>
> I looks like in bash-3.2 the following happens. *run_pacman returns 127
> which sets off the error trap. * Then the assignment fails which sets of
> another error trap. *The use of "|| ret=$?" prevents the assignment failure
> error but now there is the wrong return value.
>

This is not what anybody wants to hear, but currently the ERR trap that
everything inherits thanks to set -E is just a place holder for a traceback.

Echoing "unrecognized error" isn't helpful at all, and if this is one of the
items that is holding back bash 3.2 compat, then its inclusion should be
reconsidered.

Since trap_exit's symlink cleanup is actually useful, it would be merged to
trap 0, aka clean_up, because it triggers on every signal except kill -9.

This would also shorten other trap definitions.

>> Funny how you noticed the first, because I was about to submit a patch
>> that did not return false if there were no arguments.
>>
>> In reality though, it should be this:
>> [[ $@ ]] || return 0
>>
>> Because (( $# )) will not count emtpy arguments.
>>
>> If check_deps is passed quoted arguments that expand to nothing, it
>> will malfunction.
>
> I think that if check_deps is passed empty arguments, then there is not
> dependencies to check and it should just return.
>

Currently a coincidence with no design consideration; deplist is
unquoted and depends is handled like a unquoted array (see line 1892). What's
the point of using arrays and not quoting them? If arrays get treated as
scalars, then there's no telling if at any point a certain var needs special
consideration or not.

If deplist becomes an array, as it should since there is no reason to not quote
depends and makedepends, then check_deps needs a string check via [[ $@ ]].

Basically, anything that can be quoted, should be. That way you don't have to
hear about bugs with depends arrays with spaces and other special shell
characters, like redirection operators, 6 months from now.

Remember $startdir/4? This could have been prevented with quoting out of
practice, specially for su since its arguments are parsed just like eval's are.

And depends is where you get '>' and '<'...

Andres P
 
Old 06-22-2010, 04:23 AM
Andres P
 
Default bash-3 only error trap activation

Ok, after actually taking the time to install bash3...

$ env -i HOME="$HOME" TERM="$TERM" bash3 <<!

set -o errexit
set -o errtrace

TRIGGERED_ERR() { return $?; }

trap 'TRIGGERED_ERR' ERR

set -o xtrace

var=$(false) || true
echo $?

var=$(false || true) # only way of not triggering it...
echo $?

!

++ false # Subshell false
+++ TRIGGERED_ERR # Ignores outer "|| true"
+++ return 1
+ var=
+ true
+ echo 0
0 # But the entire command line does
# not set off errexit
++ false
++ true # Predictable second subshell...
+ var=
+ echo 0
0


So, if you want to keep the ERR trap then you'll have to modify check_deps so
that it always returns true.

Then you'd have to parse its output, similar to how in_opt_array works.


Horrible kludge, lets drop set -E.

Andres P
 
Old 06-22-2010, 05:00 AM
"Allan McRae"
 
Default bash-3 only error trap activation

On 22/06/10 14:23, Andres P wrote:

Ok, after actually taking the time to install bash3...

$ env -i HOME="$HOME" TERM="$TERM" bash3<<!

set -o errexit
set -o errtrace

TRIGGERED_ERR() { return $?; }

trap 'TRIGGERED_ERR' ERR

set -o xtrace

var=$(false) || true
echo $?

var=$(false || true) # only way of not triggering it...
echo $?

!

++ false # Subshell false
+++ TRIGGERED_ERR # Ignores outer "|| true"
+++ return 1
+ var=
+ true
+ echo 0
0 # But the entire command line does
# not set off errexit
++ false
++ true # Predictable second subshell...
+ var=
+ echo 0
0


So, if you want to keep the ERR trap then you'll have to modify check_deps so
that it always returns true.

Then you'd have to parse its output, similar to how in_opt_array works.


Horrible kludge, lets drop set -E.



I do not think that dropping 'set -E' completely is the way to go. Just
dropping it around that pacman call is enough.


This is the diff I am proposing:

@@ -382,11 +382,15 @@
}

check_deps() {
- (( $# > 0 )) || return
+ (( $# > 0 )) || return 0

+ # Disable error trap in pacman subshell call as this breaks bash-3.2
compatibility
+ # Also, a non-zero return value is not unexpected and we are manually
dealing them

+ set +E
local ret=0
- pmout=$(run_pacman -T "$@")
- ret=$?
+ pmout=$(run_pacman -T "$@") || ret=$?
+ set -E
+
if (( ret == 127 )); then #unresolved deps
echo "$pmout"
elif (( ret )); then


Allan
 
Old 06-22-2010, 05:27 AM
Andres P
 
Default bash-3 only error trap activation

On Tue, Jun 22, 2010 at 12:30 AM, Allan McRae <allan@archlinux.org> wrote:
> I do not think that dropping 'set -E' completely is the way to go. Just
> dropping it around that pacman call is enough.
>

Another candidate for the style guide since it's obfuscated enough.

btw makepkg is also skirting around -E when it sources /etc/profile...

> This is the diff I am proposing:
>
> @@ -382,11 +382,15 @@
> *}
>
> *check_deps() {
> - * * * (( $# > 0 )) || return
> + * * * (( $# > 0 )) || return 0
>
> + * * * # Disable error trap in pacman subshell call as this breaks bash-3.2
> compatibility
> + * * * # Also, a non-zero return value is not unexpected and we are
> manually dealing them
> + * * * set +E
> * * * *local ret=0
> - * * * pmout=$(run_pacman -T "$@")
> - * * * ret=$?
> + * * * pmout=$(run_pacman -T "$@") || ret=$?
> + * * * set -E
> +
> * * * *if (( ret == 127 )); then #unresolved deps
> * * * * * * * *echo "$pmout"
> * * * *elif (( ret )); then
>

Let me know what repo is this diff going to so I can rebase 'undeclared local
vars' and 'do not ignore pacman errors' against it.

Andres P
 

Thread Tools




All times are GMT. The time now is 12:41 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org