git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: Cloning empty repository uses locally configured default branch name
Date: Tue, 8 Dec 2020 10:58:51 -0500	[thread overview]
Message-ID: <X8+iu/0nPfd0lrSn@coredump.intra.peff.net> (raw)
In-Reply-To: <20201208013121.677494-1-jonathantanmy@google.com>

On Mon, Dec 07, 2020 at 05:31:20PM -0800, Jonathan Tan wrote:

> When cloning an empty repository, a local branch is created. But its
> name is not the name of the branch that the remote HEAD points to - it
> is the locally configured default branch name. This issue arose at
> $DAYJOB and, from my memory, it is also not an uncommon workflow to
> configure things online on a repo host and then use "git clone" so that
> things like remotes are automatically configured.
> 
> Has anyone looked into solutions for this? Both protocol v0 and v2 do
> not send symref information about unborn branches (v0 because, as
> protocol-capabilities.txt says, "servers SHOULD include this capability
> for the HEAD symref if it is one of the refs being sent"; v2 because
> a symref is included only if it refers to one of the refs being sent).
> In protocol v2, this could be done by adding a capability to ls-remote
> (maybe, "unborn"), and in protocol v0, this could be done either by
> updating the existing "symref" capability to be written even when the
> target branch is unborn (which is potentially backwards incompatible) or
> introducing a new capability which is like "symref".

We discussed this a few years ago, and I even wrote a small patch (for
v0 at the time, of course):

  https://lore.kernel.org/git/20170525155924.hk5jskennph6tta3@sigill.intra.peff.net/

A rebased version of that patch is below (it needed updating to handle
some namespacing stuff). Coupled with your patch here for the truly
empty repo case, it makes the server side of v0 do what you'd want.

But the client side needs to handle it, too. See the linked thread for
some discussion.

I wouldn't be too worried about the backwards incompatibility of sending
a symref line in the capabilities that doesn't point to a ref we're
sending. Old clients are quite likely to ignore it. But...

> A small issue is that upload-pack protocol v0 doesn't even write the
> blank ref line ("000...000 capabilities^{}") if HEAD points to an unborn
> branch, but that can be fixed as in the patch below.

I would worry how clients handle this bogus entry in the ref
advertisement. It looks like the actual Git client is OK, but what about
jgit, libgit2, etc? That's not necessarily a deal-breaker, but it would
be nice to know how they react.

It also only helps with v0 (and I agree with the sentiment that it would
be OK to ignore v0 at this point). For v2, we'd have to issue a HEAD
line like:

  0000000000000000000000000000000000000000 HEAD symref=refs/heads/foo

That probably would break clients, but the unborn capability should take
care of that.

Patch below (I think it only helps v0, but it could serve as a model for
doing the same thing in v2).

---
 upload-pack.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index 1006bebd50..b0cc337dcb 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1218,20 +1218,21 @@ static int send_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int find_symref(const char *refname, const struct object_id *oid,
-		       int flag, void *cb_data)
+static void find_symref(const char *refname, struct string_list *out)
 {
 	const char *symref_target;
 	struct string_list_item *item;
+	struct strbuf namespaced = STRBUF_INIT;
+	int flag;
+
+	strbuf_addf(&namespaced, "%s%s", get_git_namespace(), refname);
+	symref_target = resolve_ref_unsafe(namespaced.buf, 0, NULL, &flag);
+	strbuf_release(&namespaced);
 
-	if ((flag & REF_ISSYMREF) == 0)
-		return 0;
-	symref_target = resolve_ref_unsafe(refname, 0, NULL, &flag);
 	if (!symref_target || (flag & REF_ISSYMREF) == 0)
-		die("'%s' is a symref but it is not?", refname);
-	item = string_list_append(cb_data, strip_namespace(refname));
+		return;
+	item = string_list_append(out, refname);
 	item->util = xstrdup(strip_namespace(symref_target));
-	return 0;
 }
 
 static int parse_object_filter_config(const char *var, const char *value,
@@ -1326,7 +1327,7 @@ void upload_pack(struct upload_pack_options *options)
 	data.daemon_mode = options->daemon_mode;
 	data.timeout = options->timeout;
 
-	head_ref_namespaced(find_symref, &data.symref);
+	find_symref("HEAD", &data.symref);
 
 	if (options->advertise_refs || !data.stateless_rpc) {
 		reset_timeout(data.timeout);
-- 
2.29.2.980.g00fe049108


  parent reply	other threads:[~2020-12-08 16:02 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 [this message]
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
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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=X8+iu/0nPfd0lrSn@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).