[pacman-dev] [PATCH 1/6] repo-add: print warning if same version already exists

older
[pacman-dev] [GIT] The official...

Xavier Chantry

26 Feb 2009 26 Feb '09

8:06 p.m.

Simple fix for FS#13414 Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index c6d25aa..b12188c 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -172,12 +172,16 @@ db_write_entry() return 1 fi - # remove an existing entry if it exists, ignore failures - db_remove_entry "$pkgname" - startdir=$(pwd) pushd "$gstmpdir" 2>&1 >/dev/null + if [ -d "$pkgname-$pkgver" ]; then + warning "$(gettext "An entry for '%s' already existed")" "$pkgname-$pkgver" + fi + + # remove an existing entry if it exists, ignore failures + db_remove_entry "$pkgname" + # create package directory mkdir "$pkgname-$pkgver" cd "$pkgname-$pkgver" -- 1.6.1.3

Show replies by date

Xavier Chantry

26 Feb 26 Feb

8:06 p.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

Before this commit, the repo creation could fail after all packages have been added to the database. Now this will be detected before adding anything. Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index b12188c..a967506 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -326,9 +326,21 @@ for arg in "$@"; do fi msg "$(gettext "Extracting database to a temporary location...")" bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" - elif [ "$cmd" == "repo-remove" ]; then - error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" - exit 1 + else + case "$cmd" in + repo-remove) + error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" + exit 1 + ;; + repo-add) + # check if the file can be created (write permission, directory existence, etc) + if ! touch "$REPO_DB_FILE"; then + error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" + exit 1 + fi + rm -f "$REPO_DB_FILE" + ;; + esac fi else if [ "$cmd" == "repo-add" ]; then -- 1.6.1.3

Xavier Chantry

8:06 p.m.

New subject: [pacman-dev] [PATCH 3/6] repo-add cleanup

Refactor the main loop, which was difficult to read. Use case instead of if when appropriate. Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 130 +++++++++++++++++++++++++---------------------- 1 files changed, 69 insertions(+), 61 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index a967506..0006006 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -270,6 +270,58 @@ db_remove_entry() { popd 2>&1 >/dev/null } # end db_remove_entry +check_repo_db() +{ + 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" + exit 1 + fi + msg "$(gettext "Extracting database to a temporary location...")" + bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" + else + case "$cmd" in + repo-remove) + error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" + exit 1 + ;; + repo-add) + # check if the file can be created (write permission, directory existence, etc) + if ! touch "$REPO_DB_FILE"; then + error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" + exit 1 + fi + rm -f "$REPO_DB_FILE" + ;; + esac + fi +} + +add() +{ + pkgfile=$1 + if [ ! -f "$1" ]; then + error "$(gettext "Package '%s' not found.")" "$pkgfile" + return 1 + fi + + if ! bsdtar -tf "$pkgfile" .PKGINFO 2>&1 >/dev/null; then + error "$(gettext "'%s' is not a package file, skipping")" "$pkgfile" + return 1 + fi + + msg "$(gettext "Adding package '%s'")" "$pkgfile" + + db_write_entry "$pkgfile" +} + +remove() +{ + msg "$(gettext "Searching for package '%s'...")" "$arg" + + db_remove_entry "$arg" +} + # PROGRAM START # determine whether we have gettext; make it a no-op if we do not @@ -279,17 +331,10 @@ if [ ! $(type -t gettext) ]; then } fi -# check for help flags -if [ "$1" = "-h" -o "$1" = "--help" ]; then - usage - exit 0 -fi - -# check for version flags -if [ "$1" = "-V" -o "$1" = "--version" ]; then - version - exit 0 -fi +case "$1" in + -h|--help) usage; exit 0;; + -V|--version) version; exit 0;; +esac # check for correct number of args if [ $# -lt 2 ]; then @@ -312,64 +357,27 @@ fi success=0 # parse arguments for arg in "$@"; do - if [ "$arg" == "--force" -o "$arg" == "-f" ]; then + case "$arg" in + -q|--quiet) QUIET=1;; + -f|--force) warning "$(gettext "the -f and --force options are no longer recognized")" msg2 "$(gettext "use options=(force) in the PKGBUILD instead")" - elif [ "$arg" == "--quiet" -o "$arg" == "-q" ]; then - QUIET=1 - elif [ -z "$REPO_DB_FILE" ]; then - 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" - exit 1 - fi - msg "$(gettext "Extracting database to a temporary location...")" - bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" + ;; + *) + if [ -z "$REPO_DB_FILE" ]; then + REPO_DB_FILE="$arg" + check_repo_db else case "$cmd" in - repo-remove) - error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" - exit 1 - ;; - repo-add) - # check if the file can be created (write permission, directory existence, etc) - if ! touch "$REPO_DB_FILE"; then - error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" - exit 1 - fi - rm -f "$REPO_DB_FILE" - ;; + repo-add) add $arg && success=1 ;; + repo-remove) remove $arg && success=1 ;; esac fi - else - if [ "$cmd" == "repo-add" ]; then - if [ -f "$arg" ]; then - if ! bsdtar -tf "$arg" .PKGINFO 2>&1 >/dev/null; then - error "$(gettext "'%s' is not a package file, skipping")" "$arg" - else - msg "$(gettext "Adding package '%s'")" "$arg" - - if db_write_entry "$arg"; then - success=1 - fi - fi - else - error "$(gettext "Package '%s' not found.")" "$arg" - fi - elif [ "$cmd" == "repo-remove" ]; then - msg "$(gettext "Searching for package '%s'...")" "$arg" - - if db_remove_entry "$arg"; then - success=1 - else - error "$(gettext "Package matching '%s' not found.")" "$arg" - fi - fi - fi + ;; + esac done -# if all operations were a success, re-zip database +# if at least one operation was a success, re-zip database if [ $success -eq 1 ]; then msg "$(gettext "Creating updated database file '%s'")" "$REPO_DB_FILE" -- 1.6.1.3

