Junio C Hamano <gitster@xxxxxxxxx> writes:
> Thanks. Since this has changed significantly enough to render
> range-diff less useful, I think a fresh round of full review may be
> needed. I'll use a more recent tip of 'master' as the base when I
> queue these patches.
Having said that, here is a diff between
a/ is previous round queued with fixups merged to 'master'
b/ is the latest round queued on top of 'master'.
which may help people to see what got changed since the previous
round.
I notice that use of CALLOC_ARRAY() suggested during the review of
the last round was dropped (it should be easy to resurrect).
Documentation/git-cat-file.txt | 2 +-
builtin/cat-file.c | 4 +--
fetch-pack.c | 17 +++--------
fetch-pack.h | 4 +--
object-file.c | 1 -
protocol-caps.c | 12 ++++----
serve.c | 10 +++++-
t/t1006-cat-file.sh | 3 +-
t/t5555-http-smart-common.sh | 2 +-
t/t5701-git-serve.sh | 2 +-
transport.c | 69 +++++++++++++++++++++++++-----------------
11 files changed, 70 insertions(+), 56 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 0d9e8e6214..c073d5e50d 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -117,7 +117,7 @@ info <object>::
remote-object-info <remote> [<object>]::
Print object info for object references `<object>` at specified <remote>.
- This command may only be combined with `--buffer`.
+ This command may be combined with `--buffer`.
flush::
Used with `--buffer` to execute all preceding commands that were issued
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 48bf0ee02f..57c090f249 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -515,12 +515,12 @@ static int get_remote_info(struct batch_options *opt, int argc, const char **arg
size_t j;
int include_size = 0, include_type = 0;
- CALLOC_ARRAY(remote_object_info, object_info_oids.nr);
+ remote_object_info = xcalloc(object_info_oids.nr, sizeof(struct object_info));
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 default to a
+ * Other options that are passed in the format will fallback to a
* standard fetch request rather than object-info.
*/
if (strstr(format, "%(objectsize)")) {
diff --git a/fetch-pack.c b/fetch-pack.c
index ca06a7130f..d373aed775 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1296,14 +1296,16 @@ static void write_command_and_capabilities(struct strbuf *req_buf,
void send_object_info_request(int fd_out, struct object_info_args *args)
{
struct strbuf req_buf = STRBUF_INIT;
- struct string_list unsorted_object_info_options = *args->object_info_options;
size_t i;
write_command_and_capabilities(&req_buf, args->server_options, "object-info");
- if (unsorted_string_list_has_string(&unsorted_object_info_options, "size"))
+ if (unsorted_string_list_has_string(args->object_info_options, "size"))
packet_buf_write(&req_buf, "size");
+ if (unsorted_string_list_has_string(args->object_info_options, "type"))
+ packet_buf_write(&req_buf, "type");
+
if (args->oids) {
for (i = 0; i < args->oids->nr; i++)
packet_buf_write(&req_buf, "oid %s", oid_to_hex(&args->oids->oid[i]));
@@ -1669,7 +1671,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
/* Filter 'ref' by 'sought' and those that aren't local */
mark_complete_and_common_ref(negotiator, args, &ref);
filter_refs(args, &ref, sought, nr_sought);
- if (!args->refetch && !args->object_info && everything_local(args, &ref))
+ if (!args->refetch && everything_local(args, &ref))
state = FETCH_DONE;
else
state = FETCH_SEND_REQUEST;
@@ -2056,15 +2058,6 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
args->connectivity_checked = 1;
}
- if (args->object_info) {
- struct ref *ref_cpy_reader = ref_cpy;
- int i = 0;
- while (ref_cpy_reader) {
- oid_object_info_extended(the_repository, &ref_cpy_reader->old_oid, &(*args->object_info_data)[i], OBJECT_INFO_LOOKUP_REPLACE);
- ref_cpy_reader = ref_cpy_reader->next;
- i++;
- }
- }
update_shallow(args, sought, nr_sought, &si);
cleanup:
clear_shallow_info(&si);
diff --git a/fetch-pack.h b/fetch-pack.h
index 552fd7bde0..11c513f748 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -72,9 +72,9 @@ struct fetch_pack_args {
};
struct object_info_args {
- const struct string_list *object_info_options;
+ struct string_list *object_info_options;
const struct string_list *server_options;
- const struct oid_array *oids;
+ struct oid_array *oids;
};
/*
diff --git a/object-file.c b/object-file.c
index 333fed9f68..417c76def3 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2844,7 +2844,6 @@ 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);
diff --git a/protocol-caps.c b/protocol-caps.c
index bc7def0727..2ad9f45c59 100644
--- a/protocol-caps.c
+++ b/protocol-caps.c
@@ -11,7 +11,6 @@
struct requested_info {
unsigned size : 1;
- unsigned unknown : 1;
};
/*
@@ -41,12 +40,12 @@ static void send_info(struct repository *r, struct packet_writer *writer,
struct string_list_item *item;
struct strbuf send_buffer = STRBUF_INIT;
+ if (!oid_str_list->nr)
+ return;
+
if (info->size)
packet_writer_write(writer, "size");
- if (info->unknown || !oid_str_list->nr)
- goto release;
-
for_each_string_list_item (item, oid_str_list) {
const char *oid_str = item->string;
struct object_id oid;
@@ -73,7 +72,6 @@ static void send_info(struct repository *r, struct packet_writer *writer,
packet_writer_write(writer, "%s", send_buffer.buf);
strbuf_reset(&send_buffer);
}
-release:
strbuf_release(&send_buffer);
}
@@ -94,7 +92,9 @@ int cap_object_info(struct repository *r, struct packet_reader *request)
if (parse_oid(request->line, &oid_str_list))
continue;
- info.unknown = 1;
+ packet_writer_error(&writer,
+ "object-info: unexpected line: '%s'",
+ request->line);
}
if (request->status != PACKET_READ_FLUSH) {
diff --git a/serve.c b/serve.c
index 733347f602..1adf9df4a8 100644
--- a/serve.c
+++ b/serve.c
@@ -56,6 +56,14 @@ static int session_id_advertise(struct repository *r, struct strbuf *value)
return 1;
}
+static int object_info_advertise(struct repository *r,
+ struct strbuf *value)
+{
+ if (value)
+ strbuf_addstr(value, "size");
+ return 1;
+}
+
static void session_id_receive(struct repository *r,
const char *client_sid)
{
@@ -132,7 +140,7 @@ static struct protocol_capability capabilities[] = {
},
{
.name = "object-info",
- .advertise = always_advertise,
+ .advertise = object_info_advertise,
.command = cap_object_info,
},
};
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 7f0baea07c..55b2a1f06f 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1100,6 +1100,7 @@ test_expect_success 'batch-command flush without --buffer' '
# If a filter is not set, the filter defaults to "%(objectname) %(objectsize) %(objecttype)"
# Since "%(objecttype)" is currently not supported by the command request, object-info,
# the filters are set to "%(objectname) %(objectsize)".
+# Tests with the default filter are used to test the fallback to 'fetch' command
set_transport_variables () {
tree_sha1=$(git -C "$1" write-tree)
@@ -1163,7 +1164,7 @@ test_expect_success 'batch-command remote-object-info http:// default filter' '
echo "$tree_sha1 tree $tree_size" >>expect &&
echo "$commit_sha1 commit $commit_size" >>expect &&
echo "$tag_sha1 tag $tag_size" >>expect &&
- git cat-file --batch-command >actual <<-EOF &&
+ 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
diff --git a/t/t5555-http-smart-common.sh b/t/t5555-http-smart-common.sh
index b1cfe8b7db..5a16d4259a 100755
--- a/t/t5555-http-smart-common.sh
+++ b/t/t5555-http-smart-common.sh
@@ -131,7 +131,7 @@ test_expect_success 'git upload-pack --advertise-refs: v2' '
fetch=shallow wait-for-done
server-option
object-format=$(test_oid algo)
- object-info
+ object-info=size
0000
EOF
diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index 1896f671cb..ebb32644e3 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -20,7 +20,7 @@ test_expect_success 'test capability advertisement' '
fetch=shallow wait-for-done
server-option
object-format=$(test_oid algo)
- object-info
+ object-info=size
0000
EOF
diff --git a/transport.c b/transport.c
index 1baa1a194a..64bcc311ff 100644
--- a/transport.c
+++ b/transport.c
@@ -360,15 +360,16 @@ static int fetch_object_info(struct transport *transport, struct object_info **o
struct git_transport_data *data = transport->data;
struct object_info_args args;
struct packet_reader reader;
- struct string_list server_attributes = STRING_LIST_INIT_DUP;
memset(&args, 0, sizeof(args));
args.server_options = transport->server_options;
args.object_info_options = transport->smart_options->object_info_options;
+ args.oids = transport->smart_options->object_info_oids;
connect_setup(transport, 0);
packet_reader_init(&reader, data->fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_GENTLE_ON_EOF |
PACKET_READ_DIE_ON_ERR_PACKET);
data->version = discover_version(&reader);
@@ -378,25 +379,14 @@ static int fetch_object_info(struct transport *transport, struct object_info **o
case protocol_v2:
if (!server_supports_v2("object-info", 0))
return -1;
- /**
- * Send a request with only attributes first. If server can return all
- * of the requested attributes, then send request with object ids
- */
- send_object_info_request(data->fd[1], &args);
- if (packet_reader_read(&reader) != PACKET_READ_NORMAL) {
- check_stateless_delimiter(transport->stateless_rpc, &reader, "stateless delimiter expected");
+ if (unsorted_string_list_has_string(args.object_info_options, "size")
+ && !server_supports_feature("object-info", "size", 0)) {
return -1;
}
- string_list_split(&server_attributes, reader.line, ' ', -1);
- packet_reader_read(&reader);
- check_stateless_delimiter(transport->stateless_rpc, &reader, "stateless delimiter expected");
- if (server_attributes.nr != args.object_info_options->nr)
+ if (unsorted_string_list_has_string(args.object_info_options, "type")
+ && !server_supports_feature("object-info", "type", 0)) {
return -1;
- for (i = 0; i < server_attributes.nr; i++) {
- if (!strcmp(server_attributes.items[i].string, "size"))
- size_index = i + 1;
}
- args.oids = transport->smart_options->object_info_oids;
send_object_info_request(data->fd[1], &args);
break;
case protocol_v1:
@@ -406,18 +396,29 @@ static int fetch_object_info(struct transport *transport, struct object_info **o
BUG("unknown protocol version");
}
- if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
- die(_("error reading object info header"));
+ for (i = 0; i < args.object_info_options->nr; i++) {
+ if (packet_reader_read(&reader) != PACKET_READ_NORMAL) {
+ check_stateless_delimiter(transport->stateless_rpc, &reader, "stateless delimiter expected");
+ return -1;
+ }
+ if (unsorted_string_list_has_string(args.object_info_options, reader.line)) {
+ if (!strcmp(reader.line, "size"))
+ size_index = i;
+ continue;
+ }
+ return -1;
+ }
+
i = 0;
- while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
+ while (packet_reader_read(&reader) == PACKET_READ_NORMAL && i < args.oids->nr) {
struct string_list object_info_values = STRING_LIST_INIT_DUP;
string_list_split(&object_info_values, reader.line, ' ', -1);
- if (size_index > 0) {
- if (!strcmp(object_info_values.items[size_index].string, ""))
+ if (0 <= size_index) {
+ if (!strcmp(object_info_values.items[1 + size_index].string, ""))
die("object-info: not our ref %s",
object_info_values.items[0].string);
- *(*object_info_data)[i].sizep = strtoul(object_info_values.items[size_index].string, NULL, 10);
+ *(*object_info_data)[i].sizep = strtoul(object_info_values.items[1 + size_index].string, NULL, 10);
}
i++;
}
@@ -441,7 +442,7 @@ static int fetch_refs_via_pack(struct transport *transport,
struct ref *refs = NULL;
struct fetch_pack_args args;
struct ref *refs_tmp = NULL;
- struct ref *wanted_refs = xcalloc(1, sizeof (struct ref));
+ struct ref *object_info_refs = xcalloc(1, sizeof (struct ref));
memset(&args, 0, sizeof(args));
args.uploadpack = data->options.uploadpack;
@@ -470,11 +471,13 @@ static int fetch_refs_via_pack(struct transport *transport,
args.object_info = transport->smart_options->object_info;
if (transport->smart_options && transport->smart_options->object_info) {
- struct ref *ref = wanted_refs;
+ struct ref *ref = object_info_refs;
if (!fetch_object_info(transport, data->options.object_info_data))
goto cleanup;
args.object_info_data = data->options.object_info_data;
+ args.quiet = 1;
+ args.no_progress = 1;
for (i = 0; i < transport->smart_options->object_info_oids->nr; i++) {
struct ref *temp_ref = xcalloc(1, sizeof (struct ref));
temp_ref->old_oid = *(transport->smart_options->object_info_oids->oid + i);
@@ -482,7 +485,7 @@ static int fetch_refs_via_pack(struct transport *transport,
ref->next = temp_ref;
ref = ref->next;
}
- transport->remote_refs = wanted_refs->next;
+ transport->remote_refs = object_info_refs->next;
} else if (!data->got_remote_heads) {
int i;
int must_list_refs = 0;
@@ -500,7 +503,7 @@ static int fetch_refs_via_pack(struct transport *transport,
else if (data->version <= protocol_v1)
die_if_server_options(transport);
- if (data->options.acked_commits && !transport->smart_options->object_info) {
+ if (data->options.acked_commits) {
if (data->version < protocol_v2) {
warning(_("--negotiate-only requires protocol v2"));
ret = -1;
@@ -523,12 +526,22 @@ static int fetch_refs_via_pack(struct transport *transport,
to_fetch, nr_heads, &data->shallow,
&transport->pack_lockfiles, data->version);
+ if (args.object_info) {
+ struct ref *ref_cpy_reader = object_info_refs->next;
+ int i = 0;
+ while (ref_cpy_reader) {
+ oid_object_info_extended(the_repository, &ref_cpy_reader->old_oid, &(*args.object_info_data)[i], OBJECT_INFO_LOOKUP_REPLACE);
+ ref_cpy_reader = ref_cpy_reader->next;
+ i++;
+ }
+ }
+
data->got_remote_heads = 0;
data->options.self_contained_and_connected =
args.self_contained_and_connected;
data->options.connectivity_checked = args.connectivity_checked;
- if (!refs)
+ if (refs == NULL && !args.object_info)
ret = -1;
if (report_unmatched_refs(to_fetch, nr_heads))
ret = -1;
@@ -543,7 +556,7 @@ static int fetch_refs_via_pack(struct transport *transport,
free_refs(refs_tmp);
free_refs(refs);
- free_refs(wanted_refs);
+ free_refs(object_info_refs);
return ret;
}
