On Mon, Jun 21, 2010 at 7:33 PM, Allan McRae <[email protected]> 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
