| Message ID | 20241024205359.16376-2-eric.peijian@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cat-file: add remote-object-info to batch-command | expand |
Eric Ju <eric.peijian@gmail.com> writes: [snip] > + > +void send_object_info_request(int fd_out, struct object_info_args *args) > +{ > + struct strbuf req_buf = STRBUF_INIT; > + > + write_command_and_capabilities(&req_buf, "object-info", args->server_options); > + > + if (unsorted_string_list_has_string(args->object_info_options, "size")) > + packet_buf_write(&req_buf, "size"); > + > + if (args->oids) { > + for (size_t i = 0; i < args->oids->nr; i++) > + packet_buf_write(&req_buf, "oid %s", 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); > +} > + Was this function meant to be added here? I mean, there is no reference to it in the commit message or anywhere else. [snip]
On Fri, Oct 25, 2024 at 5:52 AM karthik nayak <karthik.188@gmail.com> wrote: > > Eric Ju <eric.peijian@gmail.com> writes: > > [snip] > > > + > > +void send_object_info_request(int fd_out, struct object_info_args *args) > > +{ > > + struct strbuf req_buf = STRBUF_INIT; > > + > > + write_command_and_capabilities(&req_buf, "object-info", args->server_options); > > + > > + if (unsorted_string_list_has_string(args->object_info_options, "size")) > > + packet_buf_write(&req_buf, "size"); > > + > > + if (args->oids) { > > + for (size_t i = 0; i < args->oids->nr; i++) > > + packet_buf_write(&req_buf, "oid %s", 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); > > +} > > + > > Was this function meant to be added here? I mean, there is no reference > to it in the commit message or anywhere else. > Thank you. The `send_object_info_request` function is used in `transport.c` `fetch_object_info()` in patch 4/6. Its functionality is similar to `send_fetch_request()`: sending the object-info command along with sub-command (e.g. size) and arguments (e.g. oids) to the remote. I guess Clavin put it here because 1. it has similar functionality as `send_fetch_request()` 2. `write_command_and_capabilities()` is only visible within `fecth-pack.c`. However, I believe your comment is valid. Adding everything to `fetch-pack.c` makes the file overly bloated with functionality unrelated to fetch-pack. For v5, I plan to address this by: I will: 1. move `write_command_and_capabilities()` a level up to connect.c 2. add a new file f`ecth-object-info.c` at the same level of `fetch-pack.c`. This new file contains the logic related to object-info command, i.e. `send_object_info_request()` and `fetch_object_info()` 3. move `fetch_object_info()` away from `transport.c` The dependency WAS like this, as I pointed out in a previous reply at https://lore.kernel.org/git/CAN2LT1AM5rYpwjZ+rhYerxDkL6mbxr7iDc=wvuhvNKS8VVXQ8w@mail.gmail.com/#t `transport.c` -> `fetch-pack.c` -> `connect.c`, where "->" means "depends on". In v5, it would be like this: `transport.c` -> `fetch-pack.c` -> `connect.c` | / -> fecth-object-info.c -> Let me know if that makes sense. > [snip]
diff --git a/fetch-pack.c b/fetch-pack.c index f752da93a8..756fb83f89 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1314,13 +1314,14 @@ static int add_haves(struct fetch_negotiator *negotiator, return haves_added; } -static void write_fetch_command_and_capabilities(struct strbuf *req_buf, - const struct string_list *server_options) +static void write_command_and_capabilities(struct strbuf *req_buf, + const char *command, + const struct string_list *server_options) { const char *hash_name; - ensure_server_supports_v2("fetch"); - packet_buf_write(req_buf, "command=fetch"); + ensure_server_supports_v2(command); + packet_buf_write(req_buf, "command=%s", command); if (server_supports_v2("agent")) packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized()); if (advertise_sid && server_supports_v2("session-id")) @@ -1346,6 +1347,28 @@ static void write_fetch_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; + + write_command_and_capabilities(&req_buf, "object-info", args->server_options); + + if (unsorted_string_list_has_string(args->object_info_options, "size")) + packet_buf_write(&req_buf, "size"); + + if (args->oids) { + for (size_t i = 0; i < args->oids->nr; i++) + packet_buf_write(&req_buf, "oid %s", 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); +} + static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, struct fetch_pack_args *args, const struct ref *wants, struct oidset *common, @@ -1356,7 +1379,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, int done_sent = 0; struct strbuf req_buf = STRBUF_INIT; - write_fetch_command_and_capabilities(&req_buf, args->server_options); + write_command_and_capabilities(&req_buf, "fetch", args->server_options); if (args->use_thin_pack) packet_buf_write(&req_buf, "thin-pack"); @@ -2174,7 +2197,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips, the_repository, "%d", negotiation_round); strbuf_reset(&req_buf); - write_fetch_command_and_capabilities(&req_buf, server_options); + write_command_and_capabilities(&req_buf, "fetch", server_options); packet_buf_write(&req_buf, "wait-for-done");