public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail•com>
To: git@vger•kernel.org
Cc: gitster@pobox•com, Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail•com>
Subject: [RFC PATCH] t5500-fetch-pack.sh: fix suppression of Git exit code in tests
Date: Mon, 12 Jan 2026 13:51:54 +0530	[thread overview]
Message-ID: <20260112085024.299018-1-shreyanshpaliwalcmsmn@gmail.com> (raw)
In-Reply-To: <xmqq8qe3pxvm.fsf@gitster.g>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1823 bytes --]

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

  reply	other threads:[~2026-01-12  8:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-11 20:00 [RFC PATCH] t5500-fetch-pack.sh: fix suppression of Git exit code in tests Shreyansh Paliwal
2026-01-11 22:50 ` Junio C Hamano
2026-01-12  8:21   ` Shreyansh Paliwal [this message]
2026-01-12  8:25   ` Patrick Steinhardt
2026-01-12  9:11     ` t5500-fetch-pack.sh and exit-code suppression Shreyansh Paliwal
2026-01-12 13:25     ` [RFC PATCH] t5500-fetch-pack.sh: fix suppression of Git exit code in tests Junio C Hamano
2026-01-13  9:53       ` Shreyansh Paliwal
2026-01-13 13:40         ` Junio C Hamano
2026-01-13 17:53           ` [GSOC][PATCH] t5500: simplify test implementation and fix git exit code suppression Shreyansh Paliwal
2026-01-15 21:28             ` Shreyansh Paliwal
2026-01-20 16:44             ` Junio C Hamano
2026-01-21 12:54               ` [GSOC][PATCH V2] " Shreyansh Paliwal

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=20260112085024.299018-1-shreyanshpaliwalcmsmn@gmail.com \
    --to=shreyanshpaliwalcmsmn@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox