From 399e4a1c560ae8caf338e13a73851089c384230a Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:51:59 +0200 Subject: t2024: Add tests verifying current DWIM behavior of 'git checkout ' 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. Improved-by: Jonathan Nieder Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh new file mode 100755 index 0000000..1452bea --- /dev/null +++ b/t/t2024-checkout-dwim.sh @@ -0,0 +1,99 @@ +#!/bin/sh + +test_description='checkout + +Ensures that checkout on an unborn branch does what the user expects' + +. ./test-lib.sh + +# 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_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' ' + 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' ' + 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 -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 -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 +' + +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 +' + +test_done -- cgit v0.10.2-6-g49f6 From ec2764ee8fb9a37cc92e0a1bce3b49ae2de0ffbc Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:00 +0200 Subject: t2024: Show failure to use refspec when DWIMming remote branch names 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. Improved-by: Jonathan Nieder Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index 1452bea..b1e3444 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -24,6 +24,7 @@ test_branch_upstream () { } test_expect_success 'setup' ' + test_commit my_master && git init repo_a && ( cd repo_a && @@ -58,7 +59,7 @@ test_expect_success 'checkout of non-existing branch fails' ' test_branch master ' -test_expect_success 'checkout of branch from multiple remotes fails' ' +test_expect_success 'checkout of branch from multiple remotes fails #1' ' git checkout -B master && test_might_fail git branch -D foo && @@ -96,4 +97,71 @@ test_expect_success '--no-guess suppresses branch auto-vivification' ' test_branch master ' +test_expect_success 'setup more remotes with unconventional refspecs' ' + git checkout -B 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 remote add repo_d repo_d && + git config remote.repo_d.fetch \ + "+refs/heads/*:refs/repo_d/*" && + git fetch --all +' + +test_expect_failure 'checkout of branch from multiple remotes fails #2' ' + git checkout -B master && + test_might_fail git branch -D bar && + + test_must_fail git checkout bar && + test_must_fail git rev-parse --verify refs/heads/bar && + test_branch master +' + +test_expect_failure 'checkout of branch from multiple remotes fails #3' ' + git checkout -B master && + test_might_fail git branch -D baz && + + test_must_fail git checkout baz && + test_must_fail git rev-parse --verify refs/heads/baz && + test_branch master +' + +test_expect_failure 'checkout of branch from a single remote succeeds #3' ' + git checkout -B master && + test_might_fail git branch -D spam && + + git checkout spam && + test_branch spam && + test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD && + test_branch_upstream spam repo_c spam +' + +test_expect_failure 'checkout of branch from a single remote succeeds #4' ' + git checkout -B master && + test_might_fail git branch -D eggs && + + git checkout eggs && + test_branch eggs && + test_cmp_rev refs/repo_d/eggs HEAD && + test_branch_upstream eggs repo_d eggs +' + test_done -- cgit v0.10.2-6-g49f6 From fa83a33b22283e465aafdacc53ad0fddea55bdf4 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:01 +0200 Subject: checkout: Use remote refspecs when DWIMming tracking branches 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 Signed-off-by: Junio C Hamano 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 b1e3444..31e3d47 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -126,7 +126,7 @@ test_expect_success 'setup more remotes with unconventional refspecs' ' git fetch --all ' -test_expect_failure 'checkout of branch from multiple remotes fails #2' ' +test_expect_success 'checkout of branch from multiple remotes fails #2' ' git checkout -B master && test_might_fail git branch -D bar && @@ -135,7 +135,7 @@ test_expect_failure 'checkout of branch from multiple remotes fails #2' ' test_branch master ' -test_expect_failure 'checkout of branch from multiple remotes fails #3' ' +test_expect_success 'checkout of branch from multiple remotes fails #3' ' git checkout -B master && test_might_fail git branch -D baz && @@ -144,7 +144,7 @@ test_expect_failure 'checkout of branch from multiple remotes fails #3' ' test_branch master ' -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 -B master && test_might_fail git branch -D spam && -- cgit v0.10.2-6-g49f6 From 9c9cd39a0c9bc1f14e9dfc29ad98475794dda6b8 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:02 +0200 Subject: t3200.39: tracking setup should fail if there is no matching refspec. 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 Signed-off-by: Junio C Hamano 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' ' -- cgit v0.10.2-6-g49f6 From 88a9f72fe0f2ae3243b3d7a53dddf856cde48aca Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:03 +0200 Subject: t7201.24: Add refspec to keep --track working 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 Signed-off-by: Junio C Hamano 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 ' ' + 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 && -- cgit v0.10.2-6-g49f6 From 983b17d4bbedd426d4fdd8a95df4dcd292c1a695 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:04 +0200 Subject: t9114.2: Don't use --track option against "svn-remote"-tracking branches 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. Acked-by: Eric Wong Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano 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 && -- cgit v0.10.2-6-g49f6 From 41c21f22d0fc06f1f22489621980396aea9f7a62 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:05 +0200 Subject: branch.c: Validate tracking branches with refspecs instead of refs/remotes/* 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 Signed-off-by: Junio C Hamano 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 31e3d47..dee55e4 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -154,7 +154,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #3' ' test_branch_upstream spam repo_c 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 -B master && test_might_fail git branch -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) && -- cgit v0.10.2-6-g49f6 From 229177aaea295a3f804f98494d7bcde9c088fc0a Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Sun, 21 Apr 2013 23:52:06 +0200 Subject: glossary: Update and rephrase the definition of a remote-tracking branch 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 Signed-off-by: Junio C Hamano 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 <> that is used to follow changes from - another <>. 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 <> in a Pull: - <>. + A <> that is used to follow changes from another + <>. 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 <>. A remote-tracking + branch should not contain direct modifications or have local + commits made to it. [[def_repository]]repository:: A collection of <> together with an -- cgit v0.10.2-6-g49f6