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

[Dan McGee]

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