[pacman-dev] [PATCH] Fix a possible bash-4.0 problem in makepkg

[Allan McRae]

Dan McGee wrote:
> On Sun, Apr 26, 2009 at 9:04 AM, Marc - A. Dahlhaus <mad at wol.de> wrote:
>   
>> Allan McRae schrieb:
>>     
>>> Marc - A. Dahlhaus wrote:
>>>       
>>>> Hello,
>>>>
>>>> i've spotted a problem in makepkg's cleanup part if the host is running
>>>> bash-4.0.
>>>> As makepkg runs bash with option -e, on bash-4.0 it fails if strip
>>>> reports an unsupported binary.
>>>> The following trivial patch fixes the problem.
>>>>
>>>> Signed-off-by: "Marc - A. Dahlhaus" <mad at wol.de>
>>>>
>>>> --- pacman-3.2.2.orig/scripts/makepkg.sh.in
>>>> +++ pacman-3.2.2/scripts/makepkg.sh.in
>>>> @@ -766,11 +766,11 @@ tidy_install() {
>>>>        find ${STRIP_DIRS[@]} -type f 2>/dev/null | while read binary ; do
>>>>            case "$(file -biz "$binary")" in
>>>>                *application/x-sharedlib*)  # Libraries (.so)
>>>> -                    /usr/bin/strip --strip-debug "$binary";;
>>>> +                    /usr/bin/strip --strip-debug "$binary" || true;;
>>>>                *application/x-archive*)    # Libraries (.a)
>>>> -                    /usr/bin/strip --strip-debug "$binary";;
>>>> +                    /usr/bin/strip --strip-debug "$binary" || true;;
>>>>                *application/x-executable*) # Binaries
>>>> -                    /usr/bin/strip "$binary";;
>>>> +                    /usr/bin/strip "$binary" || true;;
>>>>            esac
>>>>        done
>>>>    fi
>>>>         
>>> I don't think this is a good approach to the problem.  Having "|| true"
>>> means if there is a real problem, then it gets ignored.
>>>       
>> What real problem could arise?
>> An unsupported binary? Would print errors but creates the package mostly
>> stripped.
>> A missing strip command? Would print errors but creates the package
>> unstripped.
>> A damaged storage... but i think in that case makepkg would have failed in
>> the actual build step and not in cleanup.
>>     
>>> I suppose the real question is: How do you actually get an unsupported
>>> binary?  My guess is this occurs when building a package from a binary
>>> source, in which case "options=('!strip')" would be the solution.
>>>       
>> The package that triggered this, was xen.
>> xen installs some gzipped content inside /usr/lib which strip doesn't
>> support.
>>
>> $ file -biz usr/lib/xen/boot/ioemu-stubdom.gz
>> application/x-executable; charset=binary
>> compressed-encoding=application/x-gzip; charset=binary; charset=binary
>>
>> The file itself is a bootable image file for grub.
>>
>> The library strip lines are not needed.
>> What about a case which catches *"*compressed-encoding*" and continues with
>> the next file?*
>>     
>
> I'm with Allan here- we should fix the real problem rather than adding
> a "|| true" and calling it a day.
>
> I wouldn't be against an exclusion of "compressed-encoding=" when
> running strip- Allan, what do you think? The easiest way of
> implementing this would be to make it the first case and make it a
> no-op.

That is a far better solution.  I am happy with this being the first 
case statement as a no-op.

Allan