Xavier Chantry

8:06 p.m.

New subject: [pacman-dev] [PATCH 4/6] repo-add.sh.in : repo-remove improvements

* report when a package entry to be removed is not found * backup and restore eventual "deltas" files * slight optimization when looking for an entry : only look at the entries starting with $pkgname Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 50 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 38 insertions(+), 12 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 0006006..c21a08f 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -93,6 +93,20 @@ write_list_entry() { fi } +find_pkgentry() +{ + local pkgname=$1 + local pkgentry + for pkgentry in $gstmpdir/$pkgname*; do + name=${pkgentry##*/} + if [ "${name%-*-*}" = "$pkgname" ]; then + echo $pkgentry + return 0 + fi + done + return 1 +} + # write a delta entry to the pacman database # arg1 - path to delta db_write_delta() @@ -186,6 +200,9 @@ db_write_entry() mkdir "$pkgname-$pkgver" cd "$pkgname-$pkgver" + # restore an eventual deltas file + [ -f "../$pkgname.deltas" ] && mv "../$pkgname.deltas" deltas + # create desc entry msg2 "$(gettext "Creating 'desc' db entry...")" echo -e "%FILENAME%\n$(basename "$1")\n" >>desc @@ -256,18 +273,20 @@ db_write_entry() # remove existing entries from the DB # arg1 - package name db_remove_entry() { - pushd "$gstmpdir" 2>&1 >/dev/null - - # remove any other package in the DB with same name - local existing - for existing in *; do - if [ "${existing%-*-*}" = "$1" ]; then - msg2 "$(gettext "Removing existing package '%s'...")" "$existing" - rm -rf "$existing" + local pkgname=$1 + local notfound=1 + local pkgentry=$(find_pkgentry $pkgname) + while [ -n "$pkgentry" ]; do + notfound=0 + if [ -f "$pkgentry/deltas" ]; then + mv "$pkgentry/deltas" "$gstmpdir/$pkgname.deltas" fi + msg2 "$(gettext "Removing existing package '%s'...")" \ + "$(basename $pkgentry)" + rm -rf $pkgentry + pkgentry=$(find_pkgentry $pkgname) done - - popd 2>&1 >/dev/null + return $notfound } # end db_remove_entry check_repo_db() @@ -317,9 +336,16 @@ add() remove() { - msg "$(gettext "Searching for package '%s'...")" "$arg" + pkgname=$1 + msg "$(gettext "Searching for package '%s'...")" "$pkgname" - db_remove_entry "$arg" + if db_remove_entry "$pkgname"; then + rm -f "$gstmpdir/$pkgname.deltas" + return 0 + else + error "$(gettext "Package matching '%s' not found.")" "$pkgname" + return 1 + fi } # PROGRAM START -- 1.6.1.3

Xavier Chantry

8:06 p.m.

New subject: [pacman-dev] [PATCH 5/6] repo-add : drop delta support to rewrite it from scratch

The current implementation has several problems : Wrong database format All the info is taken from the filename, which is a bit ugly It looks for .delta files in the current directory when adding a package, which is not very flexible Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 59 ------------------------------------------------ 1 files changed, 0 insertions(+), 59 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index c21a08f..9cf66e5 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -107,37 +107,6 @@ find_pkgentry() return 1 } -# write a delta entry to the pacman database -# arg1 - path to delta -db_write_delta() -{ - # blank out all variables - local deltafile="$1" - local filename=$(basename "$deltafile") - local deltavars pkgname fromver tover arch csize md5sum - - # format of the delta filename: - # (package)-(fromver)_to_(tover)-(arch).delta - deltavars=( $(echo "$filename" | sed -e 's/$.*$-$.*-.*$_to_$.*-.*$-$.*$.delta/\1 \2 \3 \4/') ) - pkgname=${deltavars[0]} - fromver=${deltavars[1]} - tover=${deltavars[2]} - arch=${deltavars[3]} - - # get md5sum and size of delta - md5sum="$(openssl dgst -md5 "$deltafile" | awk '{print $NF}')" - csize=$(@SIZECMD@ "$deltafile") - - # ensure variables were found - if [ -z "$pkgname" -o -z "$fromver" -o -z "$tover" -o -z "$arch" ]; then - return 1 - fi - - # add the entry for this delta file - echo -e "$fromver $tover $csize $filename $md5sum" >>deltas -} # end db_write_delta - - # write an entry to the pacman database # arg1 - path to package db_write_entry() @@ -232,40 +201,12 @@ db_write_entry() write_list_entry "PROVIDES" "$_provides" "depends" 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 - # gcc would also grab gcc-libs. To guard against this, compare the package - # name of the delta to the current package name. - local filename=$(basename "$delta") - local dpkgname="$(echo "$filename" | sed -e 's/$.*$-.*-.*_to_.*-.*-.*.delta/\1/')" - if [ "$pkgname" = "$dpkgname" -a -f "$delta" ]; then - # create deltas file if it does not already exist - if [ ! -f "deltas" ]; then - msg2 "$(gettext "Creating 'deltas' db entry...")" - echo -e "%DELTAS%" >>deltas - fi - - # write this delta entry - if db_write_delta "$delta"; then - msg2 "$(gettext "Added delta '%s'")" "$(basename "$delta")" - else - warning "$(gettext "Could not add delta '%s'")" "$(basename "$delta")" - fi - fi - done - # add the final newline - [ -f "deltas" ] && echo -e "" >>deltas - popd 2>&1 >/dev/null # preserve the modification time # Xav : what for? pkgdir="$gstmpdir/$pkgname-$pkgver" touch -r "$pkgfile" "$pkgdir/desc" "$pkgdir/depends" - [ -f "$pkgdir/deltas" ] && touch -r "$pkgfile" "$pkgdir/deltas" return 0 } # end db_write_entry -- 1.6.1.3

Xavier Chantry

8:06 p.m.

New subject: [pacman-dev] [PATCH 6/6] repo-add : rewrite delta support

Use the correct database format Use xdelta3 to get the source and destination files from the delta itself Allow delta files to be added with repo-add just like package files. delta files can also be removed with repo-remove. This is simply done by looking for a .delta extension in the arguments, and calling the appropriate db_write_delta or db_remove_delta functions. Example usage: repo-add repo/test.db.tar.gz repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz repo-add repo/test.db.tar.gz repo/libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta repo-remove repo/test.db.tar.gz libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta Signed-off-by: Xavier Chantry --- scripts/repo-add.sh.in | 102 +++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 97 insertions(+), 5 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 9cf66e5..504adfd 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -57,8 +57,8 @@ error() { # print usage instructions usage() { printf "repo-add, repo-remove (pacman) %s\n\n" "$myver" - printf "$(gettext "Usage: repo-add [-q] <path-to-db> <package> ...\n")" - printf "$(gettext "Usage: repo-remove [-q] <path-to-db> <packagename> ...\n\n")" + printf "$(gettext "Usage: repo-add [-q] <path-to-db> ...\n")" + printf "$(gettext "Usage: repo-remove [-q] <path-to-db> ...\n\n")" printf "$(gettext "\ repo-add will update a package database by reading a package file.\n\ Multiple packages to add can be specified on the command line.\n\n")" @@ -107,6 +107,73 @@ find_pkgentry() return 1 } +# Get the package name from the delta filename +get_delta_pkgname() { + local tmp + + tmp=${1##*/} + echo ${tmp%-*-*_to*} +} + +# write a delta entry +# arg1 - path to delta file +db_write_delta() +{ + deltafile="$1" + pkgname="$(get_delta_pkgname $deltafile)" + + pkgentry=$(find_pkgentry $pkgname) + if [ -z "$pkgentry" ]; then + return 1 + fi + deltas="$pkgentry/deltas" + # create deltas file if it does not already exist + if [ ! -f "$deltas" ]; then + msg2 "$(gettext "Creating 'deltas' db entry...")" + echo -e "%DELTAS%" >>$deltas + fi + # get md5sum and compressed size of package + md5sum="$(openssl dgst -md5 "$deltafile" | awk '{print $NF}')" + csize=$(@SIZECMD@ "$deltafile") + + oldfile=$(xdelta3 printhdr $deltafile | grep "XDELTA filename (source)" | sed 's/.*: *//') + newfile=$(xdelta3 printhdr $deltafile | grep "XDELTA filename (output)" | sed 's/.*: *//') + + if grep -q "$oldfile.*$newfile" $deltas; then + warning "$(gettext "An entry for '%s' already existed")" "$deltafile" + sed -i.backup "/$oldfile.*$newfile/d" $deltas && rm -f $deltas.backup + msg2 "$(gettext "Removing existing entry '%s'...")" "$deltafile" + fi + echo ${deltafile##*/} $md5sum $csize $oldfile $newfile >> $deltas + + return 0 +} # end db_write_delta + +# remove a delta entry +# arg1 - path to delta file +db_remove_delta() +{ + deltafile="$1" + filename=${deltafile##*/} + pkgname="$(get_delta_pkgname $deltafile)" + + pkgentry=$(find_pkgentry $pkgname) + if [ -z "$pkgentry" ]; then + return 1 + fi + deltas="$pkgentry/deltas" + if [ ! -f "$deltas" ]; then + return 1 + fi + if grep -q "$filename" $deltas; then + sed -i.backup "/$filename/d" $deltas && rm -f $deltas.backup + msg2 "$(gettext "Removing existing entry '%s'...")" "$filename" + return 0 + fi + + return 1 +} # end db_remove_delta + # write an entry to the pacman database # arg1 - path to package db_write_entry() @@ -222,7 +289,7 @@ db_remove_entry() { if [ -f "$pkgentry/deltas" ]; then mv "$pkgentry/deltas" "$gstmpdir/$pkgname.deltas" fi - msg2 "$(gettext "Removing existing package '%s'...")" \ + msg2 "$(gettext "Removing existing entry '%s'...")" \ "$(basename $pkgentry)" rm -rf $pkgentry pkgentry=$(find_pkgentry $pkgname) @@ -259,12 +326,26 @@ check_repo_db() add() { - pkgfile=$1 if [ ! -f "$1" ]; then - error "$(gettext "Package '%s' not found.")" "$pkgfile" + error "$(gettext "File '%s' not found.")" "$1" return 1 fi + if [ "${1##*.}" == "delta" ]; then + deltafile=$1 + msg "$(gettext "Adding delta '%s'")" "$deltafile" + if [ ! "$(type -p xdelta3)" ]; then + error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")" + exit 1 + fi + if db_write_delta "$deltafile"; then + return 0 + else + return 1 + fi + fi + + pkgfile=$1 if ! bsdtar -tf "$pkgfile" .PKGINFO 2>&1 >/dev/null; then error "$(gettext "'%s' is not a package file, skipping")" "$pkgfile" return 1 @@ -277,6 +358,17 @@ add() remove() { + if [ "${1##*.}" == "delta" ]; then + deltafile=$1 + msg "$(gettext "Searching for delta '%s'...")" "$deltafile" + if db_remove_delta "$deltafile"; then + return 0 + else + error "$(gettext "Delta matching '%s' not found.")" "$deltafile" + return 1 + fi + fi + pkgname=$1 msg "$(gettext "Searching for package '%s'...")" "$pkgname" -- 1.6.1.3

Dan McGee

28 Feb 28 Feb

9:38 p.m.

New subject: [pacman-dev] [PATCH 3/6] repo-add cleanup

On Thu, Feb 26, 2009 at 2:06 PM, Xavier Chantry wrote:

...

Refactor the main loop, which was difficult to read.

Use case instead of if when appropriate.

Signed-off-by: Xavier Chantry

The idea is good, but make sure comments from previous patches make it into this reorg (formatting and the rm -f thing). -Dan

Allan McRae

27 Feb 27 Feb

12:56 a.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

Xavier Chantry wrote:

...

Before this commit, the repo creation could fail after all packages have been added to the database. Now this will be detected before adding anything.

Signed-off-by: Xavier Chantry

Patch looks good. Minor stylistic comments below

...

--- scripts/repo-add.sh.in | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index b12188c..a967506 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -326,9 +326,21 @@ for arg in "$@"; do fi msg "$(gettext "Extracting database to a temporary location...")" bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" - elif [ "$cmd" == "repo-remove" ]; then - error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" - exit 1 + else + case "$cmd" in + repo-remove)

indent the lines within this block some more:

...

+ error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" + exit 1 + ;; + repo-add)

and these

...

+ # check if the file can be created (write permission, directory existence, etc) + if ! touch "$REPO_DB_FILE"; then + error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" + exit 1 + fi + rm -f "$REPO_DB_FILE" + ;; + esac fi else if [ "$cmd" == "repo-add" ]; then

Allan

Xavier

12:44 p.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

On Fri, Feb 27, 2009 at 1:56 AM, Allan McRae wrote:

...

Xavier Chantry wrote:

...
Before this commit, the repo creation could fail after all packages have been added to the database. Now this will be detected before adding anything.

Signed-off-by: Xavier Chantry

Patch looks good. Minor stylistic comments below

...
--- scripts/repo-add.sh.in | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index b12188c..a967506 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -326,9 +326,21 @@ for arg in "$@"; do fi msg "$(gettext "Extracting database to a temporary location...")" bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" - elif [ "$cmd" == "repo-remove" ]; then - error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" - exit 1 + else + case "$cmd" in + repo-remove)

