[pacman-dev] [PATCH 2/4] repo-add : remove useless test_repo_db_file function

Xavier Chantry

19 Feb 2009 19 Feb '09

12:19 a.m.

This function was used only once, was basically just one line, and was also called with an unused argument. Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index c89e2a5..6b10527 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -82,15 +82,6 @@ This is free software; see the source for copying conditions.\n\ There is NO WARRANTY, to the extent permitted by law.\n")" } -# test if a file is a repository DB -test_repo_db_file () { - if bsdtar -tf "$REPO_DB_FILE" | grep -q "/desc"; then - return 0 # YES - else - return 1 # NO - fi -} - # write a list entry # arg1 - Entry name # arg2 - List @@ -328,7 +319,7 @@ for arg in "$@"; do # store absolute path to repo DB REPO_DB_FILE=$($realpath "$arg") if [ -f "$REPO_DB_FILE" ]; then - if ! test_repo_db_file $cmd; then + if ! (bsdtar -tf "$REPO_DB_FILE" | grep -q "/desc"); then error "$(gettext "Repository file '%s' is not a proper pacman database.")" "$REPO_DB_FILE" exit 1 fi -- 1.6.1.3

Show replies by date

Xavier Chantry

19 Feb 19 Feb

12:19 a.m.

New subject: [pacman-dev] [PATCH 3/4] repo-add : remove the need of realpath for REPO_DB_FILE

REPO_DB_FILE does not need to be an absolute path anymore so no need to call realpath. Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 31 +++++++++++++++---------------- 1 files changed, 15 insertions(+), 16 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 6b10527..7a271c4 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -317,7 +317,7 @@ for arg in "$@"; do QUIET=1 elif [ -z "$REPO_DB_FILE" ]; then # store absolute path to repo DB - REPO_DB_FILE=$($realpath "$arg") + REPO_DB_FILE="$arg" if [ -f "$REPO_DB_FILE" ]; then if ! (bsdtar -tf "$REPO_DB_FILE" | grep -q "/desc"); then error "$(gettext "Repository file '%s' is not a proper pacman database.")" "$REPO_DB_FILE" @@ -359,27 +359,26 @@ 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" - 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 "$REPO_DB_FILE" in - *tar.gz) TAR_OPT="z" ;; - *tar.bz2) TAR_OPT="j" ;; - *) warning "$(gettext "'%s' does not have a valid archive extension.")" \ - "$REPO_DB_FILE" ;; - esac + case "$REPO_DB_FILE" in + *tar.gz) TAR_OPT="z" ;; + *tar.bz2) TAR_OPT="j" ;; + *) warning "$(gettext "'%s' does not have a valid archive extension.")" \ + "$REPO_DB_FILE" ;; + esac + + filename=$(basename "$REPO_DB_FILE") - bsdtar -c${TAR_OPT}f "$REPO_DB_FILE" * + pushd "$gstmpdir" 2>&1 >/dev/null + if [ -n "$(ls)" ]; then + bsdtar -c${TAR_OPT}f "$filename" * else - # we should only end up with an empty db after a remove of the last package in the database error "$(gettext "All packages have been removed from the database. Deleting '%s'.")" "$REPO_DB_FILE" - rm "$REPO_DB_FILE" fi - popd 2>&1 >/dev/null + + [ -f "$REPO_DB_FILE" ] && mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" + [ -f "$gstmpdir/$filename" ] && mv "$gstmpdir/$filename" "$REPO_DB_FILE" else msg "$(gettext "No packages modified, nothing to do.")" fi -- 1.6.1.3

Xavier Chantry

12:19 a.m.

New subject: [pacman-dev] [PATCH 4/4] repo-add : remove realpath usage

