[RFD/PATCH 0/5] Improving the search for remote-tracking branches

@@ -0,0 +1,66 @@
+#!/bin/sh
+
+test_description='checkout <branch>
+
+Ensures that checkout on an unborn branch does what the user expects'
+
+. ./test-lib.sh
+
+# Arguments: <branch> <remote> <remote-tracking>
+#
+# Verify that we have checked out <branch>, and that it is at the same
+# commit as <remote-tracking>, and that has appropriate tracking config
+# setup against <remote>
+test_tracking_branch() {
+	branch=$1 &&
+	remote=$2 &&
+	remote_track=$3 &&
+	test "refs/heads/$branch" = "$(git rev-parse --symbolic-full-name HEAD)" &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify "$remote_track")" &&
+	test "$remote" = "$(git config "branch.$branch.remote")" &&
+	test "refs/heads/$branch" = "$(git config "branch.$branch.merge")"
+}
+
+test_expect_success 'setup' '
+	(git init repo_a &&
+	 cd repo_a &&
+	 test_commit a_master &&
+	 git checkout -b foo &&
+	 test_commit a_foo &&
+	 git checkout -b bar &&
+	 test_commit a_bar
+	) &&
+	(git init repo_b &&
+	 cd repo_b &&
+	 test_commit b_master &&
+	 git checkout -b foo &&
+	 test_commit b_foo &&
+	 git checkout -b baz &&
+	 test_commit b_baz
+	) &&
+	git remote add repo_a repo_a &&
+	git remote add repo_b repo_b &&
+	git config remote.repo_b.fetch \
+		"+refs/heads/*:refs/remotes/other_b/*" &&
+	git fetch --all
+'
+
+test_expect_success 'checkout of non-existing branch fails' '
+	test_must_fail git checkout xyzzy
+'
+
+test_expect_success 'checkout of branch from multiple remotes fails' '
+	test_must_fail git checkout foo
+'
+
+test_expect_success 'checkout of branch from a single remote succeeds #1' '
+	git checkout bar &&
+	test_tracking_branch bar repo_a refs/remotes/repo_a/bar
+'
+
+test_expect_success 'checkout of branch from a single remote succeeds #2' '
+	git checkout baz &&
+	test_tracking_branch baz repo_b refs/remotes/other_b/baz
+'
+
+test_done

@@ -22,6 +22,7 @@ test_tracking_branch() {
 }
 
 test_expect_success 'setup' '
+	test_commit my_master &&
 	(git init repo_a &&
 	 cd repo_a &&
 	 test_commit a_master &&

@@ -49,7 +50,7 @@ test_expect_success 'checkout of non-existing branch fails' '
 	test_must_fail git checkout xyzzy
 '
 
-test_expect_success 'checkout of branch from multiple remotes fails' '
+test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_must_fail git checkout foo
 '
 

@@ -63,4 +64,53 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 	test_tracking_branch baz repo_b refs/remotes/other_b/baz
 '
 
+test_expect_success 'setup more remotes with unconventional refspecs' '
+	git checkout master &&
+	git branch -D bar &&
+	git branch -D baz &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify my_master)" &&
+	(git init repo_c &&
+	 cd repo_c &&
+	 test_commit c_master &&
+	 git checkout -b bar &&
+	 test_commit c_bar
+	 git checkout -b spam &&
+	 test_commit c_spam
+	) &&
+	(git init repo_d &&
+	 cd repo_d &&
+	 test_commit d_master &&
+	 git checkout -b baz &&
+	 test_commit f_baz
+	 git checkout -b eggs &&
+	 test_commit c_eggs
+	) &&
+	git remote add repo_c repo_c &&
+	git config remote.repo_c.fetch \
+	    "+refs/heads/*:refs/remotes/extra_dir/repo_c/extra_dir/*" &&
+	git fetch repo_c &&
+	git remote add repo_d repo_d &&
+	git config remote.repo_d.fetch \
+	    "+refs/heads/*:refs/repo_d/*" &&
+	git fetch repo_d
+'
+
+test_expect_failure 'checkout of branch from multiple remotes fails #2' '
+	test_must_fail git checkout bar
+'
+
+test_expect_failure 'checkout of branch from multiple remotes fails #3' '
+	test_must_fail git checkout baz
+'
+
+test_expect_failure 'checkout of branch from a single remote succeeds #3' '
+	git checkout spam &&
+	test_tracking_branch spam repo_c refs/remotes/extra_dir/repo_c/extra_dir/spam
+'
+
+test_expect_failure 'checkout of branch from a single remote succeeds #4' '
+	git checkout eggs &&
+	test_tracking_branch eggs repo_d refs/repo_d/eggs
+'
+
 test_done

