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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2025, 15:46 UTC
Message-ID: <xmqqfrdftnet.fsf@gitster.g>
In-Reply-To: <aKtq47vmCrUZCUCF@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 9 quoted lines

> 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

 - The temporary files subshell creates sometimes are harder to follow

 	( cd there && git foo >../actual && ... ) &&
	test_cmp expect actual

   than they need to be.  With "git -C there", obviously paths used
   when they get created and used match:

	git -C there >actual &&
	test_cmp expect actual

 - Test framework helpers like test_when_finished and test_commit
   that rely on the global shell variables to keep track of the
   states do not work well inside subshells.

 - Some platforms have expensive forks.

but in a context that these are not huge problems, I tend to prefer the "cd in a subshell" pattern over

>> +	git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
>> +	test_config -C two remote.origin.followRemoteHEAD "never" &&
>> +	GIT_TRACE_PACKET=$PWD/trace.out git -C two fetch &&

especially where "-C there" is harder to spot.  If the above were

	git -C two do this &&
	git -C two do that >actual &&
	git -C two do something else &&

i.e., with aligned "-C two" to make it obvious that these are doing their thing in the same other place, the tradeoff might have been different, though.