[PATCH v5 0/3] Cloning with remote unborn HEAD

From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH v5 0/3] Cloning with remote unborn HEAD
Date: Tue, 26 Jan 2021 10:55:54 -0800	[thread overview]
Message-ID: <cover.1611686656.git.jonathantanmy@google.com> (raw)
In-Reply-To: <20201208013121.677494-1-jonathantanmy@google.com>

Thanks, Peff, for your review. I have addressed your comments (through
replies to your emails and here in this v5 patch set).

Jonathan Tan (3):
  ls-refs: report unborn targets of symrefs
  connect, transport: encapsulate arg in struct
  clone: respect remote unborn HEAD

 Documentation/config.txt                |  2 +
 Documentation/config/init.txt           |  2 +-
 Documentation/config/lsrefs.txt         |  3 ++
 Documentation/technical/protocol-v2.txt | 10 ++++-
 builtin/clone.c                         | 34 +++++++++++-----
 builtin/fetch-pack.c                    |  3 +-
 builtin/fetch.c                         | 18 +++++----
 builtin/ls-remote.c                     |  9 +++--
 connect.c                               | 32 +++++++++++++--
 ls-refs.c                               | 53 +++++++++++++++++++++++--
 ls-refs.h                               |  1 +
 remote.h                                |  4 +-
 serve.c                                 |  2 +-
 t/t5606-clone-options.sh                |  8 ++--
 t/t5701-git-serve.sh                    |  2 +-
 t/t5702-protocol-v2.sh                  | 25 ++++++++++++
 transport-helper.c                      |  5 ++-
 transport-internal.h                    |  9 +----
 transport.c                             | 23 ++++++-----
 transport.h                             | 29 ++++++++++----
 20 files changed, 210 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/config/lsrefs.txt

