Convert bash functions that return arrays to comply with Bash Style Guide

4 files changed, 56 insertions, 62 deletions

diff --git a/bpkg-util/manage.in b/bpkg-util/manage.in
index 8ff51e9..aa59d3b 100644
--- a/bpkg-util/manage.in
+++ b/bpkg-util/manage.in
@@ -539,9 +539,10 @@ function extract_pkg_info () # <archive>
 {
   local arc="$1"
 
-  local r
-  r=($(bpkg_util_pkg_verify_archive "$arc")) # <name> <version> <project>
-  if [[ ! -v r[2] ]]; then
+  local r # (<name> <version> <project>)
+  bpkg_util_pkg_verify_archive "$arc" | readarray -t r
+
+  if [[ -z "${r[2]}" ]]; then
     r[2]="${r[0]}"
   fi
 
@@ -579,8 +580,8 @@ function check_pkg_duplicate () # <pkg-name> <pkg-version>
     # Use <name>-<version>.* without .tar.gz in case we want to support more
     # archive types later.
     #
-    IFS=$'\n' eval \
-    'p=($(bpkg_util_pkg_find_archive "$name-$version.*" "$dst_dir/$sd"))'
+    bpkg_util_pkg_find_archive "$name-$version.*" "$dst_dir/$sd" | \
+    readarray -t p
 
     if [[ "${#p[@]}" -ne 0 ]]; then
       local a="${p[0]}"
@@ -635,12 +636,12 @@ function remove_pkg_archives ()
 
   # Search for replacement candidates.
   #
-  local pkgs=()                 # Packages to be considered for replacement.
+  local pkgs= # Packages to be considered for replacement.
 
-  IFS=$'\n' eval \
-  'pkgs=($(bpkg_util_pkg_find_archives "$name" \
-                                       "$vpat" \
-                                       "$dst_dir/${dst_sections[$dsect]}"))'
+  bpkg_util_pkg_find_archives "$name" \
+                              "$vpat" \
+                              "$dst_dir/${dst_sections[$dsect]}" | \
+  readarray -t pkgs
 
   # For each replacement candidate, ask for confirmation and, depending on the
   # answer, either remove it from the destination repository or leave it in
@@ -1374,8 +1375,9 @@ function check_drop_ownership_consistency () # <proj>
           local frel="${f#$rd/}"  # Path made relative to repo dir.
           if [[ ! "$(managed_repo "$rd")" ||
                 ! "$(contains "$frel" "${bundle_files[@]}")" ]]; then
-            local p
-            p=($(bpkg_util_pkg_verify_archive "$f"))  # (name ver proj)
+            local p # (name ver proj)
+            bpkg_util_pkg_verify_archive "$f" | readarray -t p
+
             unsel_pkg_names["${p[0]}"]=
           fi
         done < <(find "$pd" -type f -not -name "*.manifest")
diff --git a/bpkg-util/package-archive.bash.in b/bpkg-util/package-archive.bash.in
index 899e3c4..f0b88e7 100644
--- a/bpkg-util/package-archive.bash.in
+++ b/bpkg-util/package-archive.bash.in
@@ -19,9 +19,9 @@ if [ ! -v bpkg_util_bpkg ]; then
   exit 1
 fi
 
-# Extract the package information from a package archive and print it in the
-# '<name> <version> <project>' form, where the project field is empty if the
-# project value is not specified in the manifest.
+# Extract the package information from a package archive and print it to
+# stdout in the '<name>\n<version>\n<project>\n' form, where the project field
+# is empty if the project value is not specified in the manifest.
 #
 # Note that, in particular, it verifies that the archive file name matches the
 # package name and version.
@@ -57,14 +57,17 @@ function bpkg_util_pkg_verify_archive () # <path>
 
   butl_manifest_parser_finish
 
-  echo -n "$name $version $project"
+  echo "$name"
+  echo "$version"
+  echo "$project"
 }
 
 # Search for package archives in a directory using the package name and
