[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> --- Note that this adds readlink as a dependency to makepkg. That is provided by coreutils which is essential anyway so I do not think this is an issue. scripts/makepkg.sh.in | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 01d73f8..9c85d21 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1616,10 +1616,10 @@ while true; do shift done -#preserve environment variables -_PKGDEST=${PKGDEST} -_SRCDEST=${SRCDEST} -_SRCPKGDEST=${SRCPKGDEST} +# preserve environment variables and canonicalize path +[[ -n ${PKGDEST} ]] && _PKGDEST=$(readlink -f ${PKGDEST}) +[[ -n ${SRCDEST} ]] && _SRCDEST=$(readlink -f ${SRCDEST}) +[[ -n ${SRCPKGDEST} ]] && _SRCPKGDEST=$(readlink -f ${SRCPKGDEST}) # default config is makepkg.conf MAKEPKG_CONF=${MAKEPKG_CONF:-$confdir/makepkg.conf} -- 1.7.3
On 29/09/10 22:33, 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> ---
Note that this adds readlink as a dependency to makepkg. That is provided by coreutils which is essential anyway so I do not think this is an issue.
And I may be wrong... it appears readlink does different things with -f on OSX/BSD. Off to find a more compatible solution. Allan
On 29/09/10 22:41, Allan McRae wrote:
On 29/09/10 22:33, 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> ---
Note that this adds readlink as a dependency to makepkg. That is provided by coreutils which is essential anyway so I do not think this is an issue.
And I may be wrong... it appears readlink does different things with -f on OSX/BSD. Off to find a more compatible solution.
Could any BSD/OSX users report on whether "realpath ." give the full path? Thanks, Allan
On Wed, Sep 29, 2010 at 8:15 AM, Allan McRae <allan@archlinux.org> wrote:
On 29/09/10 22:41, Allan McRae wrote:
On 29/09/10 22:33, 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> ---
Note that this adds readlink as a dependency to makepkg. That is provided by coreutils which is essential anyway so I do not think this is an issue.
And I may be wrong... it appears readlink does different things with -f on OSX/BSD. Off to find a more compatible solution.
Could any BSD/OSX users report on whether "realpath ." give the full path?
It worked fine according to this: c8a41b7d6da7f820754a07cb085687ea5e110f85
On 29/09/10 23:25, Dan McGee wrote:
On Wed, Sep 29, 2010 at 8:15 AM, Allan McRae<allan@archlinux.org> wrote:
On 29/09/10 22:41, Allan McRae wrote:
On 29/09/10 22:33, 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> ---
Note that this adds readlink as a dependency to makepkg. That is provided by coreutils which is essential anyway so I do not think this is an issue.
And I may be wrong... it appears readlink does different things with -f on OSX/BSD. Off to find a more compatible solution.
Could any BSD/OSX users report on whether "realpath ." give the full path?
It worked fine according to this: c8a41b7d6da7f820754a07cb085687ea5e110f85
Just to clarify (thanks to Xavier), it worked on BSD but not in OSX which does not have either "readlink -f" or "realpath"... Argh!
On Wed, Sep 29, 2010 at 3:52 PM, Allan McRae <allan@archlinux.org> wrote:
Just to clarify (thanks to Xavier), it worked on BSD but not in OSX which does not have either "readlink -f" or "realpath"...
just found a link that might help : http://stackoverflow.com/questions/1055671/how-can-i-get-the-behavior-of-gnu... To sum up the solutions : 1) use python os.path.realpath 2) use C realpath (3) 3) use fink /sw/sbin/readlink -f 4) implement in bash Someone already proposed 4) a while ago : http://mailman.archlinux.org/pipermail/pacman-dev/2009-February/008147.html I did not compare this bash implementation with the one found on stackoverflow. The best might still be to see if there is any way to avoid using realpath, like I did for repo-add back then. But maybe there is really no way out for makepkg, I don't know.
On 29/09/10 23:58, Xavier Chantry wrote:
On Wed, Sep 29, 2010 at 3:52 PM, Allan McRae<allan@archlinux.org> wrote:
Just to clarify (thanks to Xavier), it worked on BSD but not in OSX which does not have either "readlink -f" or "realpath"...
just found a link that might help : http://stackoverflow.com/questions/1055671/how-can-i-get-the-behavior-of-gnu...
To sum up the solutions : 1) use python os.path.realpath 2) use C realpath (3) 3) use fink /sw/sbin/readlink -f 4) implement in bash
Someone already proposed 4) a while ago : http://mailman.archlinux.org/pipermail/pacman-dev/2009-February/008147.html I did not compare this bash implementation with the one found on stackoverflow.
The best might still be to see if there is any way to avoid using realpath, like I did for repo-add back then. But maybe there is really no way out for makepkg, I don't know.
Well, one way to avoid all this is to add something like: [[ $PKGDIR == "." ]] && $PKGDIR="$startdir" before we preserve the environmental variables. But that is bad as things like PKGDIR="../" would still cause issues. So I think we should just add the bash implementation of realpath as a function inside makepkg. The one at stackoverflow is very simplistic so I will take another look at the one posted to the mailing list. We should really look at libifying some/all of this sort of stuff out of makepkg... Allan
Well, one way to avoid all this is to add something like:
[[ $PKGDIR == "." ]] && $PKGDIR="$startdir"
before we preserve the environmental variables. But that is bad as things like PKGDIR="../" would still cause issues.
I rather meant a way to remove any assumption of PKGDEST being absolute. This is the only broken code, isn't it : if (( ! ret )) && [[ "$PKGDEST" != "${startdir}" ]]; then ln -sf "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" ret=$? fi Why not do something like : local link_file="$startdir/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" if [[ ! -f "$link_file" ]]; then ln -sf $pkg_file $link_file fi I don't know if there are cases where a regular file exists but is not the package we just built. If it can happen and we care, we could try to test whether pkg_file and link_file are not actually the same regular file.
So I think we should just add the bash implementation of realpath as a function inside makepkg. The one at stackoverflow is very simplistic so I will take another look at the one posted to the mailing list. We should really look at libifying some/all of this sort of stuff out of makepkg...
Allan
Btw there are actually several bash impl in that link, the last one is more complex : http://stackoverflow.com/questions/1055671/how-can-i-get-the-behavior-of-gnu...
On 30/09/10 21:26, Xavier wrote:
Well, one way to avoid all this is to add something like:
[[ $PKGDIR == "." ]]&& $PKGDIR="$startdir"
before we preserve the environmental variables. But that is bad as things like PKGDIR="../" would still cause issues.
I rather meant a way to remove any assumption of PKGDEST being absolute.
This is the only broken code, isn't it :
if (( ! ret ))&& [[ "$PKGDEST" != "${startdir}" ]]; then ln -sf "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" ret=$? fi
No. It also affects the location of the tarfile. e.g. PKGDEST="." puts the tar file in $pkgdir because we create the tarball location using $PKGDEST while in $pkgdir: cd $pkgdir ... pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" ...
Why not do something like : local link_file="$startdir/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" if [[ ! -f "$link_file" ]]; then ln -sf $pkg_file $link_file fi
I don't know if there are cases where a regular file exists but is not the package we just built. If it can happen and we care, we could try to test whether pkg_file and link_file are not actually the same regular file.
Hmm... we check if the package exist before building but do we check both PKGDEST and $startdir? We probably should.
So I think we should just add the bash implementation of realpath as a function inside makepkg. The one at stackoverflow is very simplistic so I will take another look at the one posted to the mailing list. We should really look at libifying some/all of this sort of stuff out of makepkg...
Btw there are actually several bash impl in that link, the last one is more complex : http://stackoverflow.com/questions/1055671/how-can-i-get-the-behavior-of-gnu...
Ah - I had missed that. That is definitely the best implementation I have seen so far. Although the dereferencing the symlink is simpler / more efficient in the one posted on this mailing list. Some combination might be fine. Allan
Allan McRae wrote:
No. It also affects the location of the tarfile. e.g. PKGDEST="." puts the tar file in $pkgdir because we create the tarball location using $PKGDEST while in $pkgdir:
cd $pkgdir ... pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" ...
ok but do we need to be in $pkgdir ? IIRC this is exactly what I had to change in repo-add (the current working dir). The problem is always the same : if we change dir, then relative paths become broken. Looking at the create_package function, it seems we just need to add a few $pkgdir/ here and there.
On 01/10/10 01:29, Xavier wrote:
Allan McRae wrote:
No. It also affects the location of the tarfile. e.g. PKGDEST="." puts the tar file in $pkgdir because we create the tarball location using $PKGDEST while in $pkgdir:
cd $pkgdir ... pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" ...
ok but do we need to be in $pkgdir ? IIRC this is exactly what I had to change in repo-add (the current working dir). The problem is always the same : if we change dir, then relative paths become broken. Looking at the create_package function, it seems we just need to add a few $pkgdir/ here and there.
I was looking at the makeworld script in ABS and noticed it has a nice way of dealing with this:
PKGDEST="." cd $PKGDEST PKGDEST=$(pwd) cd - &>/dev/null echo $PKGDEST /home/allan
Now that is what I call a simple solution to implementing readlink/realpath in bash! I'll adjust my patch. Allan
On Wed, Sep 29, 2010 at 11:15:45PM +1000, Allan McRae wrote:
Could any BSD/OSX users report on whether "realpath ." give the full path?
Yes, gives the full path on FreeBSD 8.1-RELEASE. Doesn't work on OpenBSD tho ("zsh: command not found: realpath").
participants (5)
-
Allan McRae
-
Dan McGee
-
Lukas Fleischer
-
Xavier
-
Xavier Chantry