Re: [PATCHv4] clone --single: limit the fetch refspec to fetched branch

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Sep 17, 2012, 12:06 UTC
Message-ID: <CACsJy8Bsps_-RaXdBd9ipWPvJukjK7mRN_wG2ezEL5d4JLyx+g@mail.gmail.com>
In-Reply-To: <1347783184-20045-1-git-send-email-ralf.thielow@gmail.com>

On Sun, Sep 16, 2012 at 3:13 PM, Ralf Thielow <ralf.thielow@gmail.com> wrote:

> +       if (option_mirror || !option_bare) {
> +               strbuf_reset(&value);

I think we should use a new strbuf local variable here to avoid resetting this. At least reviewers don't have to check if this statememt causes any effect later on because "value"'s value is gone.

Show 15 quoted lines

> +               if (option_single_branch) {
> +                       if (option_branch)
> +                               strbuf_addf(&value, "+%s%s:%s%s",
> +                                               src_ref_prefix, option_branch,
> +                                               branch_top.buf, option_branch);
> +                       else if (remote_head_points_at)
> +                               strbuf_addf(&value, "+%s:%s%s",
> +                                               remote_head_points_at->name, branch_top.buf,
> +                                               skip_prefix(remote_head_points_at->name, "refs/heads/"));
> +                       /*
> +                        * otherwise, the next "git fetch" will
> +                        * simply fetch from HEAD without updating
> +                        * any remote tracking branch, which is what
> +                        * we want.
> +                        */

Maybe document updates too? Though if it's obvious that --single-branch should prepare refspec so that only one branch is fetched later on, then maybe not.

> +               } else {
> +                       strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
> +               }

--mirror --single-branch combination does not look right. The "heads/" part is missing..

$ git branch
  master * wildmatch
$ LANG=C ./git clone --mirror --single-branch .git abc
Cloning into bare repository 'abc'...
done.
$ grep fetch abc/config
        fetch = +refs/heads/wildmatch:refs/wildmatch
$ rm -rf abc
$ LANG=C ./git clone --mirror --single-branch --branch=master .git abc
Cloning into bare repository 'abc'...
done.
$ grep fetch abc/config
        fetch = +refs/master:refs/master

-- 
Duy