Re: [PATCH 6/6] cat-file: add remote-object-info to batch-command
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jul 10, 2024, 12:08 UTC
- Message-ID
- <CAOLa=ZRL8ByTi3+8ej1Ugh75CU5Rx2grGpNx5ESKOJCrEYg2XA@mail.gmail.com>
- In-Reply-To
- <20240628190503.67389-7-eric.peijian@gmail.com>
Eric Ju <eric.peijian@gmail.com> writes:
Show 15 quoted lines
> From: Calvin Wan <calvinwan@google.com> > > Since the `info` command in cat-file --batch-command prints object info > for a given object, it is natural to add another command in cat-file > --batch-command to print object info for a given object from a remote. > Add `remote-object-info` to cat-file --batch-command. > > While `info` takes object ids one at a time, this creates overhead when > making requests to a server so `remote-object-info` instead can take > multiple object ids at once. > > cat-file --batch-command is generally implemented in the following > manner: > > - Receive and parse input from user
So this refers input delimited by newline or '\0'.
> - Call respective function attached to command > - Set batch mode state, get object info, print object info >
Doesn't the batch mode get set before the input parsing begins?
Show 24 quoted lines
> In --buffer mode, this changes to: > > - Receive and parse input from user > - Store respective function attached to command in a queue > - After flush, loop through commands in queue > - Call respective function attached to command > - Set batch mode state, get object info, print object info > > Notice how the getting and printing of object info is accomplished one > at a time. As described above, this creates a problem for making > requests to a server. Therefore, `remote-object-info` is implemented in > the following manner: > > - Receive and parse input from user > If command is `remote-object-info`: > - Get object info from remote > - Loop through object info > - Call respective function attached to `info` > - Set batch mode state, use passed in object info, print object > info > Else: > - Call respective function attached to command > - Parse input, get object info, print object info >
So this is because we want 'remote-object-info' to also use 'parse_cmd_info' similar to 'info'. But I'm not understanding why, especially since 'parse_cmd_info' calls 'batch_one_object', and we skip most of that code for 'remote-object-info'.
Wouldn't it be cleaner to just define our own 'batch_remote_object' and create 'parse_cmd_remote_info' ?
Show 142 quoted lines
> And finally for --buffer mode `remote-object-info`:
> - Receive and parse input from user
> - Store respective function attached to command in a queue
> - After flush, loop through commands in queue:
> If command is `remote-object-info`:
> - Get object info from remote
> - Loop through object info
> - Call respective function attached to `info`
> - Set batch mode state, use passed in object info, print
> object info
> Else:
> - Call respective function attached to command
> - Set batch mode state, get object info, print object info
>
> To summarize, `remote-object-info` gets object info from the remote and
> then generates multiple `info` commands with the object info passed in.
>
> In order for remote-object-info to avoid remote communication overhead
> in the non-buffer mode, the objects are passed in as such:
>
> remote-object-info <remote> <oid> <oid> ... <oid>
>
> rather than
>
> remote-object-info <remote> <oid>
> remote-object-info <remote> <oid>
> ...
> remote-object-info <remote> <oid>
>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> Helped-by: Jonathan Tan <jonathantanmy@google.com>
> Helped-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> Documentation/git-cat-file.txt | 22 +-
> builtin/cat-file.c | 231 ++++++++++----
> object-file.c | 11 +
> object-store-ll.h | 3 +
> t/t1017-cat-file-remote-object-info.sh | 412 +++++++++++++++++++++++++
> 5 files changed, 620 insertions(+), 59 deletions(-)
> create mode 100755 t/t1017-cat-file-remote-object-info.sh
>
> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index bd95a6c10a..ab0647bb39 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -149,6 +149,12 @@ info <object>::
> Print object info for object reference `<object>`. This corresponds to the
> output of `--batch-check`.
>
> +remote-object-info <remote> <object>...::
> + Print object info for object references `<object>` at specified <remote> without
> + downloading object from remote.
> + Error when no object references is provided.
> + This command may be combined with `--buffer`.
> +
> flush::
> Used with `--buffer` to execute all preceding commands that were issued
> since the beginning or since the last flush was issued. When `--buffer`
> @@ -290,7 +296,8 @@ newline. The available atoms are:
> The full hex representation of the object name.
>
> `objecttype`::
> - The type of the object (the same as `cat-file -t` reports).
> + The type of the object (the same as `cat-file -t` reports). See
> + `CAVEATS` below. Not supported by `remote-object-info`.
>
> `objectsize`::
> The size, in bytes, of the object (the same as `cat-file -s`
> @@ -298,13 +305,14 @@ newline. The available atoms are:
>
> `objectsize:disk`::
> The size, in bytes, that the object takes up on disk. See the
> - note about on-disk sizes in the `CAVEATS` section below.
> + note about on-disk sizes in the `CAVEATS` section below. Not
> + supported by `remote-object-info`.
>
> `deltabase`::
> If the object is stored as a delta on-disk, this expands to the
> full hex representation of the delta base object name.
> Otherwise, expands to the null OID (all zeroes). See `CAVEATS`
> - below.
> + below. Not supported by `remote-object-info`.
>
> `rest`::
> If this atom is used in the output string, input lines are split
> @@ -314,7 +322,9 @@ newline. The available atoms are:
> line) are output in place of the `%(rest)` atom.
>
> If no format is specified, the default format is `%(objectname)
> -%(objecttype) %(objectsize)`.
> +%(objecttype) %(objectsize)`, except remote-object-info command who uses
> +`%(objectname) %(objectsize)` for now because "%(objecttype)" is not supported yet.
> +When "%(objecttype)" is supported, default format should be unified.
>
> If `--batch` is specified, or if `--batch-command` is used with the `contents`
> command, the object information is followed by the object contents (consisting
> @@ -396,6 +406,10 @@ scripting purposes.
> CAVEATS
> -------
>
> +Note that since objecttype, objectsize:disk and deltabase are currently not supported by the
> +remote-object-info, git will error and exit when they are in the format string.
> +
> +
> Note that the sizes of objects on disk are reported accurately, but care
> should be taken in drawing conclusions about which refs or objects are
> responsible for disk usage. The size of a packed non-delta object may be
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 72a78cdc8c..34958a1747 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -24,6 +24,9 @@
> #include "promisor-remote.h"
> #include "mailmap.h"
> #include "write-or-die.h"
> +#include "alias.h"
> +#include "remote.h"
> +#include "transport.h"
>
> enum batch_mode {
> BATCH_MODE_CONTENTS,
> @@ -42,9 +45,14 @@ struct batch_options {
> char input_delim;
> char output_delim;
> const char *format;
> + int use_remote_info;
> };
>
> +#define DEFAULT_FORMAT "%(objectname) %(objecttype) %(objectsize)"
> +
> static const char *force_path;
> +static struct object_info *remote_object_info;
> +static struct oid_array object_info_oids = OID_ARRAY_INIT;
>
> static struct string_list mailmap = STRING_LIST_INIT_NODUP;
> static int use_mailmap;
> @@ -508,7 +516,6 @@ static void batch_object_write(const char *obj_name,
> }
>
> batch_write(opt, scratch->buf, scratch->len);
> -Nit: why remove this?
Show 96 quoted lines
> if (opt->batch_mode == BATCH_MODE_CONTENTS) {
> print_object_or_die(opt, data);
> batch_write(opt, &opt->output_delim, 1);
> @@ -526,51 +533,118 @@ static void batch_one_object(const char *obj_name,
> (opt->follow_symlinks ? GET_OID_FOLLOW_SYMLINKS : 0);
> enum get_oid_result result;
>
> - result = get_oid_with_context(the_repository, obj_name,
> - flags, &data->oid, &ctx);
> - if (result != FOUND) {
> - switch (result) {
> - case MISSING_OBJECT:
> - printf("%s missing%c", obj_name, opt->output_delim);
> - break;
> - case SHORT_NAME_AMBIGUOUS:
> - printf("%s ambiguous%c", obj_name, opt->output_delim);
> - break;
> - case DANGLING_SYMLINK:
> - printf("dangling %"PRIuMAX"%c%s%c",
> - (uintmax_t)strlen(obj_name),
> - opt->output_delim, obj_name, opt->output_delim);
> - break;
> - case SYMLINK_LOOP:
> - printf("loop %"PRIuMAX"%c%s%c",
> - (uintmax_t)strlen(obj_name),
> - opt->output_delim, obj_name, opt->output_delim);
> - break;
> - case NOT_DIR:
> - printf("notdir %"PRIuMAX"%c%s%c",
> - (uintmax_t)strlen(obj_name),
> - opt->output_delim, obj_name, opt->output_delim);
> - break;
> - default:
> - BUG("unknown get_sha1_with_context result %d\n",
> - result);
> - break;
> + if (!opt->use_remote_info) {
> + result = get_oid_with_context(the_repository, obj_name,
> + flags, &data->oid, &ctx);
> + if (result != FOUND) {
> + switch (result) {
> + case MISSING_OBJECT:
> + printf("%s missing%c", obj_name, opt->output_delim);
> + break;
> + case SHORT_NAME_AMBIGUOUS:
> + printf("%s ambiguous%c", obj_name, opt->output_delim);
> + break;
> + case DANGLING_SYMLINK:
> + printf("dangling %"PRIuMAX"%c%s%c",
> + (uintmax_t)strlen(obj_name),
> + opt->output_delim, obj_name, opt->output_delim);
> + break;
> + case SYMLINK_LOOP:
> + printf("loop %"PRIuMAX"%c%s%c",
> + (uintmax_t)strlen(obj_name),
> + opt->output_delim, obj_name, opt->output_delim);
> + break;
> + case NOT_DIR:
> + printf("notdir %"PRIuMAX"%c%s%c",
> + (uintmax_t)strlen(obj_name),
> + opt->output_delim, obj_name, opt->output_delim);
> + break;
> + default:
> + BUG("unknown get_sha1_with_context result %d\n",
> + result);
> + break;
> + }
> + fflush(stdout);
> + return;
> }
> - fflush(stdout);
> - return;
> - }
>
> - if (ctx.mode == 0) {
> - printf("symlink %"PRIuMAX"%c%s%c",
> - (uintmax_t)ctx.symlink_path.len,
> - opt->output_delim, ctx.symlink_path.buf, opt->output_delim);
> - fflush(stdout);
> - return;
> + if (ctx.mode == 0) {
> + printf("symlink %"PRIuMAX"%c%s%c",
> + (uintmax_t)ctx.symlink_path.len,
> + opt->output_delim, ctx.symlink_path.buf, opt->output_delim);
> + fflush(stdout);
> + return;
> + }
> }
>
> batch_object_write(obj_name, scratch, opt, data, NULL, 0);
> }
>
> +static int get_remote_info(struct batch_options *opt, int argc, const char **argv)
> +{
> + int retval = 0;
> + struct remote *remote = NULL;We need to call `remote_clear()` on this at the end.
> + struct object_id oid; > + struct string_list object_info_options = STRING_LIST_INIT_NODUP;
This needs to be cleared.
> + static struct transport *gtransport;
Shouldn't we call `transport_disconnect(transport);`?
Show 42 quoted lines
> + /*
> + * Change the format to "%(objectname) %(objectsize)" when
> + * remote-object-info command is used. Once we start supporting objecttype
> + * the default format should change to DEFAULT_FORMAT
> + */
> + if (!opt->format) {
> + opt->format = "%(objectname) %(objectsize)";
> + }
> +
> + remote = remote_get(argv[0]);
> + if (!remote)
> + die(_("must supply valid remote when using remote-object-info"));
> + oid_array_clear(&object_info_oids);
> + for (size_t i = 1; i < argc; i++) {
> + if (get_oid_hex(argv[i], &oid))
> + die(_("Not a valid object name %s"), argv[i]);
> + oid_array_append(&object_info_oids, &oid);
> + }
> +
> + gtransport = transport_get(remote, NULL);
> + if (gtransport->smart_options) {
> + int include_size = 0;
> +
> + CALLOC_ARRAY(remote_object_info, object_info_oids.nr);
> + gtransport->smart_options->object_info = 1;
> + gtransport->smart_options->object_info_oids = &object_info_oids;
> + /*
> + * 'size' is the only option currently supported.
> + * Other options that are passed in the format will exit with error.
> + */
> + if (strstr(opt->format, "%(objectsize)")) {
> + string_list_append(&object_info_options, "size");
> + include_size = 1;
> + }
> + if (strstr(opt->format, "%(objecttype)")) {
> + die(_("objecttype is currently not supported with remote-object-info"));
> + }
> + if (strstr(opt->format, "%(objectsize:disk)"))
> + die(_("objectsize:disk is currently not supported with remote-object-info"));
> + if (strstr(opt->format, "%(deltabase)"))
> + die(_("deltabase is currently not supported with remote-object-info"));
>This whole block could be replaced by an else..
if (strstr(opt->format, "%(objectsize)")) {
string_list_append(&object_info_options, "size");
include_size = 1;
} else {
die(_("%s is currently not supported with remote-object-info", opt->format));
}Show 64 quoted lines
> + if (object_info_options.nr > 0) {
> + gtransport->smart_options->object_info_options = &object_info_options;
> + for (size_t i = 0; i < object_info_oids.nr; i++) {
> + if (include_size)
> + remote_object_info[i].sizep = xcalloc(1, sizeof(long));
> + }
> + gtransport->smart_options->object_info_data = &remote_object_info;
> + retval = transport_fetch_refs(gtransport, NULL);
> + }
> + } else {
> + retval = -1;
> + }
> +
> + return retval;
> +}
> +
> struct object_cb_data {
> struct batch_options *opt;
> struct expand_data *expand;
> @@ -642,6 +716,7 @@ typedef void (*parse_cmd_fn_t)(struct batch_options *, const char *,
> struct queued_cmd {
> parse_cmd_fn_t fn;
> char *line;
> + const char *name;
> };
>
> static void parse_cmd_contents(struct batch_options *opt,
> @@ -662,6 +737,55 @@ static void parse_cmd_info(struct batch_options *opt,
> batch_one_object(line, output, opt, data);
> }
>
> +static const struct parse_cmd {
> + const char *name;
> + parse_cmd_fn_t fn;
> + unsigned takes_args;
> +} commands[] = {
> + { "contents", parse_cmd_contents, 1 },
> + { "info", parse_cmd_info, 1 },
> + { "remote-object-info", parse_cmd_info, 1 },
> + { "flush", NULL, 0 },
> +};
> +
> +static void parse_remote_info(struct batch_options *opt,
> + char *line,
> + struct strbuf *output,
> + struct expand_data *data,
> + const struct parse_cmd *p_cmd,
> + struct queued_cmd *q_cmd)
> +{
> + int count;
> + const char **argv;
> +
> + count = split_cmdline(line, &argv);
> + if (get_remote_info(opt, count, argv))
> + goto cleanup;
> + opt->use_remote_info = 1;
> + data->skip_object_info = 1;
> + data->mark_query = 0;
> + for (size_t i = 0; i < object_info_oids.nr; i++) {
> + if (remote_object_info[i].sizep)
> + data->size = *remote_object_info[i].sizep;
> + if (remote_object_info[i].typep)
> + data->type = *remote_object_info[i].typep;
> +We don't even set the type, so this shouldn't ever be possible right?
Show 14 quoted lines
> + data->oid = object_info_oids.oid[i]; > + if (p_cmd) > + p_cmd->fn(opt, argv[i+1], output, data); > + else > + q_cmd->fn(opt, argv[i+1], output, data); > + } > + opt->use_remote_info = 0; > + data->skip_object_info = 0; > + data->mark_query = 1; > + > +cleanup: > + for (size_t i = 0; i < object_info_oids.nr; i++) > + free_object_info_contents(&remote_object_info[i]); > + free(remote_object_info);
argv needs to free'd too
Show 45 quoted lines
> +}
> +
> static void dispatch_calls(struct batch_options *opt,
> struct strbuf *output,
> struct expand_data *data,
> @@ -671,8 +795,12 @@ static void dispatch_calls(struct batch_options *opt,
> if (!opt->buffer_output)
> die(_("flush is only for --buffer mode"));
>
> - for (int i = 0; i < nr; i++)
> - cmd[i].fn(opt, cmd[i].line, output, data);
> + for (int i = 0; i < nr; i++) {
> + if (!strcmp(cmd[i].name, "remote-object-info"))
> + parse_remote_info(opt, cmd[i].line, output, data, NULL, &cmd[i]);
> + else
> + cmd[i].fn(opt, cmd[i].line, output, data);
> + }
>
> fflush(stdout);
> }
> @@ -685,17 +813,6 @@ static void free_cmds(struct queued_cmd *cmd, size_t *nr)
> *nr = 0;
> }
>
> -
> -static const struct parse_cmd {
> - const char *name;
> - parse_cmd_fn_t fn;
> - unsigned takes_args;
> -} commands[] = {
> - { "contents", parse_cmd_contents, 1},
> - { "info", parse_cmd_info, 1},
> - { "flush", NULL, 0},
> -};
> -
> static void batch_objects_command(struct batch_options *opt,
> struct strbuf *output,
> struct expand_data *data)
> @@ -740,11 +857,17 @@ static void batch_objects_command(struct batch_options *opt,
> dispatch_calls(opt, output, data, queued_cmd, nr);
> free_cmds(queued_cmd, &nr);
> } else if (!opt->buffer_output) {
> - cmd->fn(opt, p, output, data);
> + if (!strcmp(cmd->name, "remote-object-info")) {
> + char *line = xstrdup_or_null(p);This needs to be free'd.
> + parse_remote_info(opt, line, output, data, cmd, NULL);
Show 83 quoted lines
> + } else {
> + cmd->fn(opt, p, output, data);
> + }
> } else {
> ALLOC_GROW(queued_cmd, nr + 1, alloc);
> call.fn = cmd->fn;
> call.line = xstrdup_or_null(p);
> + call.name = cmd->name;
> queued_cmd[nr++] = call;
> }
> }
> @@ -761,8 +884,6 @@ static void batch_objects_command(struct batch_options *opt,
> strbuf_release(&input);
> }
>
> -#define DEFAULT_FORMAT "%(objectname) %(objecttype) %(objectsize)"
> -
> static int batch_objects(struct batch_options *opt)
> {
> struct strbuf input = STRBUF_INIT;
> diff --git a/object-file.c b/object-file.c
> index d3cf4b8b2e..6aaa167942 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2988,3 +2988,14 @@ int read_loose_object(const char *path,
> munmap(map, mapsize);
> return ret;
> }
> +
> +void free_object_info_contents(struct object_info *object_info)
> +{
> + if (!object_info)
> + return;
> + free(object_info->typep);
> + free(object_info->sizep);
> + free(object_info->disk_sizep);
> + free(object_info->delta_base_oid);
> + free(object_info->type_name);
> +}
> diff --git a/object-store-ll.h b/object-store-ll.h
> index c5f2bb2fc2..333e19cd1e 100644
> --- a/object-store-ll.h
> +++ b/object-store-ll.h
> @@ -533,4 +533,7 @@ int for_each_object_in_pack(struct packed_git *p,
> int for_each_packed_object(each_packed_object_fn, void *,
> enum for_each_object_flags flags);
>
> +/* Free pointers inside of object_info, but not object_info itself */
> +void free_object_info_contents(struct object_info *object_info);
> +
> #endif /* OBJECT_STORE_LL_H */
> diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
> new file mode 100755
> index 0000000000..7a7bdfeb91
> --- /dev/null
> +++ b/t/t1017-cat-file-remote-object-info.sh
> @@ -0,0 +1,412 @@
> +#!/bin/sh
> +
> +test_description='git cat-file --batch-command with remote-object-info command'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +echo_without_newline () {
> + printf '%s' "$*"
> +}
> +
> +strlen () {
> + echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
> +}
> +
> +hello_content="Hello World"
> +hello_size=$(strlen "$hello_content")
> +hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin)
> +
> +tree_size=$(($(test_oid rawsz) + 13))
> +
> +commit_message="Initial commit"
> +commit_size=$(($(test_oid hexsz) + 137))
>Why 13 and 137?
Show 43 quoted lines
> +
> +tag_header_without_oid="type blob
> +tag hellotag
> +tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
> +tag_header_without_timestamp="object $hello_oid
> +$tag_header_without_oid"
> +tag_description="This is a tag"
> +tag_content="$tag_header_without_timestamp 0 +0000
> +
> +$tag_description"
> +
> +tag_oid=$(echo_without_newline "$tag_content" | git hash-object -t tag --stdin -w)
> +tag_size=$(strlen "$tag_content")
> +
> +# This section tests --batch-command with remote-object-info command
> +# Since "%(objecttype)" is currently not supported by the command remote-object-info ,
> +# the filters are set to "%(objectname) %(objectsize)".
> +# Tests with the default filter are used to test the fallback to 'fetch' command
> +
> +
> +# Test --batch-command remote-object-info with 'git://' transport
> +
> +. "$TEST_DIRECTORY"/lib-git-daemon.sh
> +start_git_daemon --export-all --enable=receive-pack
> +daemon_parent=$GIT_DAEMON_DOCUMENT_ROOT_PATH/parent
> +
> +test_expect_success 'create repo to be served by git-daemon' '
> + git init "$daemon_parent" &&
> +
> + echo_without_newline "$hello_content" > $daemon_parent/hello &&
> + git -C "$daemon_parent" update-index --add hello &&
> + git -C "$daemon_parent" config transfer.advertiseobjectinfo true
> +'
> +
> +set_transport_variables () {
> + hello_sha1=$(echo_without_newline "$hello_content" | git hash-object --stdin)
> + tree_sha1=$(git -C "$1" write-tree)
> + commit_sha1=$(echo_without_newline "$commit_message" | git -C "$1" commit-tree $tree_sha1)
> + tag_sha1=$(echo_without_newline "$tag_content" | git -C "$1" hash-object -t tag --stdin -w)
> + tag_size=$(strlen "$tag_content")
> +}
> +
> +extra newline here
Show 344 quoted lines
> +test_expect_success 'batch-command remote-object-info git://' ' > + ( > + set_transport_variables "$daemon_parent" && > + cd "$daemon_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF && > + remote-object-info "$GIT_DAEMON_URL/parent" $hello_sha1 > + remote-object-info "$GIT_DAEMON_URL/parent" $tree_sha1 > + remote-object-info "$GIT_DAEMON_URL/parent" $commit_sha1 > + remote-object-info "$GIT_DAEMON_URL/parent" $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command remote-object-info git:// multiple sha1 per line' ' > + ( > + set_transport_variables "$daemon_parent" && > + cd "$daemon_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF && > + remote-object-info "$GIT_DAEMON_URL/parent" $hello_sha1 $tree_sha1 $commit_sha1 $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command remote-object-info http:// default filter' ' > + ( > + set_transport_variables "$daemon_parent" && > + cd "$daemon_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + GIT_TRACE_PACKET=1 git cat-file --batch-command >actual <<-EOF && > + remote-object-info "$GIT_DAEMON_URL/parent" $hello_sha1 $tree_sha1 > + remote-object-info "$GIT_DAEMON_URL/parent" $commit_sha1 $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command --buffer remote-object-info git://' ' > + ( > + set_transport_variables "$daemon_parent" && > + cd "$daemon_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" --buffer >actual <<-EOF && > + remote-object-info "$GIT_DAEMON_URL/parent" $hello_sha1 $tree_sha1 > + remote-object-info "$GIT_DAEMON_URL/parent" $commit_sha1 $tag_sha1 > + flush > + EOF > + test_cmp expect actual > + ) > +' > + > +stop_git_daemon > + > +# Test --batch-command remote-object-info with 'http://' transport > + > +. "$TEST_DIRECTORY"/lib-httpd.sh > +start_httpd > + > +test_expect_success 'create repo to be served by http:// transport' ' > + git init "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config http.receivepack true && > + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config transfer.advertiseobjectinfo true && > + echo_without_newline "$hello_content" > $HTTPD_DOCUMENT_ROOT_PATH/http_parent/hello && > + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" update-index --add hello > +' > + > + > +test_expect_success 'batch-command remote-object-info http://' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 > + remote-object-info "$HTTPD_URL/smart/http_parent" $tree_sha1 > + remote-object-info "$HTTPD_URL/smart/http_parent" $commit_sha1 > + remote-object-info "$HTTPD_URL/smart/http_parent" $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command remote-object-info http:// one line' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 $tree_sha1 $commit_sha1 $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command --buffer remote-object-info http://' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + > + git cat-file --batch-command="%(objectname) %(objectsize)" --buffer >actual <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 $tree_sha1 > + remote-object-info "$HTTPD_URL/smart/http_parent" $commit_sha1 $tag_sha1 > + flush > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command remote-object-info http:// default filter' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + > + git cat-file --batch-command >actual <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 $tree_sha1 > + remote-object-info "$HTTPD_URL/smart/http_parent" $commit_sha1 $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'remote-object-info fails on unspported filter option (objectsize:disk)' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + test_must_fail git cat-file --batch-command="%(objectsize:disk)" 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 > + EOF > + test_grep "objectsize:disk is currently not supported with remote-object-info" err > + ) > +' > + > +test_expect_success 'remote-object-info fails on unspported filter option (deltabase)' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + test_must_fail git cat-file --batch-command="%(deltabase)" 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 > + EOF > + test_grep "deltabase is currently not supported with remote-object-info" err > + ) > +' > + > +test_expect_success 'remote-object-info fails on server with legacy protocol' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + test_must_fail git -c protocol.version=0 cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 > + EOF > + test_grep "remote-object-info requires protocol v2" err > + ) > +' > + > +test_expect_success 'remote-object-info fails on server with legacy protocol fallback' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + > + test_must_fail git -c protocol.version=0 cat-file --batch-command 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $hello_sha1 > + EOF > + test_grep "remote-object-info requires protocol v2" err > + ) > +' > + > +test_expect_success 'remote-object-info fails on malformed OID' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + malformed_object_id="this_id_is_not_valid" && > + > + test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $malformed_object_id > + EOF > + test_grep "Not a valid object name '$malformed_object_id'" err > + ) > +' > + > +test_expect_success 'remote-object-info fails on malformed OID fallback' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + malformed_object_id="this_id_is_not_valid" && > + > + test_must_fail git cat-file --batch-command 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $malformed_object_id > + EOF > + test_grep "Not a valid object name '$malformed_object_id'" err > + ) > +' > + > +test_expect_success 'remote-object-info fails on missing OID' ' > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + git clone "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" missing_oid_repo && > + test_commit -C missing_oid_repo message1 c.txt && > + ( > + cd missing_oid_repo && > + > + object_id=$(git rev-parse message1:c.txt) && > + test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $object_id > + EOF > + test_grep "object-info: not our ref $object_id" err > + ) > +' > + > +# shellcheck disable=SC2016 > +test_expect_success 'remote-object-info fails on missing OID fallback' ' > + ( > + set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && > + cd missing_oid_repo && > + object_id=$(git rev-parse message1:c.txt) && > + test_must_fail git cat-file --batch-command 2>err <<-EOF && > + remote-object-info "$HTTPD_URL/smart/http_parent" $object_id > + EOF > + test_grep "fatal: object-info: not our ref $object_id" err > + ) > +' > + > +# Test --batch-command remote-object-info with 'file://' transport > + > +# shellcheck disable=SC2016 > +test_expect_success 'create repo to be served by file:// transport' ' > + git init server && > + git -C server config protocol.version 2 && > + git -C server config transfer.advertiseobjectinfo true && > + echo_without_newline "$hello_content" > server/hello && > + git -C server update-index --add hello > +' > + > + > +test_expect_success 'batch-command remote-object-info file://' ' > + ( > + set_transport_variables "server" && > + cd server && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF && > + remote-object-info "file://$(pwd)" $hello_sha1 > + remote-object-info "file://$(pwd)" $tree_sha1 > + remote-object-info "file://$(pwd)" $commit_sha1 > + remote-object-info "file://$(pwd)" $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command remote-object-info file:// multiple sha1 per line' ' > + ( > + set_transport_variables "server" && > + cd server && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF && > + remote-object-info "file://$(pwd)" $hello_sha1 $tree_sha1 $commit_sha1 $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command --buffer remote-object-info file://' ' > + ( > + set_transport_variables "server" && > + cd server && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + git cat-file --batch-command="%(objectname) %(objectsize)" --buffer >actual <<-EOF && > + remote-object-info "file://$(pwd)" $hello_sha1 $tree_sha1 > + remote-object-info "file://$(pwd)" $commit_sha1 $tag_sha1 > + flush > + EOF > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'batch-command remote-object-info file:// default filter' ' > + ( > + set_transport_variables "server" && > + cd server && > + > + echo "$hello_sha1 $hello_size" >expect && > + echo "$tree_sha1 $tree_size" >>expect && > + echo "$commit_sha1 $commit_size" >>expect && > + echo "$tag_sha1 $tag_size" >>expect && > + > + git cat-file --batch-command >actual <<-EOF && > + remote-object-info "file://$(pwd)" $hello_sha1 $tree_sha1 > + remote-object-info "file://$(pwd)" $commit_sha1 $tag_sha1 > + EOF > + test_cmp expect actual > + ) > +' > + > +test_done
Some more tests I'd like to see - Testing against the '-Z' option. - Testing the fallback to fetch whole object when the server doesn't support 'remote-object-info'.
Thanks