[pacman-dev] [PATCH 0/3] repo-add desc/depends creation cleanup

Dave Reisner

20 Jun 2011 20 Jun '11

7:46 p.m.

This is basically what I referred to in my last patch about refactoring desc/depends writing. I snuck in an extra winner (the first) to get rid of the insane IFS tweaking and iteration of the .PKGINFO file with a for loop. Dan, I'm not sure if I can break this up any further, but I'm happy to refactor if it would make you more comfortable. I know it's an insanely important code path so I encourage you (and anyone else reading) to put on your nerd glasses and nitpick this. Dave Reisner (3): repo-add: bashify reading of .PKGINFO file repo-add: store multi-value fields as arrays repo-add: use format_entry for all desc/depends fields scripts/repo-add.sh.in | 105 +++++++++++++++++++++++------------------------ 1 files changed, 51 insertions(+), 54 deletions(-) -- 1.7.5.4

Show replies by date

Dave Reisner

20 Jun 20 Jun

7:46 p.m.

New subject: [pacman-dev] [PATCH 1/3] repo-add: bashify reading of .PKGINFO file

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 --- 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

Dan McGee

21 Jun 21 Jun

5:21 p.m.

New subject: [pacman-dev] [PATCH 1/3] repo-add: bashify reading of .PKGINFO file

On Mon, Jun 20, 2011 at 2:46 PM, Dave Reisner 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 Signed-off-by: Dan McGee

...

--- 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

Dave Reisner

20 Jun 20 Jun

7:46 p.m.

New subject: [pacman-dev] [PATCH 2/3] repo-add: store multi-value fields as arrays

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 --- 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

Dan McGee

21 Jun 21 Jun

5:21 p.m.

New subject: [pacman-dev] [PATCH 2/3] repo-add: store multi-value fields as arrays

On Mon, Jun 20, 2011 at 2:46 PM, Dave Reisner 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 Signed-off-by: Dan McGee

...

--- 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

Dave Reisner

20 Jun 20 Jun

7:46 p.m.

New subject: [pacman-dev] [PATCH 3/3] repo-add: use format_entry for all desc/depends fields

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[@]}" } >>'depends' -- 1.7.5.4

Dave Reisner

8:22 p.m.

New subject: [pacman-dev] [PATCH 3/3] repo-add: use format_entry for all desc/depends fields

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

Dan McGee

21 Jun 21 Jun

5:19 p.m.

New subject: [pacman-dev] [PATCH 3/3] repo-add: use format_entry for all desc/depends fields

On Mon, Jun 20, 2011 at 3:22 PM, Dave Reisner wrote:

...

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'

This gets a signoff from me otherwise. -Dan

Allan McRae

22 Jun 22 Jun

8:05 a.m.

On 21/06/11 05:46, Dave Reisner wrote:

...

This is basically what I referred to in my last patch about refactoring desc/depends writing. I snuck in an extra winner (the first) to get rid of the insane IFS tweaking and iteration of the .PKGINFO file with a for loop.

Dan, I'm not sure if I can break this up any further, but I'm happy to refactor if it would make you more comfortable. I know it's an insanely important code path so I encourage you (and anyone else reading) to put on your nerd glasses and nitpick this.

Dave Reisner (3): repo-add: bashify reading of .PKGINFO file repo-add: store multi-value fields as arrays repo-add: use format_entry for all desc/depends fields

scripts/repo-add.sh.in | 105 +++++++++++++++++++++++------------------------ 1 files changed, 51 insertions(+), 54 deletions(-)

No ack. Something here is entirely screwed under bash-3.2. We still have people using pacman with that version of bash. I think it does not output any of these fields... -%CONFLICTS% -%DEPENDS% -%GROUPS% -%LICENSE% -%OPTDEPENDS% -%PROVIDES% -%REPLACES% That looks like anything in an array... To test, I created a repo with all packages in Arch starting with "a". Under bash-4.2, there was only a small difference in the output: e.g. asymptote-2.11-1 %OPTDEPENDS% -python2: for the xasy GUI -python-imaging: for the xasy GUI -tix: for the xasy GUI +python2: for the xasy GUI +python-imaging: for the xasy GUI +tix: for the xasy GUI This makes the formatting the same as that used in the PKGBUILD. Not sure I like that either... Allan

Dave Reisner

1 p.m.

On Wed, Jun 22, 2011 at 06:05:06PM +1000, Allan McRae wrote:

...

On 21/06/11 05:46, Dave Reisner wrote:

...
This is basically what I referred to in my last patch about refactoring desc/depends writing. I snuck in an extra winner (the first) to get rid of the insane IFS tweaking and iteration of the .PKGINFO file with a for loop.

Dan, I'm not sure if I can break this up any further, but I'm happy to refactor if it would make you more comfortable. I know it's an insanely important code path so I encourage you (and anyone else reading) to put on your nerd glasses and nitpick this.

Dave Reisner (3): repo-add: bashify reading of .PKGINFO file repo-add: store multi-value fields as arrays repo-add: use format_entry for all desc/depends fields

scripts/repo-add.sh.in | 105 +++++++++++++++++++++++------------------------ 1 files changed, 51 insertions(+), 54 deletions(-)

No ack.

Something here is entirely screwed under bash-3.2. We still have people using pacman with that version of bash. I think it does not output any of these fields...

-%CONFLICTS% -%DEPENDS% -%GROUPS% -%LICENSE% -%OPTDEPENDS% -%PROVIDES% -%REPLACES%

That looks like anything in an array...

Good catch. I had rebased this at some point and somewhere along the way, the arrays were no longer declared as arrays. fixed locally.

...

To test, I created a repo with all packages in Arch starting with "a". Under bash-4.2, there was only a small difference in the output:

e.g. asymptote-2.11-1

%OPTDEPENDS% -python2: for the xasy GUI -python-imaging: for the xasy GUI -tix: for the xasy GUI +python2: for the xasy GUI +python-imaging: for the xasy GUI +tix: for the xasy GUI

This makes the formatting the same as that used in the PKGBUILD. Not sure I like that either...

Allan

I don't like this easier. It looks like I underestimated what the sed expression did here -- we can normalize whitespace another way though. Thanks for the test case. d

4714

Age (days ago)

4716

Last active (days ago)

List overview

Download

9 comments

3 participants

participants (3)

Allan McRae
Dan McGee
Dave Reisner

[pacman-dev] [PATCH 0/3] repo-add desc/depends creation cleanup

tags

participants (3)