| Message ID | 20240724144957.3033840-3-toon@iotcl.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fetch: use bundle URIs when having creationToken heuristic | expand |
Toon Claes <toon@iotcl.com> writes: [snip] > diff --git a/t/helper/test-bundle-uri.c b/t/helper/test-bundle-uri.c > index 0c5fa723d8..bd558d5e57 100644 > --- a/t/helper/test-bundle-uri.c > +++ b/t/helper/test-bundle-uri.c > @@ -90,7 +90,7 @@ static int cmd_ls_remote(int argc, const char **argv) > } > > transport = transport_get(remote, NULL); > - if (transport_get_remote_bundle_uri(transport) < 0) { > + if (!transport_has_remote_bundle_uri(transport)) { > error(_("could not get the bundle-uri list")); > status = 1; > goto cleanup; > diff --git a/transport.c b/transport.c > index 12cc5b4d96..1a7d86fa40 100644 > --- a/transport.c > +++ b/transport.c > @@ -1536,7 +1536,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs) > return rc; > } > > -int transport_get_remote_bundle_uri(struct transport *transport) > +static int transport_get_remote_bundle_uri(struct transport *transport) > Why make it static? > { > int value = 0; > const struct transport_vtable *vtable = transport->vtable; > @@ -1561,6 +1561,18 @@ int transport_get_remote_bundle_uri(struct transport *transport) > > if (vtable->get_bundle_uri(transport) < 0) > return error(_("could not retrieve server-advertised bundle-uri list")); > + > + return 0; > +} > + > +int transport_has_remote_bundle_uri(struct transport *transport) > +{ > + transport_get_remote_bundle_uri(transport); > + > + if (transport->bundles && > + hashmap_get_size(&transport->bundles->bundles)) > + return 1; > + > return 0; > } > > diff --git a/transport.h b/transport.h > index 6393cd9823..5ea9641558 100644 > --- a/transport.h > +++ b/transport.h > @@ -294,10 +294,11 @@ const struct ref *transport_get_remote_refs(struct transport *transport, > struct transport_ls_refs_options *transport_options); > > /** > - * Retrieve bundle URI(s) from a remote. Populates "struct > - * transport"'s "bundle_uri" and "got_remote_bundle_uri". > + * Try fetch bundle URI(s) from a remote and returns 1 if one or more Nit: s/Try/Try to/ > + * bundle URI(s) are received from the server. > + * Populates "struct transport"'s "bundles" and "got_remote_bundle_uri". > */ > -int transport_get_remote_bundle_uri(struct transport *transport); > +int transport_has_remote_bundle_uri(struct transport *transport); > Shouldn't this now be renamed to `transport_has_bundle_uri`? Earlier, we were 'getting' bundle URIs from the remote. Now, we are abstracting that away and simply asking, 'do we have bundle URIs'. > /* > * Fetch the hash algorithm used by a remote. > -- > 2.45.2
Karthik Nayak <karthik.188@gmail.com> writes: >> diff --git a/transport.c b/transport.c >> index 12cc5b4d96..1a7d86fa40 100644 >> --- a/transport.c >> +++ b/transport.c >> @@ -1536,7 +1536,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs) >> return rc; >> } >> >> -int transport_get_remote_bundle_uri(struct transport *transport) >> +static int transport_get_remote_bundle_uri(struct transport *transport) >> > > Why make it static? The reason is rather well described in the proposed log message, I think. >> + * bundle URI(s) are received from the server. >> + * Populates "struct transport"'s "bundles" and "got_remote_bundle_uri". >> */ >> -int transport_get_remote_bundle_uri(struct transport *transport); >> +int transport_has_remote_bundle_uri(struct transport *transport); >> > > Shouldn't this now be renamed to `transport_has_bundle_uri`? Earlier, we > were 'getting' bundle URIs from the remote. Now, we are abstracting that > away and simply asking, 'do we have bundle URIs'. A good question.
diff --git a/builtin/clone.c b/builtin/clone.c index aa507395a0..25535c1814 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1404,22 +1404,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix) bundle_uri); else if (has_heuristic) git_config_set_gently("fetch.bundleuri", bundle_uri); - } else { - /* - * Populate transport->got_remote_bundle_uri and - * transport->bundle_uri. We might get nothing. - */ - transport_get_remote_bundle_uri(transport); - - if (transport->bundles && - hashmap_get_size(&transport->bundles->bundles)) { - /* At this point, we need the_repository to match the cloned repo. */ - if (repo_init(the_repository, git_dir, work_tree)) - warning(_("failed to initialize the repo, skipping bundle URI")); - else if (fetch_bundle_list(the_repository, - transport->bundles)) - warning(_("failed to fetch advertised bundles")); - } + } else if (transport_has_remote_bundle_uri(transport)) { + /* At this point, we need the_repository to match the cloned repo. */ + if (repo_init(the_repository, git_dir, work_tree)) + warning(_("failed to initialize the repo, skipping bundle URI")); + else if (fetch_bundle_list(the_repository, + transport->bundles)) + warning(_("failed to fetch advertised bundles")); } if (refs) diff --git a/t/helper/test-bundle-uri.c b/t/helper/test-bundle-uri.c index 0c5fa723d8..bd558d5e57 100644 --- a/t/helper/test-bundle-uri.c +++ b/t/helper/test-bundle-uri.c @@ -90,7 +90,7 @@ static int cmd_ls_remote(int argc, const char **argv) } transport = transport_get(remote, NULL); - if (transport_get_remote_bundle_uri(transport) < 0) { + if (!transport_has_remote_bundle_uri(transport)) { error(_("could not get the bundle-uri list")); status = 1; goto cleanup; diff --git a/transport.c b/transport.c index 12cc5b4d96..1a7d86fa40 100644 --- a/transport.c +++ b/transport.c @@ -1536,7 +1536,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs) return rc; } -int transport_get_remote_bundle_uri(struct transport *transport) +static int transport_get_remote_bundle_uri(struct transport *transport) { int value = 0; const struct transport_vtable *vtable = transport->vtable; @@ -1561,6 +1561,18 @@ int transport_get_remote_bundle_uri(struct transport *transport) if (vtable->get_bundle_uri(transport) < 0) return error(_("could not retrieve server-advertised bundle-uri list")); + + return 0; +} + +int transport_has_remote_bundle_uri(struct transport *transport) +{ + transport_get_remote_bundle_uri(transport); + + if (transport->bundles && + hashmap_get_size(&transport->bundles->bundles)) + return 1; + return 0; } diff --git a/transport.h b/transport.h index 6393cd9823..5ea9641558 100644 --- a/transport.h +++ b/transport.h @@ -294,10 +294,11 @@ const struct ref *transport_get_remote_refs(struct transport *transport, struct transport_ls_refs_options *transport_options); /** - * Retrieve bundle URI(s) from a remote. Populates "struct - * transport"'s "bundle_uri" and "got_remote_bundle_uri". + * Try fetch bundle URI(s) from a remote and returns 1 if one or more + * bundle URI(s) are received from the server. + * Populates "struct transport"'s "bundles" and "got_remote_bundle_uri". */ -int transport_get_remote_bundle_uri(struct transport *transport); +int transport_has_remote_bundle_uri(struct transport *transport); /* * Fetch the hash algorithm used by a remote.
The public function transport_get_remote_bundle_uri() exists to fetch the bundle URI(s) from the remote. This function is only called from builtin/clone.c (not taking test-tool into account). There it ignores the return value, because it doesn't matter whether the server didn't return any bundles or if it failed trying to fetch them, clone can continue without bundle URIs. After calling it, it checks if anything is collected in the bundle list and starts fetching them. Add public function transport_has_remote_bundle_uri() instead. This calls the (now made private) transport_get_remote_bundle_uri() function and returns whether any bundle URI is received. This makes reuse of the code easier and avoids code duplication when we add bundle URI support to git-fetch(1). Signed-off-by: Toon Claes <toon@iotcl.com> --- builtin/clone.c | 23 +++++++---------------- t/helper/test-bundle-uri.c | 2 +- transport.c | 14 +++++++++++++- transport.h | 7 ++++--- 4 files changed, 25 insertions(+), 21 deletions(-) -- 2.45.2