{"thread":{"id":"57269","subject":"[PATCH 0/2] cat-file: add a --stdin-cmd mode","startedAt":"2022-01-19T17:58:47Z","lastAt":"2022-01-21T17:46:52Z","messageCount":5,"participants":["John Cai via GitGitGadget","Taylor Blau","John Cai"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"446510","messageId":"pull.1191.git.git.1642615122.gitgitgadget@gmail.com","threadId":"57269","inReplyTo":null,"subject":"[PATCH 0/2] cat-file: add a --stdin-cmd mode","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:58:40Z","receivedAt":"2022-01-19T17:58:47Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"This WIP patch is mostly stealing code from builtin/update-ref.c and\nimplementing the same sort of prefixed command-mode that it supports. I.e.\nin addition to --batch now supporting:\n\n<object> LF\n\n\nIt'll support with --stdin-cmd, with and without -z, respectively:\n\nobject <object> NL\nobject <object> NUL\n\n\nThe plus being that we can now implement additional commands:\n\nfflush NL\nfflush NUL\n\n\nThat command simply calls fflush(stdout), which could be done as an emergent\neffect before by feeding the input a \"NL\".\n\nI think this will be useful for other things, e.g. a not-trivial part of\n\"cat-file --batch\" time is spent on parsing its argument and seeing if it's\na revision, ref etc.\n\nSo we could e.g. add a command that only accepts a full-length 40 character\nSHA-1, or switch the --format output mid-request etc.\n\n 1. https://lore.kernel.org/git/pull.1124.git.git.1636149400.gitgitgadget@gmail.com/\n\nrequires ee4d43041d ab/cat-file\n\nJohn Cai (2):\n  strvec.c: add a strvec_split_delim()\n  cat-file: add a --stdin-cmd mode\n\n builtin/cat-file.c  | 128 +++++++++++++++++++++++++++++++++++++++++++-\n strvec.c            |  23 ++++++++\n strvec.h            |   8 +++\n t/t1006-cat-file.sh |  72 +++++++++++++++++++++++++\n 4 files changed, 230 insertions(+), 1 deletion(-)\n\n\nbase-commit: 00780c9af44409a68481c82f63a97bd18bb2593e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1191%2Fjohn-cai%2Fjc-cat-file-stdin-cmd-mode-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1191/john-cai/jc-cat-file-stdin-cmd-mode-v1\nPull-Request: https://github.com/git/git/pull/1191\n-- \ngitgitgadget\n"},{"id":"446511","messageId":"ea7408c36e515440771851c3581d7a9e3c6e4c8f.1642615122.git.gitgitgadget@gmail.com","threadId":"57269","inReplyTo":"pull.1191.git.git.1642615122.gitgitgadget@gmail.com","subject":"[PATCH 1/2] strvec.c: add a strvec_split_delim()","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:58:41Z","receivedAt":"2022-01-19T17:58:48Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"From: John Cai <johncai86@gmail.com>\n\nThis is just string_list_split() copy/pasted with the \"push\" part\nadjusted. The next commit will call this function to parse a line of\ncommand arguments.\n\nHelped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: John Cai <johncai86@gmail.com>\n---\n strvec.c | 23 +++++++++++++++++++++++\n strvec.h |  8 ++++++++\n 2 files changed, 31 insertions(+)\n\ndiff --git a/strvec.c b/strvec.c\nindex 61a76ce6cb9..7dca04bf7a5 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 9f55c8766ba..c474918b91a 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-- \ngitgitgadget\n\n"},{"id":"446512","messageId":"63fb5d156c74eb6605383a837db3e3afb024cc0e.1642615122.git.gitgitgadget@gmail.com","threadId":"57269","inReplyTo":"pull.1191.git.git.1642615122.gitgitgadget@gmail.com","subject":"[PATCH 2/2] cat-file: add a --stdin-cmd mode","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-19T17:58:42Z","receivedAt":"2022-01-19T17:58:50Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"From: John Cai <johncai86@gmail.com>\n\nTeach cat-file to accept a --stdin-cmd which causes cat-file to enter a\ncommand mode to receive commands and arguments on stdin..\n\nbatch_objects_stdin_cmd() handles a line from stdin, which it expects a\ncommand name and arguments. -z will tell --stdin-cmd to look for NUL\nas the line termination.\n\nThis commit adds two commands, fflush and object. fflush immediately\nflushes the buffer. object takes a sha as an argument and passes it to\nbatch_one_object().\n\nThe code allows for additional commands to be added as parse_cmd structs.\n\nHelped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: John Cai <johncai86@gmail.com>\n---\n builtin/cat-file.c  | 128 +++++++++++++++++++++++++++++++++++++++++++-\n t/t1006-cat-file.sh |  72 +++++++++++++++++++++++++\n 2 files changed, 199 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 7b3f42950ec..f56dbc85388 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,117 @@ 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_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{ \"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+\t\tfor (i = 0; i < args.nr; i++)\n+\t\t\tfprintf(stderr, \"%d: <%s>\\n\", i, args.v[i]);\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 +630,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 +706,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 +725,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 +805,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 +862,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/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 145eee11df9..8f339746ecf 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-- \ngitgitgadget\n"},{"id":"446518","messageId":"YehomwNiIs0l83W7@nand.local","threadId":"57269","inReplyTo":"pull.1191.git.git.1642615122.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] cat-file: add a --stdin-cmd mode","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-01-19T19:38:03Z","receivedAt":"2022-01-19T19:38:06Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 19, 2022 at 05:58:40PM +0000, John Cai via GitGitGadget wrote:\n> I think this will be useful for other things, e.g. a not-trivial part of\n> \"cat-file --batch\" time is spent on parsing its argument and seeing if it's\n> a revision, ref etc.\n>\n> So we could e.g. add a command that only accepts a full-length 40 character\n> SHA-1, or switch the --format output mid-request etc.\n\nI would like to see a more concrete proposal or need for this sort of\nthing before we go too far down adding a new mode to a command so\nfundamental as cat-file is.\n\nBetween your two proposals for other commands that you could add, I am\nnot convinced that either of them needs to be in cat-file itself. IOW,\nyou could easily inject another process in between which verifies that\nthe provided objects are 40 character SHA-1s.\n\nThe latter, changing the output format in-process, seems dubious to me.\nIs the start-up time of cat-file so slow (and you need to change formats\nso often) that the two together are unbearable? I'd be surprised if they\nwere (and if so, we should focus our efforts on improving Git's start-up\ntime).\n\nIn the meantime, this is quite an invasive way to provide callers a way\nto control the output stream. If there really is a need to just force\ncat-file to flush more often, perhaps consider designating a special\nsignal that we could treat as a request to flush the output stream?\n\nThanks,\nTaylor\n"},{"id":"446636","messageId":"820BFA8A-80D9-42F4-B8A6-D8746947C794@gmail.com","threadId":"57269","inReplyTo":"YehomwNiIs0l83W7@nand.local","subject":"Re: [PATCH 0/2] cat-file: add a --stdin-cmd mode","fromName":"John Cai","fromEmail":"johncai86@gmail.com","sentAt":"2022-01-21T17:46:43Z","receivedAt":"2022-01-21T17:46:52Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"\n\nOn 19 Jan 2022, at 14:38, Taylor Blau wrote:\n\n> On Wed, Jan 19, 2022 at 05:58:40PM +0000, John Cai via GitGitGadget wrote:\n>> I think this will be useful for other things, e.g. a not-trivial part of\n>> \"cat-file --batch\" time is spent on parsing its argument and seeing if it's\n>> a revision, ref etc.\n>>\n>> So we could e.g. add a command that only accepts a full-length 40 character\n>> SHA-1, or switch the --format output mid-request etc.\n>\n> I would like to see a more concrete proposal or need for this sort of\n> thing before we go too far down adding a new mode to a command so\n> fundamental as cat-file is.\n\nThanks for the feedback! I realized I should have made this an RFC first.\nI’ll submit an RFC separately.\n\n>\n> Between your two proposals for other commands that you could add, I am\n> not convinced that either of them needs to be in cat-file itself. IOW,\n> you could easily inject another process in between which verifies that\n> the provided objects are 40 character SHA-1s.\n>\n> The latter, changing the output format in-process, seems dubious to me.\n> Is the start-up time of cat-file so slow (and you need to change formats\n> so often) that the two together are unbearable? I'd be surprised if they\n> were (and if so, we should focus our efforts on improving Git's start-up\n> time).\n>\n> In the meantime, this is quite an invasive way to provide callers a way\n> to control the output stream. If there really is a need to just force\n> cat-file to flush more often, perhaps consider designating a special\n> signal that we could treat as a request to flush the output stream?\n>\n> Thanks,\n> Taylor\n"}]}