[PATCHv2 0/8] Improving the search for remote-tracking branches

[PATCHv2 0/8] Improving the search for remote-tracking branches

[Johan Herland]

Hi,

This is second iteration of this series. The initial three patches are
unchanged, although the commit message of #3 has been rephrased based
on Junio's comments.

Patches #4-#6 fixes existing tests in preparation for patch #7, which
changes the validation of the remote-tracking branch passed to --track:
We now require the --track argument to refer to a ref that matches a
configured refspec - otherwise, we can not reliably deduce the upstream
information to store into branch.<name>.remote and branch.<name>.merge.

Finally, patch #8 updates the paragraph on remote-tracking branches in
the glossary to be somewhat closer to the current state of things.


Have fun! :)

...Johan


Johan Herland (8):
  t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'
  t2024: Show failure to use refspec when DWIMming remote branch names
  checkout: Use remote refspecs when DWIMming tracking branches
  t3200.39: tracking setup should fail if there is no matching refspec.
  t7201.24: Add refspec to keep --track working
  t9114.2: Don't use --track option against "svn-remote"-tracking branches
  branch.c: Validate tracking branches with refspecs instead of refs/remotes/*
  glossary: Update and rephrase the definition of a remote-tracking branch

 Documentation/git-checkout.txt     |   6 +-
 Documentation/glossary-content.txt |  13 +++--
 branch.c                           |  17 +++++-
 builtin/checkout.c                 |  42 +++++++-------
 t/t2024-checkout-dwim.sh           | 116 +++++++++++++++++++++++++++++++++++++
 t/t3200-branch.sh                  |   8 +--
 t/t7201-co.sh                      |   1 +
 t/t9114-git-svn-dcommit-merge.sh   |   2 +-
 8 files changed, 170 insertions(+), 35 deletions(-)
 create mode 100755 t/t2024-checkout-dwim.sh

-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 1/8] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'

[Johan Herland]

The DWIM mode of checkout allows you to run "git checkout foo" when there is
no existing local ref or path called "foo", and there is exactly one remote
with a remote-tracking branch called "foo". Git will then automatically
create a new local branch called "foo" using the remote-tracking "foo" as
its starting point and configured upstream.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t2024-checkout-dwim.sh | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100755 t/t2024-checkout-dwim.sh

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
new file mode 100755
index 0000000..5650d21
--- /dev/null
+++ b/t/t2024-checkout-dwim.sh
@@ -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
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 2/8] t2024: Show failure to use refspec when DWIMming remote branch names

[Johan Herland]

When using "git checkout foo" to DWIM the creation of local "foo" from some
existing upstream "foo", we assume conventional refspecs as created by "git
clone" or "git remote add", and fail to work correctly if the current
refspecs do not follow the conventional "refs/remotes/$remote/*" pattern.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t2024-checkout-dwim.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 5650d21..36bf52f 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -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
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 3/8] checkout: Use remote refspecs when DWIMming tracking branches

[Johan Herland]

The DWIM mode of checkout allows you to run "git checkout foo" when there
is no existing local ref or path called "foo", and there is exactly _one_
remote with a remote-tracking branch called "foo". Git will automatically
create a new local branch called "foo" using the remote-tracking "foo" as
its starting point and configured upstream.

For example, consider the following unconventional (but perfectly valid)
remote setup:

	[remote "origin"]
		fetch = refs/heads/*:refs/remotes/origin/*
	[remote "frotz"]
		fetch = refs/heads/*:refs/remotes/frotz/nitfol/*

Case 1: Assume both "origin" and "frotz" have remote-tracking branches called
"foo", at "refs/remotes/origin/foo" and "refs/remotes/frotz/nitfol/foo"
respectively. In this case "git checkout foo" should fail, because there is
more than one remote with a "foo" branch.

Case 2: Assume only "frotz" have a remote-tracking branch called "foo". In
this case "git checkout foo" should succeed, and create a local branch "foo"
from "refs/remotes/frotz/nitfol/foo", using remote branch "foo" from "frotz"
as its upstream.

The current code hardcodes the assumption that all remote-tracking branches
must match the "refs/remotes/$remote/*" pattern (which is true for remotes
with "conventional" refspecs, but not true for the "frotz" remote above).
When running "git checkout foo", the current code looks for exactly one ref
matching "refs/remotes/*/foo", hence in the above example, it fails to find
"refs/remotes/frotz/nitfol/foo", which causes it to fail both case #1 and #2.

