Re: [PATCH v3] Limit refs to fetch to minimum in shallow clones

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Jan 7, 2012, 14:16 UTC
Message-ID: <CACsJy8DrabLzoxpmQqU13-81k=7Ni8kbg4HX3V67e3AL=L39wQ@mail.gmail.com>
In-Reply-To: <7vfwfsllny.fsf@alter.siamese.dyndns.org>

2012/1/7 Junio C Hamano <gitster@pobox.com>:

Show 15 quoted lines

> My first reaction after reading "is implied by default" was "Huh? didn't
> we just read these kick in only when --depth is given?" and I had to read
> it again. Here is my attempt to rephrase it.
>
>        Clone only the history leading to the tip of a single branch,
>        either specified by the `--branch` option or the primary branch
>        remote's `HEAD` points at. When creating a shallow clone with the
>        `--depth` option, this is the default, unless `--no-single-branch`
>        is given to fetch the histories near the tips of all branches.
>
>        Currently this option only works when creating a shallow clone and
>        does not have any effect without the `--depth` option.
>
> We might want to later enhance this to work also with a full-depth clone
> that tracks only one branch, and the above phrasing would make it clear.

Interesting. Yes that's another possibility.

Show 20 quoted lines

>> +             if (!option_branch)
>> +                     remote_head = guess_remote_head(head, refs, 0);
>> +             else {
>> +                     struct strbuf sb = STRBUF_INIT;
>> +                     strbuf_addstr(&sb, src_ref_prefix);
>> +                     strbuf_addstr(&sb, option_branch);
>> +                     remote_head = find_ref_by_name(refs, sb.buf);
>> +                     strbuf_release(&sb);
>> +             }
>> +
>> +             if (!remote_head)
>> +                     die(_("Remote branch \"%s\" not found. Nothing to clone.\n"
>> +                           "Try --no-single-branch to fetch all refs."),
>> +                         option_branch ? option_branch : "HEAD");
>
> Switching upon option_branch to tweak the message is a good idea, but
> strictly speaking, we would hit this die() when guess_remote_head() does
> not find where HEAD points at because it is detached, and in that case,
> the error is not "Nothing to clone", but "We couldn't tell which branch
> you meant to limit this cloning to".

Yeah. And in detached case without --branch, we probably should not say anything.

Show 13 quoted lines

>> @@ -642,9 +679,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>>
>>               transport_set_option(transport, TRANS_OPT_KEEP, "yes");
>>
>> -             if (option_depth)
>> +             if (option_depth) {
>>                       transport_set_option(transport, TRANS_OPT_DEPTH,
>>                                            option_depth);
>> +                     transport_set_option(transport, TRANS_OPT_FOLLOWTAGS,
>> +                                          option_single_branch ? "1" : NULL);
>
> Curious. Does anybody set FOLLOWTAGS to the transport by default becore we
> come here (just asking)?

No, I just hate another "if (option_single_branch)", which indents the call one more time. Always setting it to "1" in this case should probably be OK too. I'm just not sure if upload-pack realizes all tags are requested so include-tag extension means nothing, or it does extra work for no gain.

-- 
Duy