@@ -131,9 +131,9 @@ entries; instead, unmerged entries are ignored.
 	"--track" in linkgit:git-branch[1] for details.
 +
 If no '-b' option is given, the name of the new branch will be
-derived from the remote-tracking branch.  If "remotes/" or "refs/remotes/"
-is prefixed it is stripped away, and then the part up to the
-next slash (which would be the nickname of the remote) is removed.
+derived from the remote-tracking branch, by looking at the local part of
+the refspec configured for the corresponding remote, and then stripping
+the initial part up to the "*".
 This would tell us to use "hack" as the local branch when branching
 off of "origin/hack" (or "remotes/origin/hack", or even
 "refs/remotes/origin/hack").  If the given name has no slash, or the above

@@ -822,38 +822,40 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 }
 
 struct tracking_name_data {
-	const char *name;
-	char *remote;
+	/* const */ char *src_ref;
+	char *dst_ref;
+	unsigned char *dst_sha1;
 	int unique;
 };
 
-static int check_tracking_name(const char *refname, const unsigned char *sha1,
-			       int flags, void *cb_data)
+static int check_tracking_name(struct remote *remote, void *cb_data)
 {
 	struct tracking_name_data *cb = cb_data;
-	const char *slash;
-
-	if (prefixcmp(refname, "refs/remotes/"))
-		return 0;
-	slash = strchr(refname + 13, '/');
-	if (!slash || strcmp(slash + 1, cb->name))
+	struct refspec query;
+	memset(&query, 0, sizeof(struct refspec));
+	query.src = cb->src_ref;
+	if (remote_find_tracking(remote, &query) ||
+	    get_sha1(query.dst, cb->dst_sha1))
 		return 0;
-	if (cb->remote) {
+	if (cb->dst_ref) {
 		cb->unique = 0;
 		return 0;
 	}
-	cb->remote = xstrdup(refname);
+	cb->dst_ref = xstrdup(query.dst);
 	return 0;
 }
 
-static const char *unique_tracking_name(const char *name)
+static const char *unique_tracking_name(const char *name, unsigned char *sha1)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, 1 };
-	cb_data.name = name;
-	for_each_ref(check_tracking_name, &cb_data);
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	char src_ref[PATH_MAX];
+	snprintf(src_ref, PATH_MAX, "refs/heads/%s", name);
+	cb_data.src_ref = src_ref;
+	cb_data.dst_sha1 = sha1;
+	for_each_remote(check_tracking_name, &cb_data);
 	if (cb_data.unique)
-		return cb_data.remote;
-	free(cb_data.remote);
+		return cb_data.dst_ref;
+	free(cb_data.dst_ref);
 	return NULL;
 }
 