Rework slightly db_write_entry so that $pkgfile is no longer referenced from the temporary dir. This means $pkgfile can be a relative path and does not need to be converted with realpath anymore. Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 39 +++++++++++++++++---------------------- 1 files changed, 17 insertions(+), 22 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 7a271c4..2afb5e9 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -98,7 +98,7 @@ write_list_entry() { db_write_delta() { # blank out all variables and set deltafile to absolute path - local deltafile=$($realpath "$1") + local deltafile="$1" local filename=$(basename "$deltafile") local deltavars pkgname fromver tover arch csize md5sum @@ -129,11 +129,11 @@ db_write_delta() db_write_entry() { # blank out all variables and set pkgfile to an absolute path - local pkgfile=$($realpath "$1") + local pkgfile="$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 \ - startdir optdepend _optdepends + startdir optdepend _optdepends md5sum local OLDIFS="$IFS" # IFS (field separator) is only the newline character @@ -159,22 +159,22 @@ db_write_entry() IFS=$OLDIFS - # get compressed size of package + # get md5sum and compressed size of package + md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile") - startdir=$(pwd) - pushd "$gstmpdir" 2>&1 >/dev/null - # ensure $pkgname and $pkgver variables were found if [ -z "$pkgname" -o -z "$pkgver" ]; then error "$(gettext "Invalid package file '%s'.")" "$pkgfile" - popd 2>&1 >/dev/null return 1 fi # remove an existing entry if it exists, ignore failures db_remove_entry "$pkgname" + startdir=$(pwd) + pushd "$gstmpdir" 2>&1 >/dev/null + # create package directory mkdir "$pkgname-$pkgver" cd "$pkgname-$pkgver" @@ -191,7 +191,7 @@ db_write_entry() # compute checksums msg2 "$(gettext "Computing md5 checksums...")" - echo -e "%MD5SUM%\n$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')\n" >>desc + echo -e "%MD5SUM%\n$md5sum\n" >>desc [ -n "$url" ] && echo -e "%URL%\n$url\n" >>desc write_list_entry "LICENSE" "$_licenses" "desc" @@ -209,6 +209,7 @@ db_write_entry() write_list_entry "OPTDEPENDS" "$_optdepends" "depends" # create deltas entry if there are delta files + # Xav : why should deltas be in $startdir? for delta in $startdir/$pkgname-*-*_to_*-*-$arch.delta; do # This for loop also pulls in all files that start with the current package # name and are followed by a -whatever. For instance, running this loop for @@ -234,11 +235,15 @@ db_write_entry() # add the final newline [ -f "deltas" ] && echo -e "" >>deltas + popd 2>&1 >/dev/null + # preserve the modification time - touch -r "$pkgfile" desc depends - [ -f "deltas" ] && touch -r "$pkgfile" deltas + # Xav : what for? + pkgdir="$gstmpdir/$pkgname-$pkgver" + touch -r "$pkgfile" "$pkgdir/desc" "$pkgdir/depends" + [ -f "$pkgdir/deltas" ] && touch -r "$pkgfile" "$pkgdir/deltas" - popd 2>&1 >/dev/null + return 0 } # end db_write_entry # remove existing entries from the DB @@ -285,16 +290,6 @@ 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 - # main routine gstmpdir=$(mktemp -d /tmp/repo-tools.XXXXXXXXXX) || (\ error "$(gettext "Cannot create temp directory for database building.")"; \ -- 1.6.1.3

Dan McGee

9:24 a.m.

New subject: [pacman-dev] [PATCH 3/4] repo-add : remove the need of realpath for REPO_DB_FILE

On Wed, Feb 18, 2009 at 10:19 AM, Xavier Chantry wrote:

...

REPO_DB_FILE does not need to be an absolute path anymore so no need to call realpath.

Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 31 +++++++++++++++---------------- 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 6b10527..7a271c4 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -317,7 +317,7 @@ for arg in "$@"; do QUIET=1 elif [ -z "$REPO_DB_FILE" ]; then # store absolute path to repo DB - REPO_DB_FILE=$($realpath "$arg") + REPO_DB_FILE="$arg" if [ -f "$REPO_DB_FILE" ]; then if ! (bsdtar -tf "$REPO_DB_FILE" | grep -q "/desc"); then error "$(gettext "Repository file '%s' is not a proper pacman database.")" "$REPO_DB_FILE" @@ -359,27 +359,26 @@ 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" - 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 "$REPO_DB_FILE" in - *tar.gz) TAR_OPT="z" ;; - *tar.bz2) TAR_OPT="j" ;; - *) warning "$(gettext "'%s' does not have a valid archive extension.")" \ - "$REPO_DB_FILE" ;; - esac + case "$REPO_DB_FILE" in + *tar.gz) TAR_OPT="z" ;; + *tar.bz2) TAR_OPT="j" ;; + *) warning "$(gettext "'%s' does not have a valid archive extension.")" \ + "$REPO_DB_FILE" ;; + esac + + filename=$(basename "$REPO_DB_FILE")

- bsdtar -c${TAR_OPT}f "$REPO_DB_FILE" * + pushd "$gstmpdir" 2>&1 >/dev/null + if [ -n "$(ls)" ]; then + bsdtar -c${TAR_OPT}f "$filename" * else - # we should only end up with an empty db after a remove of the last package in the database error "$(gettext "All packages have been removed from the database. Deleting '%s'.")" "$REPO_DB_FILE" - rm "$REPO_DB_FILE" Huh? Why'd this get killed, or am I missing something?

...

fi - popd 2>&1 >/dev/null + + [ -f "$REPO_DB_FILE" ] && mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" + [ -f "$gstmpdir/$filename" ] && mv "$gstmpdir/$filename" "$REPO_DB_FILE" else msg "$(gettext "No packages modified, nothing to do.")" fi -- 1.6.1.3

_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://www.archlinux.org/mailman/listinfo/pacman-dev

Xavier

3:51 p.m.

New subject: [pacman-dev] [PATCH 3/4] repo-add : remove the need of realpath for REPO_DB_FILE

On Thu, Feb 19, 2009 at 2:24 AM, Dan McGee wrote:

...

On Wed, Feb 18, 2009 at 10:19 AM, Xavier Chantry wrote:

...
REPO_DB_FILE does not need to be an absolute path anymore so no need to call realpath.

Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 31 +++++++++++++++---------------- 1 files changed, 15 insertions(+), 16 deletions(-)

- bsdtar -c${TAR_OPT}f "$REPO_DB_FILE" * + pushd "$gstmpdir" 2>&1 >/dev/null + if [ -n "$(ls)" ]; then + bsdtar -c${TAR_OPT}f "$filename" * else - # we should only end up with an empty db after a remove of the last package in the database error "$(gettext "All packages have been removed from the database. Deleting '%s'.")" "$REPO_DB_FILE" - rm "$REPO_DB_FILE" Huh? Why'd this get killed, or am I missing something?

I really cannot blame you for it, I knew this message would be confusing, but I was not sure how to change it. Because the result is still the same. In this case, "$filename" was not created, but "$REPO_DB_FILE" exists. So the first line below will move the $repodb to $repodb.old But $filename wont be moved to $repodb. As a result the database has been deleted (at least moved to .old)

...

...
fi - popd 2>&1 >/dev/null + + [ -f "$REPO_DB_FILE" ] && mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" + [ -f "$gstmpdir/$filename" ] && mv "$gstmpdir/$filename" "$REPO_DB_FILE" else msg "$(gettext "No packages modified, nothing to do.")" fi -- 1.6.1.3

Dan McGee

9:23 p.m.

New subject: [pacman-dev] [PATCH 3/4] repo-add : remove the need of realpath for REPO_DB_FILE

On Thu, Feb 19, 2009 at 1:51 AM, Xavier wrote:

...

On Thu, Feb 19, 2009 at 2:24 AM, Dan McGee wrote:

...
On Wed, Feb 18, 2009 at 10:19 AM, Xavier Chantry wrote:

...
REPO_DB_FILE does not need to be an absolute path anymore so no need to call realpath.

Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 31 +++++++++++++++---------------- 1 files changed, 15 insertions(+), 16 deletions(-)

- bsdtar -c${TAR_OPT}f "$REPO_DB_FILE" * + pushd "$gstmpdir" 2>&1 >/dev/null + if [ -n "$(ls)" ]; then + bsdtar -c${TAR_OPT}f "$filename" * else - # we should only end up with an empty db after a remove of the last package in the database error "$(gettext "All packages have been removed from the database. Deleting '%s'.")" "$REPO_DB_FILE" - rm "$REPO_DB_FILE" Huh? Why'd this get killed, or am I missing something?

I really cannot blame you for it, I knew this message would be confusing, but I was not sure how to change it. Because the result is still the same. In this case, "$filename" was not created, but "$REPO_DB_FILE" exists. So the first line below will move the $repodb to $repodb.old But $filename wont be moved to $repodb. As a result the database has been deleted (at least moved to .old)

Mind writing that in comment form, inline in the code? That would keep me from being confused ever again. :)

...

...
...
fi - popd 2>&1 >/dev/null + + [ -f "$REPO_DB_FILE" ] && mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" + [ -f "$gstmpdir/$filename" ] && mv "$gstmpdir/$filename" "$REPO_DB_FILE" else msg "$(gettext "No packages modified, nothing to do.")" fi -- 1.6.1.3

The rest seems sane, make that fix and I'll pull it in.

Xavier

9:46 p.m.

New subject: [pacman-dev] [PATCH 3/4] repo-add : remove the need of realpath for REPO_DB_FILE

On Thu, Feb 19, 2009 at 2:23 PM, Dan McGee wrote:

...

On Thu, Feb 19, 2009 at 1:51 AM, Xavier wrote:

...
On Thu, Feb 19, 2009 at 2:24 AM, Dan McGee wrote:

...
On Wed, Feb 18, 2009 at 10:19 AM, Xavier Chantry wrote:

...
REPO_DB_FILE does not need to be an absolute path anymore so no need to call realpath.

Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 31 +++++++++++++++---------------- 1 files changed, 15 insertions(+), 16 deletions(-)

- bsdtar -c${TAR_OPT}f "$REPO_DB_FILE" * + pushd "$gstmpdir" 2>&1 >/dev/null + if [ -n "$(ls)" ]; then + bsdtar -c${TAR_OPT}f "$filename" * else - # we should only end up with an empty db after a remove of the last package in the database error "$(gettext "All packages have been removed from the database. Deleting '%s'.")" "$REPO_DB_FILE" - rm "$REPO_DB_FILE" Huh? Why'd this get killed, or am I missing something?

I really cannot blame you for it, I knew this message would be confusing, but I was not sure how to change it. Because the result is still the same. In this case, "$filename" was not created, but "$REPO_DB_FILE" exists. So the first line below will move the $repodb to $repodb.old But $filename wont be moved to $repodb. As a result the database has been deleted (at least moved to .old)

Mind writing that in comment form, inline in the code? That would keep me from being confused ever again. :)

Sure, would this work? # the database will be moved to .old below, and there will be no new one to replace it error "$(gettext "All packages have been removed from the database. Deleting '%s'.")" "$REPO_DB_FILE" If yes, it is on my working branch :)

5552

Age (days ago)

5553

Last active (days ago)

List overview

Download

6 comments

3 participants

participants (3)

Dan McGee
Xavier
Xavier Chantry