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

[Allan McRae]

Aaron Griffin wrote:
> On Sun, Apr 26, 2009 at 9:39 AM, Marc - A. Dahlhaus <mad at wol.de> wrote:
>   
>> Dan McGee schrieb:
>>     
>>> 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.
>>>
>>>       
>> This was an ugly hack, i know, hence the last few lines of my message.
>>     
>>> 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.
>>>       
>> Thats what i had in mind.
>>
>> I'll do a complete rebuild of all packages and grep for unsupported binarys
>> in the logs.
>> There might be some other types where this could bite us. I'll report back
>> after that.
>>     
>
> Is it actually strip-able? Could we actually unpack it (based on the
> compressed-encoding), strip it, and repack?

I had that thought initially and then thought that would be a major 
pain.  I'm quite happy with detecting compressed files and skipping 
them. But if someone goes ahead and implements that, at this stage I see 
no reason not to include it.  It just won't be me...

Allan