On Mon, Jun 20, 2011 at 2:46 PM, Dave Reisner <d@falconindy.com> wrote:

grep and sed aren't needed here, and this removes the truly ugly manipulation of IFS. The process substituion could just as well be a herestring, but it breaks vim's syntax highlighting. Style over substance, mang.

Signed-off-by: Dave Reisner <d@falconindy.com> Signed-off-by: Dan McGee <dan@archlinux.org>

---  scripts/repo-add.sh.in |   19 +++++--------------  1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 14506dc..3783348 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -230,19 +230,12 @@ db_write_entry()                _groups _licenses _replaces _depends _conflicts _provides _optdepends \                md5sum sha256sum pgpsig

-       local OLDIFS="$IFS" -       # IFS (field separator) is only the newline character -       IFS=" -" -        # read info from the zipped package        local line var val -       for line in $(bsdtar -xOqf "$pkgfile" .PKGINFO | -                       grep -v '^#' | sed 's|\(\w*\)\s*=\s*\(.*\)|\1 \2|'); do -               # bash awesomeness here- var is always one word, val is everything else -               var=${line%% *} -               val=${line#* } -               declare $var="$val" +       while read -r line; do +               [[ ${line:0:1} = '#' ]] && continue +               IFS=' =' read -r var val < <(printf '%s\n' "$line") +               declare "$var=$val"                case "$var" in                        group)    _groups="$_groups$group\n" ;;                        license)  _licenses="$_licenses$license\n" ;; @@ -252,9 +245,7 @@ db_write_entry()                        provides) _provides="$_provides$provides\n" ;;                        optdepend) _optdepends="$_optdepends$optdepend\n" ;;                esac -       done - -       IFS=$OLDIFS +       done< <(bsdtar -xOqf "$pkgfile" .PKGINFO)

       csize=$(@SIZECMD@ "$pkgfile")

-- 1.7.5.4

On Mon, Jun 20, 2011 at 2:46 PM, Dave Reisner <d@falconindy.com> wrote:

Fields like groups and depends should be stored as arrays. This requires rewriting our write_list_entry function to accomodate our new data type. This new function will not write to a file, but rather only format it.

Signed-off-by: Dave Reisner <d@falconindy.com> Signed-off-by: Dan McGee <dan@archlinux.org>

---  scripts/repo-add.sh.in |   46 +++++++++++++++++++++++++---------------------  1 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 3783348..fb1e24a 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -82,14 +82,16 @@ This is free software; see the source for copying conditions.\n\  There is NO WARRANTY, to the extent permitted by law.\n")"  }

-# write a list entry +# format a metadata entry  #              arg1 - Entry name -#              arg2 - List -#              arg3 - File to write to -write_list_entry() { -       if [[ -n $2 ]]; then -               echo "%$1%" >>$3 -               echo -e $2 >>$3 +#              ...  - value(s) +format_entry() { +       local field=$1; shift + +       if [[ $1 ]]; then +               printf '%%%s%%\n' "$field" +               printf '%s\n' "$@" +               printf '\n'        fi  }

@@ -237,13 +239,13 @@ db_write_entry()                IFS=' =' read -r var val < <(printf '%s\n' "$line")                declare "$var=$val"                case "$var" in -                       group)    _groups="$_groups$group\n" ;; -                       license)  _licenses="$_licenses$license\n" ;; -                       replaces) _replaces="$_replaces$replaces\n" ;; -                       depend)   _depends="$_depends$depend\n" ;; -                       conflict) _conflicts="$_conflicts$conflict\n" ;; -                       provides) _provides="$_provides$provides\n" ;; -                       optdepend) _optdepends="$_optdepends$optdepend\n" ;; +                       group)     _groups+=("$group") ;; +                       license)   _licenses+=("$license") ;; +                       replaces)  _replaces+=("$replaces") ;; +                       depend)    _depends+=("$depend") ;; +                       conflict)  _conflicts+=("$conflict") ;; +                       provides)  _provides+=("$provides") ;; +                       optdepend) _optdepends+=("$optdepend") ;;                esac        done< <(bsdtar -xOqf "$pkgfile" .PKGINFO)

@@ -297,7 +299,7 @@ db_write_entry()        [[ -n $pkgbase ]] && echo -e "%BASE%\n$pkgbase\n" >>desc        echo -e "%VERSION%\n$pkgver\n" >>desc        [[ -n $pkgdesc ]] && echo -e "%DESC%\n$pkgdesc\n" >>desc -       write_list_entry "GROUPS" "$_groups" "desc" +       format_entry "GROUPS" "${_groups[@]}" >>"desc"        [[ -n $csize ]] && echo -e "%CSIZE%\n$csize\n" >>desc        [[ -n $size ]] && echo -e "%ISIZE%\n$size\n" >>desc

