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

[Dave Reisner]

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
}

-dave