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

On Mon, Sep 17, 2012 at 2:06 PM, Nguyen Thai Ngoc Duy [off-list ref] wrote:

On Sun, Sep 16, 2012 at 3:13 PM, Ralf Thielow [off-list ref] wrote:

quoted

+       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.

It seems that we don't need this reset here because it's already
done earlier in this function. The variable "key" is also used multiple
times so I wouldn't use a new variable.

quoted

+               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.

I think it's obvious.

quoted

+               } 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

Thanks, I'll check this later and send a new version.