[pacman-dev] [PATCH] makepkg: replace basename emulation via sed with a bash substitution

[Allan McRae]

Dan McGee wrote:
> On Sun, Oct 18, 2009 at 9:52 AM, Cedric Staniewski <cedric at gmx.ca> wrote:
>   
>> Allan McRae wrote:
>>     
>>> Cedric Staniewski wrote:
>>>       
>>>> Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
>>>> ---
>>>>  scripts/makepkg.sh.in |    4 ++--
>>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>>> index 24fddf6..ad8b6da 100644
>>>> --- a/scripts/makepkg.sh.in
>>>> +++ b/scripts/makepkg.sh.in
>>>> @@ -190,7 +190,7 @@ get_filename() {
>>>>      # if a filename is specified, use it
>>>>      local filename=$(echo $1 | sed 's|::.*||')
>>>>
>>>>         
>>> We could also use bash substitution for that line.
>>>
>>>       
>>>>      # if it is just an URL, we only keep the last component
>>>> -    echo "$filename" | sed 's|^.*://.*/||g'
>>>> +    echo "${filename##*/}"
>>>>  }
>>>>
>>>>  # extract the URL from a source entry
>>>>
>>>>         
>>> and about two lines below here...
>>>
>>>       
>>>> @@ -282,7 +282,7 @@ in_array() {
>>>>  get_downloadclient() {
>>>>      # $1 = URL with valid protocol prefix
>>>>      local url=$1
>>>> -    local proto=$(echo "$url" | sed 's|://.*||')
>>>> +    local proto=$(echo ${url##*/})
>>>>
>>>>      # loop through DOWNLOAD_AGENTS variable looking for protocol
>>>>      local i
>>>>
>>>>         
>>>       
>> There are several commands which could be replaced by bash substitutions. I attached a patch which also contains the first patch of this thread. So, if we decide for the 'bash way' rather than using basename, this one could be used.
>>
>>
>>
>> >From 23b7c7346e58293e403b00a8ddff0c7e37498f46 Mon Sep 17 00:00:00 2001
>> From: Cedric Staniewski <cedric at gmx.ca>
>> Date: Sun, 18 Oct 2009 16:32:51 +0200
>> Subject: [PATCH] makepkg, repo-add: replace external commands with bash substitutions where possible
>>
>> This also removes the awk dependency from makepkg and repo-add.
>>
>> Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
>> ---
>>
>> Probably, it is better to keep the sed command for extracting the latest svn revision. The 'bash way' is imo more complex, and the slow part of this command is anyway svn info.
>>
>> Besides, I'm not sure about the change in write_pkginfo. `du` separates size and file/directory name by a tab and the output looks like this:
>>        346616  .
>> Currently, I only remove the period at the end and bash strips the trailing tab by itself. Of course, it would be possible to use size=${size%% *} with the whitespace being a tab (bash does not support \t in substitutions as it seems) which I do not really like, so the remaining possibility would be:
>>
>> local size="$(du -sk | tr '\t' ' ')"
>> size="$(( ${size%% *} * 1024 ))"
>>
>> which is not that good either.
>>     
>
> This patch seems pretty reasonable. I think leaving the svn command
> as-is would be a bit more clear, but the rest seems relatively
> reasonable. I'll let Allan comment as well though.
>   

On a brief review, it looks good to me too.   I will have some time to 
review this in more detail later this week and will comment or pull to 
my working branch as appropriate.

Allan