On Fri, Sep 30, 2011 at 01:14:02AM +0800, lolilolicon wrote:

Simply `eval'ing configurations from pacman.conf is really bad practice. Use proper string parsing instead. The parser implemented here aborts as soon as it gets the first assignment, being consistent with pacman.

I'm not really a fan of using awk, here, either -- totally unreadable. We've been bitten by numerous portability issues with awk in the past. I recommend testing against mawk and nawk if you're ever going to use awk in our codebase. Also to note bad practice: embedding shell variables in the awk expressions. You should be defining the variable within the program with the -v flag to awk. See how we do this in scripts/pacman-key in pure bash (func is get_from() -- yes, its a terrible name). Given how frequently we seem to do this, it might be a candidate for pulling out and maintaining a single implementation in the scripts/library dir. d

Signed-off-by: lolilolicon --- contrib/bacman.in | 14 +++++++++++++- contrib/pacscripts.in | 16 ++++++++++++++-- scripts/pacman-db-upgrade.sh.in | 14 +++++++++++++- scripts/pacman-optimize.sh.in | 14 +++++++++++++- 4 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/contrib/bacman.in b/contrib/bacman.in index b51a4a3..0069d1e 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -75,7 +75,19 @@ if [ ! -r @sysconfdir@/pacman.conf ]; then exit 1 fi

-eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf) +# parse value of simple, non-repeating variable assignment +conf_var_val() { + local var=${1//\//\\/} + awk ' + /^[ \t]*'"${var}"'[ \t]*=/ { + sub(/[^=]+=[ \t]*/, "") + sub(/[ \t]*$/, "") + print + exit + }' +} + +DBPath=$(conf_var_val DBPath < @sysconfdir@/pacman.conf) pac_db="${DBPath:-@localstatedir@/lib/pacman/}/local"

if [ ! -r @sysconfdir@/makepkg.conf ]; then diff --git a/contrib/pacscripts.in b/contrib/pacscripts.in index 37d3fea..0ac64a2 100755 --- a/contrib/pacscripts.in +++ b/contrib/pacscripts.in @@ -34,8 +34,20 @@ if [ ! -r "$conf" ]; then exit 1 fi

-eval $(awk '/DBPath/ {print $1$2$3}' "$conf") -eval $(awk '/CacheDir/ {print $1$2$3}' "$conf") +# parse value of simple, non-repeating variable assignment +conf_var_val() { + local var=${1//\//\\/} + awk ' + /^[ \t]*'"${var}"'[ \t]*=/ { + sub(/[^=]+=[ \t]*/, "") + sub(/[ \t]*$/, "") + print + exit + }' +} + +DBPath=$(conf_var_val DBPath < "$conf") +CacheDir=$(conf_var_val CacheDir < "$conf") pac_db="${DBPath:-@localstatedir@/lib/pacman}/local" pac_cache="${CacheDir:-@localstatedir@/cache/pacman/pkg}"

diff --git a/scripts/pacman-db-upgrade.sh.in b/scripts/pacman-db-upgrade.sh.in index 3e0d702..eb427ed 100644 --- a/scripts/pacman-db-upgrade.sh.in +++ b/scripts/pacman-db-upgrade.sh.in @@ -25,7 +25,19 @@ export TEXTDOMAINDIR='@localedir@'

myver='@PACKAGE_VERSION@'

-eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf) +# parse value of simple, non-repeating variable assignment +conf_var_val() { + local var=${1//\//\\/} + awk ' + /^[ \t]*'"${var}"'[ \t]*=/ { + sub(/[^=]+=[ \t]*/, "") + sub(/[ \t]*$/, "") + print + exit + }' +} + +DBPath=$(conf_var_val DBPath < @sysconfdir@/pacman.conf) dbroot="${DBPath:-@localstatedir@/lib/pacman/}"

m4_include(library/output_format.sh) diff --git a/scripts/pacman-optimize.sh.in b/scripts/pacman-optimize.sh.in index 5ff302e..8e2a8bb 100644 --- a/scripts/pacman-optimize.sh.in +++ b/scripts/pacman-optimize.sh.in @@ -26,7 +26,19 @@ export TEXTDOMAINDIR='@localedir@'

myver='@PACKAGE_VERSION@'

-eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf) +# parse value of simple, non-repeating variable assignment +conf_var_val() { + local var=${1//\//\\/} + awk ' + /^[ \t]*'"${var}"'[ \t]*=/ { + sub(/[^=]+=[ \t]*/, "") + sub(/[ \t]*$/, "") + print + exit + }' +} + +DBPath=$(conf_var_val DBPath < @sysconfdir@/pacman.conf) dbroot="${DBPath:-@localstatedir@/lib/pacman/}"

m4_include(library/output_format.sh) -- 1.7.6.4

On Fri, Sep 30, 2011 at 1:27 AM, Dave Reisner wrote:

On Fri, Sep 30, 2011 at 01:14:03AM +0800, lolilolicon wrote:

...

Signed-off-by: lolilolicon ---  contrib/bacman.in |   10 +++++-----  1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/bacman.in b/contrib/bacman.in index 0069d1e..d43bf78 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -58,7 +58,7 @@ if [ $EUID -gt 0 ]; then      if [ -f /usr/bin/fakeroot ]; then          echo "Entering fakeroot environment"          export INFAKEROOT="1" -        /usr/bin/fakeroot -u -- $0 $1 +        /usr/bin/fakeroot -u -- "$0" "$@"          exit $?      else          echo "WARNING: installing fakeroot or running ${progname} as root is required to" @@ -128,7 +128,7 @@ fi  # Begin  #  pkg_namver="${pkg_dir##*/}" -echo Package: ${pkg_namver} +echo "Package: ${pkg_namver}"  work_dir=$(mktemp -d -p /tmp)  cd "$work_dir" || exit 1

@@ -148,7 +148,7 @@ while read i; do          continue      fi

-    case $current in +    case "$current" in

Expansion is never performed here.

Good catch... but nevertheless just let me quote it :P

...

         %FILES%)              ret=0              if [ -e "/$i" ]; then @@ -266,7 +266,7 @@ if [ -f "$pkg_dir/install" ] ; then      cp "$pkg_dir/install" "$work_dir/.INSTALL"      comp_files+=" .INSTALL"  fi -if  [ -f $pkg_dir/changelog ] ; then +if  [ -f "$pkg_dir/changelog" ] ; then

If we took the time to actually clean up bacman to use proper bash, this wouldn't be necessary. I'd almost rather see that done than to perpetuate the POSIX syntax in what's clearly a bash program.

Yeah, bacman is full of [ "i_like_quotes" ]. I thought about it, then I decided if we want to bashify it, we better do it all in one go... maybe.

...

     cp "$pkg_dir/changelog" "$work_dir/.CHANGELOG"      comp_files+=" .CHANGELOG"  fi @@ -302,7 +302,7 @@ case "$PKGEXT" in      *tar.bz2) bzip2 -c -f ;;      *tar.xz)  xz -c -z - ;;      *tar)     cat ;; -esac > ${pkg_file} || ret=$? +esac > "${pkg_file}"; ret=$?

 if [ $ret -ne 0 ]; then      echo "ERROR: unable to write package to $pkg_dest" -- 1.7.6.4

On Thu, Sep 29, 2011 at 12:46 PM, Dave Reisner wrote:

On Fri, Sep 30, 2011 at 01:14:04AM +0800, lolilolicon wrote:

...

The original regex is also not accurate (should be ^[A-Z]+$).

Signed-off-by: lolilolicon ---  contrib/bacman.in |    4 ++--  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/bacman.in b/contrib/bacman.in index d43bf78..c69ab6f 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -143,7 +143,7 @@ while read i; do          continue      fi

-    if [[ "$i" =~ %[A-Z]*% ]] ; then +    if [[ "$i" == %+([A-Z])% ]] ; then

nak. This is affected by locale. If we're going to fix it, fix it correctly and use [[:upper:]]. See fun commits like 541c2470b8ad.

You sure about this? We're trying to match %NAME%, %VERSION%, %FILES%, etc. here, which we know will be ASCII 7-bit characters in all cases. Also note that the only purpose of this regex/glob/whatever is to short-circuit the case statement, so it isn't that big of deal if we have false positives. We just can't have false negatives. -Dan

On Fri, Sep 30, 2011 at 01:14:05AM +0800, lolilolicon wrote:

Doing so is certainly better, but we really should consider reusing write_pkginfo() from makepkg. Splitting common functions from makepkg into a library would be nice.

So let's do that instead. There's similar work in repo-add. One way or another, this should be using printf rather than echo.

Signed-off-by: lolilolicon --- contrib/bacman.in | 34 +++++++++++++++++----------------- 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/contrib/bacman.in b/contrib/bacman.in index c69ab6f..a28f8e7 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -207,57 +207,57 @@ while read i; do case "$current" in # desc %NAME%) - echo "pkgname = $i" >> .PKGINFO + echo "pkgname = $i" ;; %VERSION%) - echo "pkgver = $i" >> .PKGINFO + echo "pkgver = $i" ;; %DESC%) - echo "pkgdesc = $i" >> .PKGINFO + echo "pkgdesc = $i" ;; %URL%) - echo "url = $i" >> .PKGINFO + echo "url = $i" ;; %LICENSE%) - echo "license = $i" >> .PKGINFO + echo "license = $i" ;; %ARCH%) - echo "arch = $i" >> .PKGINFO + echo "arch = $i" ;; %BUILDDATE%) - echo "builddate = $(date -u "+%s")" >> .PKGINFO + echo "builddate = $(date -u "+%s")" ;; %PACKAGER%) - echo "packager = $pkg_pkger" >> .PKGINFO + echo "packager = $pkg_pkger" ;; %SIZE%) - echo "size = $pkg_size" >> .PKGINFO + echo "size = $pkg_size" ;; %GROUPS%) - echo "group = $i" >> .PKGINFO + echo "group = $i" ;; %REPLACES%) - echo "replaces = $i" >> .PKGINFO + echo "replaces = $i" ;; %DEPENDS%) - echo "depend = $i" >> .PKGINFO + echo "depend = $i" ;; %OPTDEPENDS%) - echo "optdepend = $i" >> .PKGINFO + echo "optdepend = $i" ;; %CONFLICTS%) - echo "conflict = $i" >> .PKGINFO + echo "conflict = $i" ;; %PROVIDES%) - echo "provides = $i" >> .PKGINFO + echo "provides = $i" ;;

# files %BACKUP%) # strip the md5sum after the tab - echo "backup = ${i%%$'\t'*}" >> .PKGINFO + echo "backup = ${i%%$'\t'*}" ;; - esac + esac >> .PKGINFO done

comp_files=".PKGINFO" -- 1.7.6.4

Another style change. The [[ expression ]] form is particularly cleaner, safer and more powerful than the [ expression ] form. Signed-off-by: lolilolicon --- contrib/bacman.in | 44 ++++++++++++++++++++++---------------------- 1 files changed, 22 insertions(+), 22 deletions(-) diff --git a/contrib/bacman.in b/contrib/bacman.in index cb1aa10..93145e4 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -26,23 +26,23 @@ readonly progver="0.2.1" # # User Friendliness # -function usage(){ +usage() { echo "This program recreates a package using pacman's db and system files" echo "Usage: $progname <installed package name>" echo "Example: $progname kernel26" } -if [ $# -ne 1 ] ; then +if (( $# != 1 )); then usage exit 1 fi -if [ "$1" = "--help" -o "$1" = "-h" ] ; then +if [[ $1 == "--help" || $1 == "-h" ]]; then usage exit 0 fi -if [ "$1" = "--version" -o "$1" = "-v" ]; then +if [[ $1 == "--version" || $1 == "-v" ]]; then echo "$progname version $progver" echo "Copyright (C) 2008 locci" exit 0 @@ -51,11 +51,11 @@ fi # # Fakeroot support # -if [ $EUID -gt 0 ]; then - if [ -f /usr/bin/fakeroot ]; then +if (( EUID )); then + if [[ -f /usr/bin/fakeroot ]]; then echo "Entering fakeroot environment" export INFAKEROOT="1" - /usr/bin/fakeroot -u -- $0 $1 + /usr/bin/fakeroot -u -- "$0" "$@" exit $? else echo "WARNING: installing fakeroot or running ${progname} as root is required to" @@ -67,7 +67,7 @@ fi # # Setting environmental variables # -if [ ! -r @sysconfdir@/pacman.conf ]; then +if [[ ! -r @sysconfdir@/pacman.conf ]]; then echo "ERROR: unable to read @sysconfdir@/pacman.conf" exit 1 fi @@ -75,13 +75,13 @@ fi eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf) pac_db="${DBPath:-@localstatedir@/lib/pacman/}/local" -if [ ! -r @sysconfdir@/makepkg.conf ]; then +if [[ ! -r @sysconfdir@/makepkg.conf ]]; then echo "ERROR: unable to read @sysconfdir@/makepkg.conf" exit 1 fi source "@sysconfdir@/makepkg.conf" -if [ -r ~/.makepkg.conf ]; then +if [[ -r ~/.makepkg.conf ]]; then source ~/.makepkg.conf fi @@ -96,12 +96,12 @@ pkg_namver="${pkg_dir##*/}" # # Checks everything is in place # -if [ ! -d "$pac_db" ] ; then +if [[ ! -d $pac_db ]]; then echo "ERROR: pacman database directory ${pac_db} not found" exit 1 fi -if [ ! -d "$pkg_dir" ] ; then +if [[ ! -d $pkg_dir ]]; then echo "ERROR: package ${pkg_name} not found in pacman database" exit 1 fi @@ -120,11 +120,11 @@ echo "Copying package files..." cat "$pkg_dir"/files | while read i; do - if [ -z "$i" ] ; then + if [[ -z $i ]]; then continue fi - if [[ "$i" =~ %[A-Z]*% ]] ; then + if [[ "$i" =~ %[A-Z]*% ]]; then current=$i continue fi @@ -132,11 +132,11 @@ while read i; do case $current in %FILES%) ret=0 - if [ -e "/$i" ]; then + if [[ -e /$i ]]; then bsdtar -cnf - "/$i" 2> /dev/null | bsdtar -xpf - # Workaround to bsdtar not reporting a missing file as an error - if [ ! -e "$work_dir/$i" -a ! -L "$work_dir/$i" ]; then + if ! [[ -e $work_dir/$i || -L $work_dir/$i ]]; then echo "" echo "ERROR: unable to add /$i to the package" echo " If your user does not have permssion to read this file then" @@ -154,7 +154,7 @@ while read i; do done ret=$? -if [ $ret -ne 0 ]; then +if (( ret )); then rm -rf "$work_dir" exit 1 fi @@ -166,7 +166,7 @@ pkg_size=$(du -sk | awk '{print $1 * 1024}') # echo Generating .PKGINFO metadata... echo "# Generated by $progname $progver" > .PKGINFO -if [ "$INFAKEROOT" = "1" ]; then +if [[ $INFAKEROOT == "1" ]]; then echo "# Using $(fakeroot -v)" >> .PKGINFO fi echo "# $(LC_ALL=C date)" >> .PKGINFO @@ -178,7 +178,7 @@ while read i; do continue; fi - if [[ "$i" =~ %[A-Z]*% ]] ; then + if [[ "$i" =~ %[A-Z]*% ]]; then current=$i continue fi @@ -241,11 +241,11 @@ done comp_files=".PKGINFO" -if [ -f "$pkg_dir/install" ] ; then +if [[ -f $pkg_dir/install ]]; then cp "$pkg_dir/install" "$work_dir/.INSTALL" comp_files+=" .INSTALL" fi -if [ -f $pkg_dir/changelog ] ; then +if [[ -f $pkg_dir/changelog ]]; then cp "$pkg_dir/changelog" "$work_dir/.CHANGELOG" comp_files+=" .CHANGELOG" fi @@ -286,7 +286,7 @@ case "$PKGEXT" in *tar) cat ;; esac > ${pkg_file} || ret=$? -if [ $ret -ne 0 ]; then +if (( ret )); then echo "ERROR: unable to write package to $pkg_dest" echo " Maybe the disk is full or you do not have write access" rm -rf "$work_dir" -- 1.7.6.4

On Sat, Oct 1, 2011 at 5:12 AM, lolilolicon wrote:

This includes:  - Quoting fixes.  - Drop deprecated mktemp option -p.  - Set extglob nullglob shell options at the top.  - Use extended globbing instead of regex to match %HEADER% in pacman db.

Signed-off-by: lolilolicon ---  contrib/bacman.in |   21 +++++++++++----------  1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/contrib/bacman.in b/contrib/bacman.in index 93145e4..63bd511 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -20,6 +20,9 @@  #   along with this program.  If not, see http://www.gnu.org/licenses/.  #

+shopt -s extglob +shopt -s nullglob +  readonly progname="bacman"  readonly progver="0.2.1"

@@ -109,8 +112,8 @@ fi  #  # Begin  # -echo Package: ${pkg_namver} -work_dir=$(mktemp -d -p /tmp) +echo "Package: ${pkg_namver}" +work_dir=$(mktemp -d /tmp/bacman-XXXXXXXXXX) We shouldn't be doing this like this: see http://projects.archlinux.org/namcap.git/commit/?id=b56ca639858f0a3cdeb418c9...

 cd "$work_dir" || exit 1

 # @@ -124,12 +127,12 @@ while read i; do                continue        fi

-       if [[ "$i" =~ %[A-Z]*% ]]; then +       if [[ "$i" == %+([A-Z])% ]]; then                current=$i                continue        fi

-       case $current in +       case "$current" in                %FILES%)                        ret=0                        if [[ -e /$i ]]; then @@ -163,6 +166,7 @@ pkg_size=$(du -sk | awk '{print $1 * 1024}')

 #  # .PKGINFO stuff +# TODO adopt makepkg's write_pkginfo() into this or scripts/library  #  echo Generating .PKGINFO metadata...  echo "# Generated by $progname $progver"    > .PKGINFO @@ -174,11 +178,11 @@ echo "#"                    >> .PKGINFO

 cat "$pkg_dir"/{desc,files} |  while read i; do -       if [[ -z "$i" ]]; then +       if [[ -z $i ]]; then                continue;        fi

-       if [[ "$i" =~ %[A-Z]*% ]]; then +       if [[ "$i" == %+([A-Z])% ]]; then                current=$i                continue        fi @@ -273,9 +277,6 @@ esac  pkg_file="$pkg_dest/$pkg_namver-$pkg_arch${PKGEXT}"  ret=0

-# when fileglobbing, we want * in an empty directory to expand to -# the null string rather than itself -shopt -s nullglob  # TODO: Maybe this can be set globally for robustness  shopt -s -o pipefail  bsdtar -cf - $comp_files * | @@ -284,7 +285,7 @@ case "$PKGEXT" in        *tar.bz2) bzip2 -c -f ;;        *tar.xz)  xz -c -z - ;;        *tar)     cat ;; -esac > ${pkg_file} || ret=$? +esac > "${pkg_file}"; ret=$?

 if (( ret )); then        echo "ERROR: unable to write package to $pkg_dest" -- 1.7.6.4

bacman should support whatever makepkg does as PKGEXT. Also remove obsolete $EXT variable. Signed-off-by: lolilolicon --- contrib/bacman.in | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-) diff --git a/contrib/bacman.in b/contrib/bacman.in index 63bd511..1bb7dc6 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -265,15 +265,6 @@ chmod 644 "$work_dir"/{.PKGINFO,.CHANGELOG,.INSTALL} 2> /dev/null # echo "Generating the package..." -case "$PKGEXT" in - *tar.gz) EXT=${PKGEXT%.gz} ;; - *tar.bz2) EXT=${PKGEXT%.bz2} ;; - *tar.xz) EXT=${PKGEXT%.xz} ;; - *tar) EXT=${PKGEXT} ;; - *) echo "WARNING: '%s' is not a valid archive extension." \ - "$PKGEXT" ; EXT=$PKGEXT ;; -esac - pkg_file="$pkg_dest/$pkg_namver-$pkg_arch${PKGEXT}" ret=0 @@ -284,7 +275,10 @@ case "$PKGEXT" in *tar.gz) gzip -c -f -n ;; *tar.bz2) bzip2 -c -f ;; *tar.xz) xz -c -z - ;; + *tar.Z) compress -c -f ;; *tar) cat ;; + *) echo "WARNING: '%s' is not a valid archive extension." \ + "$PKGEXT" >&2; cat ;; esac > "${pkg_file}"; ret=$? if (( ret )); then -- 1.7.6.4

As every piece of code in the whole project uses TAB as indentation character, bacman shouldn't be an exception. Signed-off-by: lolilolicon --- contrib/bacman.in | 278 ++++++++++++++++++++++++++--------------------------- 1 files changed, 138 insertions(+), 140 deletions(-) diff --git a/contrib/bacman.in b/contrib/bacman.in index fe13e5b..cb1aa10 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -27,62 +27,62 @@ readonly progver="0.2.1" # User Friendliness # function usage(){ - echo "This program recreates a package using pacman's db and system files" - echo "Usage: $progname <installed package name>" - echo "Example: $progname kernel26" + echo "This program recreates a package using pacman's db and system files" + echo "Usage: $progname <installed package name>" + echo "Example: $progname kernel26" } if [ $# -ne 1 ] ; then - usage - exit 1 + usage + exit 1 fi if [ "$1" = "--help" -o "$1" = "-h" ] ; then - usage - exit 0 + usage + exit 0 fi if [ "$1" = "--version" -o "$1" = "-v" ]; then - echo "$progname version $progver" - echo "Copyright (C) 2008 locci" - exit 0 + echo "$progname version $progver" + echo "Copyright (C) 2008 locci" + exit 0 fi # # Fakeroot support # if [ $EUID -gt 0 ]; then - if [ -f /usr/bin/fakeroot ]; then - echo "Entering fakeroot environment" - export INFAKEROOT="1" - /usr/bin/fakeroot -u -- $0 $1 - exit $? - else - echo "WARNING: installing fakeroot or running ${progname} as root is required to" - echo " preserve the ownership permissions of files in some packages" - echo "" - fi + if [ -f /usr/bin/fakeroot ]; then + echo "Entering fakeroot environment" + export INFAKEROOT="1" + /usr/bin/fakeroot -u -- $0 $1 + exit $? + else + echo "WARNING: installing fakeroot or running ${progname} as root is required to" + echo " preserve the ownership permissions of files in some packages" + echo "" + fi fi # # Setting environmental variables # if [ ! -r @sysconfdir@/pacman.conf ]; then - echo "ERROR: unable to read @sysconfdir@/pacman.conf" - exit 1 + echo "ERROR: unable to read @sysconfdir@/pacman.conf" + exit 1 fi eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf) pac_db="${DBPath:-@localstatedir@/lib/pacman/}/local" if [ ! -r @sysconfdir@/makepkg.conf ]; then - echo "ERROR: unable to read @sysconfdir@/makepkg.conf" - exit 1 + echo "ERROR: unable to read @sysconfdir@/makepkg.conf" + exit 1 fi source "@sysconfdir@/makepkg.conf" if [ -r ~/.makepkg.conf ]; then - source ~/.makepkg.conf + source ~/.makepkg.conf fi pkg_arch=${CARCH:-'unknown'} @@ -97,13 +97,13 @@ pkg_namver="${pkg_dir##*/}" # Checks everything is in place # if [ ! -d "$pac_db" ] ; then - echo "ERROR: pacman database directory ${pac_db} not found" - exit 1 + echo "ERROR: pacman database directory ${pac_db} not found" + exit 1 fi if [ ! -d "$pkg_dir" ] ; then - echo "ERROR: package ${pkg_name} not found in pacman database" - exit 1 + echo "ERROR: package ${pkg_name} not found in pacman database" + exit 1 fi # @@ -120,45 +120,43 @@ echo "Copying package files..." cat "$pkg_dir"/files | while read i; do - if [ -z "$i" ] ; then - continue - fi - - if [[ "$i" =~ %[A-Z]*% ]] ; then - current=$i - continue - fi - - case $current in - %FILES%) - ret=0 - if [ -e "/$i" ]; then - bsdtar -cnf - "/$i" 2> /dev/null | bsdtar -xpf - - - # Workaround to bsdtar not reporting a missing file as an error - if [ ! -e "$work_dir/$i" -a ! -L "$work_dir/$i" ]; then - echo "" - echo "ERROR: unable to add /$i to the package" - echo " If your user does not have permssion to read this file then" - echo " you will need to run $progname as root" - rm -rf "$work_dir" - exit 1 - fi - else - echo "" - echo "WARNING: package file /$i is missing" - echo "" - fi - - - ;; - esac + if [ -z "$i" ] ; then + continue + fi + + if [[ "$i" =~ %[A-Z]*% ]] ; then + current=$i + continue + fi + + case $current in + %FILES%) + ret=0 + if [ -e "/$i" ]; then + bsdtar -cnf - "/$i" 2> /dev/null | bsdtar -xpf - + + # Workaround to bsdtar not reporting a missing file as an error + if [ ! -e "$work_dir/$i" -a ! -L "$work_dir/$i" ]; then + echo "" + echo "ERROR: unable to add /$i to the package" + echo " If your user does not have permssion to read this file then" + echo " you will need to run $progname as root" + rm -rf "$work_dir" + exit 1 + fi + else + echo "" + echo "WARNING: package file /$i is missing" + echo "" + fi + ;; + esac done ret=$? if [ $ret -ne 0 ]; then - rm -rf "$work_dir" - exit 1 + rm -rf "$work_dir" + exit 1 fi pkg_size=$(du -sk | awk '{print $1 * 1024}') @@ -169,87 +167,87 @@ pkg_size=$(du -sk | awk '{print $1 * 1024}') echo Generating .PKGINFO metadata... echo "# Generated by $progname $progver" > .PKGINFO if [ "$INFAKEROOT" = "1" ]; then - echo "# Using $(fakeroot -v)" >> .PKGINFO + echo "# Using $(fakeroot -v)" >> .PKGINFO fi echo "# $(LC_ALL=C date)" >> .PKGINFO echo "#" >> .PKGINFO cat "$pkg_dir"/{desc,files} | while read i; do - if [[ -z "$i" ]]; then - continue; - fi - - if [[ "$i" =~ %[A-Z]*% ]] ; then - current=$i - continue - fi - - case "$current" in - # desc - %NAME%) - echo "pkgname = $i" >> .PKGINFO - ;; - %VERSION%) - echo "pkgver = $i" >> .PKGINFO - ;; - %DESC%) - echo "pkgdesc = $i" >> .PKGINFO - ;; - %URL%) - echo "url = $i" >> .PKGINFO - ;; - %LICENSE%) - echo "license = $i" >> .PKGINFO - ;; - %ARCH%) - echo "arch = $i" >> .PKGINFO - ;; - %BUILDDATE%) - echo "builddate = $(date -u "+%s")" >> .PKGINFO - ;; - %PACKAGER%) - echo "packager = $pkg_pkger" >> .PKGINFO - ;; - %SIZE%) - echo "size = $pkg_size" >> .PKGINFO - ;; - %GROUPS%) - echo "group = $i" >> .PKGINFO - ;; - %REPLACES%) - echo "replaces = $i" >> .PKGINFO - ;; - %DEPENDS%) - echo "depend = $i" >> .PKGINFO - ;; - %OPTDEPENDS%) - echo "optdepend = $i" >> .PKGINFO - ;; - %CONFLICTS%) - echo "conflict = $i" >> .PKGINFO - ;; - %PROVIDES%) - echo "provides = $i" >> .PKGINFO - ;; - - # files - %BACKUP%) - # strip the md5sum after the tab - echo "backup = ${i%%$'\t'*}" >> .PKGINFO - ;; - esac + if [[ -z "$i" ]]; then + continue; + fi + + if [[ "$i" =~ %[A-Z]*% ]] ; then + current=$i + continue + fi + + case "$current" in + # desc + %NAME%) + echo "pkgname = $i" >> .PKGINFO + ;; + %VERSION%) + echo "pkgver = $i" >> .PKGINFO + ;; + %DESC%) + echo "pkgdesc = $i" >> .PKGINFO + ;; + %URL%) + echo "url = $i" >> .PKGINFO + ;; + %LICENSE%) + echo "license = $i" >> .PKGINFO + ;; + %ARCH%) + echo "arch = $i" >> .PKGINFO + ;; + %BUILDDATE%) + echo "builddate = $(date -u "+%s")" >> .PKGINFO + ;; + %PACKAGER%) + echo "packager = $pkg_pkger" >> .PKGINFO + ;; + %SIZE%) + echo "size = $pkg_size" >> .PKGINFO + ;; + %GROUPS%) + echo "group = $i" >> .PKGINFO + ;; + %REPLACES%) + echo "replaces = $i" >> .PKGINFO + ;; + %DEPENDS%) + echo "depend = $i" >> .PKGINFO + ;; + %OPTDEPENDS%) + echo "optdepend = $i" >> .PKGINFO + ;; + %CONFLICTS%) + echo "conflict = $i" >> .PKGINFO + ;; + %PROVIDES%) + echo "provides = $i" >> .PKGINFO + ;; + + # files + %BACKUP%) + # strip the md5sum after the tab + echo "backup = ${i%%$'\t'*}" >> .PKGINFO + ;; + esac done comp_files=".PKGINFO" if [ -f "$pkg_dir/install" ] ; then - cp "$pkg_dir/install" "$work_dir/.INSTALL" - comp_files+=" .INSTALL" + cp "$pkg_dir/install" "$work_dir/.INSTALL" + comp_files+=" .INSTALL" fi if [ -f $pkg_dir/changelog ] ; then - cp "$pkg_dir/changelog" "$work_dir/.CHANGELOG" - comp_files+=" .CHANGELOG" + cp "$pkg_dir/changelog" "$work_dir/.CHANGELOG" + comp_files+=" .CHANGELOG" fi # @@ -282,17 +280,17 @@ shopt -s nullglob shopt -s -o pipefail bsdtar -cf - $comp_files * | case "$PKGEXT" in - *tar.gz) gzip -c -f -n ;; - *tar.bz2) bzip2 -c -f ;; - *tar.xz) xz -c -z - ;; - *tar) cat ;; + *tar.gz) gzip -c -f -n ;; + *tar.bz2) bzip2 -c -f ;; + *tar.xz) xz -c -z - ;; + *tar) cat ;; esac > ${pkg_file} || ret=$? if [ $ret -ne 0 ]; then - echo "ERROR: unable to write package to $pkg_dest" - echo " Maybe the disk is full or you do not have write access" - rm -rf "$work_dir" - exit 1 + echo "ERROR: unable to write package to $pkg_dest" + echo " Maybe the disk is full or you do not have write access" + rm -rf "$work_dir" + exit 1 fi rm -rf "$work_dir" -- 1.7.6.4

This includes: - Quoting fixes. - Drop deprecated mktemp option -p. - Set extglob nullglob shell options at the top. - Use extended globbing instead of regex to match %HEADER% in pacman db. Signed-off-by: lolilolicon --- contrib/bacman.in | 21 +++++++++++---------- 1 files changed, 11 insertions(+), 10 deletions(-) diff --git a/contrib/bacman.in b/contrib/bacman.in index 93145e4..89d7f61 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -20,6 +20,9 @@ # along with this program. If not, see http://www.gnu.org/licenses/. # +shopt -s extglob +shopt -s nullglob + readonly progname="bacman" readonly progver="0.2.1" @@ -109,8 +112,8 @@ fi # # Begin # -echo Package: ${pkg_namver} -work_dir=$(mktemp -d -p /tmp) +echo "Package: ${pkg_namver}" +work_dir=$(mktemp -d --tmpdir bacman.XXXXXXXXXX) cd "$work_dir" || exit 1 # @@ -124,12 +127,12 @@ while read i; do continue fi - if [[ "$i" =~ %[A-Z]*% ]]; then + if [[ $i == %+([A-Z])% ]]; then current=$i continue fi - case $current in + case "$current" in %FILES%) ret=0 if [[ -e /$i ]]; then @@ -163,6 +166,7 @@ pkg_size=$(du -sk | awk '{print $1 * 1024}') # # .PKGINFO stuff +# TODO adopt makepkg's write_pkginfo() into this or scripts/library # echo Generating .PKGINFO metadata... echo "# Generated by $progname $progver" > .PKGINFO @@ -174,11 +178,11 @@ echo "#" >> .PKGINFO cat "$pkg_dir"/{desc,files} | while read i; do - if [[ -z "$i" ]]; then + if [[ -z $i ]]; then continue; fi - if [[ "$i" =~ %[A-Z]*% ]]; then + if [[ $i == %+([A-Z])% ]]; then current=$i continue fi @@ -273,9 +277,6 @@ esac pkg_file="$pkg_dest/$pkg_namver-$pkg_arch${PKGEXT}" ret=0 -# when fileglobbing, we want * in an empty directory to expand to -# the null string rather than itself -shopt -s nullglob # TODO: Maybe this can be set globally for robustness shopt -s -o pipefail bsdtar -cf - $comp_files * | @@ -284,7 +285,7 @@ case "$PKGEXT" in *tar.bz2) bzip2 -c -f ;; *tar.xz) xz -c -z - ;; *tar) cat ;; -esac > ${pkg_file} || ret=$? +esac > "${pkg_file}"; ret=$? if (( ret )); then echo "ERROR: unable to write package to $pkg_dest" -- 1.7.6.4

The original code- pkg_dir="$(echo $pac_db/$pkg_name-[0-9]*)" is problematic in several ways: - $pac_db and $pkg_name should be quoted, obviously. - It assumes pkgver always starts with an integer, while in fact it just can't contain ':' and '-'. Counterexample: the code breaks on lshw B.02.15-1. - It assumes there are no more than one directory matching the pattern. While this should be the case if everything works perfectly, it certainly relies on external conditions. Counterexample: if the local db contains two packages named foo and foo-3g, even if everything else is perfect, the code will match two directories. Don't make assumptions, use what is known. Signed-off-by: lolilolicon --- contrib/bacman.in | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-) diff --git a/contrib/bacman.in b/contrib/bacman.in index adc2cb3..9636091 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -93,8 +93,8 @@ pkg_dest="${PKGDEST:-$PWD}" pkg_pkger=${PACKAGER:-'Unknown Packager'} pkg_name="$1" -pkg_dir="$(echo $pac_db/$pkg_name-[0-9]*)" -pkg_namver="${pkg_dir##*/}" +pkg_dir=("$pac_db/$pkg_name"-+([^-])-+([0-9])) +pkg_namver=("${pkg_dir[@]##*/}") # # Checks everything is in place @@ -104,8 +104,16 @@ if [[ ! -d $pac_db ]]; then exit 1 fi +if (( ${#pkg_dir[@]} != 1 )); then + printf "ERROR: %d entries for package %s found in pacman database\n" \ + ${#pkg_dir[@]} "${pkg_name}" + printf "%s\n" "${pkg_dir[@]}" + exit 1 +fi + if [[ ! -d $pkg_dir ]]; then - echo "ERROR: package ${pkg_name} not found in pacman database" + printf "ERROR: package %s is found in pacman database,\n" "${pkg_name}" + printf " but \`%s' is not a directory\n" "${pkg_dir}" exit 1 fi -- 1.7.6.4