[pacman-dev] [PATCH] makepkg: canonicalize paths from environmental variables
This prevents circular symlinks and weird final package locations when using commands like 'PKGDEST="." makepkg'. Fixes FS#20922. Signed-off-by: Allan McRae <allan@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
Allan McRae wrote:
This prevents circular symlinks and weird final package locations when using commands like 'PKGDEST="." makepkg'.
Fixes FS#20922.
Interesting, I just realized something : I made a dir symlink : bar -> foo/ [xavier@xps-m1530 ~]$ cd foo/ [xavier@xps-m1530 foo]$ echo $PWD ; readlink -f . /home/xavier/foo /home/xavier/foo [xavier@xps-m1530 foo]$ cd ../bar/ [xavier@xps-m1530 bar]$ echo $PWD ; readlink -f . /home/xavier/bar /home/xavier/foo So $PWD gives exactly what I was looking for back then for repo-add, absolute pathname without symlink resolution. I wonder if there is some weird cases where it wouldn't do what we want. But at least it seems to work for me. And just in case we ever use readlink/realpath/etc, it would be bad to forget to run it on the two paths ;)
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@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. d
On Mon, Oct 4, 2010 at 5:28 AM, Dave Reisner <d@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@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
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@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@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
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@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@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
participants (4)
-
Allan McRae
-
Dan McGee
-
Dave Reisner
-
Xavier