indent the lines within this block some more:

...
+ error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" + exit 1 + ;; + repo-add)

and these

...
+ # check if the file can be created (write permission, directory existence, etc) + if ! touch "$REPO_DB_FILE"; then + error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" + exit 1 + fi + rm -f "$REPO_DB_FILE" + ;; + esac fi else if [ "$cmd" == "repo-add" ]; then

Yeah, I don't know why vim indent the blocks inside case like that... It is quite annoying. Is there a way to write case blocks more vim friendly, or some options / tricks I could use?

Dan McGee

28 Feb 28 Feb

9:37 p.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

...

Before this commit, the repo creation could fail after all packages have been added to the database. Now this will be detected before adding anything.

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

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index b12188c..a967506 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -326,9 +326,21 @@ for arg in "$@"; do fi msg "$(gettext "Extracting database to a temporary location...")" bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" - elif [ "$cmd" == "repo-remove" ]; then - error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" - exit 1 + else + case "$cmd" in + repo-remove) + error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" + exit 1 + ;; + repo-add) + # check if the file can be created (write permission, directory existence, etc) + if ! touch "$REPO_DB_FILE"; then + error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" + exit 1 + fi + rm -f "$REPO_DB_FILE" This seems scary to me- is there any reason to blow it away like this? It also makes the repo-add process non-atomic- someone could access

