git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 10/10] fetch tests: fix needless and buggy re-quoting

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jun 21, 2022, 22:34 UTC
Message-ID
<patch-10.10-54129b94a77-20220621T222854Z-avarab@gmail.com>
In-Reply-To
<cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com>

Change the test_configured_prune_type() function to take full advantage of its arguments being passed in as a list, rather than needing hacks to work around its quoting issues.

When the test_configured_prune() function was implemented in 737c5a9cde7 (fetch: make --prune configurable, 2013-07-13) it passed in its arguments to "git fetch" as one argument (although at the time only one argument was passed)..

Then in preparation for passing more arguments the first quoting hack was added in 82f34e03e91 (fetch tests: double quote a variable for interpolation, 2018-02-09), with e1790f9245f (fetch tests: fetch <url> <spec> as well as fetch [<remote>], 2018-02-09) following after that. This was all to implement the "git fetch --prune-tags" feature in 97716d217c1 (fetch: add a --prune-tags option and fetch.pruneTags config, 2018-02-09).

At the time the edge cases introduced by this quoting were a known issue, but the alternative was a larger refactoring of this test file. In preceding commits we've done that refactoring, so let's finally take advantage of it. As reported in [1] the existing workaround(s) weren't enough.

We can now drop the "perl" (or "sed" or whatever, see [2]), this will also handle other cases, such as those mentioned in [3].

1. https://lore.kernel.org/git/00a401d884d0$32885890$979909b0$@nexbridge.com/
2. https://lore.kernel.org/git/495bd957-43dc-f252-657d-2969bb7ad5f3@github.com/
3. https://lore.kernel.org/git/YrFwcL2dRS%2Fv7xAw@coredump.intra.peff.net/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5510-fetch.sh | 79 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 62 insertions(+), 17 deletions(-)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 73964bebced..730968d4cd7 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -837,6 +837,7 @@ test_configured_prune_type () {
 		shift
 	done &&
 
+	local refspec_tags='refs/tags/*:refs/tags/*' &&
 	local refspec_heads='+refs/heads/*:refs/remotes/origin/*' &&
 
 	local fetch_prune="$1" &&
@@ -846,23 +847,59 @@ test_configured_prune_type () {
 	local expected_branch="$5" &&
 	local expected_tag="$6" &&
 	shift 6 &&
-	local cmdline="$@" &&
+	local cmdline="" &&
+	local arg_fetch_prune="" &&
+	local arg_fetch_no_prune="" &&
+	local arg_fetch_prune_tags="" &&
+	local arg_fetch_origin="" &&
+	local arg_fetch_url="" &&
+	local arg_fetch_refspec_tags="" &&
+	local arg_fetch_refspec_heads="" &&
+	while test $# != 0
+	do
+		cmdline="${cmdline:+$cmdline }$1" &&
+		case "$1" in
+		--prune)
+			arg_fetch_prune=t
+			;;
+		--no-prune)
+			arg_fetch_no_prune=t
+			;;
+		--prune-tags)
+			arg_fetch_prune_tags=t
+			;;
+		origin)
+			arg_fetch_origin=t
+			;;
+		REMOTE_URL)
+			arg_fetch_url=t
+			;;
+		$refspec_tags)
+			arg_fetch_refspec_tags=t
+			;;
+		$refspec_heads)
+			arg_fetch_refspec_heads=t
+			;;
+		*)
+			BUG "unknown argument: '$1'"
+			;;
+		esac &&
+		shift
+	done &&
 
-	remote_url="file://$TRASH_DIRECTORY/." && # NOT local yet!
-	local cmdline_setup="\"$remote_url\" \"$refspec_heads\""
+	local remote_url="file://$TRASH_DIRECTORY/." &&
 
 	if test "$mode" = 'link'
 	then
-		new_cmdline="" &&
-
 		if test -z "$cmdline"
 		then
-			new_cmdline=$cmdline_setup
-		else
-			new_cmdline=$(printf "%s" "$cmdline" | perl -pe 's[origin(?!/)]["'"$remote_url"'"]g')
-		fi &&
-
-		cmdline="$new_cmdline"
+			arg_fetch_refspec_heads=t
+			arg_fetch_url=t
+		elif test -n "$arg_fetch_origin"
+		then
+			arg_fetch_origin=
+			arg_fetch_url=t
+		fi
 	fi &&
 
 	test_expect_success "$mode prune fetch.prune=$fetch_prune remote.origin.prune=$remote_origin_prune fetch.pruneTags=$fetch_prune_tags remote.origin.pruneTags=$remote_origin_prune_tags${cmdline:+ $cmdline}; branch:$expected_branch tag:$expected_tag" '
