{"thread":{"id":"58213","subject":"[PATCH 0/2] cat-file: support NUL-delimited input with `-z`","startedAt":"2022-07-22T23:29:05Z","lastAt":"2022-08-11T11:53:48Z","messageCount":15,"participants":["Taylor Blau","Chris Torek","Junio C Hamano","Ævar Arnfjörð Bjarmason","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"459802","messageId":"cover.1658532524.git.me@ttaylorr.com","threadId":"58213","inReplyTo":null,"subject":"[PATCH 0/2] cat-file: support NUL-delimited input with `-z`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-07-22T23:28:54Z","receivedAt":"2022-07-22T23:29:05Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"This short patch series implements a `-z` mode that is compatible with\nthe `--batch`, `--batch-check`, and new `--batch-command` options in\n`cat-file`.\n\nThis came out of a request from a colleague to support `cat-file`\ninvocations that refer to a tree entry whose name includes a newline\ncharacter.\n\nThe implementation is mostly straightforward, though the second patch\n(which contains the main substance of this change) has a few additional\nthoughts on areas for future cleanup. The first patch is preparatory, but\ncould easily be squashed, too.\n\nThanks in advance for your review!\n\nTaylor Blau (2):\n  t1006: extract --batch-command inputs to variables\n  builtin/cat-file.c: support NUL-delimited input with `-z`\n\n Documentation/git-cat-file.txt |  7 +++-\n builtin/cat-file.c             | 28 +++++++++++--\n t/t1006-cat-file.sh            | 72 +++++++++++++++++++++++++++-------\n 3 files changed, 88 insertions(+), 19 deletions(-)\n\n-- \n2.37.0.1.g1379af2e9d\n"},{"id":"459803","messageId":"7c1ad1f02285841d287445abe2ba9b4cb3d8a456.1658532524.git.me@ttaylorr.com","threadId":"58213","inReplyTo":"cover.1658532524.git.me@ttaylorr.com","subject":"[PATCH 1/2] t1006: extract --batch-command inputs to variables","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-07-22T23:29:02Z","receivedAt":"2022-07-22T23:29:12Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"A future commit will want to ensure that various `--batch`-related\noptions produce the same output whether their input is newline\nterminated, or NUL terminated (and a to-be-implemented `-z` option\nexists).\n\nTo prepare for this, extract the given input(s) into separate variables\nto that their LF characters can easily be converted into NUL bytes when\ntesting the new `-z` mode.\n\nThis is consistent with other tests in t1006 (which these days is no\nlonger a shining example of our CodingGuidelines).\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n t/t1006-cat-file.sh | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex dadf3b1458..01c0535765 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -418,6 +418,12 @@ test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n     \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n '\n \n+batch_command_multiple_info=\"info $hello_sha1\n+info $tree_sha1\n+info $commit_sha1\n+info $tag_sha1\n+info deadbeef\"\n+\n test_expect_success '--batch-command with multiple info calls gives correct format' '\n \tcat >expect <<-EOF &&\n \t$hello_sha1 blob $hello_size\n@@ -427,17 +433,18 @@ test_expect_success '--batch-command with multiple info calls gives correct form\n \tdeadbeef missing\n \tEOF\n \n-\tgit cat-file --batch-command --buffer >actual <<-EOF &&\n-\tinfo $hello_sha1\n-\tinfo $tree_sha1\n-\tinfo $commit_sha1\n-\tinfo $tag_sha1\n-\tinfo deadbeef\n-\tEOF\n+\techo \"$batch_command_multiple_info\" >in &&\n+\tgit cat-file --batch-command --buffer <in >actual &&\n \n \ttest_cmp expect actual\n '\n \n+batch_command_multiple_contents=\"contents $hello_sha1\n+contents $commit_sha1\n+contents $tag_sha1\n+contents deadbeef\n+flush\"\n+\n test_expect_success '--batch-command with multiple command calls gives correct format' '\n \tremove_timestamp >expect <<-EOF &&\n \t$hello_sha1 blob $hello_size\n@@ -449,13 +456,8 @@ test_expect_success '--batch-command with multiple command calls gives correct f\n \tdeadbeef missing\n \tEOF\n \n-\tgit cat-file --batch-command --buffer >actual_raw <<-EOF &&\n-\tcontents $hello_sha1\n-\tcontents $commit_sha1\n-\tcontents $tag_sha1\n-\tcontents deadbeef\n-\tflush\n-\tEOF\n+\techo \"$batch_command_multiple_contents\" >in &&\n+\tgit cat-file --batch-command --buffer <in >actual_raw &&\n \n \tremove_timestamp <actual_raw >actual &&\n \ttest_cmp expect actual\n-- \n2.37.0.1.g1379af2e9d\n\n"},{"id":"459804","messageId":"ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com","threadId":"58213","inReplyTo":"cover.1658532524.git.me@ttaylorr.com","subject":"[PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-07-22T23:29:05Z","receivedAt":"2022-07-22T23:29:12Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When callers are using `cat-file` via one of the stdin-driven `--batch`\nmodes, all input is newline-delimited. This presents a problem when\ncallers wish to ask about, e.g. tree-entries that have a newline\ncharacter present in their filename.\n\nTo support this niche scenario, introduce a new `-z` mode to the\n`--batch`, `--batch-check`, and `--batch-command` suite of options that\ninstructs `cat-file` to treat its input as NUL-delimited, allowing the\nindividual commands themselves to have newlines present.\n\nThe refactoring here is slightly unfortunate, since we turn loops like:\n\n    while (strbuf_getline(&buf, stdin) != EOF)\n\ninto:\n\n    while (1) {\n        int ret;\n        if (opt->nul_terminated)\n            ret = strbuf_getline_nul(&input, stdin);\n        else\n            ret = strbuf_getline(&input, stdin);\n\n        if (ret == EOF)\n            break;\n    }\n\nIt's tempting to think that we could use `strbuf_getwholeline()` and\nspecify either `\\n` or `\\0` as the terminating character. But for input\non platforms that include a CR character preceeding the LF, this\nwouldn't quite be the same, since `strbuf_getline(...)` will trim any\ntrailing CR, while `strbuf_getwholeline(&buf, stdin, '\\n')` will not.\n\nIn the future, we could clean this up further by introducing a variant\nof `strbuf_getwholeline()` that addresses the aforementioned gap, but\nthat approach felt too heavy-handed for this pair of uses.\n\nSome tests are added in t1006 to ensure that `cat-file` produces the\nsame output in `--batch`, `--batch-check`, and `--batch-command` modes\nwith and without the new `-z` option.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-cat-file.txt |  7 +++++-\n builtin/cat-file.c             | 28 ++++++++++++++++++++---\n t/t1006-cat-file.sh            | 42 +++++++++++++++++++++++++++++++++-\n 3 files changed, 72 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex 24a811f0ef..3515350ed6 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n 'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n 'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n \t     [--buffer] [--follow-symlinks] [--unordered]\n-\t     [--textconv | --filters]\n+\t     [--textconv | --filters] [-z]\n 'git cat-file' (--textconv | --filters)\n \t     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\n \n@@ -207,6 +207,11 @@ respectively print:\n \t/etc/passwd\n --\n \n+-z::\n+\tOnly meaningful with `--batch`, `--batch-check`, or\n+\t`--batch-command`; input is NUL-delimited instead of\n+\tnewline-delimited.\n+\n \n OUTPUT\n ------\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex f42782e955..c3602d15df 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -31,6 +31,7 @@ struct batch_options {\n \tint all_objects;\n \tint unordered;\n \tint transform_mode; /* may be 'w' or 'c' for --filters or --textconv */\n+\tint nul_terminated;\n \tconst char *format;\n };\n \n@@ -614,12 +615,20 @@ static void batch_objects_command(struct batch_options *opt,\n \tstruct queued_cmd *queued_cmd = NULL;\n \tsize_t alloc = 0, nr = 0;\n \n-\twhile (!strbuf_getline(&input, stdin)) {\n-\t\tint i;\n+\twhile (1) {\n+\t\tint i, ret;\n \t\tconst struct parse_cmd *cmd = NULL;\n \t\tconst char *p = NULL, *cmd_end;\n \t\tstruct queued_cmd call = {0};\n \n+\t\tif (opt->nul_terminated)\n+\t\t\tret = strbuf_getline_nul(&input, stdin);\n+\t\telse\n+\t\t\tret = strbuf_getline(&input, stdin);\n+\n+\t\tif (ret)\n+\t\t\tbreak;\n+\n \t\tif (!input.len)\n \t\t\tdie(_(\"empty command in input\"));\n \t\tif (isspace(*input.buf))\n@@ -763,7 +772,16 @@ static int batch_objects(struct batch_options *opt)\n \t\tgoto cleanup;\n \t}\n \n-\twhile (strbuf_getline(&input, stdin) != EOF) {\n+\twhile (1) {\n+\t\tint ret;\n+\t\tif (opt->nul_terminated)\n+\t\t\tret = strbuf_getline_nul(&input, stdin);\n+\t\telse\n+\t\t\tret = strbuf_getline(&input, stdin);\n+\n+\t\tif (ret == EOF)\n+\t\t\tbreak;\n+\n \t\tif (data.split_on_whitespace) {\n \t\t\t/*\n \t\t\t * Split at first whitespace, tying off the beginning\n@@ -866,6 +884,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"like --batch, but don't emit <contents>\"),\n \t\t\tPARSE_OPT_OPTARG | PARSE_OPT_NONEG,\n \t\t\tbatch_option_callback),\n+\t\tOPT_BOOL('z', NULL, &batch.nul_terminated, N_(\"stdin is NUL-terminated\")),\n \t\tOPT_CALLBACK_F(0, \"batch-command\", &batch, N_(\"format\"),\n \t\t\tN_(\"read commands from stdin\"),\n \t\t\tPARSE_OPT_OPTARG | PARSE_OPT_NONEG,\n@@ -921,6 +940,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \telse if (batch.all_objects)\n \t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n \t\t\t       \"--batch-all-objects\");\n+\telse if (batch.nul_terminated)\n+\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n+\t\t\t       \"-z\");\n \n \t/* Batch defaults */\n \tif (batch.buffer_output < 0)\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 01c0535765..23b8942edb 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -88,7 +88,8 @@ done\n \n for opt in --buffer \\\n \t--follow-symlinks \\\n-\t--batch-all-objects\n+\t--batch-all-objects \\\n+\t-z\n do\n \ttest_expect_success \"usage: bad option combination: $opt without batch mode\" '\n \t\ttest_incompatible_usage git cat-file $opt &&\n@@ -100,6 +101,10 @@ echo_without_newline () {\n     printf '%s' \"$*\"\n }\n \n+echo_without_newline_nul () {\n+\techo_without_newline \"$@\" | tr '\\n' '\\0'\n+}\n+\n strlen () {\n     echo_without_newline \"$1\" | wc -c | sed -e 's/^ *//'\n }\n@@ -398,6 +403,12 @@ test_expect_success '--batch with multiple sha1s gives correct format' '\n \ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \"$(maybe_remove_timestamp \"$(echo_without_newline \"$batch_input\" | git cat-file --batch)\" 1)\"\n '\n \n+test_expect_success '--batch, -z with multiple sha1s gives correct format' '\n+\techo_without_newline_nul \"$batch_input\" >in &&\n+\ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \\\n+\t\"$(maybe_remove_timestamp \"$(git cat-file --batch -z <in)\" 1)\"\n+'\n+\n batch_check_input=\"$hello_sha1\n $tree_sha1\n $commit_sha1\n@@ -418,6 +429,24 @@ test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n     \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n '\n \n+test_expect_success \"--batch-check, -z with multiple sha1s gives correct format\" '\n+    echo_without_newline_nul \"$batch_check_input\" >in &&\n+    test \"$batch_check_output\" = \"$(git cat-file --batch-check -z <in)\"\n+'\n+\n+test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '\n+\ttouch -- \"newline${LF}embedded\" &&\n+\tgit add -- \"newline${LF}embedded\" &&\n+\tgit commit -m \"file with newline embedded\" &&\n+\ttest_tick &&\n+\n+\tprintf \"HEAD:newline${LF}embedded\" >in &&\n+\tgit cat-file --batch-check -z <in >actual &&\n+\n+\techo \"$(git rev-parse \"HEAD:newline${LF}embedded\") blob 0\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n batch_command_multiple_info=\"info $hello_sha1\n info $tree_sha1\n info $commit_sha1\n@@ -436,6 +465,11 @@ test_expect_success '--batch-command with multiple info calls gives correct form\n \techo \"$batch_command_multiple_info\" >in &&\n \tgit cat-file --batch-command --buffer <in >actual &&\n \n+\ttest_cmp expect actual &&\n+\n+\techo \"$batch_command_multiple_info\" | tr \"\\n\" \"\\0\" >in &&\n+\tgit cat-file --batch-command --buffer -z <in >actual &&\n+\n \ttest_cmp expect actual\n '\n \n@@ -459,6 +493,12 @@ test_expect_success '--batch-command with multiple command calls gives correct f\n \techo \"$batch_command_multiple_contents\" >in &&\n \tgit cat-file --batch-command --buffer <in >actual_raw &&\n \n+\tremove_timestamp <actual_raw >actual &&\n+\ttest_cmp expect actual &&\n+\n+\techo \"$batch_command_multiple_contents\" | tr \"\\n\" \"\\0\" >in &&\n+\tgit cat-file --batch-command --buffer -z <in >actual_raw &&\n+\n \tremove_timestamp <actual_raw >actual &&\n \ttest_cmp expect actual\n '\n-- \n2.37.0.1.g1379af2e9d\n"},{"id":"459805","messageId":"CAPx1GvecmCD1bJDuEks0edu_dwcH92MN4A2Lyj_Tb5W=WtzKig@mail.gmail.com","threadId":"58213","inReplyTo":"ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2022-07-22T23:41:54Z","receivedAt":"2022-07-22T23:42:11Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Fri, Jul 22, 2022 at 4:34 PM Taylor Blau <me@ttaylorr.com> wrote:\n> The refactoring here is slightly unfortunate, since we turn loops like:\n>\n>     while (strbuf_getline(&buf, stdin) != EOF)\n>\n> into:\n>\n>     while (1) {\n>         int ret;\n>         if (opt->nul_terminated)\n>             ret = strbuf_getline_nul(&input, stdin);\n>         else\n>             ret = strbuf_getline(&input, stdin);\n>\n>         if (ret == EOF)\n>             break;\n>     }\n>\n> It's tempting to think that we could use `strbuf_getwholeline()` and\n> specify either `\\n` or `\\0` as the terminating character. ...\n\nHow about:\n\n    int (*get)(struct strbuf *, FILE *);\n    get = opt->nul_terminated ? strbuf_getline_nul : strbuf_getline;\n    while (get(&buf, stdin) != EOF)\n\nor similar?\n\nChris\n"},{"id":"459823","messageId":"xmqqczdwbfpr.fsf@gitster.g","threadId":"58213","inReplyTo":"cover.1658532524.git.me@ttaylorr.com","subject":"Re: [PATCH 0/2] cat-file: support NUL-delimited input with `-z`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-23T04:44:32Z","receivedAt":"2022-07-23T04:44:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> This short patch series implements a `-z` mode that is compatible with\n> the `--batch`, `--batch-check`, and new `--batch-command` options in\n> `cat-file`.\n\nIt's a shame that we forgot that people use strange \"object name\"\nlike \"<tree>:<path>\", when we originally intended to only take\nsimple object names, like fully-spelled hexadecimal.  And if we were\nprepared to take <path>, we should have make sure we use the usual\ncquote convention.\n\n"},{"id":"459826","messageId":"xmqq4jz8bdcg.fsf@gitster.g","threadId":"58213","inReplyTo":"ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-23T05:35:43Z","receivedAt":"2022-07-23T05:35:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> @@ -14,7 +14,7 @@ SYNOPSIS\n>  'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n>  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n>  \t     [--buffer] [--follow-symlinks] [--unordered]\n> -\t     [--textconv | --filters]\n> +\t     [--textconv | --filters] [-z]\n\nIs \"-z\" useful with any other option, or is it useful only in\ncombination with one of the three --batch-*?  The above suggests the\nformer.\n\n> +test_expect_success '--batch, -z with multiple sha1s gives correct format' '\n> +\techo_without_newline_nul \"$batch_input\" >in &&\n\nI I recall [1/2] correctly, the input lacked the LF at the end.  In\nthe original \"LF terminated\" use converted to use these variables,\nbecause $batch_*_input is \"echo\"ed to create the file \"in\", the lack\nof LF at the end is a GOOD thing.\n\nBut here, echo_without_newline_nul is just a glorified \"printf %s\"\npiped into tr to turn LF into NUL.  What is fed by printf into the\npipe lacks LF at the end, so the output from tr will not have NUL at\nthe end, either.\n\nThat might happen to work (because the EOF may be enough to signal\nthe end of the entire input, thus the last input item), but it does\nnot make the test case for \"-z\" exactly parallel to the line oriented\ninput.\n\n> +test_expect_success \"--batch-check, -z with multiple sha1s gives correct format\" '\n> +    echo_without_newline_nul \"$batch_check_input\" >in &&\n> +    test \"$batch_check_output\" = \"$(git cat-file --batch-check -z <in)\"\n> +'\n> +\n> +test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '\n> +\ttouch -- \"newline${LF}embedded\" &&\n> +\tgit add -- \"newline${LF}embedded\" &&\n> +\tgit commit -m \"file with newline embedded\" &&\n> +\ttest_tick &&\n> +\n> +\tprintf \"HEAD:newline${LF}embedded\" >in &&\n> +\tgit cat-file --batch-check -z <in >actual &&\n\nAs I already said, I suspect that new users who know how our path\nquoting works would expect c-quoted path would work just fine\nwithout using \"-z\".  It is not a reason to refuse \"-z\" to exist,\nthough.\n\n> @@ -436,6 +465,11 @@ test_expect_success '--batch-command with multiple info calls gives correct form\n>  \techo \"$batch_command_multiple_info\" >in &&\n>  \tgit cat-file --batch-command --buffer <in >actual &&\n>  \n> +\ttest_cmp expect actual &&\n> +\n> +\techo \"$batch_command_multiple_info\" | tr \"\\n\" \"\\0\" >in &&\n\nThis is what I would expect.  The _info variable lacks final LF,\nwhich is supplied by \"echo\", so output from tr ends with NUL, which\nmirrors the line-oriented input we used above.\n\n> +\tgit cat-file --batch-command --buffer -z <in >actual &&\n> +\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -459,6 +493,12 @@ test_expect_success '--batch-command with multiple command calls gives correct f\n>  \techo \"$batch_command_multiple_contents\" >in &&\n>  \tgit cat-file --batch-command --buffer <in >actual_raw &&\n>  \n> +\tremove_timestamp <actual_raw >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\techo \"$batch_command_multiple_contents\" | tr \"\\n\" \"\\0\" >in &&\n> +\tgit cat-file --batch-command --buffer -z <in >actual_raw &&\n> +\n\nLikewise.\n"},{"id":"459827","messageId":"220723.86leskqtej.gmgdl@evledraar.gmail.com","threadId":"58213","inReplyTo":"ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-23T05:17:48Z","receivedAt":"2022-07-23T05:39:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 22 2022, Taylor Blau wrote:\n\n[I think John Cai would appreciate being CC'd on this]\n\n> It's tempting to think that we could use `strbuf_getwholeline()` and\n> specify either `\\n` or `\\0` as the terminating character. But for input\n> on platforms that include a CR character preceeding the LF, this\n> wouldn't quite be the same, since `strbuf_getline(...)` will trim any\n> trailing CR, while `strbuf_getwholeline(&buf, stdin, '\\n')` will not.\n\nI commend the effort to maintain bug-for-bug compatibility, but do we\ncare about this distinction, or is it just an accident that we support\n\\r\\n here in the first place?\n\nThis doesn't apply to the rest of cat-file directly, but the origin of\nthe recent --batch-command mode cdoe was to lift the same-ish code from\nbuiltin/update-ref.c, whose \\n or \\0 mode does exactly that:\n\n\twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n\nI.e. it doesn't support \\r\\n, just \\n or \\0.\n\nIsn't that fine? I may be missing something, but why isn't it OK even on\nplatforms that use \\r\\n for their normal *.txt endings to only use \\n or\n\\0 for this bit of protocol?\n\nFor the command mode at least this passes our tests:\n\t\n\tdiff --git a/builtin/cat-file.c b/builtin/cat-file.c\n\tindex f42782e955f..8646059472d 100644\n\t--- a/builtin/cat-file.c\n\t+++ b/builtin/cat-file.c\n\t@@ -614,12 +614,16 @@ static void batch_objects_command(struct batch_options *opt,\n\t \tstruct queued_cmd *queued_cmd = NULL;\n\t \tsize_t alloc = 0, nr = 0;\n\t \n\t-\twhile (!strbuf_getline(&input, stdin)) {\n\t+\twhile (!strbuf_getwholeline(&input, stdin, '\\n')) {\n\t \t\tint i;\n\t \t\tconst struct parse_cmd *cmd = NULL;\n\t \t\tconst char *p = NULL, *cmd_end;\n\t \t\tstruct queued_cmd call = {0};\n\t \n\t+\t\tif (input.len > 0 && input.buf[input.len - 1] == '\\n')\n\t+\t\t\t--input.len;\n\t+\t\tinput.buf[input.len] = '\\0';\n\t+\n\t \t\tif (!input.len)\n\t \t\t\tdie(_(\"empty command in input\"));\n\t \t\tif (isspace(*input.buf))\n\nSo maybe we should just do something like that instead? I.e. declare\nthat a mistake.\n\nAs for the rest of cat-file 05d5667fec9 (git-cat-file: Add --batch-check\noption, 2008-04-23) documents that it's LF, not CR LF, ditto\ngit-cat-file.txt.\n\nSo isn't this just an accident in of us having used the strbuf_getline()\nfunction to mean \"\\n\", but actually it also does \"\\r\\n\".\n\nWhich is a really unfortunately named function b.t.w., since it sneaks\nthis bit of Windows portability into places that may not want it in the\nfirst place.\n\n>  strlen () {\n>      echo_without_newline \"$1\" | wc -c | sed -e 's/^ *//'\n>  }\n> @@ -398,6 +403,12 @@ test_expect_success '--batch with multiple sha1s gives correct format' '\n>  \ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \"$(maybe_remove_timestamp \"$(echo_without_newline \"$batch_input\" | git cat-file --batch)\" 1)\"\n>  '\n>  \n> +test_expect_success '--batch, -z with multiple sha1s gives correct format' '\n> +\techo_without_newline_nul \"$batch_input\" >in &&\n> +\ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \\\n> +\t\"$(maybe_remove_timestamp \"$(git cat-file --batch -z <in)\" 1)\"\n\nThis...\n\n> +'\n> +\n>  batch_check_input=\"$hello_sha1\n>  $tree_sha1\n>  $commit_sha1\n> @@ -418,6 +429,24 @@ test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n>      \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n>  '\n>  \n> +test_expect_success \"--batch-check, -z with multiple sha1s gives correct format\" '\n> +    echo_without_newline_nul \"$batch_check_input\" >in &&\n> +    test \"$batch_check_output\" = \"$(git cat-file --batch-check -z <in)\"\n\nThis....\n\n> +'\n> +\n> +test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '\n> +\ttouch -- \"newline${LF}embedded\" &&\n> +\tgit add -- \"newline${LF}embedded\" &&\n> +\tgit commit -m \"file with newline embedded\" &&\n> +\ttest_tick &&\n> +\n> +\tprintf \"HEAD:newline${LF}embedded\" >in &&\n> +\tgit cat-file --batch-check -z <in >actual &&\n> +\n> +\techo \"$(git rev-parse \"HEAD:newline${LF}embedded\") blob 0\" >expect &&\n\n..and this hides git's exit code, better to pipe to a file, use test_cmp\netc. etc.\n"},{"id":"459837","messageId":"xmqqmtczafku.fsf@gitster.g","threadId":"58213","inReplyTo":"220723.86leskqtej.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-23T17:45:05Z","receivedAt":"2022-07-23T17:45:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> This doesn't apply to the rest of cat-file directly, but the origin of\n> the recent --batch-command mode cdoe was to lift the same-ish code from\n> builtin/update-ref.c, whose \\n or \\0 mode does exactly that:\n>\n> \twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n>\n> I.e. it doesn't support \\r\\n, just \\n or \\0.\n>\n> Isn't that fine? I may be missing something, but why isn't it OK even on\n> platforms that use \\r\\n for their normal *.txt endings to only use \\n or\n> \\0 for this bit of protocol?\n>\n> So maybe we should just do something like that instead? I.e. declare\n> that a mistake.\n\nGood point.\n\n> So isn't this just an accident in of us having used the strbuf_getline()\n> function to mean \"\\n\", but actually it also does \"\\r\\n\".\n>\n> Which is a really unfortunately named function b.t.w., since it sneaks\n> this bit of Windows portability into places that may not want it in the\n> first place.\n\ngetline() is to read a single \"logical\" line, so it is fine for it\nto strip CRLF on platforms with CRLF, and to leave CR at the end of\nline on a LF platform.  If the \"protocol\" is defined to use LF on\nany platform (and allow a payload that ends with CR to be passed),\nyou can argue that it is a wrong helper to call.\n\nBut does that result in a sensible behaviour?  I am not sure.  Some\neditors that are popular on LF platforms can produce CRLF files when\nthe user asks (either on purpose or by mistake), and when not using\nthe \"-z\" mode of input, I suspect that most users would expect CR at\nthe end of the \"line\" (terminated with LF on their platform of\nchoice) would be thrown away even on their LF platform, simply\nbecause it is unlikely that the kind of input they are preparing can\nusefully and legitimately end with CR, as their colleagues on CRLF\nplatforms may also have to produce similar input.  IOW, the presence\nof CRLF platforms makes text lines that end with CR much less useful\neverywhere.\n\nAnd from that point of view, \"getline() or getwholeline(NUL)\" may be\na pattern that is practically more useful than the old \"use always\ngetwholeline(NUL or LF)\" convention that I invented more than 10\nyears ago.\n"},{"id":"459940","messageId":"Yt8pohJWhhnDrG1c@nand.local","threadId":"58213","inReplyTo":"CAPx1GvecmCD1bJDuEks0edu_dwcH92MN4A2Lyj_Tb5W=WtzKig@mail.gmail.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-07-25T23:39:14Z","receivedAt":"2022-07-25T23:39:20Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 22, 2022 at 04:41:54PM -0700, Chris Torek wrote:\n> > It's tempting to think that we could use `strbuf_getwholeline()` and\n> > specify either `\\n` or `\\0` as the terminating character. ...\n>\n> How about:\n>\n>     int (*get)(struct strbuf *, FILE *);\n>     get = opt->nul_terminated ? strbuf_getline_nul : strbuf_getline;\n>     while (get(&buf, stdin) != EOF)\n>\n> or similar?\n\nI thought about it, but it seemed cleaner to lift the ternary out to\ncapture the result, too, leading to the if/else-statement I sent in the\npatch above.\n\nIf we wanted to change it, I'd probably suggest a more general-purpose\nstrbuf function that implemented this behavior through a single call.\nBut it sounds like supporting the carriage return character was a\nmistake from the beginning, which simplifies this direction quite a bit,\nanyways.\n\nThanks,\nTaylor\n"},{"id":"459941","messageId":"Yt8q8xvtXmztLIln@nand.local","threadId":"58213","inReplyTo":"xmqqmtczafku.fsf@gitster.g","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-07-25T23:44:51Z","receivedAt":"2022-07-25T23:44:58Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Junio,\n\nOn Sat, Jul 23, 2022 at 10:45:05AM -0700, Junio C Hamano wrote:\n> > So isn't this just an accident in of us having used the strbuf_getline()\n> > function to mean \"\\n\", but actually it also does \"\\r\\n\".\n> >\n> > Which is a really unfortunately named function b.t.w., since it sneaks\n> > this bit of Windows portability into places that may not want it in the\n> > first place.\n>\n> getline() is to read a single \"logical\" line, so it is fine for it\n> to strip CRLF on platforms with CRLF, and to leave CR at the end of\n> line on a LF platform.  If the \"protocol\" is defined to use LF on\n> any platform (and allow a payload that ends with CR to be passed),\n> you can argue that it is a wrong helper to call.\n\nReading your email up to this point makes me think that we should ignore\nany CR bytes we see next to a LF. So by this point I think that we\nshould take Ævar's suggestion and call:\n\n    strbuf_getwholeline(&buf, stdin, opt->nul_terminated ? '\\0' : '\\n');\n\nBut...\n\n> But does that result in a sensible behaviour?  I am not sure.  Some\n> editors that are popular on LF platforms can produce CRLF files when\n> the user asks (either on purpose or by mistake), and when not using\n> the \"-z\" mode of input, I suspect that most users would expect CR at\n> the end of the \"line\" (terminated with LF on their platform of\n> choice) would be thrown away even on their LF platform, simply\n> because it is unlikely that the kind of input they are preparing can\n> usefully and legitimately end with CR, as their colleagues on CRLF\n> platforms may also have to produce similar input.  IOW, the presence\n> of CRLF platforms makes text lines that end with CR much less useful\n> everywhere.\n>\n> And from that point of view, \"getline() or getwholeline(NUL)\" may be\n> a pattern that is practically more useful than the old \"use always\n> getwholeline(NUL or LF)\" convention that I invented more than 10\n> years ago.\n\nThis makes me think that we should retain support for dropping the CR\npreceeeding a LF and treat it as a historical wart that we are stuck\nsupporting.\n\nDo you have a preference? I am fine with either approach, FWIW.\n\nThanks,\nTaylor\n"},{"id":"459942","messageId":"Yt8sQ6kPoUUQFjle@nand.local","threadId":"58213","inReplyTo":"xmqq4jz8bdcg.fsf@gitster.g","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-07-25T23:50:27Z","receivedAt":"2022-07-25T23:50:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 22, 2022 at 10:35:43PM -0700, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> > @@ -14,7 +14,7 @@ SYNOPSIS\n> >  'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n> >  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n> >  \t     [--buffer] [--follow-symlinks] [--unordered]\n> > -\t     [--textconv | --filters]\n> > +\t     [--textconv | --filters] [-z]\n>\n> Is \"-z\" useful with any other option, or is it useful only in\n> combination with one of the three --batch-*?  The above suggests the\n> former.\n\nIt only makes sense with `--batch`-related options. But doesn't the\nabove suggest the latter, not the former? That synopsis line begins\nwith:\n\n    'git cat-file' (--batch | --batch-check | --batch-command) ...\n\nwhich made me think that this was the invocation for batch-related\noptions, and only listed options that made sense with a `--batch` mode\nof one kind or another.\n\n> > +test_expect_success '--batch, -z with multiple sha1s gives correct format' '\n> > +\techo_without_newline_nul \"$batch_input\" >in &&\n>\n> I I recall [1/2] correctly, the input lacked the LF at the end.  In\n> the original \"LF terminated\" use converted to use these variables,\n> because $batch_*_input is \"echo\"ed to create the file \"in\", the lack\n> of LF at the end is a GOOD thing.\n>\n> But here, echo_without_newline_nul is just a glorified \"printf %s\"\n> piped into tr to turn LF into NUL.  What is fed by printf into the\n> pipe lacks LF at the end, so the output from tr will not have NUL at\n> the end, either.\n>\n> That might happen to work (because the EOF may be enough to signal\n> the end of the entire input, thus the last input item), but it does\n> not make the test case for \"-z\" exactly parallel to the line oriented\n> input.\n\nI see what you're saying. And, yeah, I think it happens to work since we\ntreat EOF as marking the end of the last input element, regardless of\nwhether or not we saw a NUL byte or a LF (depending on whether or not we\npassed `-z`).\n\nI think the helper should probably be something more like:\n\n    echo_with_nul () {\n        echo \"$@\" | tr '\\n' '\\0'\n    }\n\nor similar. But as you note below, this is probably not even worth\nextracting to a helper function.\n\n'\n> > +test_expect_success \"--batch-check, -z with multiple sha1s gives correct format\" '\n> > +    echo_without_newline_nul \"$batch_check_input\" >in &&\n> > +    test \"$batch_check_output\" = \"$(git cat-file --batch-check -z <in)\"\n> > +'\n> > +\n> > +test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '\n> > +\ttouch -- \"newline${LF}embedded\" &&\n> > +\tgit add -- \"newline${LF}embedded\" &&\n> > +\tgit commit -m \"file with newline embedded\" &&\n> > +\ttest_tick &&\n> > +\n> > +\tprintf \"HEAD:newline${LF}embedded\" >in &&\n> > +\tgit cat-file --batch-check -z <in >actual &&\n>\n> As I already said, I suspect that new users who know how our path\n> quoting works would expect c-quoted path would work just fine\n> without using \"-z\".  It is not a reason to refuse \"-z\" to exist,\n> though.\n\nYeah. I think we can do both, if there is a need. I suspect that just\n`-z` support would be sufficient for now, but I agree that one doesn't\nneed to tie up the other.\n\n> > @@ -436,6 +465,11 @@ test_expect_success '--batch-command with multiple info calls gives correct form\n> >  \techo \"$batch_command_multiple_info\" >in &&\n> >  \tgit cat-file --batch-command --buffer <in >actual &&\n> >\n> > +\ttest_cmp expect actual &&\n> > +\n> > +\techo \"$batch_command_multiple_info\" | tr \"\\n\" \"\\0\" >in &&\n>\n> This is what I would expect.  The _info variable lacks final LF,\n> which is supplied by \"echo\", so output from tr ends with NUL, which\n> mirrors the line-oriented input we used above.\n\nYep.\n\n> > +\tgit cat-file --batch-command --buffer -z <in >actual &&\n> > +\n> >  \ttest_cmp expect actual\n> >  '\n> >\n> > @@ -459,6 +493,12 @@ test_expect_success '--batch-command with multiple command calls gives correct f\n> >  \techo \"$batch_command_multiple_contents\" >in &&\n> >  \tgit cat-file --batch-command --buffer <in >actual_raw &&\n> >\n> > +\tremove_timestamp <actual_raw >actual &&\n> > +\ttest_cmp expect actual &&\n> > +\n> > +\techo \"$batch_command_multiple_contents\" | tr \"\\n\" \"\\0\" >in &&\n> > +\tgit cat-file --batch-command --buffer -z <in >actual_raw &&\n> > +\n>\n> Likewise.\n\nDitto, thanks.\n\nThanks,\nTaylor\n"},{"id":"460022","messageId":"xmqqtu72tzmc.fsf@gitster.g","threadId":"58213","inReplyTo":"Yt8q8xvtXmztLIln@nand.local","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-27T14:10:51Z","receivedAt":"2022-07-27T14:10:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> Do you have a preference? I am fine with either approach, FWIW.\n\nI do not have a strong preference, but in \"text\" mode, I suspect\nthat the users may find it more convenient if we discarded CR at the\nend of the line even on LF platforms.\n\nThanks.\n"},{"id":"460023","messageId":"xmqqo7xatz5o.fsf@gitster.g","threadId":"58213","inReplyTo":"Yt8sQ6kPoUUQFjle@nand.local","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-27T14:20:51Z","receivedAt":"2022-07-27T14:21:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Fri, Jul 22, 2022 at 10:35:43PM -0700, Junio C Hamano wrote:\n>> Taylor Blau <me@ttaylorr.com> writes:\n>>\n>> > @@ -14,7 +14,7 @@ SYNOPSIS\n>> >  'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n>> >  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n>> >  \t     [--buffer] [--follow-symlinks] [--unordered]\n>> > -\t     [--textconv | --filters]\n>> > +\t     [--textconv | --filters] [-z]\n>>\n>> Is \"-z\" useful with any other option, or is it useful only in\n>> combination with one of the three --batch-*?  The above suggests the\n>> former.\n>\n> It only makes sense with `--batch`-related options. But doesn't the\n> above suggest the latter, not the former? That synopsis line begins\n> with:\n>\n>     'git cat-file' (--batch | --batch-check | --batch-command) ...\n>\n> which made me think that this was the invocation for batch-related\n> options, and only listed options that made sense with a `--batch` mode\n> of one kind or another.\n\nAh, yes you are absolutely right.  I misread the synopsis.\n\n> I think the helper should probably be something more like:\n>\n>     echo_with_nul () {\n>         echo \"$@\" | tr '\\n' '\\0'\n>     }\n\nI think you meant\n\n\tprintf \"%s\\n\" \"$@\" | tr ..\n\nbut then I wonder if\n\n\tprintf \"%s\\000\" \"$@\"\n\nwould work better?\n\n>> > +\tprintf \"HEAD:newline${LF}embedded\" >in &&\n>> > +\tgit cat-file --batch-check -z <in >actual &&\n>>\n>> As I already said, I suspect that new users who know how our path\n>> quoting works would expect c-quoted path would work just fine\n>> without using \"-z\".  It is not a reason to refuse \"-z\" to exist,\n>> though.\n>\n> Yeah. I think we can do both, if there is a need.\n\nEven though we know it is needed already, it is unfortunately way\ntoo late now X-<, because existing scripts know that paths are not\ntaken as c-quoted, even though people would naturally expect that\npaths are, and satisfying the latter expectation now means breaking\nexisting scripts.\n\nIn any case, as long as \"-z\" is designed right from the day one, it\nwould be fine ;-)\n\nThanks.\n"},{"id":"460328","messageId":"66b71194-ad0e-18d0-e43b-71e5c47ba111@gmail.com","threadId":"58213","inReplyTo":"ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-07-31T15:50:35Z","receivedAt":"2022-07-31T15:50:46Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Taylor\n\nThanks for working on this, I've been annoyed by the lack of a \"-z\" \noption but never got round to doing anything about it.\n\nOn 23/07/2022 00:29, Taylor Blau wrote:\n> When callers are using `cat-file` via one of the stdin-driven `--batch`\n> modes, all input is newline-delimited. This presents a problem when\n> callers wish to ask about, e.g. tree-entries that have a newline\n> character present in their filename.\n> \n> To support this niche scenario, introduce a new `-z` mode to the\n> `--batch`, `--batch-check`, and `--batch-command` suite of options that\n> instructs `cat-file` to treat its input as NUL-delimited, allowing the\n> individual commands themselves to have newlines present.\n\nI wonder if this should also change the output delimiter from NL to NUL.\n\nprintf 'HEAD:does\\nnot\\nexist\\000' | bin-wrappers/git cat-file \n--batch-check -z\n\nprints\n\nHEAD:does\nnot\nexist missing\n\nIf it was terminated by a NUL rather than a newline it would be much \neasier to parse.\n\nBest Wishes\n\nPhillip\n\n> The refactoring here is slightly unfortunate, since we turn loops like:\n> \n>      while (strbuf_getline(&buf, stdin) != EOF)\n> \n> into:\n> \n>      while (1) {\n>          int ret;\n>          if (opt->nul_terminated)\n>              ret = strbuf_getline_nul(&input, stdin);\n>          else\n>              ret = strbuf_getline(&input, stdin);\n> \n>          if (ret == EOF)\n>              break;\n>      }\n> \n> It's tempting to think that we could use `strbuf_getwholeline()` and\n> specify either `\\n` or `\\0` as the terminating character. But for input\n> on platforms that include a CR character preceeding the LF, this\n> wouldn't quite be the same, since `strbuf_getline(...)` will trim any\n> trailing CR, while `strbuf_getwholeline(&buf, stdin, '\\n')` will not.\n> \n> In the future, we could clean this up further by introducing a variant\n> of `strbuf_getwholeline()` that addresses the aforementioned gap, but\n> that approach felt too heavy-handed for this pair of uses.\n> \n> Some tests are added in t1006 to ensure that `cat-file` produces the\n> same output in `--batch`, `--batch-check`, and `--batch-command` modes\n> with and without the new `-z` option.\n> \n> Signed-off-by: Taylor Blau <me@ttaylorr.com>\n> ---\n>   Documentation/git-cat-file.txt |  7 +++++-\n>   builtin/cat-file.c             | 28 ++++++++++++++++++++---\n>   t/t1006-cat-file.sh            | 42 +++++++++++++++++++++++++++++++++-\n>   3 files changed, 72 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index 24a811f0ef..3515350ed6 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -14,7 +14,7 @@ SYNOPSIS\n>   'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n>   'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n>   \t     [--buffer] [--follow-symlinks] [--unordered]\n> -\t     [--textconv | --filters]\n> +\t     [--textconv | --filters] [-z]\n>   'git cat-file' (--textconv | --filters)\n>   \t     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\n>   \n> @@ -207,6 +207,11 @@ respectively print:\n>   \t/etc/passwd\n>   --\n>   \n> +-z::\n> +\tOnly meaningful with `--batch`, `--batch-check`, or\n> +\t`--batch-command`; input is NUL-delimited instead of\n> +\tnewline-delimited.\n> +\n>   \n>   OUTPUT\n>   ------\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index f42782e955..c3602d15df 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -31,6 +31,7 @@ struct batch_options {\n>   \tint all_objects;\n>   \tint unordered;\n>   \tint transform_mode; /* may be 'w' or 'c' for --filters or --textconv */\n> +\tint nul_terminated;\n>   \tconst char *format;\n>   };\n>   \n> @@ -614,12 +615,20 @@ static void batch_objects_command(struct batch_options *opt,\n>   \tstruct queued_cmd *queued_cmd = NULL;\n>   \tsize_t alloc = 0, nr = 0;\n>   \n> -\twhile (!strbuf_getline(&input, stdin)) {\n> -\t\tint i;\n> +\twhile (1) {\n> +\t\tint i, ret;\n>   \t\tconst struct parse_cmd *cmd = NULL;\n>   \t\tconst char *p = NULL, *cmd_end;\n>   \t\tstruct queued_cmd call = {0};\n>   \n> +\t\tif (opt->nul_terminated)\n> +\t\t\tret = strbuf_getline_nul(&input, stdin);\n> +\t\telse\n> +\t\t\tret = strbuf_getline(&input, stdin);\n> +\n> +\t\tif (ret)\n> +\t\t\tbreak;\n> +\n>   \t\tif (!input.len)\n>   \t\t\tdie(_(\"empty command in input\"));\n>   \t\tif (isspace(*input.buf))\n> @@ -763,7 +772,16 @@ static int batch_objects(struct batch_options *opt)\n>   \t\tgoto cleanup;\n>   \t}\n>   \n> -\twhile (strbuf_getline(&input, stdin) != EOF) {\n> +\twhile (1) {\n> +\t\tint ret;\n> +\t\tif (opt->nul_terminated)\n> +\t\t\tret = strbuf_getline_nul(&input, stdin);\n> +\t\telse\n> +\t\t\tret = strbuf_getline(&input, stdin);\n> +\n> +\t\tif (ret == EOF)\n> +\t\t\tbreak;\n> +\n>   \t\tif (data.split_on_whitespace) {\n>   \t\t\t/*\n>   \t\t\t * Split at first whitespace, tying off the beginning\n> @@ -866,6 +884,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>   \t\t\tN_(\"like --batch, but don't emit <contents>\"),\n>   \t\t\tPARSE_OPT_OPTARG | PARSE_OPT_NONEG,\n>   \t\t\tbatch_option_callback),\n> +\t\tOPT_BOOL('z', NULL, &batch.nul_terminated, N_(\"stdin is NUL-terminated\")),\n>   \t\tOPT_CALLBACK_F(0, \"batch-command\", &batch, N_(\"format\"),\n>   \t\t\tN_(\"read commands from stdin\"),\n>   \t\t\tPARSE_OPT_OPTARG | PARSE_OPT_NONEG,\n> @@ -921,6 +940,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>   \telse if (batch.all_objects)\n>   \t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n>   \t\t\t       \"--batch-all-objects\");\n> +\telse if (batch.nul_terminated)\n> +\t\tusage_msg_optf(_(\"'%s' requires a batch mode\"), usage, options,\n> +\t\t\t       \"-z\");\n>   \n>   \t/* Batch defaults */\n>   \tif (batch.buffer_output < 0)\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index 01c0535765..23b8942edb 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -88,7 +88,8 @@ done\n>   \n>   for opt in --buffer \\\n>   \t--follow-symlinks \\\n> -\t--batch-all-objects\n> +\t--batch-all-objects \\\n> +\t-z\n>   do\n>   \ttest_expect_success \"usage: bad option combination: $opt without batch mode\" '\n>   \t\ttest_incompatible_usage git cat-file $opt &&\n> @@ -100,6 +101,10 @@ echo_without_newline () {\n>       printf '%s' \"$*\"\n>   }\n>   \n> +echo_without_newline_nul () {\n> +\techo_without_newline \"$@\" | tr '\\n' '\\0'\n> +}\n> +\n>   strlen () {\n>       echo_without_newline \"$1\" | wc -c | sed -e 's/^ *//'\n>   }\n> @@ -398,6 +403,12 @@ test_expect_success '--batch with multiple sha1s gives correct format' '\n>   \ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \"$(maybe_remove_timestamp \"$(echo_without_newline \"$batch_input\" | git cat-file --batch)\" 1)\"\n>   '\n>   \n> +test_expect_success '--batch, -z with multiple sha1s gives correct format' '\n> +\techo_without_newline_nul \"$batch_input\" >in &&\n> +\ttest \"$(maybe_remove_timestamp \"$batch_output\" 1)\" = \\\n> +\t\"$(maybe_remove_timestamp \"$(git cat-file --batch -z <in)\" 1)\"\n> +'\n> +\n>   batch_check_input=\"$hello_sha1\n>   $tree_sha1\n>   $commit_sha1\n> @@ -418,6 +429,24 @@ test_expect_success \"--batch-check with multiple sha1s gives correct format\" '\n>       \"$(echo_without_newline \"$batch_check_input\" | git cat-file --batch-check)\"\n>   '\n>   \n> +test_expect_success \"--batch-check, -z with multiple sha1s gives correct format\" '\n> +    echo_without_newline_nul \"$batch_check_input\" >in &&\n> +    test \"$batch_check_output\" = \"$(git cat-file --batch-check -z <in)\"\n> +'\n> +\n> +test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '\n> +\ttouch -- \"newline${LF}embedded\" &&\n> +\tgit add -- \"newline${LF}embedded\" &&\n> +\tgit commit -m \"file with newline embedded\" &&\n> +\ttest_tick &&\n> +\n> +\tprintf \"HEAD:newline${LF}embedded\" >in &&\n> +\tgit cat-file --batch-check -z <in >actual &&\n> +\n> +\techo \"$(git rev-parse \"HEAD:newline${LF}embedded\") blob 0\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>   batch_command_multiple_info=\"info $hello_sha1\n>   info $tree_sha1\n>   info $commit_sha1\n> @@ -436,6 +465,11 @@ test_expect_success '--batch-command with multiple info calls gives correct form\n>   \techo \"$batch_command_multiple_info\" >in &&\n>   \tgit cat-file --batch-command --buffer <in >actual &&\n>   \n> +\ttest_cmp expect actual &&\n> +\n> +\techo \"$batch_command_multiple_info\" | tr \"\\n\" \"\\0\" >in &&\n> +\tgit cat-file --batch-command --buffer -z <in >actual &&\n> +\n>   \ttest_cmp expect actual\n>   '\n>   \n> @@ -459,6 +493,12 @@ test_expect_success '--batch-command with multiple command calls gives correct f\n>   \techo \"$batch_command_multiple_contents\" >in &&\n>   \tgit cat-file --batch-command --buffer <in >actual_raw &&\n>   \n> +\tremove_timestamp <actual_raw >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\techo \"$batch_command_multiple_contents\" | tr \"\\n\" \"\\0\" >in &&\n> +\tgit cat-file --batch-command --buffer -z <in >actual_raw &&\n> +\n>   \tremove_timestamp <actual_raw >actual &&\n>   \ttest_cmp expect actual\n>   '\n"},{"id":"461074","messageId":"220811.86o7wrov26.gmgdl@evledraar.gmail.com","threadId":"58213","inReplyTo":"ed1583223f63cfde99829069f14af62e4f0f2a82.1658532524.git.me@ttaylorr.com","subject":"Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-11T11:52:23Z","receivedAt":"2022-08-11T11:53:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 22 2022, Taylor Blau wrote:\n\nThis change:\n\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index 24a811f0ef..3515350ed6 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -14,7 +14,7 @@ SYNOPSIS\n>  'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n>  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n>  \t     [--buffer] [--follow-symlinks] [--unordered]\n> -\t     [--textconv | --filters]\n> +\t     [--textconv | --filters] [-z]\n>  'git cat-file' (--textconv | --filters)\n>  \t     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\n>  \n> [...]\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index f42782e955..c3602d15df 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -31,6 +31,7 @@ struct batch_options {\n>  \tint all_objects;\n>  \tint unordered;\n>  \tint transform_mode; /* may be 'w' or 'c' for --filters or --textconv */\n> +\tint nul_terminated;\n>  \tconst char *format;\n>  };\n\nIs missing a corresponding change to the -h output here. Before this\nthey were the same, but now:\n\t\n\t+ diff -u txt help\n\t--- txt 2022-08-11 11:53:00.235221628 +0000\n\t+++ help        2022-08-11 11:53:00.239221594 +0000\n\t@@ -3,6 +3,6 @@\n\t git cat-file (-t | -s) [--allow-unknown-type] <object>\n\t git cat-file (--batch | --batch-check | --batch-command) [--batch-all-objects]\n\t              [--buffer] [--follow-symlinks] [--unordered]\n\t-             [--textconv | --filters] [-z]\n\t+             [--textconv | --filters]\n\t git cat-file (--textconv | --filters)\n\t              [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\n\n(Spotted with some local patches of mine which automatically check this)\n"}]}