Re: [pacman-dev] [PATCH 1/4] Add delta information to the repository database.

I'll just go through one patch at a time and make some inline comments. Overall they look very good though. On 10/15/07, Nathan Jones wrote:

Signed-off-by: Nathan Jones --- scripts/repo-add.sh.in | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index cb741d7..67fa4f1 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -97,6 +97,34 @@ write_list_entry() { fi }

+# write a delta entry to the pacman database +# arg1 - path to delta +db_write_delta() +{ + # blank out all variables and set deltafile + local deltafile=$(readlink -f "$1") + local filename=$(basename "$deltafile") + local pkgname fromver tover arch csize md5sum + + pkgname="$(echo "$filename" | sed -e 's/\(.*\)-\(.*-.*\)_to_\(.*-.*\)-\(.*\).delta/\1/')" + fromver="$(echo "$filename" | sed -e 's/\(.*\)-\(.*-.*\)_to_\(.*-.*\)-\(.*\).delta/\2/')" + tover="$(echo "$filename" | sed -e 's/\(.*\)-\(.*-.*\)_to_\(.*-.*\)-\(.*\).delta/\3/')" + arch="$(echo "$filename" | sed -e 's/\(.*\)-\(.*-.*\)_to_\(.*-.*\)-\(.*\).delta/\4/')" + md5sum="$(md5sum "$deltafile" | cut -d ' ' -f 1)"

Is there any way to do it without so much sed-magic, or at least make it look slightly cleaner? If not, then we'll have to live with it. Can you add some comments above it explaining the regex in case someone else ever has to change it though?

+ + # get size of delta + csize=$(du -b -L "$deltafile" | cut -f 1) + + # ensure $pkgname and $pkgver variables were found + if [ -z "$pkgname" -o -z "$fromver" ]; 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() @@ -106,6 +134,7 @@ db_write_entry() local pkgname pkgver pkgdesc url builddate packager csize size \ group depend backup license replaces provides conflict \ _groups _depends _backups _licenses _replaces _provides _conflicts + local startdir

Might as well omit the 'local' and stick it in somewhere in the big list that makes sense.

local OLDIFS="$IFS" # IFS (field seperator) is only the newline character @@ -133,6 +162,7 @@ db_write_entry() # get compressed size of package csize=$(du -b -L "$pkgfile" | cut -f 1)

+ startdir=`pwd`

Please use the $() format instead of ``, we've removed all `` escapes throughout the pacman toolset.

pushd "$gstmpdir" 2>&1 >/dev/null

# ensure $pkgname and $pkgver variables were found @@ -183,8 +213,22 @@ db_write_entry() write_list_entry "CONFLICTS" "$_conflicts" "depends" write_list_entry "PROVIDES" "$_provides" "depends"

+ # create deltas entry + msg2 "$(gettext "Creating 'deltas' db entry...")" + echo -e "%DELTAS%" >>deltas + for delta in $startdir/$pkgname-*-*_to_*-*-$arch.delta; do + if [ -e "$delta" ]; then + if db_write_delta "$delta"; then + msg2 "$(gettext "Added delta '%s'")" "$(basename "$delta")" + else + msg2 "$(gettext "Could not add delta '%s'")" "$(basename "$delta")" + fi + fi + done + echo -e "" >>deltas + # preserve the modification time - touch -r "$pkgfile" desc depends + touch -r "$pkgfile" desc depends deltas

Will this create an empty deltas file if it doesn't exist? We probably don't want that (just db clutter).

popd 2>&1 >/dev/null } # end db_write_entry -- 1.5.3.4

-Dan