Re: [PATCHv2] clone: fix refspec on "--single-branch" option

Ralf Thielow [off-list ref] writes:

+			else if (remote_head_points_at)
+					strbuf_addf(&value, "+%s:%s%s", our_head_points_at->name,
+									branch_top.buf, prettify_refname(remote_head_points_at->name));

As refspec maps names that appear on the source side to names that
appear on the destination side, and for fetch, the "soruce side"
is the remote, using "our_head_points_at" on the source side makes
it look very fishy (even though it may be a name derived from
remote_head_points_at and has the correct and appropriate value).

"prettify" also is very questionable.  It is meant to strip commonly
known prefix to make it easier to read by humans, and we can change
its result based solely on aesthetics in the future.  It is not
suitable for coming up with a value for configuration in the longer
term.

Can we make the part you moved de-dented a bit, perhaps by making it
into a small helper function or something?  It is extremely hard to
read with overly looooooong lines.

After a repo was cloned with the "--single-branch"
option, the configured refspec looks like
"+refs/heads/*:refs/remotes/origin/*".
After fetching from this repo again, it'll receive
all refs instead of just the ref from the single
branch. Fixing this by configure exactly the ref
of the branch the user specified in the "git clone"
command.

Signed-off-by: Ralf Thielow <redacted>
---

As refspec maps names that appear on the source side to names that
appear on the destination side, and for fetch, the "soruce side"
is the remote, using "our_head_points_at" on the source side makes
it look very fishy (even though it may be a name derived from
remote_head_points_at and has the correct and appropriate value).

"prettify" also is very questionable.  It is meant to strip commonly
known prefix to make it easier to read by humans, and we can change
its result based solely on aesthetics in the future.  It is not
suitable for coming up with a value for configuration in the longer
term.

Thanks. I've fixed.

Can we make the part you moved de-dented a bit, perhaps by making it
into a small helper function or something?  It is extremely hard to
read with overly looooooong lines.

Would be nice but the list of arguments will become too long.
I did a bit of reformating and think it now looks a bit nicer.

 builtin/clone.c | 43 +++++++++++++++++++++++++++++--------------
 1 Datei geändert, 29 Zeilen hinzugefügt(+), 14 Zeilen entfernt(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 5e8f3ba..06e3d3a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c

@@ -755,20 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	}
 
 	strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
-
-	if (option_mirror || !option_bare) {
-		/* Configure the remote */
-		strbuf_addf(&key, "remote.%s.fetch", option_origin);
-		git_config_set_multivar(key.buf, value.buf, "^$", 0);
-		strbuf_reset(&key);
-
-		if (option_mirror) {
-			strbuf_addf(&key, "remote.%s.mirror", option_origin);
-			git_config_set(key.buf, "true");
-			strbuf_reset(&key);
-		}
-	}
-
 	strbuf_addf(&key, "remote.%s.url", option_origin);
 	git_config_set(key.buf, repo);
 	strbuf_reset(&key);

@@ -853,6 +839,35 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					      "refs/heads/master");
 	}
 
+	if (option_mirror || !option_bare) {
+		strbuf_reset(&value);
+		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/"));
+		} else {
+			strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
+		}
+		/* Configure the remote */
+		if (value.len) {
+			strbuf_reset(&key);
+			strbuf_addf(&key, "remote.%s.fetch", option_origin);
+			git_config_set_multivar(key.buf, value.buf, "^$", 0);
+			strbuf_reset(&key);
+
+			if (option_mirror) {
+				strbuf_addf(&key, "remote.%s.mirror", option_origin);
+				git_config_set(key.buf, "true");
+				strbuf_reset(&key);
+			}
+		}
+	}
+
 	if (is_local)
 		clone_local(path, git_dir);
 	else if (refs && complete_refs_before_fetch)

-- 
1.7.12.396.g3cff853.dirty