[pacman-dev] [PATCH 05/11] makepkg: undeclared local variables

[Andres P]

On Sun, Jun 20, 2010 at 7:43 AM, Allan McRae <allan at archlinux.org> wrote:
> On 17/06/10 22:44, Andres P wrote:
>>
>> Tight variable scoping should avoid further regressions with new patches
>> and
>> variable overriding (see what ac5c2fd09 fixed).
>>
>> Signed-off-by: Andres P<aepd87 at gmail.com>
>> ---
>>  scripts/makepkg.sh.in |   30 ++++++++++++++++++++++--------
>>  1 files changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>> index 37241bd..630d9c2 100644
>> --- a/scripts/makepkg.sh.in
>> +++ b/scripts/makepkg.sh.in
>
> <snip>
>
>> @@ -384,6 +386,9 @@ run_pacman() {
>>  check_deps() {
>>        (( $#>  0 )) || return
>>
>> +       # XXX: Due to a bash bug, pmout's subshell cannot be declared
>> sensibly:
>> +       #      local pmout=$(run_pacman -T "$@")
>
> Can you explain that comment?
>
>> +       local pmout
>>        local ret=0
>>        pmout=$(run_pacman -T "$@")
>>        ret=$?
>> @@ -651,7 +656,7 @@ extract_sources() {
>>        msg "$(gettext "Extracting Sources...")"
>>        local netfile
>>        for netfile in "${source[@]}"; do
>> -               file=$(get_filename "$netfile")
>> +               local file=$(get_filename "$netfile")
>
> And why it does not apply here...
>

Because get_filename isn't being checked for a return value.

I posted this on yesterday's patch's comments, but I guess it should
be in a comment aswell:
$ fn() { foo=$(false); echo $?; local bar=$(false); echo $?; }
$ fn
1
0

This means that local bar=$(false), or local bar=$(exit 255) returns 0.


It would be more organized to change all local subshells instead of spending
time figuring out if it gets checked for a return value or not.

But that belongs in a style guide type document or something...

Just yesterday I saw someone submit a patch with [[ $(type -t "foo") ==
"function" ]], so I'm guessing there's going to be more of these flashbacks.

Andres P