Re: [PATCH 3/4] t5510: prefer "git -C" to subshell for followRemoteHEAD tests

From

Jeff King <peff@peff.net>

Date

Aug 26, 2025, 03:44 UTC

Message-ID

<20250826034434.GB388997@coredump.intra.peff.net>

In-Reply-To

<xmqqfrdftnet.fsf@gitster.g>

On Mon, Aug 25, 2025 at 08:46:02AM -0700, Junio C Hamano wrote:

> > I for one think that the original is much more readable.
> >
> > With the subshell it's quite clear, even at a cursory glance, which
> > commands are executed in a subdirectory, but when using '-C dir' all
> > over we have to look closely.  Furthermore, when there is a command
> > outside of the subshell, we can be fairly sure that it's intentional,
> > but when a command without '-C dir' lurks among many others using '-C
> > dir', then we can't be so sure, but have to investigate whether that
> > was intentional or oversight.
> 
> Unfortunately I tend to agree.  A few downsides I find a bit
> problematic in the subshell solution are
> [...]

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 6e8b741491..bac464a9ec 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -269,12 +269,16 @@ test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
 '
 
 test_expect_success 'followRemoteHEAD create does not overwrite dangling symref' '
-	git -C two remote add -m does-not-exist custom-head ../one &&
-	test_config -C two remote.custom-head.followRemoteHEAD create &&
-	git -C two fetch custom-head &&
-	echo refs/remotes/custom-head/does-not-exist >expect &&
-	git -C two symbolic-ref refs/remotes/custom-head/HEAD >actual &&
-	test_cmp expect actual
+	test_when_finished "git -C two config unset remote.custom-head.followRemoteHEAD" &&
+	(
+		cd two &&
+		git remote add -m does-not-exist custom-head ../one &&
+		git config remote.custom-head.followRemoteHEAD create &&
+		git fetch custom-head &&
+		echo refs/remotes/custom-head/does-not-exist >expect &&
+		git symbolic-ref refs/remotes/custom-head/HEAD >actual &&
+		test_cmp expect actual
+	)
 '
 
 test_expect_success 'fetch --prune on its own works as expected' '


But both patches are already in 'next'. How do you want to proceed? I
can prepare a patch on top converting back to sub-shells. Or if we are
going to do the post-release rewind of next, that is an opportunity to
fix things cleanly. Or we could leave it as-is if it is not worth the
bother at this point.

-Peff