-# version pattern and printing their paths newline-separated. If the version
-# argument is '*', then print archives for all package versions. Otherwise if
-# the version contains the trailing '*', then print archives for all revisions
-# of the specified version and for the exact version otherwise. For example:
+# version pattern and printing their paths one per line to stdout. If the
+# version argument is '*', then print archives for all package versions.
+# Otherwise if the version contains the trailing '*', then print archives for
+# all revisions of the specified version and for the exact version otherwise.
+# For example:
 #
 # bpkg_util_pkg_find_archives foo '*'    dir/
 # bpkg_util_pkg_find_archives foo '1.0*' dir/
@@ -73,20 +76,14 @@ function bpkg_util_pkg_verify_archive () # <path>
 # Note that the resulting archive paths include the specified directory as a
 # prefix.
 #
-# NOTE: this function can be called with overriden IFS.
-#
 function bpkg_util_pkg_find_archives () # <name> <version> <dir>
 {
-  IFS=$' \t\n' bpkg_util_pkg_find_archives_impl "$@"
-}
-
-function bpkg_util_pkg_find_archives_impl ()
-{
   local nam="$1"
   local ver="$2"
   local dir="$3"
 
-  local r=""
+  local r=()
+  local f
 
   if [ -d "$dir" ]; then
     local vr # Version with the revision stripped, if search for revisions.
@@ -103,10 +100,9 @@ function bpkg_util_pkg_find_archives_impl ()
     # '1.2.3+2*': foo-1.2.3.tar.gz, foo-1.2.3+1.tar.gz, foo-1.2.30.tar.gz,
     # etc) and return those which package name and version match properly.
     #
-    local f
     while read f; do
       local p
-      p=($(bpkg_util_pkg_verify_archive "$f"))
+      bpkg_util_pkg_verify_archive "$f" | readarray -t p
 
       local n="${p[0]}"
       local v="${p[1]}"
@@ -116,37 +112,26 @@ function bpkg_util_pkg_find_archives_impl ()
               "$v" == "$ver" || \
               ( -n "$vr" && "$v" =~ ^"$vr"(\+[0-9]+)?$ )) ]]; then
 
-        if [ -n "$r" ]; then
-          r="$r"$'\n'"$f"
-        else
-          r="$f"
-        fi
+        r+=("$f")
       fi
     done < <(find "$dir" -type f -name "$np")
   fi
 
-  if [ -n "$r" ]; then
-    echo -n "$r"
-  fi
+  for f in "${r[@]}"; do
+    echo "$f"
+  done
 }
 
 # Search for a package archive in a directory using a file name pattern. If
-# the archive is found, then print the package information in the
-# '<path>\n<name>\n<version>\n<project>' form, where the project field is
+# the archive is found, then print the package information to stdout in the
+# '<path>\n<name>\n<version>\n<project>\n' form, where the project field is
 # empty if the project value is not specified in the manifest.
 #
 # Note that if there are multiple archives matching the pattern, then it is
 # unspecified which one is picked.
 #
