Re: [arch-projects] [dbscripts] [PATCH v2 2/3] test: Fixup glob matching

[Eli Schwartz via arch-projects]

On 02/22/2018 09:15 PM, Luke Shumaker wrote:
>  - common.bash: Globbing happens on the RHS of a [[ = ]] test.
>This means that we must quote variables on the RHS that are to be taken
>verbatim.  This is surprising, because we don't need to quote the LHS.

Unless we intend to do a general style cleanup, it is unnecessary to
"fix" cases where variables with known static content aren't quoted.

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature

[arch-projects] [dbscripts] [PATCH v2 2/3] test: Fixup glob matching

[Luke Shumaker]

From: Luke Shumaker 

 - ftpdir-cleanup.bats: Glob expansion does not occur in [[ -f ]] tests.
   The [[ ! -f .../${pkgname}-*${PKGEXT} ]] checks were checking that there
   were no files containing a literal '*' for that part of their name.
   Obviously, this isn't what was intended.

 - sourceballs.bats: [ -r ] checks explode if the glob returns >1 file.
   Avoid using them if the path being checked contains a glob.

 - common.bash: Globbing happens on the RHS of a [[ = ]] test.
   This means that we must quote variables on the RHS that are to be taken
   verbatim.  This is surprising, because we don't need to quote the LHS.
---
 test/cases/ftpdir-cleanup.bats |  4 ++--
 test/cases/sourceballs.bats|  4 ++--
 test/lib/common.bash   | 12 +++-
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/test/cases/ftpdir-cleanup.bats b/test/cases/ftpdir-cleanup.bats
index fd485f3..8c713c6 100644
--- a/test/cases/ftpdir-cleanup.bats
+++ b/test/cases/ftpdir-cleanup.bats
@@ -13,8 +13,8 @@ __checkRepoRemovedPackage() {
local pkgname
 
for pkgname in $(__getPackageNamesFromPackageBase ${pkgbase}); do
-   [[ ! -f ${FTP_BASE}/${PKGPOOL}/${pkgname}-*${PKGEXT} ]]
-   [[ ! -f 
${FTP_BASE}/${repo}/os/${repoarch}/${pkgname}-*${PKGEXT} ]]
+   ! __isGlobfile "${FTP_BASE}/${PKGPOOL}/${pkgname}"-*"${PKGEXT}"
+   ! __isGlobfile 
"${FTP_BASE}/${repo}/os/${repoarch}/${pkgname}"-*"${PKGEXT}"
done
 }
 
diff --git a/test/cases/sourceballs.bats b/test/cases/sourceballs.bats
index a0a2999..df7ddd4 100644
--- a/test/cases/sourceballs.bats
+++ b/test/cases/sourceballs.bats
@@ -2,12 +2,12 @@ load ../lib/common
 
 __checkSourcePackage() {
local pkgbase=$1
-   [ -r ${FTP_BASE}/${SRCPOOL}/${pkgbase}-*${SRCEXT} ]
+   __isGlobfile "${FTP_BASE}/${SRCPOOL}/${pkgbase}"-*"${SRCEXT}"
 }
 
 __checkRemovedSourcePackage() {
local pkgbase=$1
-   [ ! -r ${FTP_BASE}/${SRCPOOL}/${pkgbase}-*${SRCEXT} ]
+   ! __isGlobfile "${FTP_BASE}/${SRCPOOL}/${pkgbase}"-*"${SRCEXT}"
 }
 
 @test "create simple package sourceballs" {
diff --git a/test/lib/common.bash b/test/lib/common.bash
index 5411641..6e2b3df 100644
--- a/test/lib/common.bash
+++ b/test/lib/common.bash
@@ -14,6 +14,16 @@ __getCheckSum() {
echo "${result%% *}"
 }
 
+# Proxy function to check if a file exists. Using [[ -f ... ]] directly is not
+# always wanted because we might want to expand bash globs first. This way we
+# can pass unquoted globs to __isGlobfile() and have them expanded as function
+# arguments before being checked.
+#
+# This is a copy of db-functions is_globfile
+__isGlobfile() {
+   [[ -f $1 ]]
+}
+
 __buildPackage() {
local pkgdest=${1:-.}
local p
@@ -203,7 +213,7 @@ checkPackageDB() {
for repoarch in ${repoarches[@]}; do
# Only 'any' packages can be found in repos of 
both arches
if [[ $pkgarch != any ]]; then
-   if [[ $pkgarch != ${repoarch} ]]; then
+   if [[ $pkgarch != "$repoarch" ]]; then
continue
fi
fi
-- 
2.16.1