The better way to handle the above example is to actually study the fetch
refspecs to deduce the candidate remote-tracking branches for "foo"; i.e.
assume "foo" is a remote branch being fetched, and then map "refs/heads/foo"
through the refspecs in order to get the corresponding remote-tracking
branches "refs/remotes/origin/foo" and "refs/remotes/frotz/nitfol/foo".
Finally we check which of these happens to exist in the local repo, and
if there is exactly one, we have an unambiguous match for "git checkout foo",
and may proceed.

This fixes most of the failing tests introduced in the previous patch.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-checkout.txt |  6 +++---
 builtin/checkout.c             | 42 ++++++++++++++++++++++--------------------
 t/t2024-checkout-dwim.sh       |  6 +++---
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8edcdca..bf0c99c 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -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
diff --git a/builtin/checkout.c b/builtin/checkout.c
index eb51872..bcb18c8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -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;
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 36bf52f..fc6edc9 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -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
 '
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 4/8] t3200.39: tracking setup should fail if there is no matching refspec.

[Johan Herland]

We are formalizing a requirement that any remote-tracking branch to be used
as an upstream (i.e. as an argument to --track), _must_ "belong" to a
configured remote by being matched by the "dst" side of a fetch refspec.

This patch encodes the new expected behavior of this test, and marks the
test with "test_expect_failure" in anticipation of a following patch to
introduce the new behavior.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3200-branch.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index d969f0e..1bfdadc 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -317,13 +317,13 @@ 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 'tracking setup fails on non-matching refspec' '
 	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) &&
-	git branch --track my5 local/master &&
-	! test "$(git config branch.my5.remote)" = local &&
-	! test "$(git config branch.my5.merge)" = refs/heads/master
+	test_must_fail git branch --track my5 local/master &&
+	test_must_fail git config branch.my5.remote &&
+	test_must_fail git config branch.my5.merge
 '
 
 test_expect_success 'test tracking setup via config' '
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 5/8] t7201.24: Add refspec to keep --track working

[Johan Herland]

We are formalizing a requirement that any remote-tracking branch to be used
as an upstream (i.e. as an argument to --track), _must_ "belong" to a
configured remote by being matched by the "dst" side of a fetch refspec.

Without this patch, this test would start failing when the new behavior is
introduced.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t7201-co.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index be9672e..0c9ec0a 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -431,6 +431,7 @@ test_expect_success 'detach a symbolic link HEAD' '
 
 test_expect_success \
     'checkout with --track fakes a sensible -b <name>' '
+    git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" &&
     git update-ref refs/remotes/origin/koala/bear renamer &&
 
     git checkout --track origin/koala/bear &&
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 6/8] t9114.2: Don't use --track option against "svn-remote"-tracking branches

[Johan Herland]

We are formalizing a requirement that any remote-tracking branch to be used
as an upstream (i.e. as an argument to --track), _must_ "belong" to a
configured remote by being matched by the "dst" side of a fetch refspec.

This test uses --track against a "remotes/trunk" ref which does not belong
to any configured (git) remotes, but is instead created by "git svn fetch"
operating on an svn-remote. It does not make sense to use an svn-remote as
an upstream for a local branch, as a regular "git pull" from (or "git push"
to) it would obviously fail (instead you would need to use "git svn" to
communicate with this remote). Furthermore, the usage of --track in this
case is unnecessary, since the upstreaming config that would be created is
never used.

Simply removing --track fixes the issue without changing the expected
behavior of the test.

Cc: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t9114-git-svn-dcommit-merge.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 3077851..f524d2f 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -48,7 +48,7 @@ test_expect_success 'setup svn repository' '
 test_expect_success '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 &&