Range-diff against v4:
1:  d7d2ba597e ! 1:  32e16dfdbd ls-refs: report unborn targets of symrefs
    @@ Commit message

         Currently, symrefs that have unborn targets (such as in this case) are
         not communicated by the protocol. Teach Git to advertise and support the
    -    "unborn" feature in "ls-refs" (guarded by the lsrefs.allowunborn
    +    "unborn" feature in "ls-refs" (by default, this is advertised, but
    +    server administrators may turn this off through the lsrefs.allowunborn
         config). This feature indicates that "ls-refs" supports the "unborn"
         argument; when it is specified, "ls-refs" will send the HEAD symref with
         the name of its unborn target.
    @@ ls-refs.c: static int send_ref(const char *refname, const struct object_id *oid,
     +	int flag;
     +	int oid_is_null;
     +
    -+	memset(&oid, 0, sizeof(oid));
     +	strbuf_addf(&namespaced, "%sHEAD", get_git_namespace());
    -+	resolve_ref_unsafe(namespaced.buf, 0, &oid, &flag);
    ++	if (!resolve_ref_unsafe(namespaced.buf, 0, &oid, &flag))
    ++		return; /* bad ref */
     +	oid_is_null = is_null_oid(&oid);
     +	if (!oid_is_null ||
     +	    (data->unborn && data->symrefs && (flag & REF_ISSYMREF)))
    @@ ls-refs.c: int ls_refs(struct repository *r, struct strvec *keys,
      	memset(&data, 0, sizeof(data));

     -	git_config(ls_refs_config, NULL);
    ++	data.allow_unborn = 1;
     +	git_config(ls_refs_config, &data);

      	while (packet_reader_read(request) == PACKET_READ_NORMAL) {
    @@ ls-refs.c: int ls_refs(struct repository *r, struct strvec *keys,
     +	if (value) {
     +		int allow_unborn_value;
     +
    -+		if (!repo_config_get_bool(the_repository,
    ++		if (repo_config_get_bool(the_repository,
     +					 "lsrefs.allowunborn",
    -+					 &allow_unborn_value) &&
    ++					 &allow_unborn_value) ||
     +		    allow_unborn_value)
     +			strbuf_addstr(value, "unborn");
     +	}
    @@ serve.c: struct protocol_capability {
      	{ "fetch", upload_pack_advertise, upload_pack_v2 },
      	{ "server-option", always_advertise, NULL },
      	{ "object-format", object_format_advertise, NULL },
    +
    + ## t/t5701-git-serve.sh ##
    +@@ t/t5701-git-serve.sh: test_expect_success 'test capability advertisement' '
    + 	cat >expect <<-EOF &&
    + 	version 2
    + 	agent=git/$(git version | cut -d" " -f3)
    +-	ls-refs
    ++	ls-refs=unborn
    + 	fetch=shallow
    + 	server-option
    + 	object-format=$(test_oid algo)
2:  51d8a359c7 < -:  ---------- connect, transport: add no-op arg for future patch
-:  ---------- > 2:  4eec551668 connect, transport: encapsulate arg in struct
3:  896be550f1 ! 3:  922e8c229c clone: respect remote unborn HEAD
    @@ Documentation/config/init.txt: init.templateDir::
     +	a new repository.

      ## builtin/clone.c ##
    -@@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
    - 	int submodule_progress;
    - 
    - 	struct strvec ref_prefixes = STRVEC_INIT;
    -+	char *unborn_head_target = NULL;
    - 
    - 	packet_trace_identity("clone");
    - 
    -@@ builtin/clone.c: 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, NULL);
    -+	refs = transport_get_remote_refs(transport, &ref_prefixes,
    -+					 &unborn_head_target);
    - 
    - 	if (refs) {
    - 		int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
     @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
      		remote_head = NULL;
      		option_no_checkout = 1;
    @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
     +			const char *branch;
     +			char *ref;
     +
    -+			if (unborn_head_target &&
    -+			    skip_prefix(unborn_head_target, "refs/heads/", &branch)) {
    -+				ref = unborn_head_target;
    -+				unborn_head_target = NULL;
    ++			if (transport_ls_refs_options.unborn_head_target &&
    ++			    skip_prefix(transport_ls_refs_options.unborn_head_target,
    ++					"refs/heads/", &branch)) {
    ++				ref = transport_ls_refs_options.unborn_head_target;
    ++				transport_ls_refs_options.unborn_head_target = NULL;
     +			} else {
     +				branch = git_default_branch_name();
     +				ref = xstrfmt("refs/heads/%s", branch);
    @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
      		}
      	}
     @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
    - 	strbuf_release(&key);
      	junk_mode = JUNK_LEAVE_ALL;

    -+	free(unborn_head_target);
    - 	strvec_clear(&ref_prefixes);
    + 	strvec_clear(&transport_ls_refs_options.ref_prefixes);
    ++	free(transport_ls_refs_options.unborn_head_target);
      	return err;
      }

    @@ connect.c: static int process_ref_v2(struct packet_reader *reader, struct ref **
      	if (parse_oid_hex_algop(line_sections.items[i++].string, &old_oid, &end, reader->hash_algo) ||
      	    *end) {
      		ret = 0;
    +@@ connect.c: struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
    + 	const char *hash_name;
    + 	struct strvec *ref_prefixes = transport_options ?
    + 		&transport_options->ref_prefixes : NULL;
    ++	char **unborn_head_target = transport_options ?
    ++		&transport_options->unborn_head_target : NULL;
    + 	*list = NULL;
    + 
    + 	if (server_supports_v2("ls-refs", 1))
     @@ connect.c: struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
      	if (!for_push)
      		packet_write_fmt(fd_out, "peel\n");
    @@ connect.c: 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))
     +		if (!process_ref_v2(reader, &list, unborn_head_target))
      			die(_("invalid ls-refs response: %s"), reader->line);
    @@ t/t5702-protocol-v2.sh: test_expect_success 'clone with file:// using protocol v
      '

     +test_expect_success 'clone of empty repo propagates name of default branch' '
    ++	test_when_finished "rm -rf file_empty_parent file_empty_child" &&
    ++
     +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
     +	git -c init.defaultBranch=mydefaultbranch init file_empty_parent &&
    -+	test_config -C file_empty_parent lsrefs.allowUnborn true &&
     +
     +	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
     +	git -c init.defaultBranch=main -c protocol.version=2 \
     +		clone "file://$(pwd)/file_empty_parent" file_empty_child &&
     +	grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
     +'
    ++
    ++test_expect_success '...but not if explicitly forbidden by config' '
    ++	test_when_finished "rm -rf file_empty_parent file_empty_child" &&
    ++
    ++	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
    ++	git -c init.defaultBranch=mydefaultbranch init file_empty_parent &&
    ++	test_config -C file_empty_parent lsrefs.allowUnborn false &&
    ++
    ++	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
    ++	git -c init.defaultBranch=main -c protocol.version=2 \
    ++		clone "file://$(pwd)/file_empty_parent" file_empty_child &&
    ++	! grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
    ++'
     +
      test_expect_success 'fetch with file:// using protocol v2' '
      	test_when_finished "rm -f log" &&

    +
    + ## transport.h ##
    +@@ transport.h: struct transport_ls_refs_options {
    + 	 * provided ref_prefixes.
    + 	 */
    + 	struct strvec ref_prefixes;
    ++
    ++	/*
    ++	 * If unborn_head_target is not NULL, and the remote reports HEAD as
    ++	 * pointing to an unborn branch, transport_get_remote_refs() stores the
    ++	 * unborn branch in unborn_head_target. It should be freed by the
    ++	 * caller.
    ++	 */
    ++	char *unborn_head_target;
    + };
    + #define TRANSPORT_LS_REFS_OPTIONS_INIT { STRVEC_INIT }
    + 
-- 
2.30.0.280.ga3ce27912f-goog