On Fri, Jul 22, 2022 at 10:35:43PM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

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

It only makes sense with `--batch`-related options. But doesn't the
above suggest the latter, not the former? That synopsis line begins
with:

    'git cat-file' (--batch | --batch-check | --batch-command) ...

which made me think that this was the invocation for batch-related
options, and only listed options that made sense with a `--batch` mode
of one kind or another.

quoted

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

I see what you're saying. And, yeah, I think it happens to work since we
treat EOF as marking the end of the last input element, regardless of
whether or not we saw a NUL byte or a LF (depending on whether or not we
passed `-z`).

I think the helper should probably be something more like:

    echo_with_nul () {
        echo "$@" | tr '\n' '\0'
    }

or similar. But as you note below, this is probably not even worth
extracting to a helper function.

'

quoted

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

Yeah. I think we can do both, if there is a need. I suspect that just
`-z` support would be sufficient for now, but I agree that one doesn't
need to tie up the other.

quoted

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

Yep.

quoted

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

Ditto, thanks.

Thanks,
Taylor