[pacman-dev] repo-add error in BSD
repo-add uses readlink -f in linux to find out file's real path. readlink is part of coreutils. I've found that readlink in BSD don't support -f option so we need to use realpath instead. Attached you can find a mini-patch. Cheers, Antonio Huete
On Tue, Jun 3, 2008 at 4:53 PM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
repo-add uses readlink -f in linux to find out file's real path. readlink is part of coreutils. I've found that readlink in BSD don't support -f option so we need to use realpath instead.
Attached you can find a mini-patch.
Please use inline patches next time so we can more easily comment inline, thanks. You may want to even read up on using git-send-email. I've marked my comments below with **** since I don't have the benefit of reply markers. Besides what I mention below, I think this patch looks OK and we can apply this. I would also ask that you add it to repo-remove and replace the use of readlink there as well with a call to fullpath.
From b1142fa6d3b182161d6348bf0942eb3b94f40e3a Mon Sep 17 00:00:00 2001 From: Antonio Huete Jimenez <ahuete.devel@gmail.com> Date: Tue, 3 Jun 2008 00:26:06 +0200 Subject: [PATCH] Let repo-add to use realpath instead readlink in BSD environments.
--- po/es.po | 3 +++ scripts/repo-add.sh.in | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/po/es.po b/po/es.po index e53b92b..5a98562 100644 --- a/po/es.po +++ b/po/es.po @@ -1780,6 +1780,9 @@ msgstr "%s no fue encontrado" msgid "could not find %s-%s-%s-%s%s - aborting" msgstr "No se pudo encontrar %s-%s-%s-%s%s - abortando" +msgid "Couldn't use neither realpath nor readlink to canonicalize." +msgstr "Fallo al encontrar realpath o readlink." + #~ msgid "error: failed to add target '%s' (%s)" #~ msgstr "error: fallo al procesar '%s' (%s)" diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index e90f0e8..9b77e39 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -31,6 +31,18 @@ REPO_DB_FILE="" # ensure we have a sane umask set umask 0022 +fullpath() { + if type -p realpath; then + realpath "$@" + echo soy de realpath ****Hmm? Debugging stuff? + elif type -p readlink; then + readlink -f "$@" + echo soy de readlink + else + error "$(gettext "Couldn't use neither realpath nor readlink to canonicalize.")" ***Either the realpath or readlink utility is required! and probaly also add an exit 1 here? + fi +} + msg() { [ $QUIET -ne 0 ] && return local mesg=$1; shift @@ -104,7 +116,7 @@ write_list_entry() { db_write_delta() { # blank out all variables and set deltafile - local deltafile=$(readlink -f "$1") + local deltafile=$(fullpath "$1") local filename=$(basename "$deltafile") local deltavars pkgname fromver tover arch csize md5sum @@ -135,7 +147,7 @@ db_write_delta() db_write_entry() { # blank out all variables and set pkgfile - local pkgfile=$(readlink -f "$1") + local pkgfile=$(fullpath "$1") local pkgname pkgver pkgdesc url builddate packager csize size \ group depend backup license replaces provides conflict force \ _groups _depends _backups _licenses _replaces _provides _conflicts \ @@ -304,7 +316,7 @@ for arg in "$@"; do elif [ "$arg" == "--quiet" -o "$arg" == "-q" ]; then QUIET=1 elif [ -z "$REPO_DB_FILE" ]; then - REPO_DB_FILE=$(readlink -f "$arg") + REPO_DB_FILE=$(fullpath -f "$arg") **** You left the -f arg in here by accident I'm assuming. if ! test_repo_db_file; then error "$(gettext "Repository file '%s' is not a proper pacman database.")" "$REPO_DB_FILE" exit 1 -- 1.5.5.1
On Tue, Jun 3, 2008 at 11:53 PM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
repo-add uses readlink -f in linux to find out file's real path. readlink is part of coreutils. I've found that readlink in BSD don't support -f option so we need to use realpath instead.
Can anyone explain why we need readlink at all? Otherwise, it is used in repo-remove too but repo-add and repo-remove should be combined.
Xavier escribió:
On Tue, Jun 3, 2008 at 11:53 PM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
repo-add uses readlink -f in linux to find out file's real path. readlink is part of coreutils. I've found that readlink in BSD don't support -f option so we need to use realpath instead.
Can anyone explain why we need readlink at all?
Otherwise, it is used in repo-remove too but repo-add and repo-remove should be combined.
Ooops! I forgot this. Should I still submit the full patch?
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev
Antonio Huete Jimenez wrote:
Xavier escribió:
On Tue, Jun 3, 2008 at 11:53 PM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
repo-add uses readlink -f in linux to find out file's real path. readlink is part of coreutils. I've found that readlink in BSD don't support -f option so we need to use realpath instead.
Can anyone explain why we need readlink at all?
Otherwise, it is used in repo-remove too but repo-add and repo-remove should be combined.
Ooops! I forgot this. Should I still submit the full patch?
Not before the above question is answered : what do we need readlink for?
Can anyone explain why we need readlink at all?
Otherwise, it is used in repo-remove too but repo-add and repo-remove should be combined.
Ooops! I forgot this. Should I still submit the full patch?
Not before the above question is answered : what do we need readlink for?
In repo-add/repo-remove, readlink is used to calculate the full path of a file (canonicalize with option -f). But in BSD there's no such option in readlink, so you have to use realpath instead
On Thu, Jun 12, 2008 at 10:47 AM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
In repo-add/repo-remove, readlink is used to calculate the full path of a file (canonicalize with option -f). But in BSD there's no such option in readlink, so you have to use realpath instead
Sorry, I didn't realize my question was that obscure, let me rephrase it : Why the hell do we need to calculate the full path of these files? If I ask that question, it is because I am personally not able to answer it myself after looking at this repo-add script. If someone can explain why this is necessary, then we will have to use your readlink/realpath patch. If no one can, then it is probably useless so we can remove readlink usage altogether and solve this portability problem in a much simpler way.
On Thu, Jun 12, 2008 at 3:58 AM, Xavier <shiningxc@gmail.com> wrote:
On Thu, Jun 12, 2008 at 10:47 AM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
In repo-add/repo-remove, readlink is used to calculate the full path of a file (canonicalize with option -f). But in BSD there's no such option in readlink, so you have to use realpath instead
Sorry, I didn't realize my question was that obscure, let me rephrase it : Why the hell do we need to calculate the full path of these files?
If I ask that question, it is because I am personally not able to answer it myself after looking at this repo-add script. If someone can explain why this is necessary, then we will have to use your readlink/realpath patch. If no one can, then it is probably useless so we can remove readlink usage altogether and solve this portability problem in a much simpler way.
Without looking at the script, I imagine it has to do with calculating the path used in %FILENAME% in the db entries. it *does* support full paths: %FILENAME% foo/bar/somepkg-1.0.pkg.tar.gz
On Thu, Jun 12, 2008 at 5:03 PM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
Without looking at the script, I imagine it has to do with calculating the path used in %FILENAME% in the db entries. it *does* support full paths: %FILENAME% foo/bar/somepkg-1.0.pkg.tar.gz
Actually, I didn't check that carefully, but here is how filename is generated by repo-add : echo -e "%FILENAME%\n$(basename "$1")\n" >>desc And at the beginning of db_write_entry function : local pkgfile=$(readlink -f "$1") So that means readlink is not used for the FILENAME, and only the basename of the file is put there, so not the full path.
On Thu, Jun 12, 2008 at 10:03 AM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Thu, Jun 12, 2008 at 3:58 AM, Xavier <shiningxc@gmail.com> wrote:
On Thu, Jun 12, 2008 at 10:47 AM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
In repo-add/repo-remove, readlink is used to calculate the full path of a file (canonicalize with option -f). But in BSD there's no such option in readlink, so you have to use realpath instead
Sorry, I didn't realize my question was that obscure, let me rephrase it : Why the hell do we need to calculate the full path of these files?
If I ask that question, it is because I am personally not able to answer it myself after looking at this repo-add script. If someone can explain why this is necessary, then we will have to use your readlink/realpath patch. If no one can, then it is probably useless so we can remove readlink usage altogether and solve this portability problem in a much simpler way.
Without looking at the script, I imagine it has to do with calculating the path used in %FILENAME% in the db entries. it *does* support full paths: %FILENAME% foo/bar/somepkg-1.0.pkg.tar.gz
That doesn't even use the result of readlink, haha: local pkgfile=$(readlink -f "$1") ..... echo -e "%FILENAME%\n$(basename "$1")\n" >>desc So FILENAME doesn't support any full paths- this has come up on the list before. I really see little need for this readlink usage, but Aaron, you did include it originally. If you get a chance to look closer, let us know what you think it may have been for. -Dan
On Thu, Jun 12, 2008 at 10:13 AM, Dan McGee <dpmcgee@gmail.com> wrote:
On Thu, Jun 12, 2008 at 10:03 AM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Thu, Jun 12, 2008 at 3:58 AM, Xavier <shiningxc@gmail.com> wrote:
On Thu, Jun 12, 2008 at 10:47 AM, Antonio Huete Jimenez <ahuete.devel@gmail.com> wrote:
In repo-add/repo-remove, readlink is used to calculate the full path of a file (canonicalize with option -f). But in BSD there's no such option in readlink, so you have to use realpath instead
Sorry, I didn't realize my question was that obscure, let me rephrase it : Why the hell do we need to calculate the full path of these files?
If I ask that question, it is because I am personally not able to answer it myself after looking at this repo-add script. If someone can explain why this is necessary, then we will have to use your readlink/realpath patch. If no one can, then it is probably useless so we can remove readlink usage altogether and solve this portability problem in a much simpler way.
Without looking at the script, I imagine it has to do with calculating the path used in %FILENAME% in the db entries. it *does* support full paths: %FILENAME% foo/bar/somepkg-1.0.pkg.tar.gz
That doesn't even use the result of readlink, haha: local pkgfile=$(readlink -f "$1") ..... echo -e "%FILENAME%\n$(basename "$1")\n" >>desc
So FILENAME doesn't support any full paths- this has come up on the list before. I really see little need for this readlink usage, but Aaron, you did include it originally. If you get a chance to look closer, let us know what you think it may have been for.
Well if it's not done that way then it's possible I intended to do it that way and just forgot. The only time I used readlink in scripts is when I need to create a relative path from a to b (or, when I need an absolute path). So, the options are either: a) keep readlink there and allow full paths b) remove it entirely I kinda like the idea of using full paths, but it's not a huge deal
Aaron Griffin wrote:
Well if it's not done that way then it's possible I intended to do it that way and just forgot. The only time I used readlink in scripts is when I need to create a relative path from a to b (or, when I need an absolute path).
Finally, it looks like we need absolute path because we change the directory. So after changing directory, the eventual relative paths would no longer be valid. I still don't know if it is possible to handle this situation without using readlink -f.
On Sun, Jun 15, 2008 at 1:34 PM, Xavier <shiningxc@gmail.com> wrote:
Finally, it looks like we need absolute path because we change the directory. So after changing directory, the eventual relative paths would no longer be valid. I still don't know if it is possible to handle this situation without using readlink -f.
Every alternatives I could think of would be much uglier, so forget it. That patch using both realpath and readlink seems to be the cleanest way.
Every alternatives I could think of would be much uglier, so forget it. That patch using both realpath and readlink seems to be the cleanest way.
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev
Sorry, I've been out for a while. It's nice to see that the patch is already commited :-)
Linux coreutils provides readlink, and BSD systems tend to have realpath available. Both commands provide similar functionality but of course have different names. Add a check for either and use what is available. While doing this, also unify some of the differences that have cropped up between repo-add and repo-remove. Signed-off-by: Dan McGee <dan@archlinux.org> --- scripts/repo-add.sh.in | 23 +++++++++++++++++------ scripts/repo-remove.sh.in | 20 ++++++++++++++++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index e90f0e8..53a7da5 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -103,8 +103,8 @@ write_list_entry() { # arg1 - path to delta db_write_delta() { - # blank out all variables and set deltafile - local deltafile=$(readlink -f "$1") + # blank out all variables and set deltafile to absolute path + local deltafile=$($realpath "$1") local filename=$(basename "$deltafile") local deltavars pkgname fromver tover arch csize md5sum @@ -134,8 +134,8 @@ db_write_delta() # arg1 - path to package db_write_entry() { - # blank out all variables and set pkgfile - local pkgfile=$(readlink -f "$1") + # blank out all variables and set pkgfile to an absolute path + local pkgfile=$($realpath "$1") local pkgname pkgver pkgdesc url builddate packager csize size \ group depend backup license replaces provides conflict force \ _groups _depends _backups _licenses _replaces _provides _conflicts \ @@ -278,6 +278,16 @@ if [ $# -lt 2 ]; then exit 1 fi +# check for and store the name of a realpath-like program +if [ $(type -t realpath) ]; then + realpath='realpath' +elif [ $(type -t readlink) ]; then + realpath='readlink -f' +else + error "$(gettext "Either realpath or readlink are required by repo-add.")" + exit 1 # $E_MISSING_PROGRAM +fi + # source system and user makepkg.conf if [ -r "$confdir/makepkg.conf" ]; then source "$confdir/makepkg.conf" @@ -304,7 +314,8 @@ for arg in "$@"; do elif [ "$arg" == "--quiet" -o "$arg" == "-q" ]; then QUIET=1 elif [ -z "$REPO_DB_FILE" ]; then - REPO_DB_FILE=$(readlink -f "$arg") + # store absolute path to repo DB + REPO_DB_FILE=$($realpath "$arg") if ! test_repo_db_file; then error "$(gettext "Repository file '%s' is not a proper pacman database.")" "$REPO_DB_FILE" exit 1 @@ -331,7 +342,7 @@ done # if all operations were a success, re-zip database if [ $success -eq 1 ]; then - msg "$(gettext "Creating updated database file %s")" "$REPO_DB_FILE" + msg "$(gettext "Creating updated database file '%s'")" "$REPO_DB_FILE" pushd "$gstmpdir" 2>&1 >/dev/null if [ -n "$(ls)" ]; then diff --git a/scripts/repo-remove.sh.in b/scripts/repo-remove.sh.in index 08786ee..a650bcf 100644 --- a/scripts/repo-remove.sh.in +++ b/scripts/repo-remove.sh.in @@ -126,6 +126,16 @@ if [ $# -lt 2 ]; then exit 1 fi +# check for and store the name of a realpath-like program +if [ $(type -t realpath) ]; then + realpath='realpath' +elif [ $(type -t readlink) ]; then + realpath='readlink -f' +else + error "$(gettext "Either realpath or readlink are required by repo-add.")" + exit 1 # $E_MISSING_PROGRAM +fi + # source system and user makepkg.conf if [ -r "$confdir/makepkg.conf" ]; then source "$confdir/makepkg.conf" @@ -149,9 +159,10 @@ for arg in "$@"; do if [ "$arg" == "--quiet" -o "$arg" == "-q" ]; then QUIET=1 elif [ -z "$REPO_DB_FILE" ]; then - REPO_DB_FILE=$(readlink -f "$arg") + # store absolute path to repo DB + REPO_DB_FILE=$($realpath "$arg") if ! test_repo_db_file; then - error "$(gettext "Repository file '%s' is not a proper pacman database.")\n" "$REPO_DB_FILE" + error "$(gettext "Repository file '%s' is not a proper pacman database.")" "$REPO_DB_FILE" exit 1 elif [ -f "$REPO_DB_FILE" ]; then msg "$(gettext "Extracting database to a temporary location...")" @@ -170,12 +181,13 @@ done # if all operations were a success, re-zip database if [ $success -eq 1 ]; then - msg "$(gettext "Creating updated database file '%s'...")" "$REPO_DB_FILE" + msg "$(gettext "Creating updated database file '%s'")" "$REPO_DB_FILE" pushd "$gstmpdir" 2>&1 >/dev/null if [ -n "$(ls)" ]; then [ -f "${REPO_DB_FILE}.old" ] && rm "${REPO_DB_FILE}.old" [ -f "$REPO_DB_FILE" ] && mv "$REPO_DB_FILE" "${REPO_DB_FILE}.old" + case "$DB_COMPRESSION" in gz) TAR_OPT="z" ;; bz2) TAR_OPT="j" ;; @@ -194,6 +206,6 @@ else fi # remove the temp directory used to unzip -[ -d "$gstmpdir" ] && rm -rf $gstmpdir +[ -d "$gstmpdir" ] && rm -rf "$gstmpdir" # vim: set ts=2 sw=2 noet: -- 1.5.5.3
participants (5)
-
Aaron Griffin
-
Antonio Huete Jimenez
-
Dan McGee
-
Dan McGee
-
Xavier