+	git checkout -b svn remotes/trunk &&
 	git checkout -b merge &&
 	echo new file > new_file &&
 	git add new_file &&
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 7/8] branch.c: Validate tracking branches with refspecs instead of refs/remotes/*

[Johan Herland]

The current code for validating tracking branches (e.g. the argument to
the -t/--track option) hardcodes refs/heads/* and refs/remotes/* as the
potential locations for tracking branches. This works with the refspecs
created by "git clone" or "git remote add", but is suboptimal in other
cases:

 - If "refs/remotes/foo/bar" exists without any association to a remote
   (i.e. there is no remote named "foo", or no remote with a refspec
   that matches "refs/remotes/foo/bar"), then it is impossible to set up
   a valid upstream config that tracks it. Currently, the code defaults
   to using "refs/remotes/foo/bar" from repo "." as the upstream, which
   works, but is probably not what the user had in mind when running
   "git branch baz --track foo/bar".

 - If the user has tweaked the fetch refspec for a remote to put its
   remote-tracking branches outside of refs/remotes/*, e.g. by running
       git config remote.foo.fetch "+refs/heads/*:refs/foo_stuff/*"
   then the current code will refuse to use its remote-tracking branches
   as --track arguments, since they do not match refs/remotes/*.

This patch removes the "refs/remotes/*" requirement for upstream branches,
and replaces it with explicit checking of the refspecs for each remote to
determine whether a given --track argument is a valid remote-tracking
branch. This solves both of the above problems, since the matching refspec
guarantees that there is a both a remote name and a remote branch name
that can be used for the upstream config.

However, this means that refs located within refs/remotes/* without a
corresponding remote/refspec will no longer be usable as upstreams.
The few existing tests which depended on this behavioral quirk has
already been fixed in the preceding patches.

This patch fixes the last remaining test failure in t2024-checkout-dwim.

Signed-off-by: Johan Herland <johan@herland.net>
---
 branch.c                 | 17 ++++++++++++++++-
 t/t2024-checkout-dwim.sh |  2 +-
 t/t3200-branch.sh        |  2 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/branch.c b/branch.c
index 6ae6a4c..beaf11d 100644
--- a/branch.c
+++ b/branch.c
@@ -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,7 @@ 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);
 			else
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index fc6edc9..a8f0a90 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -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
 '
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 1bfdadc..44ec6a4 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -317,7 +317,7 @@ test_expect_success 'test tracking setup (non-wildcard, matching)' '
 	test $(git config branch.my4.merge) = refs/heads/master
 '
 
-test_expect_failure 'tracking setup fails on non-matching refspec' '
+test_expect_success 'tracking setup fails on non-matching refspec' '
 	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) &&
-- 
1.8.1.3.704.g33f7d4f

[PATCHv2 8/8] glossary: Update and rephrase the definition of a remote-tracking branch

[Johan Herland]

The definition of a remote-tracking branch in the glossary have been
out-of-date for a while (by e.g. referring to "Pull:" from old-style
$GIT_DIR/remotes files).

Also, the preceding patches have formalized that a remote-tracking branch
must match a configured refspec in order to be usable as an upstream.

This patch rewrites the paragraph on remote-tracking branches accordingly.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/glossary-content.txt | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 2478a39..7a79f26 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -423,12 +423,13 @@ should not be combined with other pathspec.
 	linkgit:git-push[1].
 
 [[def_remote_tracking_branch]]remote-tracking branch::
-	A regular Git <<def_branch,branch>> that is used to follow changes from
-	another <<def_repository,repository>>. A remote-tracking
-	branch should not contain direct modifications or have local commits
-	made to it. A remote-tracking branch can usually be
-	identified as the right-hand-side <<def_ref,ref>> in a Pull:
-	<<def_refspec,refspec>>.
+	A <<def_ref,ref>> that is used to follow changes from another
+	<<def_repository,repository>>. It typically looks like
+	'refs/remotes/foo/bar' (indicating that it tracks a branch named
+	'bar' in a remote named 'foo'), and matches the right-hand-side of
+	a configured fetch <<def_refspec,refspec>>. A remote-tracking
+	branch should not contain direct modifications or have local
+	commits made to it.
 
 [[def_repository]]repository::
 	A collection of <<def_ref,refs>> together with an
-- 
1.8.1.3.704.g33f7d4f

Re: [PATCHv2 1/8] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'

[Jonathan Nieder]

Johan Herland wrote:

> The DWIM mode of checkout allows you to run "git checkout foo" when there is
> no existing local ref or path called "foo" and there is exactly one remote
> with a remote-tracking branch called "foo".

Thanks for testing this.  I'm surprised no one suggested a test since
v1.7.0-rc0~51^2~6 (2009-10-18).

Maybe it would also be worthwhile to also test --no-guess?  (c.f.
46148dd7, 2009-10-18)

[...]
> +++ b/t/t2024-checkout-dwim.sh
> @@ -0,0 +1,66 @@
[...]
> +# 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")"

Stylistic tweaks:

 * setting all local vars on one line
 * avoiding command substitution so we notice if commands fail
 * using test_cmp in place of test $foo = $bar for better output
   when the test fails

	# Is the current branch "refs/heads/$1"?
	test_branch () {
		printf "%s\n" "refs/heads/$1" >expect.HEAD &&
		git symbolic-ref HEAD >actual.HEAD &&
		test_cmp expect.HEAD actual.HEAD
	}

	# Is branch "refs/heads/$1" set to pull from "$2/$3"?
	test_branch_upstream () {
		printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
		{
			git config "branch.$1.remote" &&
			git config "branch.$1.merge"
		} >actual.upstream &&
		test_cmp expect.upstream actual.upstream
	}

	test_tracking_branch () {
		branch=$1 remote=$2 remote_branch=$3 &&

		test_branch "$branch" &&
		test_cmp_rev "refs/remotes/$remote/$remote_branch" HEAD &&
		test_branch_upstream "$branch" "$remote" "$remote_branch"
	}

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

Style: indenting code in subshells.

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

Maybe, to defend against state from previous tests and confirm that
the checkout didn't do anything:

	git checkout -B master &&
	test_might_fail git branch -D xyzzy &&

	test_must_fail git checkout xyzzy &&
	test_must_fail git rev-parse --verify refs/heads/xyzzy &&
	test_branch master

> +
> +test_expect_success 'checkout of branch from multiple remotes fails' '
> +	test_must_fail git checkout foo
> +'

Likewise:

	git checkout -B master &&
	test_might_fail git branch -D foo &&

	test_must_fail git checkout foo &&
	test_must_fail git rev-parse --verify refs/heads/foo &&
	test_branch master

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

	git checkout -B master &&
	test_might_fail git branch -D bar &&

	git checkout bar &&
	test_branch bar &&
	test_cmp_rev remotes/repo_a/bar HEAD &&
	test_branch_upstream bar 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

	git checkout -B master &&
	test_might_fail git branch -D baz &&

	git checkout baz &&
	test_branch baz &&
	test_cmp_rev remotes/other_b/baz HEAD &&
	test_branch_upstream baz repo_b baz

And for --no-guess:

	test_expect_success '--no-guess suppresses branch auto-vivification' '
		git checkout -B master &&
		test_might_fail git branch -D bar &&

		test_must_fail git checkout --no-guess bar &&
		test_must_fail git rev-parse --verify refs/heads/bar &&
		test_branch master
	'

Sane?

Thanks,
Jonathan

Re: [PATCHv2 1/8] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'

[Johan Herland]

On Sat, Apr 20, 2013 at 10:44 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Johan Herland wrote:
>
>> The DWIM mode of checkout allows you to run "git checkout foo" when there is
>> no existing local ref or path called "foo" and there is exactly one remote
>> with a remote-tracking branch called "foo".
>
> Thanks for testing this.  I'm surprised no one suggested a test since
> v1.7.0-rc0~51^2~6 (2009-10-18).
>
> Maybe it would also be worthwhile to also test --no-guess?  (c.f.
> 46148dd7, 2009-10-18)

[...]

> Sane?

Yes. Thanks!

Will incorporate your suggestions into the next iteration.


...Johan

--
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCHv2 6/8] t9114.2: Don't use --track option against "svn-remote"-tracking branches

[Eric Wong]

Acked-by: Eric Wong <normalperson@yhbt.net>