@@ -309,20 +311,22 @@ db_write_entry()        [[ -n $pgpsig ]] && echo -e "%PGPSIG%\n$pgpsig\n" >>desc

       [[ -n $url ]] && echo -e "%URL%\n$url\n" >>desc -       write_list_entry "LICENSE" "$_licenses" "desc" +       format_entry "LICENSE" "${_licenses[@]}" >>"desc"        [[ -n $arch ]] && echo -e "%ARCH%\n$arch\n" >>desc        [[ -n $builddate ]] && echo -e "%BUILDDATE%\n$builddate\n" >>desc        [[ -n $packager ]] && echo -e "%PACKAGER%\n$packager\n" >>desc -       write_list_entry "REPLACES" "$_replaces" "desc" +       format_entry "REPLACES" "${_replaces[@]}" >>"desc"

       # create depends entry        msg2 "$(gettext "Creating '%s' db entry...")" 'depends'        # create the file even if it will remain empty        touch "depends" -       write_list_entry "DEPENDS" "$_depends" "depends" -       write_list_entry "CONFLICTS" "$_conflicts" "depends" -       write_list_entry "PROVIDES" "$_provides" "depends" -       write_list_entry "OPTDEPENDS" "$_optdepends" "depends" +       { +               format_entry "DEPENDS" "${_depends[@]}" +               format_entry "CONFLICTS" "${_conflicts[@]}" +               format_entry "PROVIDES" "${_provides[@]}" +               format_entry "OPTDEPENDS" "${_optdepends[@]}" +       } >>'depends'

       popd >/dev/null        popd >/dev/null -- 1.7.5.4

On Mon, Jun 20, 2011 at 03:46:34PM -0400, Dave Reisner wrote:

This ranks high on the code readability scale. The same function formats all of our data and writes to the metadata file at once. --- scripts/repo-add.sh.in | 52 ++++++++++++++++++++++++----------------------- 1 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index fb1e24a..174efa4 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -294,37 +294,39 @@ db_write_entry()

# create desc entry msg2 "$(gettext "Creating '%s' db entry...")" 'desc' - echo -e "%FILENAME%\n${1##*/}\n" >>desc - echo -e "%NAME%\n$pkgname\n" >>desc - [[ -n $pkgbase ]] && echo -e "%BASE%\n$pkgbase\n" >>desc - echo -e "%VERSION%\n$pkgver\n" >>desc - [[ -n $pkgdesc ]] && echo -e "%DESC%\n$pkgdesc\n" >>desc - format_entry "GROUPS" "${_groups[@]}" >>"desc" - [[ -n $csize ]] && echo -e "%CSIZE%\n$csize\n" >>desc - [[ -n $size ]] && echo -e "%ISIZE%\n$size\n" >>desc - - # add checksums - echo -e "%MD5SUM%\n$md5sum\n" >>desc - echo -e "%SHA256SUM%\n$sha256sum\n" >>desc - - # add PGP sig - [[ -n $pgpsig ]] && echo -e "%PGPSIG%\n$pgpsig\n" >>desc - - [[ -n $url ]] && echo -e "%URL%\n$url\n" >>desc - format_entry "LICENSE" "${_licenses[@]}" >>"desc" - [[ -n $arch ]] && echo -e "%ARCH%\n$arch\n" >>desc - [[ -n $builddate ]] && echo -e "%BUILDDATE%\n$builddate\n" >>desc - [[ -n $packager ]] && echo -e "%PACKAGER%\n$packager\n" >>desc - format_entry "REPLACES" "${_replaces[@]}" >>"desc" + { + format_entry "FILENAME" "${1##*/}" + format_entry "NAME" "$pkgname" + format_entry "BASE" "$pkgbase" + format_entry "VERSION" "$pkgver" + format_entry "DESC" "$pkgdesc" + format_entry "GROUPS" "${_groups[@]}" + format_entry "CSIZE" "$csize" + format_entry "ISIZE" "$size" + + # add checksums + format_entry "MD5SUM" "$md5sum" + format_entry "SHA256SUM" "$sha256sum" + + # add PGP sig + format_entry "PGPSIG" "$pgpsig" + + format_entry "URL" "$url" + format_entry "LICENSE" "${_licenses[@]}" + format_entry "ARCH" "$arch" + format_entry "BUILDDATE" "$builddate" + format_entry "PACKAGER" "$packager" + format_entry "REPLACES" "${_replaces[@]}" + } >>'desc'

# create depends entry msg2 "$(gettext "Creating '%s' db entry...")" 'depends' # create the file even if it will remain empty touch "depends" { - format_entry "DEPENDS" "${_depends[@]}" - format_entry "CONFLICTS" "${_conflicts[@]}" - format_entry "PROVIDES" "${_provides[@]}" + format_entry "DEPENDS" "${_depends[@]}" + format_entry "CONFLICTS" "${_conflicts[@]}" + format_entry "PROVIDES" "${_provides[@]}" format_entry "OPTDEPENDS" "${_optdepends[@]}"

ewww whitespace error... fixed in git.

} >>'depends'

-- 1.7.5.4