From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Date: Mon, 12 Jan 2026 08:21:54 GMT
Subject: [RFC PATCH] t5500-fetch-pack.sh: fix suppression of Git exit code in tests
Message-ID: <20260112085024.299018-1-shreyanshpaliwalcmsmn@gmail.com>
In-Reply-To: <xmqq8qe3pxvm.fsf@gitster.g>

> > +	test_oid algo >oid_algo &&
> > +	git rev-parse other >oid_other &&
> > +	git rev-parse main >oid_main &&
>
> It is unusual to take these to temporary files.  If you want to
> reuse the value more than once, it is more common to take them in
> variables.

Actually I referenced a previous patch [1] where temporary files were
used for similar values, so I followed the same pattern here,
but I agree that variables would make more sense.

> > 	GIT_PROTOCOL=version=2 git upload-pack . <<-EOF >/dev/null
> >  	0012command=fetch
> > -	$(echo "object-format=$(test_oid algo)" | packetize)
> > +	$(echo "object-format=$(<oid_algo)" | packetize)
>
> The construct $(<file) is bashism, that does not work if your shell
> is not bash, isn't it?  If you used a variable, e.g.,
>
> 	$(echo "object-format=$oid_algo" | packetize)
>
> that would make the result more portable.

Right, agreed.

> In any case, since the output of "echo" is sent to "| packetize",
> the exit code of $(test_oid algo) would not affect the bigger
> picture, and so would a failure from $(<oid_algo).  I am not sure if
> this conversion has any value wrt to "suppression of exit code".  If
> $(<oid_algo) construct fails to read the oid_algo file, the upstream
> of "| packetize" may exit with non-zero code, but the downstream of
> the pipe would hide it.
>
> THe same comment applies to other two uses of $(<file) construct.

Yes, that makes sense now, sorry I hadn’t considered it this way.
I now get that since the output is piped into packetize,
failure in the command would still be hidden.

Now to fix this can we explicitly check the git commands beforehand,
like for e.g.,

	oid_other=$(git rev-parse other) || exit 1

I believe that would prevent the suppression.

Best,
Shreyansh

[1] - https://github.com/git/git/commit/c6f44e1da5e88e34