Re: [PATCH v3 1/8] pack-objects: allow `--filter` without `--stdout`

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2023, 23:51 UTC
Message-ID: <xmqqcz0fh7o0.fsf@gitster.g>
In-Reply-To: <ZMBO5A0WxWystYyv@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 19 quoted lines

> On Mon, Jul 24, 2023 at 10:59:02AM +0200, Christian Couder wrote:
>> diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh
>> index b26d476c64..2ff3eef9a3 100755
>> --- a/t/t5317-pack-objects-filter-objects.sh
>> +++ b/t/t5317-pack-objects-filter-objects.sh
>> @@ -53,6 +53,14 @@ test_expect_success 'verify blob:none packfile has no blobs' '
>>  	! grep blob verify_result
>>  '
>>
>> +test_expect_success 'verify blob:none packfile without --stdout' '
>> +	git -C r1 pack-objects --revs --filter=blob:none mypackname >packhash <<-EOF &&
>> +	HEAD
>> +	EOF
>> +	git -C r1 verify-pack -v "mypackname-$(cat packhash).pack" >verify_result &&
>> +	! grep blob verify_result
>> +'
>
> Just a couple of style nits here. It's a little strange (for me, at
> least) to see the heredoc into a git process.

FWIW, "git" is after all no different from any other command and redirecting here-doc, especially with an option like "--stdin" is in effect, is perfectly sensible, I think. Preparing an input in a file (e.g. "cat >file <<EOF" followed by "git cmd <file") might give you slightly a better debuggability, but I do not sense that it is what you are worried about.

> ... I am less certain
> about redirecting the output into a file "packhash", only to cat it back
> out.

But that would make the syntax awkward. Do you mean something along this line?

	var=$(git ... <<-EOF
		here text
	EOF
	) &&
	git ... mypackname-$var.pack &&
	...

Somehow here-doc and $(command subsitution) does not visually mix well.

Also, $var will not be inspectable when running this test under "-i -v", so it hurts debuggability without taking the output in a temporary file. You could do "-x", of course, but that would make everything ultra verbose, so...