| Message ID | 20220502170904.2770649-7-calvinwan@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cat-file: add --batch-command remote-object-info command | expand |
Calvin Wan <calvinwan@google.com> writes: > If a v2 server does not support object-info or if the client requests > information that isn't supported by object-info, fetch the objects as if > it were a standard v2 fetch (however without changing any refs). What do you mean by "however without changing any refs"? (I would have expected that no refs would be changed, so this, to me, implies that we would expect some refs to be changed.) > diff --git a/fetch-pack.c b/fetch-pack.c > index 506ca28e05..938d082b80 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -1639,6 +1639,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, > if (args->depth > 0 || args->deepen_since || args->deepen_not) > args->deepen = 1; > > + if (args->object_info) > + state = FETCH_SEND_REQUEST; > + > while (state != FETCH_DONE) { > switch (state) { > case FETCH_CHECK_LOCAL: > @@ -1648,7 +1651,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 && everything_local(args, &ref)) > + if (!args->refetch && !args->object_info && everything_local(args, &ref)) > state = FETCH_DONE; > else > state = FETCH_SEND_REQUEST; I think that the purpose of all these changes is to skip certain steps when we know that we're fetching as a fallback for object-info, but I don't think they're necessary - it's fine to not fetch certain objects if we have them present. > @@ -2035,6 +2038,15 @@ 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++; > + } > + } Could this part be done in the same function that falls back (fetch_refs_via_pack(), I believe)? That would make the code easier to understand - here, we don't know why we would need to copy the OIDs, but in the function that falls back, we can look up and see that this is done because we couldn't do something else. > diff --git a/transport.c b/transport.c > index 08c505e1d0..b85e52d9a8 100644 > --- a/transport.c > +++ b/transport.c > @@ -436,10 +436,12 @@ static int fetch_refs_via_pack(struct transport *transport, > int nr_heads, struct ref **to_fetch) > { > int ret = 0; > + size_t i; > struct git_transport_data *data = transport->data; > struct ref *refs = NULL; > struct fetch_pack_args args; > struct ref *refs_tmp = NULL; > + struct ref *wanted_refs = xcalloc(1, sizeof (struct ref)); If you only need these new variables in a block, declare them there (and free them at the end of that block). > @@ -489,7 +500,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) { > + if (data->options.acked_commits && !transport->smart_options->object_info) { > if (data->version < protocol_v2) { > warning(_("--negotiate-only requires protocol v2")); > ret = -1; Is this addition necessary? --negotiate-only and object-info do not work together.
diff --git a/fetch-pack.c b/fetch-pack.c index 506ca28e05..938d082b80 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1639,6 +1639,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, if (args->depth > 0 || args->deepen_since || args->deepen_not) args->deepen = 1; + if (args->object_info) + state = FETCH_SEND_REQUEST; + while (state != FETCH_DONE) { switch (state) { case FETCH_CHECK_LOCAL: @@ -1648,7 +1651,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 && everything_local(args, &ref)) + if (!args->refetch && !args->object_info && everything_local(args, &ref)) state = FETCH_DONE; else state = FETCH_SEND_REQUEST; @@ -2035,6 +2038,15 @@ 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 c27018d48c..552fd7bde0 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -17,6 +17,7 @@ struct fetch_pack_args { const struct string_list *deepen_not; struct list_objects_filter_options filter_options; const struct string_list *server_options; + struct object_info **object_info_data; /* * If not NULL, during packfile negotiation, fetch-pack will send "have" @@ -43,6 +44,7 @@ struct fetch_pack_args { unsigned reject_shallow_remote:1; unsigned deepen:1; unsigned refetch:1; + unsigned object_info:1; /* * Indicate that the remote of this request is a promisor remote. The diff --git a/transport.c b/transport.c index 08c505e1d0..b85e52d9a8 100644 --- a/transport.c +++ b/transport.c @@ -436,10 +436,12 @@ static int fetch_refs_via_pack(struct transport *transport, int nr_heads, struct ref **to_fetch) { int ret = 0; + size_t i; struct git_transport_data *data = transport->data; struct ref *refs = NULL; struct fetch_pack_args args; struct ref *refs_tmp = NULL; + struct ref *wanted_refs = xcalloc(1, sizeof (struct ref)); memset(&args, 0, sizeof(args)); args.uploadpack = data->options.uploadpack; @@ -468,10 +470,19 @@ 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; + if (!fetch_object_info(transport, data->options.object_info_data)) goto cleanup; - ret = -1; - goto cleanup; + args.object_info_data = data->options.object_info_data; + 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); + temp_ref->exact_oid = 1; + ref->next = temp_ref; + ref = ref->next; + } + transport->remote_refs = wanted_refs->next; } else if (!data->got_remote_heads) { int i; int must_list_refs = 0; @@ -489,7 +500,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) { + if (data->options.acked_commits && !transport->smart_options->object_info) { if (data->version < protocol_v2) { warning(_("--negotiate-only requires protocol v2")); ret = -1; @@ -532,6 +543,7 @@ static int fetch_refs_via_pack(struct transport *transport, free_refs(refs_tmp); free_refs(refs); + free_refs(wanted_refs); return ret; }