On Thu, Feb 26, 2009 at 2:06 PM, Xavier Chantry wrote: the database and fail while you are adding 10 packages, correct?

...

+ ;; + esac fi else if [ "$cmd" == "repo-add" ]; then -- 1.6.1.3

Also agree with Allan's style comments, and I am not sure why vim indents like it does. -Dan

Allan McRae

1 Mar 1 Mar

5:57 a.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

Dan McGee wrote:

...

On Thu, Feb 26, 2009 at 2:06 PM, Xavier Chantry wrote:

...
Before this commit, the repo creation could fail after all packages have been added to the database. Now this will be detected before adding anything.

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

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index b12188c..a967506 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -326,9 +326,21 @@ for arg in "$@"; do fi msg "$(gettext "Extracting database to a temporary location...")" bsdtar -xf "$REPO_DB_FILE" -C "$gstmpdir" - elif [ "$cmd" == "repo-remove" ]; then - error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" - exit 1 + else + case "$cmd" in + repo-remove) + error "$(gettext "Repository file '%s' was not found.")" "$REPO_DB_FILE" + exit 1 + ;; + repo-add) + # check if the file can be created (write permission, directory existence, etc) + if ! touch "$REPO_DB_FILE"; then + error "$(gettext "Repository file '%s' could not be created.")" "$REPO_DB_FILE" + exit 1 + fi + rm -f "$REPO_DB_FILE"

