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

[Dan McGee]

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.

-Dan