Re: [PATCH v2 2/3] transport: introduce transport_has_remote_bundle_uri()

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jul 26, 2024, 08:58 UTC
Message-ID: <CAOLa=ZTuM9XsroDKizvtOB0d1mJ4t8ffi6_bMuX+R8qzUEW6Rw@mail.gmail.com>
In-Reply-To: <20240724144957.3033840-3-toon@iotcl.com>

Toon Claes <toon@iotcl.com> writes:

[snip]

Show 24 quoted lines

> 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?

Show 33 quoted lines

>  {
>  	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/

Show 6 quoted lines

> + * 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