From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org, peff@peff.net, felipe.contreras@gmail.com,
avarab@gmail.com
Subject: Re: [PATCH v2 2/3] connect, transport: add no-op arg for future patch
Date: Tue, 15 Dec 2020 22:20:50 -0800 [thread overview]
Message-ID: <xmqqwnxi46gt.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <e24fb6d746279a71f8d6a532790231c2a46468aa.1608084282.git.jonathantanmy@google.com> (Jonathan Tan's message of "Tue, 15 Dec 2020 18:07:57 -0800")
Jonathan Tan <jonathantanmy@google.com> writes:
> A future patch will require transport_get_remote_refs() and
> get_remote_refs() to gain a new argument. Add the argument in this
> patch, with no effect on execution, so that the future patch only needs
> to concern itself with new logic.
Please give at least a hint about what this "new argument" will be
passing around through the callchain. E.g.
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(). Add the parameter to functions
involved in the callchain, but no caller passes an actual
argument yet in this step ...
or something like that.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> builtin/clone.c | 2 +-
> builtin/fetch-pack.c | 3 ++-
> builtin/fetch.c | 2 +-
> builtin/ls-remote.c | 2 +-
> builtin/remote.c | 2 +-
> connect.c | 5 ++++-
> remote.h | 3 ++-
> transport-helper.c | 7 +++++--
> transport-internal.h | 13 +++++--------
> transport.c | 29 ++++++++++++++++++-----------
> transport.h | 7 ++++++-
> 11 files changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index a0841923cf..70f9450db4 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1264,7 +1264,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> if (!option_no_tags)
> strvec_push(&ref_prefixes, "refs/tags/");
>
> - refs = transport_get_remote_refs(transport, &ref_prefixes);
> + refs = transport_get_remote_refs(transport, &ref_prefixes, NULL);
>
> if (refs) {
> int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index 58b7c1fbdc..9f921dfab4 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, NULL);
> break;
> case protocol_v1:
> case protocol_v0:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index ecf8537605..a7ef59acfc 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1393,7 +1393,7 @@ static int do_fetch(struct transport *transport,
>
> if (must_list_refs) {
> trace2_region_enter("fetch", "remote_refs", the_repository);
> - remote_refs = transport_get_remote_refs(transport, &ref_prefixes);
> + remote_refs = transport_get_remote_refs(transport, &ref_prefixes, NULL);
> trace2_region_leave("fetch", "remote_refs", the_repository);
> } else
> remote_refs = NULL;
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 092917eca2..4cf3f60b1b 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -118,7 +118,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> if (server_options.nr)
> transport->server_options = &server_options;
>
> - ref = transport_get_remote_refs(transport, &ref_prefixes);
> + ref = transport_get_remote_refs(transport, &ref_prefixes, NULL);
> if (ref) {
> int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
> repo_set_hash_algo(the_repository, hash_algo);
> diff --git a/builtin/remote.c b/builtin/remote.c
> index c1b211b272..246e62f118 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -950,7 +950,7 @@ static int get_remote_ref_states(const char *name,
> if (query) {
> transport = transport_get(states->remote, states->remote->url_nr > 0 ?
> states->remote->url[0] : NULL);
> - remote_refs = transport_get_remote_refs(transport, NULL);
> + remote_refs = transport_get_remote_refs(transport, NULL, NULL);
> transport_disconnect(transport);
>
> states->queried = 1;
> diff --git a/connect.c b/connect.c
> index 8b8f56cf6d..99d9052365 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -455,7 +455,8 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
> struct ref **list, int for_push,
> const struct strvec *ref_prefixes,
> const struct string_list *server_options,
> - int stateless_rpc)
> + int stateless_rpc,
> + char **unborn_head_target)
> {
> int i;
> const char *hash_name;
> @@ -496,6 +497,8 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
>
> /* Process response from server */
> while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
> + if (unborn_head_target)
> + BUG("NEEDSWORK: provide unborn HEAD target to caller while reading refs");
> if (!process_ref_v2(reader, &list))
> die(_("invalid ls-refs response: %s"), reader->line);
> }
> diff --git a/remote.h b/remote.h
> index 3211abdf05..967f2178d8 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -198,7 +198,8 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
> struct ref **list, int for_push,
> const struct strvec *ref_prefixes,
> const struct string_list *server_options,
> - int stateless_rpc);
> + int stateless_rpc,
> + char **unborn_head_target);
>
> int resolve_remote_symref(struct ref *ref, struct ref *list);
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 5f6e0b3bd8..5d97eba935 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -1162,13 +1162,16 @@ static int has_attribute(const char *attrs, const char *attr)
> }
>
> static struct ref *get_refs_list(struct transport *transport, int for_push,
> - const struct strvec *ref_prefixes)
> + const struct strvec *ref_prefixes,
> + char **unborn_head_target)
> {
> get_helper(transport);
>
> if (process_connect(transport, for_push)) {
> do_take_over(transport);
> - return transport->vtable->get_refs_list(transport, for_push, ref_prefixes);
> + return transport->vtable->get_refs_list(transport, for_push,
> + ref_prefixes,
> + unborn_head_target);
> }
>
> return get_refs_list_using_list(transport, for_push);
> diff --git a/transport-internal.h b/transport-internal.h
> index 27c9daffc4..5037f6197d 100644
> --- a/transport-internal.h
> +++ b/transport-internal.h
> @@ -18,19 +18,16 @@ struct transport_vtable {
> * the transport to try to share connections, for_push is a
> * hint as to whether the ultimate operation is a push or a fetch.
> *
> - * If communicating using protocol v2 a list of prefixes can be
> - * provided to be sent to the server to enable it to limit the ref
> - * advertisement. Since ref filtering is done on the server's end, and
> - * only when using protocol v2, this list will be ignored when not
> - * using protocol v2 meaning this function can return refs which don't
> - * match the provided ref_prefixes.
> - *
> * If the transport is able to determine the remote hash for
> * the ref without a huge amount of effort, it should store it
> * in the ref's old_sha1 field; otherwise it should be all 0.
> + *
> + * See transport_get_remote_refs() for information on ref_prefixes and
> + * unborn_head_target.
> **/
> struct ref *(*get_refs_list)(struct transport *transport, int for_push,
> - const struct strvec *ref_prefixes);
> + const struct strvec *ref_prefixes,
> + char **unborn_head_target);
>
> /**
> * Fetch the objects for the given refs. Note that this gets
> diff --git a/transport.c b/transport.c
> index 47da955e4f..815e175017 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -127,7 +127,8 @@ struct bundle_transport_data {
>
> static struct ref *get_refs_from_bundle(struct transport *transport,
> int for_push,
> - const struct strvec *ref_prefixes)
> + const struct strvec *ref_prefixes,
> + char **unborn_head_target)
> {
> struct bundle_transport_data *data = transport->data;
> struct ref *result = NULL;
> @@ -163,7 +164,7 @@ static int fetch_refs_from_bundle(struct transport *transport,
> int ret;
>
> if (!data->get_refs_from_bundle_called)
> - get_refs_from_bundle(transport, 0, NULL);
> + get_refs_from_bundle(transport, 0, NULL, NULL);
> ret = unbundle(the_repository, &data->header, data->fd,
> transport->progress ? BUNDLE_VERBOSE : 0);
> transport->hash_algo = data->header.hash_algo;
> @@ -281,7 +282,7 @@ static void die_if_server_options(struct transport *transport)
> */
> static struct ref *handshake(struct transport *transport, int for_push,
> const struct strvec *ref_prefixes,
> - int must_list_refs)
> + int must_list_refs, char **unborn_head_target)
> {
> struct git_transport_data *data = transport->data;
> struct ref *refs = NULL;
> @@ -301,7 +302,8 @@ static struct ref *handshake(struct transport *transport, int for_push,
> get_remote_refs(data->fd[1], &reader, &refs, for_push,
> ref_prefixes,
> transport->server_options,
> - transport->stateless_rpc);
> + transport->stateless_rpc,
> + unborn_head_target);
> break;
> case protocol_v1:
> case protocol_v0:
> @@ -324,9 +326,11 @@ static struct ref *handshake(struct transport *transport, int for_push,
> }
>
> static struct ref *get_refs_via_connect(struct transport *transport, int for_push,
> - const struct strvec *ref_prefixes)
> + const struct strvec *ref_prefixes,
> + char **unborn_head_target)
> {
> - return handshake(transport, for_push, ref_prefixes, 1);
> + return handshake(transport, for_push, ref_prefixes, 1,
> + unborn_head_target);
> }
>
> static int fetch_refs_via_pack(struct transport *transport,
> @@ -370,7 +374,7 @@ static int fetch_refs_via_pack(struct transport *transport,
> break;
> }
> }
> - refs_tmp = handshake(transport, 0, NULL, must_list_refs);
> + refs_tmp = handshake(transport, 0, NULL, must_list_refs, NULL);
> }
>
> if (data->version == protocol_unknown_version)
> @@ -765,7 +769,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
> return -1;
>
> if (!data->got_remote_heads)
> - get_refs_via_connect(transport, 1, NULL);
> + get_refs_via_connect(transport, 1, NULL, NULL);
>
> memset(&args, 0, sizeof(args));
> args.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);
> @@ -1251,7 +1255,8 @@ int transport_push(struct repository *r,
>
> trace2_region_enter("transport_push", "get_refs_list", r);
> remote_refs = transport->vtable->get_refs_list(transport, 1,
> - &ref_prefixes);
> + &ref_prefixes,
> + NULL);
> trace2_region_leave("transport_push", "get_refs_list", r);
>
> strvec_clear(&ref_prefixes);
> @@ -1370,12 +1375,14 @@ int transport_push(struct repository *r,
> }
>
> const struct ref *transport_get_remote_refs(struct transport *transport,
> - const struct strvec *ref_prefixes)
> + const struct strvec *ref_prefixes,
> + char **unborn_head_target)
> {
> if (!transport->got_remote_refs) {
> transport->remote_refs =
> transport->vtable->get_refs_list(transport, 0,
> - ref_prefixes);
> + ref_prefixes,
> + unborn_head_target);
> transport->got_remote_refs = 1;
> }
>
> diff --git a/transport.h b/transport.h
> index 24558c027d..65de0c9c00 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -241,9 +241,14 @@ int transport_push(struct repository *repo,
> * advertisement. Since ref filtering is done on the server's end (and only
> * when using protocol v2), this can return refs which don't match the provided
> * ref_prefixes.
> + *
> + * If unborn_head_target is not NULL, and the remote reports HEAD as pointing
> + * to an unborn branch, this function stores the unborn branch in
> + * unborn_head_target. It should be freed by the caller.
> */
> const struct ref *transport_get_remote_refs(struct transport *transport,
> - const struct strvec *ref_prefixes);
> + const struct strvec *ref_prefixes,
> + char **unborn_head_target);
>
> /*
> * Fetch the hash algorithm used by a remote.
next prev parent reply other threads:[~2020-12-16 6:21 UTC|newest]
Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-08 1:31 Cloning empty repository uses locally configured default branch name Jonathan Tan
2020-12-08 2:16 ` Junio C Hamano
2020-12-08 2:32 ` brian m. carlson
2020-12-08 18:55 ` Jonathan Tan
2020-12-08 21:00 ` Junio C Hamano
2020-12-08 15:58 ` Jeff King
2020-12-08 20:06 ` Jonathan Tan
2020-12-08 21:15 ` Jeff King
2020-12-11 21:05 ` [PATCH] clone: in protocol v2, use remote's default branch Jonathan Tan
2020-12-11 23:41 ` Junio C Hamano
2020-12-14 12:38 ` Ævar Arnfjörð Bjarmason
2020-12-14 15:51 ` Felipe Contreras
2020-12-14 16:30 ` Junio C Hamano
2020-12-15 1:41 ` Ævar Arnfjörð Bjarmason
2020-12-15 2:22 ` Junio C Hamano
2020-12-15 2:38 ` Jeff King
2020-12-15 2:55 ` Junio C Hamano
2020-12-15 4:36 ` Jeff King
2020-12-16 3:09 ` Junio C Hamano
2020-12-16 18:39 ` Jeff King
2020-12-16 20:56 ` Junio C Hamano
2020-12-18 6:19 ` Jeff King
2020-12-15 3:22 ` Felipe Contreras
2020-12-14 19:25 ` Jonathan Tan
2020-12-14 19:42 ` Felipe Contreras
2020-12-15 1:27 ` Jeff King
2020-12-15 19:10 ` Jonathan Tan
2020-12-16 2:07 ` [PATCH v2 0/3] Cloning with remote unborn HEAD Jonathan Tan
2020-12-16 2:07 ` [PATCH v2 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2020-12-16 6:16 ` Junio C Hamano
2020-12-16 23:49 ` Jonathan Tan
2020-12-16 18:23 ` Jeff King
2020-12-16 23:54 ` Jonathan Tan
2020-12-17 1:32 ` Junio C Hamano
2020-12-18 6:16 ` Jeff King
2020-12-16 2:07 ` [PATCH v2 2/3] connect, transport: add no-op arg for future patch Jonathan Tan
2020-12-16 6:20 ` Junio C Hamano [this message]
2020-12-16 2:07 ` [PATCH v2 3/3] clone: respect remote unborn HEAD Jonathan Tan
2020-12-21 22:30 ` [PATCH v3 0/3] Cloning with " Jonathan Tan
2020-12-21 22:30 ` [PATCH v3 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2020-12-21 22:31 ` [PATCH v3 2/3] connect, transport: add no-op arg for future patch Jonathan Tan
2020-12-21 22:31 ` [PATCH v3 3/3] clone: respect remote unborn HEAD Jonathan Tan
2020-12-21 23:48 ` [PATCH v3 0/3] Cloning with " Junio C Hamano
2021-01-21 20:14 ` Jeff King
2020-12-22 21:54 ` [PATCH v4 " Jonathan Tan
2020-12-22 21:54 ` [PATCH v4 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-01-21 20:48 ` Jeff King
2021-01-26 18:13 ` Jonathan Tan
2021-01-26 23:16 ` Jeff King
2020-12-22 21:54 ` [PATCH v4 2/3] connect, transport: add no-op arg for future patch Jonathan Tan
2021-01-21 20:55 ` Jeff King
2021-01-26 18:16 ` Jonathan Tan
2020-12-22 21:54 ` [PATCH v4 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-01-21 21:02 ` Jeff King
2021-01-26 18:22 ` Jonathan Tan
2021-01-26 23:04 ` Jeff King
2021-01-28 5:50 ` Junio C Hamano
2020-12-22 22:06 ` [PATCH v4 0/3] Cloning with " Junio C Hamano
2021-01-26 18:55 ` [PATCH v5 " Jonathan Tan
2021-01-26 18:55 ` [PATCH v5 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-01-26 21:38 ` Junio C Hamano
2021-01-26 23:03 ` Junio C Hamano
2021-01-30 3:55 ` Jonathan Tan
2021-01-26 23:20 ` Jeff King
2021-01-26 23:38 ` Junio C Hamano
2021-01-29 20:23 ` Jonathan Tan
2021-01-29 22:04 ` Junio C Hamano
2021-02-02 2:20 ` Jonathan Tan
2021-02-02 5:00 ` Junio C Hamano
2021-01-27 1:28 ` Ævar Arnfjörð Bjarmason
2021-01-30 4:04 ` Jonathan Tan
2021-01-26 18:55 ` [PATCH v5 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-01-26 21:54 ` Junio C Hamano
2021-01-30 4:06 ` Jonathan Tan
2021-01-26 18:55 ` [PATCH v5 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-01-26 22:24 ` Junio C Hamano
2021-01-30 4:27 ` Jonathan Tan
2021-01-27 1:11 ` [PATCH v5 0/3] Cloning with " Junio C Hamano
2021-01-27 4:25 ` Jeff King
2021-01-27 6:14 ` Junio C Hamano
2021-01-27 1:41 ` Ævar Arnfjörð Bjarmason
2021-01-30 4:41 ` Jonathan Tan
2021-01-30 11:13 ` Ævar Arnfjörð Bjarmason
2021-02-02 2:22 ` Jonathan Tan
2021-02-03 14:23 ` Ævar Arnfjörð Bjarmason
2021-02-05 22:28 ` Junio C Hamano
2021-02-02 2:14 ` [PATCH v6 " Jonathan Tan
2021-02-02 2:14 ` [PATCH v6 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-02-02 16:55 ` Junio C Hamano
2021-02-02 18:34 ` Jonathan Tan
2021-02-02 22:17 ` Junio C Hamano
2021-02-03 1:04 ` Jonathan Tan
2021-02-02 2:15 ` [PATCH v6 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-02-02 2:15 ` [PATCH v6 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-02-05 4:58 ` [PATCH v7 0/3] Cloning with " Jonathan Tan
2021-02-05 4:58 ` [PATCH v7 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-02-05 16:10 ` Jeff King
2021-02-05 4:58 ` [PATCH v7 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-02-05 4:58 ` [PATCH v7 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-02-05 5:25 ` [PATCH v7 0/3] Cloning with " Junio C Hamano
2021-02-05 16:15 ` Jeff King
2021-02-05 21:15 ` Ævar Arnfjörð Bjarmason
2021-02-05 23:07 ` Junio C Hamano
2021-02-05 20:48 ` [PATCH v8 " Jonathan Tan
2021-02-05 20:48 ` [PATCH v8 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-02-05 20:48 ` [PATCH v8 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-02-05 20:48 ` [PATCH v8 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-02-06 18:51 ` [PATCH v8 0/3] Cloning with " Junio C Hamano
2021-02-08 22:28 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwnxi46gt.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).