On 2022.03.28 19:11, Calvin Wan wrote:
> Sometimes it is useful to get information about an object without
> having to download it completely. The server logic has already been
> implemented as “a2ba162cda (object-info: support for retrieving object
> info, 2021-04-20)”. This patch implements the client option for
> it. Currently, only 'size' is supported, however future patches can
> implement additional options.
>
> Add ‘--object-info’ option to fetch. This option allows the client to
> make an object-info command request to a server that supports protocol
> v2. If the server is v2, but does not allow for the object-info
> command request, fetch the objects as if it were a standard fetch
> (however without changing any refs). Therefore, hook `fetch
> object-info` into transport_fetch_refs() to easily fallback if needed.
>
> A previous patch added the `transfer.advertiseObjectInfo` config
> option, of which this patch utilizes to test the fallback.
>
> ---
> Documentation/fetch-options.txt | 5 ++
> Documentation/git-fetch.txt | 1 +
> builtin/fetch.c | 36 ++++++++-
> fetch-pack.c | 42 +++++++++-
> fetch-pack.h | 9 +++
> t/t5583-fetch-object-info.sh | 138 ++++++++++++++++++++++++++++++++
> transport-helper.c | 8 +-
> transport-internal.h | 1 +
> transport.c | 75 ++++++++++++++++-
> transport.h | 9 +++
> 10 files changed, 315 insertions(+), 9 deletions(-)
> create mode 100755 t/t5583-fetch-object-info.sh
> diff --git a/fetch-pack.c b/fetch-pack.c
> index b709a61baf..36e3d1c5d0 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1269,6 +1269,27 @@ static void write_command_and_capabilities(struct strbuf *req_buf,
> packet_buf_delim(req_buf);
> }
>
> +void send_object_info_request(int fd_out, struct object_info_args *args)
> +{
> + struct strbuf req_buf = STRBUF_INIT;
> + int i;
> +
> + write_command_and_capabilities(&req_buf, args->server_options, "object-info");
> +
> + if (string_list_has_string(args->object_info_options, "size"))
> + packet_buf_write(&req_buf, "size");
Do we need a newline after "size" here? It's not clear to me that it's
necessary; Documentation/technical/protocol-v2.txt just says
"command-specific-args are packet line framed arguments defined by each
individual command", but the oid arguments below have a newline, so we
should be consistent one way or the other.
It looks like the server-side implementation wants just a bare "size"
string (no newline), but I suspect that either is OK because the
packet_reader is probably using PACKET_READ_CHOMP_NEWLINE.
> + for (i = 0; i < args->oids->nr; i++) {
> + packet_buf_write(&req_buf, "oid %s\n", oid_to_hex(&args->oids->oid[i]));
> + }
> +
> + packet_buf_flush(&req_buf);
> + if (write_in_full(fd_out, req_buf.buf, req_buf.len) < 0)
> + die_errno(_("unable to write request to remote"));
> +
> + strbuf_release(&req_buf);
> +}
> +
> diff --git a/transport.c b/transport.c
> index 70e9840a90..65a1b1fdb3 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -350,6 +350,67 @@ static struct ref *handshake(struct transport *transport, int for_push,
> return refs;
> }
>
> +/*
> + * Fetches object info if server supports object-info command
> + * Make sure to fallback to normal fetch if this fails
> + */
> +static int fetch_object_info(struct transport *transport)
> +{
> + struct git_transport_data *data = transport->data;
> + struct object_info_args args;
> + struct packet_reader reader;
> +
> + 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_DIE_ON_ERR_PACKET);
> + data->version = discover_version(&reader);
> +
> + transport->hash_algo = reader.hash_algo;
> +
> + switch (data->version) {
> + case protocol_v2:
> + if (!server_supports_v2("object-info", 0))
> + return 0;
> + send_object_info_request(data->fd[1], &args);
> + break;
> + case protocol_v1:
> + case protocol_v0:
> + die(_("wrong protocol version. expected v2"));
The comment at the top of this function says that callers should be
prepared to fallback to normal fetch if this function fails. The only
way it can currently fail is if we are using protocol V2 but the server
doesn't support object-info. Rather than die() if we're on protocol V1
or V0, can we also return a failure here and let callers fallback to
fetch?
> + case protocol_unknown_version:
> + BUG("unknown protocol version");
> + }
> +
> + if (packet_reader_read(&reader) != PACKET_READ_NORMAL) {
> + die(_("error reading object info header"));
> + }
> + if (strcmp(reader.line, "size")) {
> + die(_("expected 'size', received '%s'"), reader.line);
> + }
> + while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
> + printf("%s\n", reader.line);
> + }
> + check_stateless_delimiter(transport->stateless_rpc, &reader, "stateless delimiter expected");
> +
> + return 1;
> +}
> +
