diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 8994b04d15..44a32a1df4 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -338,14 +338,19 @@ struct expand_data { * Flags about when an object info is being fetched from remote. */ unsigned is_remote:1; + + /* + * List of atoms (i.e. "objectsize") that the server supports. Built + * from the server's object-info advertised capabilities. + */ + struct string_list remote_allowed_atoms; }; -#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD } - -static const char *remote_object_info_atoms[] = { - "objectname", - "objectsize", -}; +#define EXPAND_DATA_INIT { \ + .mode = S_IFINVALID, \ + .type = OBJ_BAD, \ + .remote_allowed_atoms = STRING_LIST_INIT_NODUP, \ +} static int is_atom(const char *atom, const char *s, int slen) { @@ -357,17 +362,12 @@ static int expand_atom(struct strbuf *sb, const char *atom, int len, struct expand_data *data) { if (data->is_remote) { - size_t i, allowed_nr = ARRAY_SIZE(remote_object_info_atoms); - for (i = 0; i < allowed_nr; i++) - if (is_atom(remote_object_info_atoms[i], atom, len)) + size_t i; + for (i = 0; i < data->remote_allowed_atoms.nr; i++) + if (is_atom(data->remote_allowed_atoms.items[i].string, + atom, len)) break; - - /* - * On remote, skip unsupported atoms returning an empty sb, - * honoring how for-each-ref handles known but inapplicable - * atoms (e.g. %(tagger)). - */ - if (i == allowed_nr) + if (i == data->remote_allowed_atoms.nr) return 1; } @@ -683,12 +683,12 @@ out: static int get_remote_info(int argc, const char **argv, struct object_info **remote_object_info, - struct oid_array *object_info_oids) + struct oid_array *object_info_oids, + struct string_list *object_info_options) { int retval = 0; struct remote *remote = NULL; struct object_id oid; - struct string_list object_info_options = STRING_LIST_INIT_NODUP; struct transport *gtransport; remote = remote_get(argv[0]); @@ -728,13 +728,10 @@ static int get_remote_info(int argc, CALLOC_ARRAY(*remote_object_info, object_info_oids->nr); gtransport->smart_options->object_info_oids = object_info_oids; - string_list_append(&object_info_options, "size"); - - gtransport->smart_options->object_info_options = &object_info_options; + gtransport->smart_options->object_info_options = object_info_options; gtransport->smart_options->object_info_data = *remote_object_info; retval = transport_fetch_object_info(gtransport); cleanup: - string_list_clear(&object_info_options, 0); transport_disconnect(gtransport); return retval; } @@ -820,6 +817,21 @@ static void parse_cmd_mailmap(struct batch_options *opt UNUSED, load_mailmap(); } +struct protocol_placeholder_entry { + const char *option; + const char *atom; +}; + +static const struct protocol_placeholder_entry remote_atom_map[] = { + {"size", "objectsize"}, + {"type", "objecttype"}, + /* + * Add new protocol options here. Even if the server doesn't support + * them the allow_list will drop them if the server doesn't advertise + * them. + */ +}; + static void parse_cmd_remote_object_info(struct batch_options *opt, const char *line, struct strbuf *output, struct expand_data *data) @@ -829,6 +841,7 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, char *line_to_split; struct object_info *remote_object_info = NULL; struct oid_array object_info_oids = OID_ARRAY_INIT; + struct string_list object_info_options = STRING_LIST_INIT_NODUP; const char *saved_format = opt->format; if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE) @@ -848,10 +861,22 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, die(_("remote-object-info supports at most %d objects"), MAX_ALLOWED_OBJ_LIMIT); + if (data->info.sizep) + string_list_append(&object_info_options, "size"); + if (data->info.typep) + string_list_append(&object_info_options, "type"); + if (get_remote_info(count, argv, &remote_object_info, - &object_info_oids)) + &object_info_oids, &object_info_options)) die(_("failed to get object info from the remote: %s"), argv[0]); + string_list_clear(&data->remote_allowed_atoms, 0); + string_list_append(&data->remote_allowed_atoms, "objectname"); + for (size_t i = 0; i < ARRAY_SIZE(remote_atom_map); i++) + if (unsorted_string_list_has_string(&object_info_options, remote_atom_map[i].option)) + string_list_append(&data->remote_allowed_atoms, + remote_atom_map[i].atom); + data->skip_object_info = 1; for (size_t i = 0; i < object_info_oids.nr; i++) { data->oid = object_info_oids.oid[i]; @@ -862,25 +887,29 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, continue; } + /* + * When reaching here, it means remote-object-info can retrieve + * information from server without downloading them. + */ if (remote_object_info[i].sizep) { - /* - * When reaching here, it means remote-object-info can retrieve - * information from server without downloading them. - */ data->size = *remote_object_info[i].sizep; - opt->batch_mode = BATCH_MODE_INFO; - data->is_remote = 1; - batch_object_write(argv[i + 1], output, opt, data, NULL, 0); - data->is_remote = 0; - } else { - report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing"); } + + if (remote_object_info[i].typep) { + data->type = *remote_object_info[i].typep; + } + + opt->batch_mode = BATCH_MODE_INFO; + data->is_remote = 1; + batch_object_write(argv[i + 1], output, opt, data, NULL, 0); + data->is_remote = 0; } data->skip_object_info = 0; opt->format = saved_format; for (size_t i = 0; i < object_info_oids.nr; i++) free_object_info_contents(&remote_object_info[i]); + string_list_clear(&object_info_options, 0); free(line_to_split); free(argv); free(remote_object_info); @@ -1200,6 +1229,7 @@ static int batch_objects(struct batch_options *opt) cleanup: strbuf_release(&input); strbuf_release(&output); + string_list_clear(&data.remote_allowed_atoms, 0); cfg->warn_on_object_refname_ambiguity = save_warning; return retval; } diff --git a/fetch-object-info.c b/fetch-object-info.c index 30475a1e87..ba7e179c44 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -55,6 +55,24 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar case protocol_v2: if (!server_supports_v2("object-info")) die(_("object-info capability is not enabled on the server")); + /* + * When removing an element from the list it gets swapped by the + * last element, iterate backwards to prevent elements skipping + * evaluation. + * + * object_info_options->nr can be safely casted without overflow + * because the number of options is a small known number (the + * supported placeholders which currently are size and type). + */ + for (int i = (int)args->object_info_options->nr - 1; i >= 0; i--) + if (!server_supports_feature("object-info", + args->object_info_options->items[i].string, 0)) + unsorted_string_list_delete_item(args->object_info_options, i, 0); + + /* + * Even if no options are left, we still send the oid so we get + * at least an existence check. + */ send_object_info_request(fd_out, args); break; case protocol_v1: @@ -71,7 +89,7 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar return -1; } - if (!string_list_has_string(args->object_info_options, reader->line)) + if (!unsorted_string_list_has_string(args->object_info_options, reader->line)) return -1; if (!strcmp(reader->line, "size")) { diff --git a/fetch-object-info.h b/fetch-object-info.h index 31aad98408..269cebb3f7 100644 --- a/fetch-object-info.h +++ b/fetch-object-info.h @@ -14,6 +14,9 @@ struct object_info; /* * Sends git-cat-file object-info command into the request buf and read the * results from packets. + * + * Modifies args->object_info_options, on return it contains only the supported + * options by the server. */ int fetch_object_info(enum protocol_version version, struct object_info_args *args, struct packet_reader *reader, struct object_info *object_info_data, diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh index edc20394d8..116862f9d0 100755 --- a/t/t1017-cat-file-remote-object-info.sh +++ b/t/t1017-cat-file-remote-object-info.sh @@ -271,6 +271,34 @@ test_expect_success 'unsupported placeholder on remote returns empty string' ' ) ' +test_expect_success 'requesting only objectname echoes back' ' + ( + set_transport_variables "$daemon_parent" && + cd "$daemon_parent/daemon_client_empty" && + + echo $hello_oid >expect && + git cat-file --batch-command="%(objectname)" >actual <<-EOF && + remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid + EOF + test_cmp expect actual + ) +' + +test_expect_success 'objectname goes through existence check' ' + ( + set_transport_variables "$daemon_parent" && + cd "$daemon_parent/daemon_client_empty" && + + echo "$unstored_oid missing" >expect && + + git cat-file --batch-command="%(objectname)" >actual <<-EOF && + remote-object-info "$GIT_DAEMON_URL/parent" $unstored_oid + EOF + + test_cmp expect actual + ) +' + # Test --batch-command remote-object-info with 'git://' and # transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability test_expect_success 'batch-command remote-object-info git:// fails when transfer.advertiseobjectinfo=false' ' diff --git a/transport.c b/transport.c index 9342680531..f0a6a45547 100644 --- a/transport.c +++ b/transport.c @@ -443,7 +443,6 @@ static int fetch_object_info_via_pack(struct transport *transport) args.server_options = transport->server_options; args.oids = transport->smart_options->object_info_oids; args.object_info_options = transport->smart_options->object_info_options; - string_list_sort(args.object_info_options); connect_setup(transport, 0); packet_reader_init(&reader, data->fd[0], NULL, 0,