@@ -916,8 +918,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 		if (dwim_new_local_branch_ok &&
 		    !check_filename(NULL, arg) &&
 		    argc == 1) {
-			const char *remote = unique_tracking_name(arg);
-			if (!remote || get_sha1(remote, rev))
+			const char *remote = unique_tracking_name(arg, rev);
+			if (!remote)
 				return argcount;
 			*new_branch = arg;
 			arg = remote;

@@ -95,15 +95,15 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 	git fetch repo_d
 '
 
-test_expect_failure 'checkout of branch from multiple remotes fails #2' '
+test_expect_success 'checkout of branch from multiple remotes fails #2' '
 	test_must_fail git checkout bar
 '
 
-test_expect_failure 'checkout of branch from multiple remotes fails #3' '
+test_expect_success 'checkout of branch from multiple remotes fails #3' '
 	test_must_fail git checkout baz
 '
 
-test_expect_failure 'checkout of branch from a single remote succeeds #3' '
+test_expect_success 'checkout of branch from a single remote succeeds #3' '
 	git checkout spam &&
 	test_tracking_branch spam repo_c refs/remotes/extra_dir/repo_c/extra_dir/spam
 '

@@ -274,7 +274,6 @@ void create_branch(const char *head,
 	case 1:
 		/* Unique completion -- good, only if it is a real branch */
 		if (prefixcmp(real_ref, "refs/heads/") &&
-		    prefixcmp(real_ref, "refs/remotes/") &&
 		    validate_remote_tracking_branch(real_ref)) {
 			if (explicit_tracking)
 				die(_(upstream_not_branch), start_name);

@@ -317,7 +317,7 @@ test_expect_success 'test tracking setup (non-wildcard, matching)' '
 	test $(git config branch.my4.merge) = refs/heads/master
 '
 
-test_expect_success 'test tracking setup (non-wildcard, not matching)' '
+test_expect_failure 'test tracking setup (non-wildcard, not matching)' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&

@@ -429,7 +429,7 @@ test_expect_success 'detach a symbolic link HEAD' '
     test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
 '
 
-test_expect_success \
+test_expect_failure \
     'checkout with --track fakes a sensible -b <name>' '
     git update-ref refs/remotes/origin/koala/bear renamer &&
 

@@ -45,7 +45,7 @@ test_expect_success 'setup svn repository' '
 	)
 	'
 
-test_expect_success 'setup git mirror and merge' '
+test_expect_failure 'setup git mirror and merge' '
 	git svn init "$svnrepo" -t tags -T trunk -b branches &&
 	git svn fetch &&
 	git checkout --track -b svn remotes/trunk &&

@@ -67,7 +67,7 @@ test_expect_success 'setup git mirror and merge' '
 
 test_debug 'gitk --all & sleep 1'
 
-test_expect_success 'verify pre-merge ancestry' "
+test_expect_failure 'verify pre-merge ancestry' "
 	test x\`git rev-parse --verify refs/heads/svn^2\` = \
 	     x\`git rev-parse --verify refs/heads/merge\` &&
 	git cat-file commit refs/heads/svn^ | grep '^friend$'

@@ -79,7 +79,7 @@ test_expect_success 'git svn dcommit merges' "
 
 test_debug 'gitk --all & sleep 1'
 
-test_expect_success 'verify post-merge ancestry' "
+test_expect_failure 'verify post-merge ancestry' "
 	test x\`git rev-parse --verify refs/heads/svn\` = \
 	     x\`git rev-parse --verify refs/remotes/trunk \` &&
 	test x\`git rev-parse --verify refs/heads/svn^2\` = \

@@ -87,7 +87,7 @@ test_expect_success 'verify post-merge ancestry' "
 	git cat-file commit refs/heads/svn^ | grep '^friend$'
 	"
 
-test_expect_success 'verify merge commit message' "
+test_expect_failure 'verify merge commit message' "
 	git rev-list --pretty=raw -1 refs/heads/svn | \
 	  grep \"    Merge branch 'merge' into svn\"
 	"

@@ -197,6 +197,21 @@ int validate_new_branchname(const char *name, struct strbuf *ref,
 	return 1;
 }
 
+static int check_tracking_branch(struct remote *remote, void *cb_data)
+{
+	char *tracking_branch = cb_data;
+	struct refspec query;
+	memset(&query, 0, sizeof(struct refspec));
+	query.dst = tracking_branch;
+	return !(remote_find_tracking(remote, &query) ||
+		 prefixcmp(query.src, "refs/heads/"));
+}
+
+static int validate_remote_tracking_branch(char *ref)
+{
+	return !for_each_remote(check_tracking_branch, ref);
+}
+
 static const char upstream_not_branch[] =
 N_("Cannot setup tracking information; starting point '%s' is not a branch.");
 static const char upstream_missing[] =

@@ -259,7 +274,8 @@ void create_branch(const char *head,
 	case 1:
 		/* Unique completion -- good, only if it is a real branch */
 		if (prefixcmp(real_ref, "refs/heads/") &&
-		    prefixcmp(real_ref, "refs/remotes/")) {
+		    prefixcmp(real_ref, "refs/remotes/") &&
+		    validate_remote_tracking_branch(real_ref)) {
 			if (explicit_tracking)
 				die(_(upstream_not_branch), start_name);
 			else

@@ -108,7 +108,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #3' '
 	test_tracking_branch spam repo_c refs/remotes/extra_dir/repo_c/extra_dir/spam
 '
 
-test_expect_failure 'checkout of branch from a single remote succeeds #4' '
+test_expect_success 'checkout of branch from a single remote succeeds #4' '
 	git checkout eggs &&
 	test_tracking_branch eggs repo_d refs/repo_d/eggs
 '