[pacman-dev] [PATCH] makepkg: canonicalize paths from environmental variables

[Allan McRae]

On 04/10/10 23:11, Dave Reisner wrote:
> On Mon, Oct 04, 2010 at 06:57:17AM -0500, Dan McGee wrote:
>> On Mon, Oct 4, 2010 at 5:28 AM, Dave Reisner<d at falconindy.com>  wrote:
>>> On Mon, Oct 04, 2010 at 03:01:50PM +1000, Allan McRae wrote:
>>>> This prevents circular symlinks and weird final package locations
>>>> when using commands like  'PKGDEST="." makepkg'.
>>>>
>>>> Fixes FS#20922.
>>>>
>>>> Signed-off-by: Allan McRae<allan at archlinux.org>
>>>> ---
>>>>   scripts/makepkg.sh.in |   21 +++++++++++++++++----
>>>>   1 files changed, 17 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>>> index 9bcd446..ccf4213 100644
>>>> --- a/scripts/makepkg.sh.in
>>>> +++ b/scripts/makepkg.sh.in
>>>> @@ -1410,6 +1410,19 @@ run_split_packaging() {
>>>>        done
>>>>   }
>>>>
>>>> +# Canonicalize a path if it exists
>>>> +canonicalize_path() {
>>>> +     local path=$1;
>>>> +
>>>> +     if [[ -d $path ]]; then
>>>> +             cd $path
>>>> +             path=$(pwd)
>>>> +             cd -&>/dev/null
>>>> +     fi
>>>> +
>>>> +     echo $path
>>>> +}
>>>> +
>>>>   # getopt like parser
>>>>   parse_options() {
>>>>        local short_options=$1; shift;
>>>> @@ -1615,10 +1628,10 @@ while true; do
>>>>        shift
>>>>   done
>>>>
>>>> -#preserve environment variables
>>>> -_PKGDEST=${PKGDEST}
>>>> -_SRCDEST=${SRCDEST}
>>>> -_SRCPKGDEST=${SRCPKGDEST}
>>>> +# preserve environment variables and canonicalize path
>>>> +[[ -n ${PKGDEST} ]]&&  _PKGDEST=$(canonicalize_path ${PKGDEST})
>>>> +[[ -n ${SRCDEST} ]]&&  _SRCDEST=$(canonicalize_path ${SRCDEST})
>>>> +[[ -n ${SRCPKGDEST} ]]&&  _SRCPKGDEST=$(canonicalize_path ${SRCPKGDEST})
>>>>
>>>>   # default config is makepkg.conf
>>>>   MAKEPKG_CONF=${MAKEPKG_CONF:-$confdir/makepkg.conf}
>>>> --
>>>> 1.7.3.1
>>>>
>>>>
>>>>
>>>
>>> Your cd inside the if needs quoting, and you can get away with just
>>> referencing $PWD instead of forking to pwd (GNU coreutils just returns
>>> $PWD anyways). However, using pwd might have an advantage. The -P flag
>>> will resolve symlinks, and it seems to be common in other 'nixes. This
>>> would solve the case of multiple nested symlinks.
>>
>> pwd is actually a shell builtin, no? I agree with the quoting
>> suggestions, and also wonder if it is worth using a subshell construct
>> here so you don't even have to worry about the 'cd -' business.
>>
>> $ type pwd
>> pwd is a shell builtin
>>
>> -Dan
>>
>
> Hmm, I might have interpreted things incorrectly based on the man pages
> I found. If it's a builtin, that's even better.
>
> Using a subshell would destroy any variable set inside of it. You'd need
> to alter Allan's solution slightly, e.g.
>
> canonicalize_path() {
>    path=$1
>
>    if [[ -d $path ]]; then
>      (
>        cd "$path"
>        pwd -P
>      )
>    else
>      echo "$path"
>    fi
> }

Updated patch taking this approach:
http://projects.archlinux.org/users/allan/pacman.git/commit/?h=working&id=8c3d2959

Allan