git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`
Date: Fri, 22 Jul 2022 22:35:43 -0700	[thread overview]
Message-ID: <xmqq4jz8bdcg.fsf@gitster.g> (raw)
In-Reply-To: <ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com> (Taylor Blau's message of "Fri, 22 Jul 2022 19:29:05 -0400")

Taylor Blau <me@ttaylorr.com> writes:

> @@ -14,7 +14,7 @@ SYNOPSIS
>  'git cat-file' (-t | -s) [--allow-unknown-type] <object>
>  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]
>  	     [--buffer] [--follow-symlinks] [--unordered]
> -	     [--textconv | --filters]
> +	     [--textconv | --filters] [-z]

Is "-z" useful with any other option, or is it useful only in
combination with one of the three --batch-*?  The above suggests the
former.

> +test_expect_success '--batch, -z with multiple sha1s gives correct format' '
> +	echo_without_newline_nul "$batch_input" >in &&

I I recall [1/2] correctly, the input lacked the LF at the end.  In
the original "LF terminated" use converted to use these variables,
because $batch_*_input is "echo"ed to create the file "in", the lack
of LF at the end is a GOOD thing.

But here, echo_without_newline_nul is just a glorified "printf %s"
piped into tr to turn LF into NUL.  What is fed by printf into the
pipe lacks LF at the end, so the output from tr will not have NUL at
the end, either.

That might happen to work (because the EOF may be enough to signal
the end of the entire input, thus the last input item), but it does
not make the test case for "-z" exactly parallel to the line oriented
input.

> +test_expect_success "--batch-check, -z with multiple sha1s gives correct format" '
> +    echo_without_newline_nul "$batch_check_input" >in &&
> +    test "$batch_check_output" = "$(git cat-file --batch-check -z <in)"
> +'
> +
> +test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
> +	touch -- "newline${LF}embedded" &&
> +	git add -- "newline${LF}embedded" &&
> +	git commit -m "file with newline embedded" &&
> +	test_tick &&
> +
> +	printf "HEAD:newline${LF}embedded" >in &&
> +	git cat-file --batch-check -z <in >actual &&

As I already said, I suspect that new users who know how our path
quoting works would expect c-quoted path would work just fine
without using "-z".  It is not a reason to refuse "-z" to exist,
though.

> @@ -436,6 +465,11 @@ test_expect_success '--batch-command with multiple info calls gives correct form
>  	echo "$batch_command_multiple_info" >in &&
>  	git cat-file --batch-command --buffer <in >actual &&
>  
> +	test_cmp expect actual &&
> +
> +	echo "$batch_command_multiple_info" | tr "\n" "\0" >in &&

This is what I would expect.  The _info variable lacks final LF,
which is supplied by "echo", so output from tr ends with NUL, which
mirrors the line-oriented input we used above.

> +	git cat-file --batch-command --buffer -z <in >actual &&
> +
>  	test_cmp expect actual
>  '
>  
> @@ -459,6 +493,12 @@ test_expect_success '--batch-command with multiple command calls gives correct f
>  	echo "$batch_command_multiple_contents" >in &&
>  	git cat-file --batch-command --buffer <in >actual_raw &&
>  
> +	remove_timestamp <actual_raw >actual &&
> +	test_cmp expect actual &&
> +
> +	echo "$batch_command_multiple_contents" | tr "\n" "\0" >in &&
> +	git cat-file --batch-command --buffer -z <in >actual_raw &&
> +

Likewise.

  parent reply	other threads:[~2022-07-23  5:35 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-22 23:28 [PATCH 0/2] cat-file: support NUL-delimited input with `-z` Taylor Blau
2022-07-22 23:29 ` [PATCH 1/2] t1006: extract --batch-command inputs to variables Taylor Blau
2022-07-22 23:29 ` [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z` Taylor Blau
2022-07-22 23:41   ` Chris Torek
2022-07-25 23:39     ` Taylor Blau
2022-07-23  5:17   ` Ævar Arnfjörð Bjarmason
2022-07-23 17:45     ` Junio C Hamano
2022-07-25 23:44       ` Taylor Blau
2022-07-27 14:10         ` Junio C Hamano
2022-07-23  5:35   ` Junio C Hamano [this message]
2022-07-25 23:50     ` Taylor Blau
2022-07-27 14:20       ` Junio C Hamano
2022-07-31 15:50   ` Phillip Wood
2022-08-11 11:52   ` Ævar Arnfjörð Bjarmason
2022-07-23  4:44 ` [PATCH 0/2] cat-file: " Junio C Hamano

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=xmqq4jz8bdcg.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).