From: Nguyen Thai Ngoc Duy Date: Thu, 12 Jan 2012 03:00:17 GMT Subject: Re: [PATCH v3 07/10] clone: --branch= always means refs/heads/ Message-ID: In-Reply-To: <7v4nw26mbe.fsf@alter.siamese.dyndns.org> 2012/1/12 Junio C Hamano : > Nguyễn Thái Ngọc Duy writes: > >> It does not make sense to look outside refs/heads for HEAD's target >> (src_ref_prefix can be set to "refs/" if --mirror is used) because ref >> code only allows symref in form refs/heads/... >> >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- >>  builtin/clone.c |    4 ++-- >>  1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/builtin/clone.c b/builtin/clone.c >> index 05224d7..960242f 100644 >> --- a/builtin/clone.c >> +++ b/builtin/clone.c >> @@ -463,7 +463,7 @@ static void update_remote_refs(const struct ref *refs, >>  static void update_head(const struct ref *our, const struct ref *remote, >>                       const char *msg) >>  { >> -     if (our) { >> +     if (our && !prefixcmp(our->name, "refs/heads/")) { >>               /* Local default branch link */ >>               create_symref("HEAD", our->name, NULL); >>               if (!option_bare) { > > I think this makes sense. In the following three casees: > >  - When cloning without --branch, if we could not find a branch that match >   HEAD at the remote; > >  - When cloning with --branch, if we did not see such a branch at the >   remote; or > >  - When cloning from an empty repository > > we come here with "our" set to NULL. Additionally, if the remote HEAD > points outside refs/heads/ and the transport could detect that case > (e.g. a helper that reads from ls-remote output), we can see our->name > outside refs/heads/. Is there any other case where our is not NULL and > our->name does not start with refs/heads/? No, this patch pretty much guarantees that our->name must be inside refs/heads, unless remote's HEAD points outside, which makes it a broken remote and git-upload-pack should refuse to serve in the first place. Until --branch=tag comes into the picture. > The "else if (remote)" clause (not shown, outside the context) that > follows still has comments that says it is a case for "Source had detached > HEAD pointing somewhere", and needs to be adjusted for this change, no? It > is "(1) we know the HEAD points at a non-branch, (2) HEAD may point at a > branch but we do not know which one, or (3) we asked for a specific branch > but it did not exist there" (cloning from an empty will have NULL in > remote and the comment would not apply to that case). Yes. -- Duy