Re: [PATCH v5 2/3] connect, transport: encapsulate arg in struct
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 26, 2021, 21:54 UTC
- Message-ID
- <xmqq4kj31hzn.fsf@gitster.c.googlers.com>
- In-Reply-To
- <4eec55166889f16e41b10ae65002fd2c821b6762.1611686656.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 36 quoted lines
> In a future patch we plan to return the name of an unborn current branch
> from deep in the callchain to a caller via a new pointer parameter that
> points at a variable in the caller when the caller calls
> get_remote_refs() and transport_get_remote_refs().
>
> In preparation for that, encapsulate the existing ref_prefixes
> parameter into a struct. The aforementioned unborn current branch will
> go into this new struct in the future patch.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/clone.c | 18 +++++++++++-------
> builtin/fetch-pack.c | 3 ++-
> builtin/fetch.c | 18 +++++++++++-------
> builtin/ls-remote.c | 9 +++++----
> connect.c | 4 +++-
> remote.h | 4 +++-
> transport-helper.c | 5 +++--
> transport-internal.h | 9 +--------
> transport.c | 23 ++++++++++++-----------
> transport.h | 21 ++++++++++++++-------
> 10 files changed, 65 insertions(+), 49 deletions(-)
>
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index 58b7c1fbdc..c2d96f4c89 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -220,7 +220,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
> version = discover_version(&reader);
> switch (version) {
> case protocol_v2:
> - get_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL, args.stateless_rpc);
> + get_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL,
> + args.stateless_rpc);
> break;This seems to be an unrelated line-wrapping, but there are overlong lines that are longer than this line in the same function.
Everything else looks sensible to change the assumption that a strvec is sufficient for the communication in the codepaths and make it more easily extended by passing a ls-refs-options structure, which makes quite a lot of sense.
Show 21 quoted lines
> diff --git a/transport.h b/transport.h
> index 24558c027d..1f5b60e4d3 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -233,17 +233,24 @@ int transport_push(struct repository *repo,
> struct refspec *rs, int flags,
> unsigned int * reject_reasons);
>
> +struct transport_ls_refs_options {
> + /*
> + * Optionally, a list of ref prefixes can be provided which can be sent
> + * to the server (when communicating using protocol v2) to enable it to
> + * limit the ref advertisement. Since ref filtering is done on the
> + * server's end (and only when using protocol v2),
> + * transport_get_remote_refs() could return refs which don't match the
> + * provided ref_prefixes.
> + */
> + struct strvec ref_prefixes;
> +};
> +#define TRANSPORT_LS_REFS_OPTIONS_INIT { STRVEC_INIT }
> +And of course, the first step only carries the strvec we have been passing around, i.e. does not lose or gain features.
Looking good. Thanks.