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

[Dan McGee]

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.

-Dan