-# NOTE: this function can be called with overriden IFS.
-#
 function bpkg_util_pkg_find_archive () # <pattern> <dir>
 {
-  IFS=$' \t\n' bpkg_util_pkg_find_archive_impl "$@"
-}
-
-function bpkg_util_pkg_find_archive_impl ()
-{
   local pat="$1"
   local dir="$2"
 
@@ -156,14 +141,17 @@ function bpkg_util_pkg_find_archive_impl ()
     # We could probably use -print -quit but this is not portable (NetBSD
     # needs -exit instead of -quit).
     #
-    f="$(find "$dir" -type f -name "$pat" | head -n 1)"
+    f="$(find "$dir" -type f -name "$pat" | sed -n -e '1p')"
 
     if [ -n "$f" ]; then
 
       local p
-      p=($(bpkg_util_pkg_verify_archive "$f"))
+      bpkg_util_pkg_verify_archive "$f" | readarray -t p
 
-      printf "$f\n${p[0]}\n${p[1]}\n${p[2]}"
+      echo "$f"
+      echo "${p[0]}"
+      echo "${p[1]}"
+      echo "${p[2]}"
       return
     fi
   fi
diff --git a/tests/package-archive/driver.in b/tests/package-archive/driver.in
index a32e571..5adecf1 100644
--- a/tests/package-archive/driver.in
+++ b/tests/package-archive/driver.in
@@ -8,13 +8,13 @@
 bpkg_util_bpkg=bpkg
 
 trap "{ exit 1; }" ERR
-set -o errtrace # Trap ERR in functions.
+set -o errtrace     # Trap in functions and subshells.
+set -o pipefail     # Fail if any pipeline command fails.
+shopt -s lastpipe   # Execute last pipeline command in the current shell.
+shopt -s nullglob   # Expand no-match globs to nothing rather than themselves.
 
 @import bpkg-util/package-archive@
 
 # Call the function passed on the command line.
 #
-# Note that we reset IFS to make sure that the function being tested is not
-# affected by its value set by the caller.
-#
-IFS= "$@"
+"$@"
diff --git a/tests/package-archive/testscript b/tests/package-archive/testscript
index b2eb7e3..f845401 100644
--- a/tests/package-archive/testscript
+++ b/tests/package-archive/testscript
@@ -22,7 +22,11 @@ clone_arcs = \
 
   : success
   :
-  $* $src_base/libhello-0.1.0.tar.gz >:'libhello 0.1.0 hello'
+  $* $src_base/libhello-0.1.0.tar.gz >>EOO
+    libhello
+    0.1.0
+    hello
+    EOO
 }
 
 : pkg-find-archives
@@ -43,7 +47,7 @@ clone_arcs = \
   {
     $clone_arcs;
 
-    $* 'libhello' '*' $~ >>:/~"%EOO%"
+    $* 'libhello' '*' $~ >>/~"%EOO%"
       %\(
       $~/libhello-0.1.0.tar.gz
       $~/libhello-0.1.0+1.tar.gz
@@ -59,7 +63,7 @@ clone_arcs = \
   {
     $clone_arcs;
 
-    $* 'libhello' '0.1.0' $~ >:/"$~/libhello-0.1.0.tar.gz"
+    $* 'libhello' '0.1.0' $~ >/"$~/libhello-0.1.0.tar.gz"
   }
 
   : package-revision
@@ -67,7 +71,7 @@ clone_arcs = \
   {
     $clone_arcs;
 
-    $* 'libhello' '0.1.0+1' $~ >:/"$~/libhello-0.1.0+1.tar.gz"
+    $* 'libhello' '0.1.0+1' $~ >/"$~/libhello-0.1.0+1.tar.gz"
   }
 
   : package-revisions1
@@ -75,7 +79,7 @@ clone_arcs = \
   {
     $clone_arcs;
 
-    $* 'libhello' '0.1.0*' $~ >>:/~"%EOO%"
+    $* 'libhello' '0.1.0*' $~ >>/~"%EOO%"
       %\(
       $~/libhello-0.1.0.tar.gz
       $~/libhello-0.1.0+1.tar.gz
@@ -91,7 +95,7 @@ clone_arcs = \
   {
     $clone_arcs;
 
-    $* 'libhello' '0.1.0+2*' $~ >>:/~"%EOO%"
+    $* 'libhello' '0.1.0+2*' $~ >>/~"%EOO%"
       %\(
       $~/libhello-0.1.0.tar.gz
       $~/libhello-0.1.0+1.tar.gz
@@ -117,7 +121,7 @@ clone_arcs = \
   {
     $clone_arcs;
 
-    $* 'libhello-0.1.0.*' $~ >>:/"EOO"
+    $* 'libhello-0.1.0.*' $~ >>/"EOO"
       $~/libhello-0.1.0.tar.gz
       libhello
       0.1.0