@@ -871,7 +908,7 @@ test_configured_prune_type () {
 		git tag -f newtag &&
 		(
 			cd one &&
-			git fetch '"$cmdline_setup"' &&
+			git fetch "$remote_url" "$refspec_heads" &&
 			git rev-parse --verify refs/remotes/origin/newbranch &&
 			git rev-parse --verify refs/tags/newtag
 		) &&
@@ -893,7 +930,15 @@ test_configured_prune_type () {
 			then
 				git_fetch_c=""
 			fi &&
-			git$git_fetch_c fetch '"$cmdline"' &&
+			git$git_fetch_c fetch \
+				${arg_fetch_prune:+--prune} \
+				${arg_fetch_no_prune:+--no-prune} \
+				${arg_fetch_prune_tags:+--prune-tags} \
+				${arg_fetch_origin:+origin} \
+				${arg_fetch_url:+"$remote_url"} \
+				${arg_fetch_refspec_tags:+"refs/tags/*:refs/tags/*"} \
+				${arg_fetch_refspec_heads:+"+refs/heads/*:refs/remotes/origin/*"} &&
+
 			case "$expected_branch" in
 			pruned)
 				test_must_fail git rev-parse --verify refs/remotes/origin/newbranch
@@ -993,22 +1038,22 @@ test_configured_prune unset unset unset unset pruned pruned --prune --prune-tags
 
 test_configured_prune_type --mode name true  unset true  unset pruned pruned
 test_configured_prune_type --mode link true  unset true  unset pruned pruned \
-	"\"$remote_url\"" \
+	REMOTE_URL \
 	"refs/tags/*:refs/tags/*" "+refs/heads/*:refs/remotes/origin/*"
 test_configured_prune_type --mode name unset true  unset true  pruned pruned
 test_configured_prune_type --mode link unset true  unset true  pruned pruned \
-	"\"$remote_url\"" \
+	REMOTE_URL \
 	"refs/tags/*:refs/tags/*" "+refs/heads/*:refs/remotes/origin/*"
 
 # remote.<name>.pruneTags overrides fetch.pruneTags, just like
 # remote.<name>.prune overrides fetch.prune if set.
 test_configured_prune_type --mode name true  unset true unset pruned pruned
 test_configured_prune_type --mode link true  unset true unset pruned pruned \
-	"\"$remote_url\"" \
+	REMOTE_URL \
 	"refs/tags/*:refs/tags/*" "+refs/heads/*:refs/remotes/origin/*"
 test_configured_prune_type --mode name false true  false true pruned pruned
 test_configured_prune_type --mode link false true  false true pruned pruned \
-	"\"$remote_url\"" \
+	REMOTE_URL \
 	"refs/tags/*:refs/tags/*" "+refs/heads/*:refs/remotes/origin/*"
 test_configured_prune true  false true false kept kept
 

-- 
2.36.1.1239.gfba91521d90
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 32 of 37 in “Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1”
  1. rsbecker@nexbridge.comJun 20, 2022
  2. Derrick StoleeJun 20, 2022
  3. rsbecker@nexbridge.comJun 20, 2022
  4. Derrick StoleeJun 20, 2022
  5. SZEDER GáborJun 20, 2022
  6. Ævar Arnfjörð BjarmasonJun 20, 2022
  7. Junio C HamanoJun 20, 2022
  8. rsbecker@nexbridge.comJun 20, 2022
  9. Ævar Arnfjörð BjarmasonJun 20, 2022
  10. rsbecker@nexbridge.comJun 20, 2022
  11. t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)Derrick Stolee, Jun 20, 2022
  12. Johannes SixtJun 21, 2022
  13. Jeff KingJun 21, 2022
  14. SZEDER GáborJun 21, 2022
  15. Jeff KingJun 21, 2022
  16. Junio C HamanoJun 21, 2022
  17. Junio C HamanoJun 21, 2022
  18. Derrick StoleeJun 21, 2022
  19. Junio C HamanoJun 21, 2022
  20. 00/10 t5510: fix the quoting messÆvar Arnfjörð Bjarmason, Jun 21, 2022
  21. 01/10 fetch tests: remove redundant test_unconfig()Ævar Arnfjörð Bjarmason, Jun 21, 2022
  22. Junio C HamanoJun 22, 2022
  23. 04/10 fetch tests: add a helper to avoid boilerplateÆvar Arnfjörð Bjarmason, Jun 21, 2022
  24. 03/10 fetch tests: use "local", &&-chaining, style etc.Ævar Arnfjörð Bjarmason, Jun 21, 2022
  25. 02/10 fetch tests: use named, not positional parametersÆvar Arnfjörð Bjarmason, Jun 21, 2022
  26. 05/10 fetch tests: pass "mode" parameter first, pave way for "$@"Ævar Arnfjörð Bjarmason, Jun 21, 2022
  27. Junio C HamanoJun 22, 2022
  28. 06/10 fetch tests: pass a list, not a string of argumentsÆvar Arnfjörð Bjarmason, Jun 21, 2022
  29. 07/10 fetch tests: remove lazy variable setupÆvar Arnfjörð Bjarmason, Jun 21, 2022
  30. 08/10 fetch tests: remove shelling out for previously "lazy" variablesÆvar Arnfjörð Bjarmason, Jun 21, 2022
  31. 09/10 fetch tests: stop implicitly adding refspecsÆvar Arnfjörð Bjarmason, Jun 21, 2022
  32. 10/10 fetch tests: fix needless and buggy re-quotingÆvar Arnfjörð Bjarmason, Jun 21, 2022
  33. Junio C HamanoJun 22, 2022
  34. Derrick StoleeJun 22, 2022
  35. Ævar Arnfjörð BjarmasonJun 22, 2022
  36. Junio C HamanoJun 22, 2022
  37. rsbecker@nexbridge.comJun 21, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.