{"thread":{"id":"57283","subject":"[RFC] cat-file: add a --stdin-cmd mode","startedAt":"2022-01-21T17:54:23Z","lastAt":"2022-01-25T14:00:06Z","messageCount":4,"participants":["John Cai","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"446637","messageId":"20220121175405.45753-1-johncai86@gmail.com","threadId":"57283","inReplyTo":null,"subject":"[RFC] cat-file: add a --stdin-cmd mode","fromName":"John Cai","fromEmail":"johncai86@gmail.com","sentAt":"2022-01-21T17:54:05Z","receivedAt":"2022-01-21T17:54:23Z","isPatch":false,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"This RFC patch proposes a new flag --stdin-cmd that works with\ngit-cat-file --batch. Similar to git-update-ref --stdin, it will accept\ncommands and arguments from stdin.\n\nThe start of this idea was discussed in [1], where the original\nmotivation was to be able to control when the buffer was flushed to\nstdout in --buffer mode.\n\nHowever, this can actually be much more useful in situations when\ngit-cat-file --batch is being used as a long lived backend query\nprocess. At GitLab, we use a pair of cat-file processes. One for\niterating over object metadata with --batch-check, and the other to grab\nobject contents with --batch. However, if we had --stdin-cmd, we could\nget rid of the second --batch-check process, and just have one progress\nwhere we can flip between getting object info, and getting object contents.\nThis can lead to huge savings.\n\ngit cat-file --batch --stdin-cmd\n\n$ <command> [arg1] [arg2] NL\n\nWe can also add a -z mode to allow for NUL-terminated lines\n\n$ <command> [arg1] [arg2] NUL\n\nThis patch adds three commands: object, info, fflush\n\n$ object <sha1> NL\n$ info <sha1> NL\n$ fflush NL\n\nThese three would be immediately useful in GitLab's context, but one can\nimagine this mode to be further extended for other things.\n\nFor instance, a non-trivial part of \"cat-file --batch\" time is spent\non parsing its argument and seeing if it's a revision, ref etc. So we\ncould add a command that only accepts a full-length 40\ncharacter SHA-1.\n\nThis would be the first step in adding such an interface to\ngit-cat-file.\n\n[1] https://lore.kernel.org/git/pull.1124.git.git.1636149400.gitgitgadget@gmail.com/\n\nHelped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: John Cai <johncai86@gmail.com>\n---\n builtin/cat-file.c  | 137 +++++++++++++++++++++++++++++++++++++++++++-\n strvec.c            |  23 ++++++++\n strvec.h            |   8 +++\n t/t1006-cat-file.sh |  72 +++++++++++++++++++++++\n 4 files changed, 239 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 7b3f42950e..2679b34b43 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -16,6 +16,7 @@\n #include \"packfile.h\"\n #include \"object-store.h\"\n #include \"promisor-remote.h\"\n+#include \"strvec.h\"\n \n struct batch_options {\n \tint enabled;\n@@ -26,7 +27,10 @@ struct batch_options {\n \tint unordered;\n \tint cmdmode; /* may be 'w' or 'c' for --filters or --textconv */\n \tconst char *format;\n+\tint stdin_cmd;\n+\tint end_null;\n };\n+static char line_termination = '\\n';\n \n static const char *force_path;\n \n@@ -508,6 +512,126 @@ static int batch_unordered_packed(const struct object_id *oid,\n \t\t\t\t      data);\n }\n \n+enum batch_state {\n+\t/* Non-transactional state open for commands. */\n+\tBATCH_STATE_OPEN,\n+};\n+\n+static void parse_cmd_object(struct batch_options *opt,\n+\t\t\t     const int argc, const char **argv,\n+\t\t\t     struct strbuf *output,\n+\t\t\t     struct expand_data *data)\n+{\n+\tbatch_one_object(argv[0], output, opt, data);\n+}\n+\n+static void parse_cmd_info(struct batch_options *opt,\n+\t\t\t     const int argc, const char **argv,\n+\t\t\t     struct strbuf *output,\n+\t\t\t     struct expand_data *data)\n+{\n+\topt->print_contents = 0;\n+\tbatch_one_object(argv[0], output, opt, data);\n+}\n+\n+static void parse_cmd_fflush(struct batch_options *opt,\n+\t\t\t     const int argc, const char **argv,\n+\t\t\t     struct strbuf *output,\n+\t\t\t     struct expand_data *data)\n+{\n+\tfflush(stdout);\n+}\n+\n+typedef void (*parse_cmd_fn_t)(struct batch_options *, const int,\n+\t\t\t       const char **, struct strbuf *,\n+\t\t\t       struct expand_data *);\n+\n+static const struct parse_cmd {\n+\tconst char *prefix;\n+\tparse_cmd_fn_t fn;\n+\tunsigned args;\n+\tenum batch_state state;\n+} command[] = {\n+\t{ \"object\", parse_cmd_object, 1, BATCH_STATE_OPEN },\n+\t{ \"info\", parse_cmd_info, 1, BATCH_STATE_OPEN },\n+\t{ \"fflush\", parse_cmd_fflush, 0, BATCH_STATE_OPEN },\n+};\n+\n+static void batch_objects_stdin_cmd(struct batch_options *opt,\n+\t\t\t\t    struct strbuf *output,\n+\t\t\t\t    struct expand_data *data)\n+{\n+\tstruct strbuf input = STRBUF_INIT;\n+\tenum batch_state state = BATCH_STATE_OPEN;\n+\n+\t/* Read each line dispatch its command */\n+\twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n+\t\tint i;\n+\t\tconst struct parse_cmd *cmd = NULL;\n+\t\tstruct strvec args = STRVEC_INIT;\n+\t\tsize_t n;\n+\t\tconst char *p;\n+\n+\t\tif (*input.buf == line_termination)\n+\t\t\tdie(\"empty command in input\");\n+\t\telse if (isspace(*input.buf))\n+\t\t\tdie(\"whitespace before command: %s\", input.buf);\n+\n+\t\tfor (i = 0; i < ARRAY_SIZE(command); i++) {\n+\t\t\tconst char *prefix = command[i].prefix;\n+\t\t\tchar c;\n+\n+\t\t\tif (!starts_with(input.buf, prefix))\n+\t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * If the command has arguments, verify that it's\n+\t\t\t * followed by a space. Otherwise, it shall be followed\n+\t\t\t * by a line terminator.\n+\t\t\t */\n+\t\t\tc = command[i].args ? ' ' : line_termination;\n+\t\t\tif (input.buf[strlen(prefix)] != c)\n+\t\t\t\tcontinue;\n+\n+\t\t\tcmd = &command[i];\n+\t\t\tbreak;\n+\t\t}\n+\t\tif (!cmd)\n+\t\t\tdie(\"unknown command: %s\", input.buf);\n+\n+\t\t/*\n+\t\t * Read additional arguments. Do not raise an error in\n+\t\t * case there is an early EOF to let the command\n+\t\t * handle missing arguments with a proper error message\n+\t\t */\n+\t\tp = input.buf + strlen(cmd->prefix) + 1;\n+\t\tif (*p == line_termination || !*p) {\n+\t\t\tn = 0;\n+\t\t} else {\n+\t\t\tconst char *pos = strstr(p, \"\\n\");\n+\t\t\tchar *str = xstrndup(p, pos - p);\n+\n+\t\t\tn = strvec_split_delim(&args, str, ' ', - 1);\n+\t\t\tfree(str);\n+\t\t}\n+\n+\t\tif (n < cmd->args)\n+\t\t\tdie(\"%s: too few arguments\", cmd->prefix);\n+\t\tif (n > cmd->args)\n+\t\t\tdie(\"%s: too many arguments\", cmd->prefix);\n+\n+\t\tswitch (state) {\n+\t\tcase BATCH_STATE_OPEN:\n+\t\t\t/* TODO: command state management */\n+\t\t\tbreak;\n+\t\t}\n+\t\tcmd->fn(opt, args.nr, args.v, output, data);\n+\t\tstrvec_clear(&args);\n+\t}\n+\n+\tstrbuf_release(&input);\n+}\n+\n static int batch_objects(struct batch_options *opt)\n {\n \tstruct strbuf input = STRBUF_INIT;\n@@ -515,6 +639,7 @@ static int batch_objects(struct batch_options *opt)\n \tstruct expand_data data;\n \tint save_warning;\n \tint retval = 0;\n+\tconst int stdin_cmd = opt->stdin_cmd;\n \n \tif (!opt->format)\n \t\topt->format = \"%(objectname) %(objecttype) %(objectsize)\";\n@@ -590,7 +715,8 @@ static int batch_objects(struct batch_options *opt)\n \tsave_warning = warn_on_object_refname_ambiguity;\n \twarn_on_object_refname_ambiguity = 0;\n \n-\twhile (strbuf_getline(&input, stdin) != EOF) {\n+\twhile (!stdin_cmd &&\n+\t       strbuf_getline(&input, stdin) != EOF) {\n \t\tif (data.split_on_whitespace) {\n \t\t\t/*\n \t\t\t * Split at first whitespace, tying off the beginning\n@@ -608,6 +734,9 @@ static int batch_objects(struct batch_options *opt)\n \t\tbatch_one_object(input.buf, &output, opt, &data);\n \t}\n \n+\tif (stdin_cmd)\n+\t\tbatch_objects_stdin_cmd(opt, &output, &data);\n+\n \tstrbuf_release(&input);\n \tstrbuf_release(&output);\n \twarn_on_object_refname_ambiguity = save_warning;\n@@ -685,6 +814,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\t\tbatch_option_callback),\n \t\tOPT_CMDMODE(0, \"batch-all-objects\", &opt,\n \t\t\t    N_(\"with --batch[-check]: ignores stdin, batches all known objects\"), 'b'),\n+\t\tOPT_BOOL(0, \"stdin-cmd\", &batch.stdin_cmd,\n+\t\t\t N_(\"with --batch[-check]: enters stdin 'command mode\")),\n+\t\tOPT_BOOL('z', NULL, &batch.end_null, N_(\"with --stdin-cmd, use NUL termination\")),\n+\n \t\t/* Batch-specific options */\n \t\tOPT_GROUP(N_(\"Change or optimize batch output\")),\n \t\tOPT_BOOL(0, \"buffer\", &batch.buffer_output, N_(\"buffer --batch output\")),\n@@ -738,6 +871,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t/* Batch defaults */\n \tif (batch.buffer_output < 0)\n \t\tbatch.buffer_output = batch.all_objects;\n+\tif (batch.end_null)\n+\t\tline_termination = '\\0';\n \n \t/* Return early if we're in batch mode? */\n \tif (batch.enabled) {\ndiff --git a/strvec.c b/strvec.c\nindex 61a76ce6cb..7dca04bf7a 100644\n--- a/strvec.c\n+++ b/strvec.c\n@@ -85,6 +85,29 @@ void strvec_split(struct strvec *array, const char *to_split)\n \t}\n }\n \n+size_t strvec_split_delim(struct strvec *array, const char *string,\n+\t\t\t  int delim, int maxsplit)\n+{\n+\tsize_t count = 0;\n+\tconst char *p = string, *end;\n+\n+\tfor (;;) {\n+\t\tcount++;\n+\t\tif (maxsplit >= 0 && count > maxsplit) {\n+\t\t\tstrvec_push(array, p);\n+\t\t\treturn count;\n+\t\t}\n+\t\tend = strchr(p, delim);\n+\t\tif (end) {\n+\t\t\tstrvec_push_nodup(array, xmemdupz(p, end - p));\n+\t\t\tp = end + 1;\n+\t\t} else {\n+\t\t\tstrvec_push(array, p);\n+\t\t\treturn count;\n+\t\t}\n+\t}\n+}\n+\n void strvec_clear(struct strvec *array)\n {\n \tif (array->v != empty_strvec) {\ndiff --git a/strvec.h b/strvec.h\nindex 9f55c8766b..c474918b91 100644\n--- a/strvec.h\n+++ b/strvec.h\n@@ -73,6 +73,14 @@ void strvec_pop(struct strvec *);\n /* Splits by whitespace; does not handle quoted arguments! */\n void strvec_split(struct strvec *, const char *);\n \n+/**\n+ * strvec_split_delim() is a split function that behaves more like its\n+ * string_list_split() cousin than the whitespace-splitting\n+ * strvec_split().\n+ */\n+size_t strvec_split_delim(struct strvec *array, const char *string,\n+\t\t\t  int delim, int maxsplit);\n+\n /**\n  * Free all memory associated with the array and return it to the\n  * initial, empty state.\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 145eee11df..8f339746ec 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -964,4 +964,76 @@ test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace'\n \ttest_cmp expect actual\n '\n \n+F='%s\\0'\n+\n+test_expect_success 'stdin-cmd not enough arguments' '\n+\techo \"object \" >cmd &&\n+\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n+\tgrep -E \"^fatal:.*too few arguments\" err\n+'\n+\n+test_expect_success 'stdin-cmd too many arguments' '\n+\techo \"object foo bar\" >cmd &&\n+\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n+\tgrep -E \"^fatal:.*too many arguments\" err\n+'\n+\n+test_expect_success 'stdin-cmd unknown command' '\n+\techo unknown_command >cmd &&\n+\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n+\tgrep -E \"^fatal:.*unknown command.*\" err &&\n+\ttest_expect_code 128 git cat-file --batch --stdin-cmd -z < cmd 2>err &&\n+\tgrep -E \"^fatal:.*unknown command.*\" err\n+'\n+\n+test_expect_success 'setup object data' '\n+\tcontent=\"Object Data\" &&\n+\tsize=$(strlen \"$content\") &&\n+\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n+'\n+\n+test_expect_success 'stdin-cmd calling object works' '\n+\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n+\techo \"$sha1 blob $size\" >expect &&\n+\techo `git cat-file -p \"$sha1\"` >>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin-cmd -z calling object works' '\n+\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n+\techo \"$sha1 blob $size\" >expect &&\n+\techo `git cat-file -p \"$sha1\"` >>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'setup object data' '\n+\tcontent=\"Object Data\" &&\n+\tsize=$(strlen \"$content\") &&\n+\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n+'\n+\n+test_expect_success 'stdin-cmd calling object works' '\n+\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n+\techo \"$sha1 blob $size\" >expect &&\n+\techo `git cat-file -p \"$sha1\"` >>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin-cmd -z calling object works' '\n+\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n+\techo \"$sha1 blob $size\" >expect &&\n+\techo `git cat-file -p \"$sha1\"` >>expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin-cmd fflush works' '\n+\tprintf \"fflush\\n\" > cmd &&\n+\ttest_expect_code 0 git cat-file --batch --stdin-cmd < cmd 2>err\n+'\n+\n+test_expect_success 'stdin-cmd -z fflush works' '\n+\tprintf $F 'fflush' > cmd &&\n+\ttest_expect_code 0 git cat-file --batch --stdin-cmd -z < cmd 2>err\n+'\n+\n test_done\n-- \n2.34.1\n\n"},{"id":"446717","messageId":"289ffedd-3cf1-408a-88f5-8461842cb6be@gmail.com","threadId":"57283","inReplyTo":"20220121175405.45753-1-johncai86@gmail.com","subject":"Re: [RFC] cat-file: add a --stdin-cmd mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-01-23T15:11:02Z","receivedAt":"2022-01-23T15:11:12Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi John\n\nOn 21/01/2022 17:54, John Cai wrote:\n> This RFC patch proposes a new flag --stdin-cmd that works with\n> git-cat-file --batch. Similar to git-update-ref --stdin, it will accept\n> commands and arguments from stdin.\n\nLets start by bike shedding about the name! I think all the other \ncat-file options that read from stdin are called --batch[-something] so \nperhaps this could follow the existing pattern.\n\n> The start of this idea was discussed in [1], where the original\n> motivation was to be able to control when the buffer was flushed to\n> stdout in --buffer mode.\n\nThat's useful background, thanks\n\n> However, this can actually be much more useful in situations when\n> git-cat-file --batch is being used as a long lived backend query\n> process. At GitLab, we use a pair of cat-file processes. One for\n> iterating over object metadata with --batch-check, and the other to grab\n> object contents with --batch. However, if we had --stdin-cmd, we could\n> get rid of the second --batch-check process, and just have one progress\n> where we can flip between getting object info, and getting object contents.\n> This can lead to huge savings.\n\nIt is just getting rid of a single extra process so are the huge savings \ncoming because there are lots of these pairs of cat-file processes?\n\n> git cat-file --batch --stdin-cmd\n> \n> $ <command> [arg1] [arg2] NL\n> \n> We can also add a -z mode to allow for NUL-terminated lines\n\nThe -z flag is a very welcome addition and will help address some \nexisting problems with cat-file --batch[1]. This should be an \nuncontroversial change and could be submitted separately from the rest \nof the changes in this patch.\n\n> $ <command> [arg1] [arg2] NUL\n> \n> This patch adds three commands: object, info, fflush\n> \n> $ object <sha1> NL\n> $ info <sha1> NL\n> $ fflush NL\n> \n> These three would be immediately useful in GitLab's context, but one can\n> imagine this mode to be further extended for other things.\n\nI think it would be good to define what input format we want to support. \nThe code in this patch does not handle input like \"object HEAD:name with \nspaces\" which is a shame. It would be good to retain the ability to add \ncommands that take arguments with spaces in them in the future, for \nexample one might want to pass a format string as an argument to some \nfuture command. Making each command responsible for parsing its own \narguments would help fix this rather than just splitting on input on \neach space and passing that list to the commands.\n\nThe basic premise of adding a new command line option that reads a \ncommand and arguments seems like a sensible way to extend the \nfunctionality of cat-file.\n\n[For future reference I think the command name parsing could be cleaner \nif it used skip_prefix() rather than starts_with()]\n\n> For instance, a non-trivial part of \"cat-file --batch\" time is spent\n> on parsing its argument and seeing if it's a revision, ref etc. So we\n> could add a command that only accepts a full-length 40\n> character SHA-1.\n\nIt would be good to get a better understanding of what fraction of the \ntime git spends parsing the argument and what it is doing in that time. \nIf we can speed up the existing code then everyone will benefit from the \nchange, not just those who switch to the new command.\n\n\nBest Wishes\n\nPhillip\n\n[1] \nhttps://lore.kernel.org/git/20191008200050.GA26453@sigill.intra.peff.net/\n\n> This would be the first step in adding such an interface to\n> git-cat-file.\n> \n> [1] https://lore.kernel.org/git/pull.1124.git.git.1636149400.gitgitgadget@gmail.com/\n> \n> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> Signed-off-by: John Cai <johncai86@gmail.com>\n> ---\n>   builtin/cat-file.c  | 137 +++++++++++++++++++++++++++++++++++++++++++-\n>   strvec.c            |  23 ++++++++\n>   strvec.h            |   8 +++\n>   t/t1006-cat-file.sh |  72 +++++++++++++++++++++++\n>   4 files changed, 239 insertions(+), 1 deletion(-)\n> \n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 7b3f42950e..2679b34b43 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -16,6 +16,7 @@\n>   #include \"packfile.h\"\n>   #include \"object-store.h\"\n>   #include \"promisor-remote.h\"\n> +#include \"strvec.h\"\n>   \n>   struct batch_options {\n>   \tint enabled;\n> @@ -26,7 +27,10 @@ struct batch_options {\n>   \tint unordered;\n>   \tint cmdmode; /* may be 'w' or 'c' for --filters or --textconv */\n>   \tconst char *format;\n> +\tint stdin_cmd;\n> +\tint end_null;\n>   };\n> +static char line_termination = '\\n';\n>   \n>   static const char *force_path;\n>   \n> @@ -508,6 +512,126 @@ static int batch_unordered_packed(const struct object_id *oid,\n>   \t\t\t\t      data);\n>   }\n>   \n> +enum batch_state {\n> +\t/* Non-transactional state open for commands. */\n> +\tBATCH_STATE_OPEN,\n> +};\n> +\n> +static void parse_cmd_object(struct batch_options *opt,\n> +\t\t\t     const int argc, const char **argv,\n> +\t\t\t     struct strbuf *output,\n> +\t\t\t     struct expand_data *data)\n> +{\n> +\tbatch_one_object(argv[0], output, opt, data);\n> +}\n> +\n> +static void parse_cmd_info(struct batch_options *opt,\n> +\t\t\t     const int argc, const char **argv,\n> +\t\t\t     struct strbuf *output,\n> +\t\t\t     struct expand_data *data)\n> +{\n> +\topt->print_contents = 0;\n> +\tbatch_one_object(argv[0], output, opt, data);\n> +}\n> +\n> +static void parse_cmd_fflush(struct batch_options *opt,\n> +\t\t\t     const int argc, const char **argv,\n> +\t\t\t     struct strbuf *output,\n> +\t\t\t     struct expand_data *data)\n> +{\n> +\tfflush(stdout);\n> +}\n> +\n> +typedef void (*parse_cmd_fn_t)(struct batch_options *, const int,\n> +\t\t\t       const char **, struct strbuf *,\n> +\t\t\t       struct expand_data *);\n> +\n> +static const struct parse_cmd {\n> +\tconst char *prefix;\n> +\tparse_cmd_fn_t fn;\n> +\tunsigned args;\n> +\tenum batch_state state;\n> +} command[] = {\n> +\t{ \"object\", parse_cmd_object, 1, BATCH_STATE_OPEN },\n> +\t{ \"info\", parse_cmd_info, 1, BATCH_STATE_OPEN },\n> +\t{ \"fflush\", parse_cmd_fflush, 0, BATCH_STATE_OPEN },\n> +};\n> +\n> +static void batch_objects_stdin_cmd(struct batch_options *opt,\n> +\t\t\t\t    struct strbuf *output,\n> +\t\t\t\t    struct expand_data *data)\n> +{\n> +\tstruct strbuf input = STRBUF_INIT;\n> +\tenum batch_state state = BATCH_STATE_OPEN;\n> +\n> +\t/* Read each line dispatch its command */\n> +\twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n> +\t\tint i;\n> +\t\tconst struct parse_cmd *cmd = NULL;\n> +\t\tstruct strvec args = STRVEC_INIT;\n> +\t\tsize_t n;\n> +\t\tconst char *p;\n> +\n> +\t\tif (*input.buf == line_termination)\n> +\t\t\tdie(\"empty command in input\");\n> +\t\telse if (isspace(*input.buf))\n> +\t\t\tdie(\"whitespace before command: %s\", input.buf);\n> +\n> +\t\tfor (i = 0; i < ARRAY_SIZE(command); i++) {\n> +\t\t\tconst char *prefix = command[i].prefix;\n> +\t\t\tchar c;\n> +\n> +\t\t\tif (!starts_with(input.buf, prefix))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\t/*\n> +\t\t\t * If the command has arguments, verify that it's\n> +\t\t\t * followed by a space. Otherwise, it shall be followed\n> +\t\t\t * by a line terminator.\n> +\t\t\t */\n> +\t\t\tc = command[i].args ? ' ' : line_termination;\n> +\t\t\tif (input.buf[strlen(prefix)] != c)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tcmd = &command[i];\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tif (!cmd)\n> +\t\t\tdie(\"unknown command: %s\", input.buf);\n> +\n> +\t\t/*\n> +\t\t * Read additional arguments. Do not raise an error in\n> +\t\t * case there is an early EOF to let the command\n> +\t\t * handle missing arguments with a proper error message\n> +\t\t */\n> +\t\tp = input.buf + strlen(cmd->prefix) + 1;\n> +\t\tif (*p == line_termination || !*p) {\n> +\t\t\tn = 0;\n> +\t\t} else {\n> +\t\t\tconst char *pos = strstr(p, \"\\n\");\n> +\t\t\tchar *str = xstrndup(p, pos - p);\n> +\n> +\t\t\tn = strvec_split_delim(&args, str, ' ', - 1);\n> +\t\t\tfree(str);\n> +\t\t}\n> +\n> +\t\tif (n < cmd->args)\n> +\t\t\tdie(\"%s: too few arguments\", cmd->prefix);\n> +\t\tif (n > cmd->args)\n> +\t\t\tdie(\"%s: too many arguments\", cmd->prefix);\n> +\n> +\t\tswitch (state) {\n> +\t\tcase BATCH_STATE_OPEN:\n> +\t\t\t/* TODO: command state management */\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcmd->fn(opt, args.nr, args.v, output, data);\n> +\t\tstrvec_clear(&args);\n> +\t}\n> +\n> +\tstrbuf_release(&input);\n> +}\n> +\n>   static int batch_objects(struct batch_options *opt)\n>   {\n>   \tstruct strbuf input = STRBUF_INIT;\n> @@ -515,6 +639,7 @@ static int batch_objects(struct batch_options *opt)\n>   \tstruct expand_data data;\n>   \tint save_warning;\n>   \tint retval = 0;\n> +\tconst int stdin_cmd = opt->stdin_cmd;\n>   \n>   \tif (!opt->format)\n>   \t\topt->format = \"%(objectname) %(objecttype) %(objectsize)\";\n> @@ -590,7 +715,8 @@ static int batch_objects(struct batch_options *opt)\n>   \tsave_warning = warn_on_object_refname_ambiguity;\n>   \twarn_on_object_refname_ambiguity = 0;\n>   \n> -\twhile (strbuf_getline(&input, stdin) != EOF) {\n> +\twhile (!stdin_cmd &&\n> +\t       strbuf_getline(&input, stdin) != EOF) {\n>   \t\tif (data.split_on_whitespace) {\n>   \t\t\t/*\n>   \t\t\t * Split at first whitespace, tying off the beginning\n> @@ -608,6 +734,9 @@ static int batch_objects(struct batch_options *opt)\n>   \t\tbatch_one_object(input.buf, &output, opt, &data);\n>   \t}\n>   \n> +\tif (stdin_cmd)\n> +\t\tbatch_objects_stdin_cmd(opt, &output, &data);\n> +\n>   \tstrbuf_release(&input);\n>   \tstrbuf_release(&output);\n>   \twarn_on_object_refname_ambiguity = save_warning;\n> @@ -685,6 +814,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>   \t\t\tbatch_option_callback),\n>   \t\tOPT_CMDMODE(0, \"batch-all-objects\", &opt,\n>   \t\t\t    N_(\"with --batch[-check]: ignores stdin, batches all known objects\"), 'b'),\n> +\t\tOPT_BOOL(0, \"stdin-cmd\", &batch.stdin_cmd,\n> +\t\t\t N_(\"with --batch[-check]: enters stdin 'command mode\")),\n> +\t\tOPT_BOOL('z', NULL, &batch.end_null, N_(\"with --stdin-cmd, use NUL termination\")),\n> +\n>   \t\t/* Batch-specific options */\n>   \t\tOPT_GROUP(N_(\"Change or optimize batch output\")),\n>   \t\tOPT_BOOL(0, \"buffer\", &batch.buffer_output, N_(\"buffer --batch output\")),\n> @@ -738,6 +871,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>   \t/* Batch defaults */\n>   \tif (batch.buffer_output < 0)\n>   \t\tbatch.buffer_output = batch.all_objects;\n> +\tif (batch.end_null)\n> +\t\tline_termination = '\\0';\n>   \n>   \t/* Return early if we're in batch mode? */\n>   \tif (batch.enabled) {\n> diff --git a/strvec.c b/strvec.c\n> index 61a76ce6cb..7dca04bf7a 100644\n> --- a/strvec.c\n> +++ b/strvec.c\n> @@ -85,6 +85,29 @@ void strvec_split(struct strvec *array, const char *to_split)\n>   \t}\n>   }\n>   \n> +size_t strvec_split_delim(struct strvec *array, const char *string,\n> +\t\t\t  int delim, int maxsplit)\n> +{\n> +\tsize_t count = 0;\n> +\tconst char *p = string, *end;\n> +\n> +\tfor (;;) {\n> +\t\tcount++;\n> +\t\tif (maxsplit >= 0 && count > maxsplit) {\n> +\t\t\tstrvec_push(array, p);\n> +\t\t\treturn count;\n> +\t\t}\n> +\t\tend = strchr(p, delim);\n> +\t\tif (end) {\n> +\t\t\tstrvec_push_nodup(array, xmemdupz(p, end - p));\n> +\t\t\tp = end + 1;\n> +\t\t} else {\n> +\t\t\tstrvec_push(array, p);\n> +\t\t\treturn count;\n> +\t\t}\n> +\t}\n> +}\n> +\n>   void strvec_clear(struct strvec *array)\n>   {\n>   \tif (array->v != empty_strvec) {\n> diff --git a/strvec.h b/strvec.h\n> index 9f55c8766b..c474918b91 100644\n> --- a/strvec.h\n> +++ b/strvec.h\n> @@ -73,6 +73,14 @@ void strvec_pop(struct strvec *);\n>   /* Splits by whitespace; does not handle quoted arguments! */\n>   void strvec_split(struct strvec *, const char *);\n>   \n> +/**\n> + * strvec_split_delim() is a split function that behaves more like its\n> + * string_list_split() cousin than the whitespace-splitting\n> + * strvec_split().\n> + */\n> +size_t strvec_split_delim(struct strvec *array, const char *string,\n> +\t\t\t  int delim, int maxsplit);\n> +\n>   /**\n>    * Free all memory associated with the array and return it to the\n>    * initial, empty state.\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index 145eee11df..8f339746ec 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -964,4 +964,76 @@ test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace'\n>   \ttest_cmp expect actual\n>   '\n>   \n> +F='%s\\0'\n> +\n> +test_expect_success 'stdin-cmd not enough arguments' '\n> +\techo \"object \" >cmd &&\n> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n> +\tgrep -E \"^fatal:.*too few arguments\" err\n> +'\n> +\n> +test_expect_success 'stdin-cmd too many arguments' '\n> +\techo \"object foo bar\" >cmd &&\n> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n> +\tgrep -E \"^fatal:.*too many arguments\" err\n> +'\n> +\n> +test_expect_success 'stdin-cmd unknown command' '\n> +\techo unknown_command >cmd &&\n> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n> +\tgrep -E \"^fatal:.*unknown command.*\" err &&\n> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd -z < cmd 2>err &&\n> +\tgrep -E \"^fatal:.*unknown command.*\" err\n> +'\n> +\n> +test_expect_success 'setup object data' '\n> +\tcontent=\"Object Data\" &&\n> +\tsize=$(strlen \"$content\") &&\n> +\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n> +'\n> +\n> +test_expect_success 'stdin-cmd calling object works' '\n> +\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n> +\techo \"$sha1 blob $size\" >expect &&\n> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'stdin-cmd -z calling object works' '\n> +\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n> +\techo \"$sha1 blob $size\" >expect &&\n> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'setup object data' '\n> +\tcontent=\"Object Data\" &&\n> +\tsize=$(strlen \"$content\") &&\n> +\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n> +'\n> +\n> +test_expect_success 'stdin-cmd calling object works' '\n> +\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n> +\techo \"$sha1 blob $size\" >expect &&\n> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'stdin-cmd -z calling object works' '\n> +\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n> +\techo \"$sha1 blob $size\" >expect &&\n> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'stdin-cmd fflush works' '\n> +\tprintf \"fflush\\n\" > cmd &&\n> +\ttest_expect_code 0 git cat-file --batch --stdin-cmd < cmd 2>err\n> +'\n> +\n> +test_expect_success 'stdin-cmd -z fflush works' '\n> +\tprintf $F 'fflush' > cmd &&\n> +\ttest_expect_code 0 git cat-file --batch --stdin-cmd -z < cmd 2>err\n> +'\n> +\n>   test_done\n"},{"id":"446750","messageId":"55726CC9-7734-40A0-88B4-9D24F41C01B6@gmail.com","threadId":"57283","inReplyTo":"289ffedd-3cf1-408a-88f5-8461842cb6be@gmail.com","subject":"Re: [RFC] cat-file: add a --stdin-cmd mode","fromName":"John Cai","fromEmail":"johncai86@gmail.com","sentAt":"2022-01-24T14:29:47Z","receivedAt":"2022-01-24T14:30:10Z","isPatch":false,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"\n\nOn 23 Jan 2022, at 10:11, Phillip Wood wrote:\n\n> Hi John\n>\n> On 21/01/2022 17:54, John Cai wrote:\n>> This RFC patch proposes a new flag --stdin-cmd that works with\n>> git-cat-file --batch. Similar to git-update-ref --stdin, it will accept\n>> commands and arguments from stdin.\n>\n> Lets start by bike shedding about the name! I think all the other cat-file options that read from stdin are called --batch[-something] so perhaps this could follow the existing pattern.\n\nYeah perhaps it would be more clear for this to have its own name. Maybe --batch-cmd? --batch-stdin-cmd seems too long and clunky.\n>\n>> The start of this idea was discussed in [1], where the original\n>> motivation was to be able to control when the buffer was flushed to\n>> stdout in --buffer mode.\n>\n> That's useful background, thanks\n>\n>> However, this can actually be much more useful in situations when\n>> git-cat-file --batch is being used as a long lived backend query\n>> process. At GitLab, we use a pair of cat-file processes. One for\n>> iterating over object metadata with --batch-check, and the other to grab\n>> object contents with --batch. However, if we had --stdin-cmd, we could\n>> get rid of the second --batch-check process, and just have one progress\n>> where we can flip between getting object info, and getting object contents.\n>> This can lead to huge savings.\n>\n> It is just getting rid of a single extra process so are the huge savings coming because there are lots of these pairs of cat-file processes?\n\nRight, I left out that detail. So we have many such processes at any given time that grab all sorts of objects from the odb. So this would allow us to cut our number of git processes by about half.\n>\n>> git cat-file --batch --stdin-cmd\n>>\n>> $ <command> [arg1] [arg2] NL\n>>\n>> We can also add a -z mode to allow for NUL-terminated lines\n>\n> The -z flag is a very welcome addition and will help address some existing problems with cat-file --batch[1]. This should be an uncontroversial change and could be submitted separately from the rest of the changes in this patch.\n\nSounds good to me. That can be a later iteration.\n>\n>> $ <command> [arg1] [arg2] NUL\n>>\n>> This patch adds three commands: object, info, fflush\n>>\n>> $ object <sha1> NL\n>> $ info <sha1> NL\n>> $ fflush NL\n>>\n>> These three would be immediately useful in GitLab's context, but one can\n>> imagine this mode to be further extended for other things.\n>\n> I think it would be good to define what input format we want to support. The code in this patch does not handle input like \"object HEAD:name with spaces\" which is a shame. It would be good to retain the ability to add commands that take arguments with spaces in them in the future, for example one might want to pass a format string as an argument to some future command. Making each command responsible for parsing its own arguments would help fix this rather than just splitting on input on each space and passing that list to the commands.\n\nThis is good feedback. I can see the value of a more flexible interface for the handler functions.\n\n>\n> The basic premise of adding a new command line option that reads a command and arguments seems like a sensible way to extend the functionality of cat-file.\n>\n> [For future reference I think the command name parsing could be cleaner if it used skip_prefix() rather than starts_with()]\n>\n>> For instance, a non-trivial part of \"cat-file --batch\" time is spent\n>> on parsing its argument and seeing if it's a revision, ref etc. So we\n>> could add a command that only accepts a full-length 40\n>> character SHA-1.\n>\n> It would be good to get a better understanding of what fraction of the time git spends parsing the argument and what it is doing in that time. If we can speed up the existing code then everyone will benefit from the change, not just those who switch to the new command.\n\nGood question. I would need to do some investigation. This is still only a possible future enhancement so it wouldn’t be included in the first iteration.\n\nThanks for the thoughtful review!\n\n>\n>\n> Best Wishes\n>\n> Phillip\n>\n> [1] https://lore.kernel.org/git/20191008200050.GA26453@sigill.intra.peff.net/\n>\n>> This would be the first step in adding such an interface to\n>> git-cat-file.\n>>\n>> [1] https://lore.kernel.org/git/pull.1124.git.git.1636149400.gitgitgadget@gmail.com/\n>>\n>> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>> Signed-off-by: John Cai <johncai86@gmail.com>\n>> ---\n>>   builtin/cat-file.c  | 137 +++++++++++++++++++++++++++++++++++++++++++-\n>>   strvec.c            |  23 ++++++++\n>>   strvec.h            |   8 +++\n>>   t/t1006-cat-file.sh |  72 +++++++++++++++++++++++\n>>   4 files changed, 239 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n>> index 7b3f42950e..2679b34b43 100644\n>> --- a/builtin/cat-file.c\n>> +++ b/builtin/cat-file.c\n>> @@ -16,6 +16,7 @@\n>>   #include \"packfile.h\"\n>>   #include \"object-store.h\"\n>>   #include \"promisor-remote.h\"\n>> +#include \"strvec.h\"\n>>    struct batch_options {\n>>   \tint enabled;\n>> @@ -26,7 +27,10 @@ struct batch_options {\n>>   \tint unordered;\n>>   \tint cmdmode; /* may be 'w' or 'c' for --filters or --textconv */\n>>   \tconst char *format;\n>> +\tint stdin_cmd;\n>> +\tint end_null;\n>>   };\n>> +static char line_termination = '\\n';\n>>    static const char *force_path;\n>>  @@ -508,6 +512,126 @@ static int batch_unordered_packed(const struct object_id *oid,\n>>   \t\t\t\t      data);\n>>   }\n>>  +enum batch_state {\n>> +\t/* Non-transactional state open for commands. */\n>> +\tBATCH_STATE_OPEN,\n>> +};\n>> +\n>> +static void parse_cmd_object(struct batch_options *opt,\n>> +\t\t\t     const int argc, const char **argv,\n>> +\t\t\t     struct strbuf *output,\n>> +\t\t\t     struct expand_data *data)\n>> +{\n>> +\tbatch_one_object(argv[0], output, opt, data);\n>> +}\n>> +\n>> +static void parse_cmd_info(struct batch_options *opt,\n>> +\t\t\t     const int argc, const char **argv,\n>> +\t\t\t     struct strbuf *output,\n>> +\t\t\t     struct expand_data *data)\n>> +{\n>> +\topt->print_contents = 0;\n>> +\tbatch_one_object(argv[0], output, opt, data);\n>> +}\n>> +\n>> +static void parse_cmd_fflush(struct batch_options *opt,\n>> +\t\t\t     const int argc, const char **argv,\n>> +\t\t\t     struct strbuf *output,\n>> +\t\t\t     struct expand_data *data)\n>> +{\n>> +\tfflush(stdout);\n>> +}\n>> +\n>> +typedef void (*parse_cmd_fn_t)(struct batch_options *, const int,\n>> +\t\t\t       const char **, struct strbuf *,\n>> +\t\t\t       struct expand_data *);\n>> +\n>> +static const struct parse_cmd {\n>> +\tconst char *prefix;\n>> +\tparse_cmd_fn_t fn;\n>> +\tunsigned args;\n>> +\tenum batch_state state;\n>> +} command[] = {\n>> +\t{ \"object\", parse_cmd_object, 1, BATCH_STATE_OPEN },\n>> +\t{ \"info\", parse_cmd_info, 1, BATCH_STATE_OPEN },\n>> +\t{ \"fflush\", parse_cmd_fflush, 0, BATCH_STATE_OPEN },\n>> +};\n>> +\n>> +static void batch_objects_stdin_cmd(struct batch_options *opt,\n>> +\t\t\t\t    struct strbuf *output,\n>> +\t\t\t\t    struct expand_data *data)\n>> +{\n>> +\tstruct strbuf input = STRBUF_INIT;\n>> +\tenum batch_state state = BATCH_STATE_OPEN;\n>> +\n>> +\t/* Read each line dispatch its command */\n>> +\twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n>> +\t\tint i;\n>> +\t\tconst struct parse_cmd *cmd = NULL;\n>> +\t\tstruct strvec args = STRVEC_INIT;\n>> +\t\tsize_t n;\n>> +\t\tconst char *p;\n>> +\n>> +\t\tif (*input.buf == line_termination)\n>> +\t\t\tdie(\"empty command in input\");\n>> +\t\telse if (isspace(*input.buf))\n>> +\t\t\tdie(\"whitespace before command: %s\", input.buf);\n>> +\n>> +\t\tfor (i = 0; i < ARRAY_SIZE(command); i++) {\n>> +\t\t\tconst char *prefix = command[i].prefix;\n>> +\t\t\tchar c;\n>> +\n>> +\t\t\tif (!starts_with(input.buf, prefix))\n>> +\t\t\t\tcontinue;\n>> +\n>> +\t\t\t/*\n>> +\t\t\t * If the command has arguments, verify that it's\n>> +\t\t\t * followed by a space. Otherwise, it shall be followed\n>> +\t\t\t * by a line terminator.\n>> +\t\t\t */\n>> +\t\t\tc = command[i].args ? ' ' : line_termination;\n>> +\t\t\tif (input.buf[strlen(prefix)] != c)\n>> +\t\t\t\tcontinue;\n>> +\n>> +\t\t\tcmd = &command[i];\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tif (!cmd)\n>> +\t\t\tdie(\"unknown command: %s\", input.buf);\n>> +\n>> +\t\t/*\n>> +\t\t * Read additional arguments. Do not raise an error in\n>> +\t\t * case there is an early EOF to let the command\n>> +\t\t * handle missing arguments with a proper error message\n>> +\t\t */\n>> +\t\tp = input.buf + strlen(cmd->prefix) + 1;\n>> +\t\tif (*p == line_termination || !*p) {\n>> +\t\t\tn = 0;\n>> +\t\t} else {\n>> +\t\t\tconst char *pos = strstr(p, \"\\n\");\n>> +\t\t\tchar *str = xstrndup(p, pos - p);\n>> +\n>> +\t\t\tn = strvec_split_delim(&args, str, ' ', - 1);\n>> +\t\t\tfree(str);\n>> +\t\t}\n>> +\n>> +\t\tif (n < cmd->args)\n>> +\t\t\tdie(\"%s: too few arguments\", cmd->prefix);\n>> +\t\tif (n > cmd->args)\n>> +\t\t\tdie(\"%s: too many arguments\", cmd->prefix);\n>> +\n>> +\t\tswitch (state) {\n>> +\t\tcase BATCH_STATE_OPEN:\n>> +\t\t\t/* TODO: command state management */\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tcmd->fn(opt, args.nr, args.v, output, data);\n>> +\t\tstrvec_clear(&args);\n>> +\t}\n>> +\n>> +\tstrbuf_release(&input);\n>> +}\n>> +\n>>   static int batch_objects(struct batch_options *opt)\n>>   {\n>>   \tstruct strbuf input = STRBUF_INIT;\n>> @@ -515,6 +639,7 @@ static int batch_objects(struct batch_options *opt)\n>>   \tstruct expand_data data;\n>>   \tint save_warning;\n>>   \tint retval = 0;\n>> +\tconst int stdin_cmd = opt->stdin_cmd;\n>>    \tif (!opt->format)\n>>   \t\topt->format = \"%(objectname) %(objecttype) %(objectsize)\";\n>> @@ -590,7 +715,8 @@ static int batch_objects(struct batch_options *opt)\n>>   \tsave_warning = warn_on_object_refname_ambiguity;\n>>   \twarn_on_object_refname_ambiguity = 0;\n>>  -\twhile (strbuf_getline(&input, stdin) != EOF) {\n>> +\twhile (!stdin_cmd &&\n>> +\t       strbuf_getline(&input, stdin) != EOF) {\n>>   \t\tif (data.split_on_whitespace) {\n>>   \t\t\t/*\n>>   \t\t\t * Split at first whitespace, tying off the beginning\n>> @@ -608,6 +734,9 @@ static int batch_objects(struct batch_options *opt)\n>>   \t\tbatch_one_object(input.buf, &output, opt, &data);\n>>   \t}\n>>  +\tif (stdin_cmd)\n>> +\t\tbatch_objects_stdin_cmd(opt, &output, &data);\n>> +\n>>   \tstrbuf_release(&input);\n>>   \tstrbuf_release(&output);\n>>   \twarn_on_object_refname_ambiguity = save_warning;\n>> @@ -685,6 +814,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>>   \t\t\tbatch_option_callback),\n>>   \t\tOPT_CMDMODE(0, \"batch-all-objects\", &opt,\n>>   \t\t\t    N_(\"with --batch[-check]: ignores stdin, batches all known objects\"), 'b'),\n>> +\t\tOPT_BOOL(0, \"stdin-cmd\", &batch.stdin_cmd,\n>> +\t\t\t N_(\"with --batch[-check]: enters stdin 'command mode\")),\n>> +\t\tOPT_BOOL('z', NULL, &batch.end_null, N_(\"with --stdin-cmd, use NUL termination\")),\n>> +\n>>   \t\t/* Batch-specific options */\n>>   \t\tOPT_GROUP(N_(\"Change or optimize batch output\")),\n>>   \t\tOPT_BOOL(0, \"buffer\", &batch.buffer_output, N_(\"buffer --batch output\")),\n>> @@ -738,6 +871,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>>   \t/* Batch defaults */\n>>   \tif (batch.buffer_output < 0)\n>>   \t\tbatch.buffer_output = batch.all_objects;\n>> +\tif (batch.end_null)\n>> +\t\tline_termination = '\\0';\n>>    \t/* Return early if we're in batch mode? */\n>>   \tif (batch.enabled) {\n>> diff --git a/strvec.c b/strvec.c\n>> index 61a76ce6cb..7dca04bf7a 100644\n>> --- a/strvec.c\n>> +++ b/strvec.c\n>> @@ -85,6 +85,29 @@ void strvec_split(struct strvec *array, const char *to_split)\n>>   \t}\n>>   }\n>>  +size_t strvec_split_delim(struct strvec *array, const char *string,\n>> +\t\t\t  int delim, int maxsplit)\n>> +{\n>> +\tsize_t count = 0;\n>> +\tconst char *p = string, *end;\n>> +\n>> +\tfor (;;) {\n>> +\t\tcount++;\n>> +\t\tif (maxsplit >= 0 && count > maxsplit) {\n>> +\t\t\tstrvec_push(array, p);\n>> +\t\t\treturn count;\n>> +\t\t}\n>> +\t\tend = strchr(p, delim);\n>> +\t\tif (end) {\n>> +\t\t\tstrvec_push_nodup(array, xmemdupz(p, end - p));\n>> +\t\t\tp = end + 1;\n>> +\t\t} else {\n>> +\t\t\tstrvec_push(array, p);\n>> +\t\t\treturn count;\n>> +\t\t}\n>> +\t}\n>> +}\n>> +\n>>   void strvec_clear(struct strvec *array)\n>>   {\n>>   \tif (array->v != empty_strvec) {\n>> diff --git a/strvec.h b/strvec.h\n>> index 9f55c8766b..c474918b91 100644\n>> --- a/strvec.h\n>> +++ b/strvec.h\n>> @@ -73,6 +73,14 @@ void strvec_pop(struct strvec *);\n>>   /* Splits by whitespace; does not handle quoted arguments! */\n>>   void strvec_split(struct strvec *, const char *);\n>>  +/**\n>> + * strvec_split_delim() is a split function that behaves more like its\n>> + * string_list_split() cousin than the whitespace-splitting\n>> + * strvec_split().\n>> + */\n>> +size_t strvec_split_delim(struct strvec *array, const char *string,\n>> +\t\t\t  int delim, int maxsplit);\n>> +\n>>   /**\n>>    * Free all memory associated with the array and return it to the\n>>    * initial, empty state.\n>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n>> index 145eee11df..8f339746ec 100755\n>> --- a/t/t1006-cat-file.sh\n>> +++ b/t/t1006-cat-file.sh\n>> @@ -964,4 +964,76 @@ test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace'\n>>   \ttest_cmp expect actual\n>>   '\n>>  +F='%s\\0'\n>> +\n>> +test_expect_success 'stdin-cmd not enough arguments' '\n>> +\techo \"object \" >cmd &&\n>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n>> +\tgrep -E \"^fatal:.*too few arguments\" err\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd too many arguments' '\n>> +\techo \"object foo bar\" >cmd &&\n>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n>> +\tgrep -E \"^fatal:.*too many arguments\" err\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd unknown command' '\n>> +\techo unknown_command >cmd &&\n>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n>> +\tgrep -E \"^fatal:.*unknown command.*\" err &&\n>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd -z < cmd 2>err &&\n>> +\tgrep -E \"^fatal:.*unknown command.*\" err\n>> +'\n>> +\n>> +test_expect_success 'setup object data' '\n>> +\tcontent=\"Object Data\" &&\n>> +\tsize=$(strlen \"$content\") &&\n>> +\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd calling object works' '\n>> +\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n>> +\techo \"$sha1 blob $size\" >expect &&\n>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd -z calling object works' '\n>> +\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n>> +\techo \"$sha1 blob $size\" >expect &&\n>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'setup object data' '\n>> +\tcontent=\"Object Data\" &&\n>> +\tsize=$(strlen \"$content\") &&\n>> +\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd calling object works' '\n>> +\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n>> +\techo \"$sha1 blob $size\" >expect &&\n>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd -z calling object works' '\n>> +\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n>> +\techo \"$sha1 blob $size\" >expect &&\n>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd fflush works' '\n>> +\tprintf \"fflush\\n\" > cmd &&\n>> +\ttest_expect_code 0 git cat-file --batch --stdin-cmd < cmd 2>err\n>> +'\n>> +\n>> +test_expect_success 'stdin-cmd -z fflush works' '\n>> +\tprintf $F 'fflush' > cmd &&\n>> +\ttest_expect_code 0 git cat-file --batch --stdin-cmd -z < cmd 2>err\n>> +'\n>> +\n>>   test_done\n"},{"id":"446847","messageId":"7bb67a30-3f51-bf40-604b-12d96ad65737@gmail.com","threadId":"57283","inReplyTo":"55726CC9-7734-40A0-88B4-9D24F41C01B6@gmail.com","subject":"Re: [RFC] cat-file: add a --stdin-cmd mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-01-25T13:57:20Z","receivedAt":"2022-01-25T14:00:06Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi John\n\nOn 24/01/2022 14:29, John Cai wrote:\n> On 23 Jan 2022, at 10:11, Phillip Wood wrote:\n> \n>> Hi John\n>>\n>> On 21/01/2022 17:54, John Cai wrote:\n>>> This RFC patch proposes a new flag --stdin-cmd that works with\n>>> git-cat-file --batch. Similar to git-update-ref --stdin, it will accept\n>>> commands and arguments from stdin.\n>>\n>> Lets start by bike shedding about the name! I think all the other cat-file options that read from stdin are called --batch[-something] so perhaps this could follow the existing pattern.\n> \n> Yeah perhaps it would be more clear for this to have its own name. Maybe --batch-cmd? --batch-stdin-cmd seems too long and clunky.\n\nI agree --batch-stdin-cmd is a bit clunky, --batch-command might work\n\n>>> The start of this idea was discussed in [1], where the original\n>>> motivation was to be able to control when the buffer was flushed to\n>>> stdout in --buffer mode.\n>>\n>> That's useful background, thanks\n>>\n>>> However, this can actually be much more useful in situations when\n>>> git-cat-file --batch is being used as a long lived backend query\n>>> process. At GitLab, we use a pair of cat-file processes. One for\n>>> iterating over object metadata with --batch-check, and the other to grab\n>>> object contents with --batch. However, if we had --stdin-cmd, we could\n>>> get rid of the second --batch-check process, and just have one progress\n>>> where we can flip between getting object info, and getting object contents.\n>>> This can lead to huge savings.\n>>\n>> It is just getting rid of a single extra process so are the huge savings coming because there are lots of these pairs of cat-file processes?\n> \n> Right, I left out that detail. So we have many such processes at any given time that grab all sorts of objects from the odb. So this would allow us to cut our number of git processes by about half.\n\nThanks for explaining, that make sense\n\n>>> git cat-file --batch --stdin-cmd\n>>>\n>>> $ <command> [arg1] [arg2] NL\n>>>\n>>> We can also add a -z mode to allow for NUL-terminated lines\n>>\n>> The -z flag is a very welcome addition and will help address some existing problems with cat-file --batch[1]. This should be an uncontroversial change and could be submitted separately from the rest of the changes in this patch.\n> \n> Sounds good to me. That can be a later iteration.\n\nOr an earlier separate series if you want\n\nBest Wishes\n\nPhillip\n\n>>> $ <command> [arg1] [arg2] NUL\n>>>\n>>> This patch adds three commands: object, info, fflush\n>>>\n>>> $ object <sha1> NL\n>>> $ info <sha1> NL\n>>> $ fflush NL\n>>>\n>>> These three would be immediately useful in GitLab's context, but one can\n>>> imagine this mode to be further extended for other things.\n>>\n>> I think it would be good to define what input format we want to support. The code in this patch does not handle input like \"object HEAD:name with spaces\" which is a shame. It would be good to retain the ability to add commands that take arguments with spaces in them in the future, for example one might want to pass a format string as an argument to some future command. Making each command responsible for parsing its own arguments would help fix this rather than just splitting on input on each space and passing that list to the commands.\n> \n> This is good feedback. I can see the value of a more flexible interface for the handler functions.\n> \n>>\n>> The basic premise of adding a new command line option that reads a command and arguments seems like a sensible way to extend the functionality of cat-file.\n>>\n>> [For future reference I think the command name parsing could be cleaner if it used skip_prefix() rather than starts_with()]\n>>\n>>> For instance, a non-trivial part of \"cat-file --batch\" time is spent\n>>> on parsing its argument and seeing if it's a revision, ref etc. So we\n>>> could add a command that only accepts a full-length 40\n>>> character SHA-1.\n>>\n>> It would be good to get a better understanding of what fraction of the time git spends parsing the argument and what it is doing in that time. If we can speed up the existing code then everyone will benefit from the change, not just those who switch to the new command.\n> \n> Good question. I would need to do some investigation. This is still only a possible future enhancement so it wouldn’t be included in the first iteration.\n> \n> Thanks for the thoughtful review!\n> \n>>\n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n>> [1] https://lore.kernel.org/git/20191008200050.GA26453@sigill.intra.peff.net/\n>>\n>>> This would be the first step in adding such an interface to\n>>> git-cat-file.\n>>>\n>>> [1] https://lore.kernel.org/git/pull.1124.git.git.1636149400.gitgitgadget@gmail.com/\n>>>\n>>> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>>> Signed-off-by: John Cai <johncai86@gmail.com>\n>>> ---\n>>>    builtin/cat-file.c  | 137 +++++++++++++++++++++++++++++++++++++++++++-\n>>>    strvec.c            |  23 ++++++++\n>>>    strvec.h            |   8 +++\n>>>    t/t1006-cat-file.sh |  72 +++++++++++++++++++++++\n>>>    4 files changed, 239 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n>>> index 7b3f42950e..2679b34b43 100644\n>>> --- a/builtin/cat-file.c\n>>> +++ b/builtin/cat-file.c\n>>> @@ -16,6 +16,7 @@\n>>>    #include \"packfile.h\"\n>>>    #include \"object-store.h\"\n>>>    #include \"promisor-remote.h\"\n>>> +#include \"strvec.h\"\n>>>     struct batch_options {\n>>>    \tint enabled;\n>>> @@ -26,7 +27,10 @@ struct batch_options {\n>>>    \tint unordered;\n>>>    \tint cmdmode; /* may be 'w' or 'c' for --filters or --textconv */\n>>>    \tconst char *format;\n>>> +\tint stdin_cmd;\n>>> +\tint end_null;\n>>>    };\n>>> +static char line_termination = '\\n';\n>>>     static const char *force_path;\n>>>   @@ -508,6 +512,126 @@ static int batch_unordered_packed(const struct object_id *oid,\n>>>    \t\t\t\t      data);\n>>>    }\n>>>   +enum batch_state {\n>>> +\t/* Non-transactional state open for commands. */\n>>> +\tBATCH_STATE_OPEN,\n>>> +};\n>>> +\n>>> +static void parse_cmd_object(struct batch_options *opt,\n>>> +\t\t\t     const int argc, const char **argv,\n>>> +\t\t\t     struct strbuf *output,\n>>> +\t\t\t     struct expand_data *data)\n>>> +{\n>>> +\tbatch_one_object(argv[0], output, opt, data);\n>>> +}\n>>> +\n>>> +static void parse_cmd_info(struct batch_options *opt,\n>>> +\t\t\t     const int argc, const char **argv,\n>>> +\t\t\t     struct strbuf *output,\n>>> +\t\t\t     struct expand_data *data)\n>>> +{\n>>> +\topt->print_contents = 0;\n>>> +\tbatch_one_object(argv[0], output, opt, data);\n>>> +}\n>>> +\n>>> +static void parse_cmd_fflush(struct batch_options *opt,\n>>> +\t\t\t     const int argc, const char **argv,\n>>> +\t\t\t     struct strbuf *output,\n>>> +\t\t\t     struct expand_data *data)\n>>> +{\n>>> +\tfflush(stdout);\n>>> +}\n>>> +\n>>> +typedef void (*parse_cmd_fn_t)(struct batch_options *, const int,\n>>> +\t\t\t       const char **, struct strbuf *,\n>>> +\t\t\t       struct expand_data *);\n>>> +\n>>> +static const struct parse_cmd {\n>>> +\tconst char *prefix;\n>>> +\tparse_cmd_fn_t fn;\n>>> +\tunsigned args;\n>>> +\tenum batch_state state;\n>>> +} command[] = {\n>>> +\t{ \"object\", parse_cmd_object, 1, BATCH_STATE_OPEN },\n>>> +\t{ \"info\", parse_cmd_info, 1, BATCH_STATE_OPEN },\n>>> +\t{ \"fflush\", parse_cmd_fflush, 0, BATCH_STATE_OPEN },\n>>> +};\n>>> +\n>>> +static void batch_objects_stdin_cmd(struct batch_options *opt,\n>>> +\t\t\t\t    struct strbuf *output,\n>>> +\t\t\t\t    struct expand_data *data)\n>>> +{\n>>> +\tstruct strbuf input = STRBUF_INIT;\n>>> +\tenum batch_state state = BATCH_STATE_OPEN;\n>>> +\n>>> +\t/* Read each line dispatch its command */\n>>> +\twhile (!strbuf_getwholeline(&input, stdin, line_termination)) {\n>>> +\t\tint i;\n>>> +\t\tconst struct parse_cmd *cmd = NULL;\n>>> +\t\tstruct strvec args = STRVEC_INIT;\n>>> +\t\tsize_t n;\n>>> +\t\tconst char *p;\n>>> +\n>>> +\t\tif (*input.buf == line_termination)\n>>> +\t\t\tdie(\"empty command in input\");\n>>> +\t\telse if (isspace(*input.buf))\n>>> +\t\t\tdie(\"whitespace before command: %s\", input.buf);\n>>> +\n>>> +\t\tfor (i = 0; i < ARRAY_SIZE(command); i++) {\n>>> +\t\t\tconst char *prefix = command[i].prefix;\n>>> +\t\t\tchar c;\n>>> +\n>>> +\t\t\tif (!starts_with(input.buf, prefix))\n>>> +\t\t\t\tcontinue;\n>>> +\n>>> +\t\t\t/*\n>>> +\t\t\t * If the command has arguments, verify that it's\n>>> +\t\t\t * followed by a space. Otherwise, it shall be followed\n>>> +\t\t\t * by a line terminator.\n>>> +\t\t\t */\n>>> +\t\t\tc = command[i].args ? ' ' : line_termination;\n>>> +\t\t\tif (input.buf[strlen(prefix)] != c)\n>>> +\t\t\t\tcontinue;\n>>> +\n>>> +\t\t\tcmd = &command[i];\n>>> +\t\t\tbreak;\n>>> +\t\t}\n>>> +\t\tif (!cmd)\n>>> +\t\t\tdie(\"unknown command: %s\", input.buf);\n>>> +\n>>> +\t\t/*\n>>> +\t\t * Read additional arguments. Do not raise an error in\n>>> +\t\t * case there is an early EOF to let the command\n>>> +\t\t * handle missing arguments with a proper error message\n>>> +\t\t */\n>>> +\t\tp = input.buf + strlen(cmd->prefix) + 1;\n>>> +\t\tif (*p == line_termination || !*p) {\n>>> +\t\t\tn = 0;\n>>> +\t\t} else {\n>>> +\t\t\tconst char *pos = strstr(p, \"\\n\");\n>>> +\t\t\tchar *str = xstrndup(p, pos - p);\n>>> +\n>>> +\t\t\tn = strvec_split_delim(&args, str, ' ', - 1);\n>>> +\t\t\tfree(str);\n>>> +\t\t}\n>>> +\n>>> +\t\tif (n < cmd->args)\n>>> +\t\t\tdie(\"%s: too few arguments\", cmd->prefix);\n>>> +\t\tif (n > cmd->args)\n>>> +\t\t\tdie(\"%s: too many arguments\", cmd->prefix);\n>>> +\n>>> +\t\tswitch (state) {\n>>> +\t\tcase BATCH_STATE_OPEN:\n>>> +\t\t\t/* TODO: command state management */\n>>> +\t\t\tbreak;\n>>> +\t\t}\n>>> +\t\tcmd->fn(opt, args.nr, args.v, output, data);\n>>> +\t\tstrvec_clear(&args);\n>>> +\t}\n>>> +\n>>> +\tstrbuf_release(&input);\n>>> +}\n>>> +\n>>>    static int batch_objects(struct batch_options *opt)\n>>>    {\n>>>    \tstruct strbuf input = STRBUF_INIT;\n>>> @@ -515,6 +639,7 @@ static int batch_objects(struct batch_options *opt)\n>>>    \tstruct expand_data data;\n>>>    \tint save_warning;\n>>>    \tint retval = 0;\n>>> +\tconst int stdin_cmd = opt->stdin_cmd;\n>>>     \tif (!opt->format)\n>>>    \t\topt->format = \"%(objectname) %(objecttype) %(objectsize)\";\n>>> @@ -590,7 +715,8 @@ static int batch_objects(struct batch_options *opt)\n>>>    \tsave_warning = warn_on_object_refname_ambiguity;\n>>>    \twarn_on_object_refname_ambiguity = 0;\n>>>   -\twhile (strbuf_getline(&input, stdin) != EOF) {\n>>> +\twhile (!stdin_cmd &&\n>>> +\t       strbuf_getline(&input, stdin) != EOF) {\n>>>    \t\tif (data.split_on_whitespace) {\n>>>    \t\t\t/*\n>>>    \t\t\t * Split at first whitespace, tying off the beginning\n>>> @@ -608,6 +734,9 @@ static int batch_objects(struct batch_options *opt)\n>>>    \t\tbatch_one_object(input.buf, &output, opt, &data);\n>>>    \t}\n>>>   +\tif (stdin_cmd)\n>>> +\t\tbatch_objects_stdin_cmd(opt, &output, &data);\n>>> +\n>>>    \tstrbuf_release(&input);\n>>>    \tstrbuf_release(&output);\n>>>    \twarn_on_object_refname_ambiguity = save_warning;\n>>> @@ -685,6 +814,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>>>    \t\t\tbatch_option_callback),\n>>>    \t\tOPT_CMDMODE(0, \"batch-all-objects\", &opt,\n>>>    \t\t\t    N_(\"with --batch[-check]: ignores stdin, batches all known objects\"), 'b'),\n>>> +\t\tOPT_BOOL(0, \"stdin-cmd\", &batch.stdin_cmd,\n>>> +\t\t\t N_(\"with --batch[-check]: enters stdin 'command mode\")),\n>>> +\t\tOPT_BOOL('z', NULL, &batch.end_null, N_(\"with --stdin-cmd, use NUL termination\")),\n>>> +\n>>>    \t\t/* Batch-specific options */\n>>>    \t\tOPT_GROUP(N_(\"Change or optimize batch output\")),\n>>>    \t\tOPT_BOOL(0, \"buffer\", &batch.buffer_output, N_(\"buffer --batch output\")),\n>>> @@ -738,6 +871,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n>>>    \t/* Batch defaults */\n>>>    \tif (batch.buffer_output < 0)\n>>>    \t\tbatch.buffer_output = batch.all_objects;\n>>> +\tif (batch.end_null)\n>>> +\t\tline_termination = '\\0';\n>>>     \t/* Return early if we're in batch mode? */\n>>>    \tif (batch.enabled) {\n>>> diff --git a/strvec.c b/strvec.c\n>>> index 61a76ce6cb..7dca04bf7a 100644\n>>> --- a/strvec.c\n>>> +++ b/strvec.c\n>>> @@ -85,6 +85,29 @@ void strvec_split(struct strvec *array, const char *to_split)\n>>>    \t}\n>>>    }\n>>>   +size_t strvec_split_delim(struct strvec *array, const char *string,\n>>> +\t\t\t  int delim, int maxsplit)\n>>> +{\n>>> +\tsize_t count = 0;\n>>> +\tconst char *p = string, *end;\n>>> +\n>>> +\tfor (;;) {\n>>> +\t\tcount++;\n>>> +\t\tif (maxsplit >= 0 && count > maxsplit) {\n>>> +\t\t\tstrvec_push(array, p);\n>>> +\t\t\treturn count;\n>>> +\t\t}\n>>> +\t\tend = strchr(p, delim);\n>>> +\t\tif (end) {\n>>> +\t\t\tstrvec_push_nodup(array, xmemdupz(p, end - p));\n>>> +\t\t\tp = end + 1;\n>>> +\t\t} else {\n>>> +\t\t\tstrvec_push(array, p);\n>>> +\t\t\treturn count;\n>>> +\t\t}\n>>> +\t}\n>>> +}\n>>> +\n>>>    void strvec_clear(struct strvec *array)\n>>>    {\n>>>    \tif (array->v != empty_strvec) {\n>>> diff --git a/strvec.h b/strvec.h\n>>> index 9f55c8766b..c474918b91 100644\n>>> --- a/strvec.h\n>>> +++ b/strvec.h\n>>> @@ -73,6 +73,14 @@ void strvec_pop(struct strvec *);\n>>>    /* Splits by whitespace; does not handle quoted arguments! */\n>>>    void strvec_split(struct strvec *, const char *);\n>>>   +/**\n>>> + * strvec_split_delim() is a split function that behaves more like its\n>>> + * string_list_split() cousin than the whitespace-splitting\n>>> + * strvec_split().\n>>> + */\n>>> +size_t strvec_split_delim(struct strvec *array, const char *string,\n>>> +\t\t\t  int delim, int maxsplit);\n>>> +\n>>>    /**\n>>>     * Free all memory associated with the array and return it to the\n>>>     * initial, empty state.\n>>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n>>> index 145eee11df..8f339746ec 100755\n>>> --- a/t/t1006-cat-file.sh\n>>> +++ b/t/t1006-cat-file.sh\n>>> @@ -964,4 +964,76 @@ test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace'\n>>>    \ttest_cmp expect actual\n>>>    '\n>>>   +F='%s\\0'\n>>> +\n>>> +test_expect_success 'stdin-cmd not enough arguments' '\n>>> +\techo \"object \" >cmd &&\n>>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n>>> +\tgrep -E \"^fatal:.*too few arguments\" err\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd too many arguments' '\n>>> +\techo \"object foo bar\" >cmd &&\n>>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n>>> +\tgrep -E \"^fatal:.*too many arguments\" err\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd unknown command' '\n>>> +\techo unknown_command >cmd &&\n>>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd < cmd 2>err &&\n>>> +\tgrep -E \"^fatal:.*unknown command.*\" err &&\n>>> +\ttest_expect_code 128 git cat-file --batch --stdin-cmd -z < cmd 2>err &&\n>>> +\tgrep -E \"^fatal:.*unknown command.*\" err\n>>> +'\n>>> +\n>>> +test_expect_success 'setup object data' '\n>>> +\tcontent=\"Object Data\" &&\n>>> +\tsize=$(strlen \"$content\") &&\n>>> +\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd calling object works' '\n>>> +\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n>>> +\techo \"$sha1 blob $size\" >expect &&\n>>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>>> +\ttest_cmp expect actual\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd -z calling object works' '\n>>> +\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n>>> +\techo \"$sha1 blob $size\" >expect &&\n>>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>>> +\ttest_cmp expect actual\n>>> +'\n>>> +\n>>> +test_expect_success 'setup object data' '\n>>> +\tcontent=\"Object Data\" &&\n>>> +\tsize=$(strlen \"$content\") &&\n>>> +\tsha1=$(echo_without_newline \"$content\" | git hash-object -w --stdin)\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd calling object works' '\n>>> +\techo \"object $sha1\" | git cat-file --batch --stdin-cmd >actual &&\n>>> +\techo \"$sha1 blob $size\" >expect &&\n>>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>>> +\ttest_cmp expect actual\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd -z calling object works' '\n>>> +\tprintf $F \"object $sha1\" | git cat-file --batch --stdin-cmd -z >actual &&\n>>> +\techo \"$sha1 blob $size\" >expect &&\n>>> +\techo `git cat-file -p \"$sha1\"` >>expect &&\n>>> +\ttest_cmp expect actual\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd fflush works' '\n>>> +\tprintf \"fflush\\n\" > cmd &&\n>>> +\ttest_expect_code 0 git cat-file --batch --stdin-cmd < cmd 2>err\n>>> +'\n>>> +\n>>> +test_expect_success 'stdin-cmd -z fflush works' '\n>>> +\tprintf $F 'fflush' > cmd &&\n>>> +\ttest_expect_code 0 git cat-file --batch --stdin-cmd -z < cmd 2>err\n>>> +'\n>>> +\n>>>    test_done\n\n"}]}