[pacman-dev] [PATCH] makepkg: use basename instead of a sed command

[Cedric Staniewski]

Allan McRae wrote:
> Cedric Staniewski wrote:
>> Xavier wrote:
>>  
>>> On Sat, Oct 17, 2009 at 7:41 PM, Cedric Staniewski <cedric at gmx.ca>
>>> wrote:
>>>    
>>>> The basename command is easier to understand than a sed command and it
>>>> is even slightly faster than sed.
>>>>
>>>> Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
>>>> ---
>>>> or if you prefer bashisms, see next patch
>>>>
>>>>       
>>> I actually saw that code recently and thought exactly the same.
>>> I prefer the bash way for code that is executed many times (for
>>> performance reason), but it's not really the case here, so basename
>>> should be fine.
>>> Is basename available on all the os we support ?
>>>     
>>
>> I do not know if it is available on all the required os, but it was
>> already used before in makepkg and makepkg's header states:
>>
>>  
>>> # makepkg uses quite a few external programs during its execution. You
>>> # need to have at least the following installed for makepkg to function:
>>> #   awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, find
>>> (findutils),
>>> #   getopt (util-linux), gettext, grep, gzip, openssl, sed
>>>     
>>
>> with basename being part of coreutils.
>>   
> 
> I'd prefer the bash substituion for
> 
> 1) it is a bash internal so does not require starting another process

That's sure, but for basename it makes not so much difference:

$ url="http://mailman.archlinux.org/mailman/listinfo/pacman-dev"
$ time echo $url | sed 's|^.*://.*/||g'
pacman-dev

real	0m0.012s
user	0m0.010s
sys	0m0.003s
$ time echo $(basename "$url")
pacman-dev

real	0m0.003s
user	0m0.003s
sys	0m0.000s
$ time echo ${url##*/}
pacman-dev

real	0m0.000s
user	0m0.000s
sys	0m0.000s


In my opinion, the use of basename would make the code a little bit easier to understand, but if we use bash substitutions for anything else, it would not make much sense to use basename here.

> 2) the only use of basename so far in makepkg is in an error message
> which I doubt many people ever hit.  So there is no real guarantee this
> is portable across platforms (though it should be...)


If you thing this could be an issue, we should use bash substitutions.


> Allan
> 
>