All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Jeff King <peff@peff.net>,
	 git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH 3/4] t5510: prefer "git -C" to subshell for followRemoteHEAD tests
Date: Mon, 25 Aug 2025 08:46:02 -0700	[thread overview]
Message-ID: <xmqqfrdftnet.fsf@gitster.g> (raw)
In-Reply-To: <aKtq47vmCrUZCUCF@szeder.dev> ("SZEDER Gábor"'s message of "Sun, 24 Aug 2025 21:41:23 +0200")

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

> 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.

  reply	other threads:[~2025-08-25 15:46 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-19 19:20 [PATCH 0/4] dangling symrefs and fetchRemoteHEAD=create Jeff King
2025-08-19 19:23 ` Jeff King
2025-08-19 19:24 ` [PATCH 1/4] t5510: make confusing config cleanup more explicit Jeff King
2025-08-19 20:03   ` Eric Sunshine
2025-08-19 20:16     ` Eric Sunshine
2025-08-19 20:53     ` Jeff King
2025-08-19 19:26 ` [PATCH 2/4] t5510: stop changing top-level working directory Jeff King
2025-08-19 19:27 ` [PATCH 3/4] t5510: prefer "git -C" to subshell for followRemoteHEAD tests Jeff King
2025-08-24 19:41   ` SZEDER Gábor
2025-08-25 15:46     ` Junio C Hamano [this message]
2025-08-26  3:44       ` Jeff King
2025-08-26 14:58         ` Junio C Hamano
2025-08-19 19:29 ` [PATCH 4/4] refs: do not clobber dangling symrefs Jeff King
2025-08-20  7:27   ` Patrick Steinhardt
2025-08-20 19:14     ` Jeff King
2025-09-22 12:23   ` Toon Claes
2025-09-22 15:54     ` Junio C Hamano
2025-09-22 17:21       ` Jeff King
2025-09-22 17:35         ` Junio C Hamano
2025-09-22 17:12     ` Jeff King
2025-09-23  9:36       ` Toon Claes
2025-09-23 17:33         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfrdftnet.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.