Re: [PATCH v3 07/10] clone: --branch=<branch> always means refs/heads/<branch>
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Jan 12, 2012, 03:00 UTC
- Message-ID
- <CACsJy8DRCqhMqp9n1qrXakRHTBk1SWeCWu5jp0bngXuMQPY88w@mail.gmail.com>
- In-Reply-To
- <7v4nw26mbe.fsf@alter.siamese.dyndns.org>
2012/1/12 Junio C Hamano <gitster@pobox.com>:
Show 40 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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 <pclouds@gmail.com>
>> ---
>> 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.
Show 7 quoted lines
> 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