From: Nguyen Thai Ngoc Duy Date: Sat, 07 Jan 2012 14:16:22 GMT Subject: Re: [PATCH v3] Limit refs to fetch to minimum in shallow clones Message-ID: In-Reply-To: <7vfwfsllny.fsf@alter.siamese.dyndns.org> 2012/1/7 Junio C Hamano : > 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. >> +             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. >> @@ -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