diff mbox series

[07/19] t9107: use shell parameter expansion to avoid breaking &&-chain

Message ID 20211209051115.52629-8-sunshine@sunshineco.com (mailing list archive)
State Accepted
Commit 2041b0e8e87adca9f33b499f136445dc015339c8
Headers show
Series tests: fix broken &&-chains & abort loops on error | expand

Commit Message

Eric Sunshine Dec. 9, 2021, 5:11 a.m. UTC
This test intentionally breaks the &&-chain when using `expr` to parse
"[<path>]:<ref>" since the pattern matching operation will return 1
(failure) when <path> is empty even though an empty <path> is legitimate
in this test and should not cause the test to fail. However, it is
possible to parse the input without breaking the &&-chain by using shell
parameter expansion (i.e. `${i%%...}`). Other ways to avoid the problem
would be `{ expr $i : ... ||:; }` or test_might_fail(), however,
parameter expansion seems simplest.

IMPLEMENTATION NOTE

The rewritten `if` expression:

    if test "$ref" = "${ref#refs/remotes/}"`; then continue; fi

is perhaps a bit subtle. At first glance, it looks like it will
`continue` the loop if $ref starts with "refs/remotes/", but in fact
it's the opposite: the loop will `continue` if $ref does not start with
"refs/remotes/".

In the original, `expr` would only match if the ref started with
"refs/remotes/", and $ref would end up empty if it didn't, so `test -z`
would `continue` the loop if the ref did not start with "refs/remotes/".

With parameter expansion, ${ref#refs/remotes/} attempts to strip
"refs/remotes/" from $ref. If it fails, meaning that $ref does not start
with "refs/remotes/", then the expansion will just be $ref unchanged,
and it will `continue` the loop. On the other hand, if stripping
succeeds, meaning that $ref begins with "refs/remotes/", then the
expansion will be the value of $ref with "refs/remotes/" removed, hence
`continue` will not be taken.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t9107-git-svn-migrate.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh
index ceaa5bad10..aa908bbc2f 100755
--- a/t/t9107-git-svn-migrate.sh
+++ b/t/t9107-git-svn-migrate.sh
@@ -98,10 +98,10 @@  test_expect_success 'migrate --minimize on old inited layout' '
 	rm -rf "$GIT_DIR"/svn &&
 	for i in $(cat fetch.out)
 	do
-		path=$(expr $i : "\([^:]*\):.*$")
-		ref=$(expr $i : "[^:]*:\(refs/remotes/.*\)$")
-		if test -z "$ref"; then continue; fi
-		if test -n "$path"; then path="/$path"; fi
+		path=${i%%:*} &&
+		ref=${i#*:} &&
+		if test "$ref" = "${ref#refs/remotes/}"; then continue; fi &&
+		if test -n "$path"; then path="/$path"; fi &&
 		mkdir -p "$GIT_DIR"/svn/$ref/info/ &&
 		echo "$svnrepo"$path >"$GIT_DIR"/svn/$ref/info/url ||
 		return 1