| Message ID | a6dd5d72fce8cec430c0ec5da2dd9f0969a7fd91.1644862989.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add cat-file --batch-command flag | expand |
On Mon, Feb 14, 2022 at 1:23 PM John Cai via GitGitGadget <gitgitgadget@gmail.com> wrote: > Add a new flag --batch-command that accepts commands and arguments > from stdin, similar to git-update-ref --stdin. Some relatively minor comments below. Not sure any of them are serious enough to warrant a reroll... > The contents command takes an <object> argument and prints out the object > contents. > > The info command takes a <object> argument and prints out the object > metadata. s/a <object>/an <object>/ > Signed-off-by: John Cai <johncai86@gmail.com> > --- > diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt > @@ -96,6 +96,33 @@ OPTIONS > +contents `<object>`:: > + Print object contents for object reference `<object>`. This corresponds to > + the output of `--batch`. > + > +info `<object>`:: > + Print object info for object reference `<object>`. This corresponds to the > + output of `--batch-check`. Sorry if I wasn't clear in my earlier review, but when I suggested s/<object>/`<object>`/, I was referring only to the body of each item, not to the item itself (for which we do not -- I think -- ever use `<...>`). So: content <object>:: Print object contents ... `<object>`. ... As mentioned in my earlier review, I think the SYNOPSIS also needs an update to mention --batch-command. > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > @@ -513,6 +514,129 @@ static int batch_unordered_packed(const struct object_id *oid, > +static void dispatch_calls(struct batch_options *opt, > + struct strbuf *output, > + struct expand_data *data, > + struct queued_cmd *cmd, > + int nr) > +{ > + int i; > + > + for (i = 0; i < nr; i++){ Style: for (...) { > + cmd[i].fn(opt, cmd[i].line, output, data); > + free(cmd[i].line); > + } > + > + fflush(stdout); > +} If I recall correctly, Junio suggested calling free() within this loop, but this feels like an incorrect separation of concerns since it doesn't also reset the caller's `nr` to 0. If (for some reason), dispatch_calls() is invoked twice in a row without resetting `nr` to 0 in between the calls, then the dispatched commands will be called with a pointer to freed memory. One somewhat ugly way to fix this potential problem would be for this function to clear `nr`: static void dispatch_calls(..., int *nr) { for (...) { cmd[i].fn(...); free(cmd[i].line); } *nr = 0; flush(stdout); } But, as this is a private helper, the code as presented in the patch may be "good enough" even though it's a bit fragile. > +static void batch_objects_command(struct batch_options *opt, > + struct strbuf *output, > + struct expand_data *data) > +{ > + while (!strbuf_getline(&input, stdin)) { > + if (!input.len) > + die(_("empty command in input")); > + if (isspace(*input.buf)) > + die(_("whitespace before command: '%s'"), input.buf); > + > + if (skip_prefix(input.buf, "flush", &cmd_end)) { > + if (!opt->buffer_output) > + die(_("flush is only for --buffer mode")); > + if (*cmd_end) > + die(_("flush takes no arguments")); I didn't articulate it in my previous review since the thought was only half-formed, but given "flushify", this will incorrectly complain that "flush takes no arguments" instead of complaining "unknown command: flushify" as is done below (or it will incorrectly complain "flush is only for --buffer mode" if --buffer wasn't specified). If I'm reading the code correctly, it seems as if these problems could be avoided by treating `flush` as just another parse_cmd::commands[] item so that it gets all the same parsing/checking as the other commands rather than parsing it separately here. > + dispatch_calls(opt, output, data, queued_cmd, nr); > + nr = 0; > + continue; > + } > + > + for (i = 0; i < ARRAY_SIZE(commands); i++) { > + if (!skip_prefix(input.buf, commands[i].prefix, &cmd_end)) > + continue; > + > + cmd = &commands[i]; > + if (cmd->takes_args) { > + if (*cmd_end != ' ') > + die(_("%s requires arguments"), > + commands[i].prefix); > + > + p = cmd_end + 1; > + } else if (*cmd_end) { > + die(_("%s takes no arguments"), > + commands[i].prefix); > + } Good. Appears to be correctly handling the full matrix of command-requires-arguments and the actual input having or not having arguments. > + break; > + } > + > + if (!cmd) > + die(_("unknown command: '%s'"), input.buf); If you treat `flush` as just another parse_cmd::commands[], then right here is where you would handle it (I think): if (strcmp(cmd->prefix, "flush")) { dispatch_calls(opt, output, data, queued_cmd, nr); nr = 0; continue; } > + if (!opt->buffer_output) { > + cmd->fn(opt, p, output, data); > + continue; > + } > + > + ALLOC_GROW(queued_cmd, nr + 1, alloc); > + call.fn = cmd->fn; > + call.line = xstrdup_or_null(p); > + queued_cmd[nr++] = call; > + } > + > + if (opt->buffer_output && > + nr && > + !git_env_bool("GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT", 0)) > + dispatch_calls(opt, output, data, queued_cmd, nr); > + > + free(queued_cmd); > + strbuf_release(&input); > +}
Hi Eric, Thanks for taking another look! On 15 Feb 2022, at 14:39, Eric Sunshine wrote: > On Mon, Feb 14, 2022 at 1:23 PM John Cai via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> Add a new flag --batch-command that accepts commands and arguments >> from stdin, similar to git-update-ref --stdin. > > Some relatively minor comments below. Not sure any of them are serious > enough to warrant a reroll... > >> The contents command takes an <object> argument and prints out the object >> contents. >> >> The info command takes a <object> argument and prints out the object >> metadata. > > s/a <object>/an <object>/ > >> Signed-off-by: John Cai <johncai86@gmail.com> >> --- >> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt >> @@ -96,6 +96,33 @@ OPTIONS >> +contents `<object>`:: >> + Print object contents for object reference `<object>`. This corresponds to >> + the output of `--batch`. >> + >> +info `<object>`:: >> + Print object info for object reference `<object>`. This corresponds to the >> + output of `--batch-check`. > > Sorry if I wasn't clear in my earlier review, but when I suggested > s/<object>/`<object>`/, I was referring only to the body of each item, > not to the item itself (for which we do not -- I think -- ever use > `<...>`). So: > > content <object>:: > Print object contents ... `<object>`. ... > > As mentioned in my earlier review, I think the SYNOPSIS also needs an > update to mention --batch-command. :face-palm: yes I forgot about that in the last version. > >> diff --git a/builtin/cat-file.c b/builtin/cat-file.c >> @@ -513,6 +514,129 @@ static int batch_unordered_packed(const struct object_id *oid, >> +static void dispatch_calls(struct batch_options *opt, >> + struct strbuf *output, >> + struct expand_data *data, >> + struct queued_cmd *cmd, >> + int nr) >> +{ >> + int i; >> + >> + for (i = 0; i < nr; i++){ > > Style: for (...) { > >> + cmd[i].fn(opt, cmd[i].line, output, data); >> + free(cmd[i].line); >> + } >> + >> + fflush(stdout); >> +} > > If I recall correctly, Junio suggested calling free() within this > loop, but this feels like an incorrect separation of concerns since it > doesn't also reset the caller's `nr` to 0. If (for some reason), > dispatch_calls() is invoked twice in a row without resetting `nr` to 0 > in between the calls, then the dispatched commands will be called with > a pointer to freed memory. > > One somewhat ugly way to fix this potential problem would be for this > function to clear `nr`: > > static void dispatch_calls(..., int *nr) > { > for (...) { > cmd[i].fn(...); > free(cmd[i].line); > } > *nr = 0; > flush(stdout); > } > > But, as this is a private helper, the code as presented in the patch > may be "good enough" even though it's a bit fragile. What you suggested makes sense from a separation of concerns point of view. I'm still learning what looks ugly in C :), but I think this is easier to read overall than what I had before. > >> +static void batch_objects_command(struct batch_options *opt, >> + struct strbuf *output, >> + struct expand_data *data) >> +{ >> + while (!strbuf_getline(&input, stdin)) { >> + if (!input.len) >> + die(_("empty command in input")); >> + if (isspace(*input.buf)) >> + die(_("whitespace before command: '%s'"), input.buf); >> + >> + if (skip_prefix(input.buf, "flush", &cmd_end)) { >> + if (!opt->buffer_output) >> + die(_("flush is only for --buffer mode")); >> + if (*cmd_end) >> + die(_("flush takes no arguments")); > > I didn't articulate it in my previous review since the thought was > only half-formed, but given "flushify", this will incorrectly complain > that "flush takes no arguments" instead of complaining "unknown > command: flushify" as is done below (or it will incorrectly complain > "flush is only for --buffer mode" if --buffer wasn't specified). > > If I'm reading the code correctly, it seems as if these problems could > be avoided by treating `flush` as just another parse_cmd::commands[] > item so that it gets all the same parsing/checking as the other > commands rather than parsing it separately here. This is a good idea. I like the reduced complexity. > >> + dispatch_calls(opt, output, data, queued_cmd, nr); >> + nr = 0; >> + continue; >> + } >> + >> + for (i = 0; i < ARRAY_SIZE(commands); i++) { >> + if (!skip_prefix(input.buf, commands[i].prefix, &cmd_end)) >> + continue; >> + >> + cmd = &commands[i]; >> + if (cmd->takes_args) { >> + if (*cmd_end != ' ') >> + die(_("%s requires arguments"), >> + commands[i].prefix); >> + >> + p = cmd_end + 1; >> + } else if (*cmd_end) { >> + die(_("%s takes no arguments"), >> + commands[i].prefix); >> + } > > Good. Appears to be correctly handling the full matrix of > command-requires-arguments and the actual input having or not having > arguments. > >> + break; >> + } >> + >> + if (!cmd) >> + die(_("unknown command: '%s'"), input.buf); > > If you treat `flush` as just another parse_cmd::commands[], then right > here is where you would handle it (I think): > > if (strcmp(cmd->prefix, "flush")) { > dispatch_calls(opt, output, data, queued_cmd, nr); > nr = 0; > continue; > } > >> + if (!opt->buffer_output) { >> + cmd->fn(opt, p, output, data); >> + continue; >> + } >> + >> + ALLOC_GROW(queued_cmd, nr + 1, alloc); >> + call.fn = cmd->fn; >> + call.line = xstrdup_or_null(p); >> + queued_cmd[nr++] = call; >> + } >> + >> + if (opt->buffer_output && >> + nr && >> + !git_env_bool("GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT", 0)) >> + dispatch_calls(opt, output, data, queued_cmd, nr); >> + >> + free(queued_cmd); >> + strbuf_release(&input); >> +}
On Tue, Feb 15, 2022 at 5:58 PM John Cai <johncai86@gmail.com> wrote: > On 15 Feb 2022, at 14:39, Eric Sunshine wrote: > > On Mon, Feb 14, 2022 at 1:23 PM John Cai via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > >> +static void dispatch_calls(struct batch_options *opt, > >> + int nr) > >> +{ > >> + for (i = 0; i < nr; i++){ > >> + cmd[i].fn(opt, cmd[i].line, output, data); > >> + free(cmd[i].line); > >> + } > >> + fflush(stdout); > >> +} > > > > If I recall correctly, Junio suggested calling free() within this > > loop, but this feels like an incorrect separation of concerns since it > > doesn't also reset the caller's `nr` to 0. If (for some reason), > > dispatch_calls() is invoked twice in a row without resetting `nr` to 0 > > in between the calls, then the dispatched commands will be called with > > a pointer to freed memory. > > > > One somewhat ugly way to fix this potential problem would be for this > > function to clear `nr`: > > > > static void dispatch_calls(..., int *nr) > > { > > for (...) { > > cmd[i].fn(...); > > free(cmd[i].line); > > } > > *nr = 0; > > flush(stdout); > > } > > > > But, as this is a private helper, the code as presented in the patch > > may be "good enough" even though it's a bit fragile. > > What you suggested makes sense from a separation of concerns point of view. I'm > still learning what looks ugly in C :), but I think this is easier to read > overall than what I had before. Even with my suggestion, it's still rather ugly for a "dispatcher" function to be freeing resources allocated by some other entity and for it to be clearing the other entity's `nr` variable, but at least it's less fragile. It would be less ugly, perhaps, if this function was named dispatch_and_free(). A better way to make it less ugly would probably be to introduce a structure which holds the array of batch_options* and the `nr` and `alloc` variables, and then have a dedicated function for clearing/freeing that structure. However, while such an approach is fine for reusable containers but is probably way overkill for this one-off case. > > If I'm reading the code correctly, it seems as if these problems could > > be avoided by treating `flush` as just another parse_cmd::commands[] > > item so that it gets all the same parsing/checking as the other > > commands rather than parsing it separately here. > > This is a good idea. I like the reduced complexity. > > > If you treat `flush` as just another parse_cmd::commands[], then right > > here is where you would handle it (I think): > > > > if (strcmp(cmd->prefix, "flush")) { > > dispatch_calls(opt, output, data, queued_cmd, nr); > > nr = 0; > > continue; > > } One other point which would make this suggested code even clearer is to rename the parse_cmd::prefix member to `command` or `name` or `token` or something other than "prefix" since it really _is_ the command name, not a prefix of the command. (You're only treating it as a prefix semantically at parse time.) Thus: if (strcmp(cmd->name, "flush")) { dispatch_calls(opt, output, data, queued_cmd, nr); nr = 0; continue; } is easier to understand at a glance than `strcmp(cmd->prefix, "flush"))`.
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index bef76f4dd06..1553f65c656 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -96,6 +96,33 @@ OPTIONS need to specify the path, separated by whitespace. See the section `BATCH OUTPUT` below for details. +--batch-command:: +--batch-command=<format>:: + Enter a command mode that reads commands and arguments from stdin. May + only be combined with `--buffer`, `--textconv` or `--filters`. In the + case of `--textconv` or `--filters`, the input lines also need to specify + the path, separated by whitespace. See the section `BATCH OUTPUT` below + for details. ++ +`--batch-command` recognizes the following commands: ++ +-- +contents `<object>`:: + Print object contents for object reference `<object>`. This corresponds to + the output of `--batch`. + +info `<object>`:: + Print object info for object reference `<object>`. This corresponds to the + output of `--batch-check`. + +flush:: + Used with `--buffer` to execute all preceding commands that were issued + since the beginning or since the last flush was issued. When `--buffer` + is used, no output will come until a `flush` is issued. When `--buffer` + is not used, commands are flushed each time without issuing `flush`. +-- ++ + --batch-all-objects:: Instead of reading a list of objects on stdin, perform the requested batch operation on all objects in the repository and @@ -110,7 +137,7 @@ OPTIONS that a process can interactively read and write from `cat-file`. With this option, the output uses normal stdio buffering; this is much more efficient when invoking - `--batch-check` on a large number of objects. + `--batch-check` or `--batch-command` on a large number of objects. --unordered:: When `--batch-all-objects` is in use, visit objects in an @@ -202,6 +229,13 @@ from stdin, one per line, and print information about them. By default, the whole line is considered as an object, as if it were fed to linkgit:git-rev-parse[1]. +When `--batch-command` is given, `cat-file` will read commands from stdin, +one per line, and print information based on the command given. With +`--batch-command`, the `info` command followed by an object will print +information about the object the same way `--batch-check` would, and the +`contents` command followed by an object prints contents in the same way +`--batch` would. + You can specify the information shown for each object by using a custom `<format>`. The `<format>` is copied literally to stdout for each object, with placeholders of the form `%(atom)` expanded, followed by a @@ -237,9 +271,9 @@ newline. The available atoms are: If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`. -If `--batch` is specified, the object information is followed by the -object contents (consisting of `%(objectsize)` bytes), followed by a -newline. +If `--batch` is specified, or if `--batch-command` is used with the `contents` +command, the object information is followed by the object contents (consisting +of `%(objectsize)` bytes), followed by a newline. For example, `--batch` without a custom format would produce: diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 5e38af82af1..2d433bb6180 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -20,6 +20,7 @@ enum batch_mode { BATCH_MODE_CONTENTS, BATCH_MODE_INFO, + BATCH_MODE_QUEUE_AND_DISPATCH, }; struct batch_options { @@ -513,6 +514,129 @@ static int batch_unordered_packed(const struct object_id *oid, data); } +typedef void (*parse_cmd_fn_t)(struct batch_options *, const char *, + struct strbuf *, struct expand_data *); + +struct queued_cmd { + parse_cmd_fn_t fn; + char *line; +}; + +static void parse_cmd_contents(struct batch_options *opt, + const char *line, + struct strbuf *output, + struct expand_data *data) +{ + opt->batch_mode = BATCH_MODE_CONTENTS; + batch_one_object(line, output, opt, data); +} + +static void parse_cmd_info(struct batch_options *opt, + const char *line, + struct strbuf *output, + struct expand_data *data) +{ + opt->batch_mode = BATCH_MODE_INFO; + batch_one_object(line, output, opt, data); +} + +static void dispatch_calls(struct batch_options *opt, + struct strbuf *output, + struct expand_data *data, + struct queued_cmd *cmd, + int nr) +{ + int i; + + for (i = 0; i < nr; i++){ + cmd[i].fn(opt, cmd[i].line, output, data); + free(cmd[i].line); + } + + fflush(stdout); +} + +static const struct parse_cmd { + const char *prefix; + parse_cmd_fn_t fn; + unsigned takes_args; +} commands[] = { + { "contents", parse_cmd_contents, 1}, + { "info", parse_cmd_info, 1}, +}; + +static void batch_objects_command(struct batch_options *opt, + struct strbuf *output, + struct expand_data *data) +{ + struct strbuf input = STRBUF_INIT; + struct queued_cmd *queued_cmd = NULL; + size_t alloc = 0, nr = 0; + + while (!strbuf_getline(&input, stdin)) { + int i; + const struct parse_cmd *cmd = NULL; + const char *p = NULL, *cmd_end; + struct queued_cmd call = {0}; + + if (!input.len) + die(_("empty command in input")); + if (isspace(*input.buf)) + die(_("whitespace before command: '%s'"), input.buf); + + if (skip_prefix(input.buf, "flush", &cmd_end)) { + if (!opt->buffer_output) + die(_("flush is only for --buffer mode")); + if (*cmd_end) + die(_("flush takes no arguments")); + + dispatch_calls(opt, output, data, queued_cmd, nr); + nr = 0; + continue; + } + + for (i = 0; i < ARRAY_SIZE(commands); i++) { + if (!skip_prefix(input.buf, commands[i].prefix, &cmd_end)) + continue; + + cmd = &commands[i]; + if (cmd->takes_args) { + if (*cmd_end != ' ') + die(_("%s requires arguments"), + commands[i].prefix); + + p = cmd_end + 1; + } else if (*cmd_end) { + die(_("%s takes no arguments"), + commands[i].prefix); + } + + break; + } + + if (!cmd) + die(_("unknown command: '%s'"), input.buf); + + if (!opt->buffer_output) { + cmd->fn(opt, p, output, data); + continue; + } + + ALLOC_GROW(queued_cmd, nr + 1, alloc); + call.fn = cmd->fn; + call.line = xstrdup_or_null(p); + queued_cmd[nr++] = call; + } + + if (opt->buffer_output && + nr && + !git_env_bool("GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT", 0)) + dispatch_calls(opt, output, data, queued_cmd, nr); + + free(queued_cmd); + strbuf_release(&input); +} + static int batch_objects(struct batch_options *opt) { struct strbuf input = STRBUF_INIT; @@ -595,6 +719,10 @@ static int batch_objects(struct batch_options *opt) save_warning = warn_on_object_refname_ambiguity; warn_on_object_refname_ambiguity = 0; + if (opt->batch_mode == BATCH_MODE_QUEUE_AND_DISPATCH) { + batch_objects_command(opt, &output, &data); + goto cleanup; + } while (strbuf_getline(&input, stdin) != EOF) { if (data.split_on_whitespace) { /* @@ -613,6 +741,7 @@ static int batch_objects(struct batch_options *opt) batch_one_object(input.buf, &output, opt, &data); } + cleanup: strbuf_release(&input); strbuf_release(&output); warn_on_object_refname_ambiguity = save_warning; @@ -645,6 +774,8 @@ static int batch_option_callback(const struct option *opt, bo->batch_mode = BATCH_MODE_CONTENTS; else if (!strcmp(opt->long_name, "batch-check")) bo->batch_mode = BATCH_MODE_INFO; + else if (!strcmp(opt->long_name, "batch-command")) + bo->batch_mode = BATCH_MODE_QUEUE_AND_DISPATCH; else BUG("%s given to batch-option-callback", opt->long_name); @@ -695,6 +826,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) N_("like --batch, but don't emit <contents>"), PARSE_OPT_OPTARG | PARSE_OPT_NONEG, batch_option_callback), + OPT_CALLBACK_F(0, "batch-command", &batch, N_("format"), + N_("read commands from stdin"), + PARSE_OPT_OPTARG | PARSE_OPT_NONEG, + batch_option_callback), OPT_CMDMODE(0, "batch-all-objects", &opt, N_("with --batch[-check]: ignores stdin, batches all known objects"), 'b'), /* Batch-specific options */ diff --git a/t/README b/t/README index f48e0542cdc..bcd813b0c59 100644 --- a/t/README +++ b/t/README @@ -472,6 +472,9 @@ a test and then fails then the whole test run will abort. This can help to make sure the expected tests are executed and not silently skipped when their dependency breaks or is simply not present in a new environment. +GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=<boolean>, when true will prevent cat-file +--batch-command from flushing to output on exit. + Naming Tests ------------ diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 2d52851dadc..3575cc63cff 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -182,6 +182,24 @@ $content" test_cmp expect actual ' + for opt in --buffer --no-buffer + do + test -z "$content" || + test_expect_success "--batch-command $opt output of $type content is correct" ' + maybe_remove_timestamp "$batch_output" $no_ts >expect && + maybe_remove_timestamp "$(test_write_lines "contents $sha1" \ + | git cat-file --batch-command $opt)" $no_ts >actual && + test_cmp expect actual + ' + + test_expect_success "--batch-command $opt output of $type info is correct" ' + echo "$sha1 $type $size" >expect && + test_write_lines "info $sha1" \ + | git cat-file --batch-command $opt >actual && + test_cmp expect actual + ' + done + test_expect_success "custom --batch-check format" ' echo "$type $sha1" >expect && echo $sha1 | git cat-file --batch-check="%(objecttype) %(objectname)" >actual && @@ -229,6 +247,22 @@ test_expect_success "setup" ' run_tests 'blob' $hello_sha1 $hello_size "$hello_content" "$hello_content" +test_expect_success '--batch-command --buffer with flush for blob info' ' + echo "$hello_sha1 blob $hello_size" >expect && + test_write_lines "info $hello_sha1" "flush" | \ + GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 \ + git cat-file --batch-command --buffer >actual && + test_cmp expect actual +' + +test_expect_success '--batch-command --buffer without flush for blob info' ' + touch output && + test_write_lines "info $hello_sha1" | \ + GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 \ + git cat-file --batch-command --buffer >>output && + test_must_be_empty output +' + test_expect_success '--batch-check without %(rest) considers whole line' ' echo "$hello_sha1 blob $hello_size" >expect && git update-index --add --cacheinfo 100644 $hello_sha1 "white space" && @@ -272,7 +306,7 @@ test_expect_success \ "Reach a blob from a tag pointing to it" \ "test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\"" -for batch in batch batch-check +for batch in batch batch-check batch-command do for opt in t s e p do @@ -378,6 +412,42 @@ test_expect_success "--batch-check with multiple sha1s gives correct format" ' "$(echo_without_newline "$batch_check_input" | git cat-file --batch-check)" ' +test_expect_success '--batch-command with multiple info calls gives correct format' ' + cat >expect <<-EOF && + $hello_sha1 blob $hello_size + $tree_sha1 tree $tree_size + $commit_sha1 commit $commit_size + $tag_sha1 tag $tag_size + deadbeef missing + EOF + + test_write_lines "info $hello_sha1"\ + "info $tree_sha1"\ + "info $commit_sha1"\ + "info $tag_sha1"\ + "info deadbeef" | git cat-file --batch-command --buffer >actual && + test_cmp expect actual +' + +test_expect_success '--batch-command with multiple command calls gives correct format' ' + remove_timestamp >expect <<-EOF && + $hello_sha1 blob $hello_size + $hello_content + $commit_sha1 commit $commit_size + $commit_content + $tag_sha1 tag $tag_size + $tag_content + deadbeef missing + EOF + + test_write_lines "contents $hello_sha1"\ + "contents $commit_sha1"\ + "contents $tag_sha1"\ + "contents deadbeef"\ + "flush" | git cat-file --batch-command --buffer | remove_timestamp >actual && + test_cmp expect actual +' + test_expect_success 'setup blobs which are likely to delta' ' test-tool genrandom foo 10240 >foo && { cat foo && echo plus; } >foo-plus && @@ -968,5 +1038,40 @@ test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace' echo "$orig commit $orig_size" >expect && test_cmp expect actual ' +test_expect_success 'batch-command empty command' ' + echo "" >cmd && + test_expect_code 128 git cat-file --batch-command <cmd 2>err && + grep "^fatal:.*empty command in input.*" err +' + +test_expect_success 'batch-command whitespace before command' ' + echo " info deadbeef" >cmd && + test_expect_code 128 git cat-file --batch-command <cmd 2>err && + grep "^fatal:.*whitespace before command.*" err +' + +test_expect_success 'batch-command unknown command' ' + echo unknown_command >cmd && + test_expect_code 128 git cat-file --batch-command <cmd 2>err && + grep "^fatal:.*unknown command.*" err +' + +test_expect_success 'batch-command missing arguments' ' + echo "info" >cmd && + test_expect_code 128 git cat-file --batch-command <cmd 2>err && + grep "^fatal:.*info requires arguments.*" err +' + +test_expect_success 'batch-command flush with arguments' ' + echo "flush arg" >cmd && + test_expect_code 128 git cat-file --batch-command --buffer <cmd 2>err && + grep "^fatal:.*flush takes no arguments.*" err +' + +test_expect_success 'batch-command flush without --buffer' ' + echo "flush arg" >cmd && + test_expect_code 128 git cat-file --batch-command <cmd 2>err && + grep "^fatal:.*flush is only for --buffer mode.*" err +' test_done