This seems scary to me- is there any reason to blow it away like this? It also makes the repo-add process non-atomic- someone could access the database and fail while you are adding 10 packages, correct?

I thought about this too but I was less scared when I noticed that this is only done when the db file is not found. So you are only removing the db file created in the touch statement.

Xavier

10:49 a.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

On Sun, Mar 1, 2009 at 6:57 AM, Allan McRae wrote:

...

I thought about this too but I was less scared when I noticed that this is only done when the db file is not found. So you are only removing the db file created in the touch statement.

Indeed. However when the db file is found, I didn't even check it was also writable in that case. And we have a race condition huge like a mountain here. There can be several minutes spent between this write/creation check is made, and the actual write of the new database. It would probably be a good idea to write a lock file for each database ($REPO.lck) so that there is never more than one repo-add instance running for one given db. For these reasons, I will just drop that patch for now, for rewriting it later.

Xavier

11:33 a.m.

New subject: [pacman-dev] [PATCH 2/6] repo-add: fail early if repo can not be created

On Sat, Feb 28, 2009 at 10:37 PM, Dan McGee wrote:

...

Also agree with Allan's style comments, and I am not sure why vim indents like it does.

I guess vim considers case blocks just like any other blocks : " Add a 'shiftwidth' after if, while, else, case, until, for, function() So it indents all the lines inside a case/esac at the same level. And it apparently does not consider case statements as blocks. That indentation is made even worse by the fact that I didn't even put a newline between the different case statements, so it is even harder to see them. Anyway my updated patches have manual indentation now.

Dan McGee

28 Feb 28 Feb

9:33 p.m.

New subject: [pacman-dev] [PATCH 1/6] repo-add: print warning if same version already exists

On Thu, Feb 26, 2009 at 2:06 PM, Xavier Chantry wrote:

...

Simple fix for FS#13414

Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee

5535

Age (days ago)

5538

Last active (days ago)

List overview

Download

13 comments

4 participants

participants (4)

Allan McRae
Dan McGee
Xavier
Xavier Chantry