{"thread":{"id":"33542","subject":"[RFD/PATCH 0/5] Improving the search for remote-tracking branches","startedAt":"2013-04-19T06:20:37Z","lastAt":"2013-04-20T08:05:55Z","messageCount":9,"participants":["Johan Herland","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"214810","messageId":"1366352442-501-1-git-send-email-johan@herland.net","threadId":"33542","inReplyTo":null,"subject":"[RFD/PATCH 0/5] Improving the search for remote-tracking branches","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-19T06:20:37Z","receivedAt":"2013-04-19T06:20:37Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Hi,\n\nThe following patch series has three uncontroversial patches (I hope)\nto improve the DWIM behavivor \"git checkout foo\", followed by two more\nexperimental patches meant to trigger discussion about how Git decides\non what is a remote-tracking branch, and what is not:\n\nThe first three patches concern the DWIMming of \"git checkout foo\" into\n\"git checkout -b foo --track refs/remotes/$remote/foo\". This DWIMming\ndepends on finding exactly _one_ remote \"$remote\" having a remote-\ntracking branch called \"foo\". Currently we find remote-tracking branches\nby pattern-matching refs against \"refs/remotes/*/foo\", but instead we\nshould look at the configured refspecs to find the real set of remote-\ntracking branch candidates. This fixes the DWIM behavior when the user\nhas tweaked refspecs to place remote-tracking branches elsewhere (e.g.\nrefs/remotes/$remote/heads/foo). The argument is explained more closely\nin patch #3.\n\nThe remaining two patches tries to apply the same argument to the\n-t/--track argument to checkout/branch: This option takes a remote-\ntracking branch (or a local branch) as argument, but validating this\nargument is currently done by a simple match against refs/remotes/*\n(or refs/heads/*). By the same reasoning as above, we should instead\nlook at the configured refspecs to determine whether the argument is\na valid remote-tracking branch. However, as shown in the final patch,\nthis would break a few tests where we currently try to --track refs\nthat are not \"proper\" remote-tracking branches (in that they do not\ncorrespond to a refspec of some remote).\n\n- Should we simply \"fix\" these tests (t3200.39, t7201.24, t9114.*), and\n  _require_ that all remote-tracking branches must have corresponding\n  refspecs configured?\n\n- Are there valid real-world use cases that would be broken by such a\n  requirement?\n\n- Is it preferable to stop after patch #4, and accept the superset of\n  refs/remotes/* and refspec-matching refs as valid remote-tracking\n  branches?\n\n\nHave fun! :)\n\n...Johan\n\n\nJohan Herland (5):\n  t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'\n  t2024: Show failure to use refspec when DWIMming remote branch names\n  checkout: Use remote refspecs when DWIMming tracking branches\n  branch.c: Look up refspecs to validate tracking branches\n  RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream\n\n Documentation/git-checkout.txt   |   6 +-\n branch.c                         |  17 +++++-\n builtin/checkout.c               |  42 +++++++-------\n t/t2024-checkout-dwim.sh         | 116 +++++++++++++++++++++++++++++++++++++++\n t/t3200-branch.sh                |   2 +-\n t/t7201-co.sh                    |   2 +-\n t/t9114-git-svn-dcommit-merge.sh |   8 +--\n 7 files changed, 163 insertions(+), 30 deletions(-)\n create mode 100755 t/t2024-checkout-dwim.sh\n\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"214811","messageId":"1366352442-501-2-git-send-email-johan@herland.net","threadId":"33542","inReplyTo":"1366352442-501-1-git-send-email-johan@herland.net","subject":"[RFD/PATCH 1/5] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-19T06:20:38Z","receivedAt":"2013-04-19T06:20:38Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"The DWIM mode of checkout allows you to run \"git checkout foo\" when there is\nno existing local ref or path called \"foo\", and there is exactly one remote\nwith a remote-tracking branch called \"foo\". Git will then automatically\ncreate a new local branch called \"foo\" using the remote-tracking \"foo\" as\nits starting point and configured upstream.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n t/t2024-checkout-dwim.sh | 66 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 66 insertions(+)\n create mode 100755 t/t2024-checkout-dwim.sh\n\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nnew file mode 100755\nindex 0000000..5650d21\n--- /dev/null\n+++ b/t/t2024-checkout-dwim.sh\n@@ -0,0 +1,66 @@\n+#!/bin/sh\n+\n+test_description='checkout <branch>\n+\n+Ensures that checkout on an unborn branch does what the user expects'\n+\n+. ./test-lib.sh\n+\n+# Arguments: <branch> <remote> <remote-tracking>\n+#\n+# Verify that we have checked out <branch>, and that it is at the same\n+# commit as <remote-tracking>, and that has appropriate tracking config\n+# setup against <remote>\n+test_tracking_branch() {\n+\tbranch=$1 &&\n+\tremote=$2 &&\n+\tremote_track=$3 &&\n+\ttest \"refs/heads/$branch\" = \"$(git rev-parse --symbolic-full-name HEAD)\" &&\n+\ttest \"$(git rev-parse --verify HEAD)\" = \"$(git rev-parse --verify \"$remote_track\")\" &&\n+\ttest \"$remote\" = \"$(git config \"branch.$branch.remote\")\" &&\n+\ttest \"refs/heads/$branch\" = \"$(git config \"branch.$branch.merge\")\"\n+}\n+\n+test_expect_success 'setup' '\n+\t(git init repo_a &&\n+\t cd repo_a &&\n+\t test_commit a_master &&\n+\t git checkout -b foo &&\n+\t test_commit a_foo &&\n+\t git checkout -b bar &&\n+\t test_commit a_bar\n+\t) &&\n+\t(git init repo_b &&\n+\t cd repo_b &&\n+\t test_commit b_master &&\n+\t git checkout -b foo &&\n+\t test_commit b_foo &&\n+\t git checkout -b baz &&\n+\t test_commit b_baz\n+\t) &&\n+\tgit remote add repo_a repo_a &&\n+\tgit remote add repo_b repo_b &&\n+\tgit config remote.repo_b.fetch \\\n+\t\t\"+refs/heads/*:refs/remotes/other_b/*\" &&\n+\tgit fetch --all\n+'\n+\n+test_expect_success 'checkout of non-existing branch fails' '\n+\ttest_must_fail git checkout xyzzy\n+'\n+\n+test_expect_success 'checkout of branch from multiple remotes fails' '\n+\ttest_must_fail git checkout foo\n+'\n+\n+test_expect_success 'checkout of branch from a single remote succeeds #1' '\n+\tgit checkout bar &&\n+\ttest_tracking_branch bar repo_a refs/remotes/repo_a/bar\n+'\n+\n+test_expect_success 'checkout of branch from a single remote succeeds #2' '\n+\tgit checkout baz &&\n+\ttest_tracking_branch baz repo_b refs/remotes/other_b/baz\n+'\n+\n+test_done\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"214812","messageId":"1366352442-501-3-git-send-email-johan@herland.net","threadId":"33542","inReplyTo":"1366352442-501-1-git-send-email-johan@herland.net","subject":"[RFD/PATCH 2/5] t2024: Show failure to use refspec when DWIMming remote branch names","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-19T06:20:39Z","receivedAt":"2013-04-19T06:20:39Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"When using \"git checkout foo\" to DWIM the creation of local \"foo\" from some\nexisting upstream \"foo\", we assume conventional refspecs as created by \"git\nclone\" or \"git remote add\", and fail to work correctly if the current\nrefspecs do not follow the conventional \"refs/remotes/$remote/*\" pattern.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n t/t2024-checkout-dwim.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 51 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex 5650d21..36bf52f 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -22,6 +22,7 @@ test_tracking_branch() {\n }\n \n test_expect_success 'setup' '\n+\ttest_commit my_master &&\n \t(git init repo_a &&\n \t cd repo_a &&\n \t test_commit a_master &&\n@@ -49,7 +50,7 @@ test_expect_success 'checkout of non-existing branch fails' '\n \ttest_must_fail git checkout xyzzy\n '\n \n-test_expect_success 'checkout of branch from multiple remotes fails' '\n+test_expect_success 'checkout of branch from multiple remotes fails #1' '\n \ttest_must_fail git checkout foo\n '\n \n@@ -63,4 +64,53 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '\n \ttest_tracking_branch baz repo_b refs/remotes/other_b/baz\n '\n \n+test_expect_success 'setup more remotes with unconventional refspecs' '\n+\tgit checkout master &&\n+\tgit branch -D bar &&\n+\tgit branch -D baz &&\n+\ttest \"$(git rev-parse --verify HEAD)\" = \"$(git rev-parse --verify my_master)\" &&\n+\t(git init repo_c &&\n+\t cd repo_c &&\n+\t test_commit c_master &&\n+\t git checkout -b bar &&\n+\t test_commit c_bar\n+\t git checkout -b spam &&\n+\t test_commit c_spam\n+\t) &&\n+\t(git init repo_d &&\n+\t cd repo_d &&\n+\t test_commit d_master &&\n+\t git checkout -b baz &&\n+\t test_commit f_baz\n+\t git checkout -b eggs &&\n+\t test_commit c_eggs\n+\t) &&\n+\tgit remote add repo_c repo_c &&\n+\tgit config remote.repo_c.fetch \\\n+\t    \"+refs/heads/*:refs/remotes/extra_dir/repo_c/extra_dir/*\" &&\n+\tgit fetch repo_c &&\n+\tgit remote add repo_d repo_d &&\n+\tgit config remote.repo_d.fetch \\\n+\t    \"+refs/heads/*:refs/repo_d/*\" &&\n+\tgit fetch repo_d\n+'\n+\n+test_expect_failure 'checkout of branch from multiple remotes fails #2' '\n+\ttest_must_fail git checkout bar\n+'\n+\n+test_expect_failure 'checkout of branch from multiple remotes fails #3' '\n+\ttest_must_fail git checkout baz\n+'\n+\n+test_expect_failure 'checkout of branch from a single remote succeeds #3' '\n+\tgit checkout spam &&\n+\ttest_tracking_branch spam repo_c refs/remotes/extra_dir/repo_c/extra_dir/spam\n+'\n+\n+test_expect_failure 'checkout of branch from a single remote succeeds #4' '\n+\tgit checkout eggs &&\n+\ttest_tracking_branch eggs repo_d refs/repo_d/eggs\n+'\n+\n test_done\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"214813","messageId":"1366352442-501-4-git-send-email-johan@herland.net","threadId":"33542","inReplyTo":"1366352442-501-1-git-send-email-johan@herland.net","subject":"[RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-19T06:20:40Z","receivedAt":"2013-04-19T06:20:40Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"The DWIM mode of checkout allows you to run \"git checkout foo\" when there is\nno existing local ref or path called \"foo\", and there is exactly one remote\nwith a remote-tracking branch called \"foo\". Git will then automatically\ncreate a new local branch called \"foo\" using the remote-tracking \"foo\" as\nits starting point and configured upstream.\n\nHowever, the current code hardcodes the assumption that all remote-tracking\nbranches are located at refs/remotes/$remote/*, and that \"git checkout foo\"\nmust find exactly one ref matching \"refs/remotes/*/foo\" to succeed.\nThis approach fails if a user has customized the refspec of a given remote to\nplace remote-tracking branches elsewhere.\n\nThe better way to find a tracking branch is to use the fetch refspecs for the\nconfigured remotes to deduce the available candidate remote-tracking branches\ncorresponding to a remote branch of the requested name, and if exactly one of\nthese exists (and points to a valid SHA1), then that is the remote-tracking\nbranch we will use.\n\nFor example, in the case of \"git checkout foo\", we map \"refs/heads/foo\"\nthrough each remote's refspec to find the available candidate remote-tracking\nbranches, and if exactly one of these candidates exist in our local repo, then\nwe have found the upstream for the new local branch \"foo\".\n\nThis fixes most of the failing tests introduced in the previous patch.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n Documentation/git-checkout.txt |  6 +++---\n builtin/checkout.c             | 42 ++++++++++++++++++++++--------------------\n t/t2024-checkout-dwim.sh       |  6 +++---\n 3 files changed, 28 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 8edcdca..bf0c99c 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -131,9 +131,9 @@ entries; instead, unmerged entries are ignored.\n \t\"--track\" in linkgit:git-branch[1] for details.\n +\n If no '-b' option is given, the name of the new branch will be\n-derived from the remote-tracking branch.  If \"remotes/\" or \"refs/remotes/\"\n-is prefixed it is stripped away, and then the part up to the\n-next slash (which would be the nickname of the remote) is removed.\n+derived from the remote-tracking branch, by looking at the local part of\n+the refspec configured for the corresponding remote, and then stripping\n+the initial part up to the \"*\".\n This would tell us to use \"hack\" as the local branch when branching\n off of \"origin/hack\" (or \"remotes/origin/hack\", or even\n \"refs/remotes/origin/hack\").  If the given name has no slash, or the above\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f8033f4..d6f9c01 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -822,38 +822,40 @@ static int git_checkout_config(const char *var, const char *value, void *cb)\n }\n \n struct tracking_name_data {\n-\tconst char *name;\n-\tchar *remote;\n+\t/* const */ char *src_ref;\n+\tchar *dst_ref;\n+\tunsigned char *dst_sha1;\n \tint unique;\n };\n \n-static int check_tracking_name(const char *refname, const unsigned char *sha1,\n-\t\t\t       int flags, void *cb_data)\n+static int check_tracking_name(struct remote *remote, void *cb_data)\n {\n \tstruct tracking_name_data *cb = cb_data;\n-\tconst char *slash;\n-\n-\tif (prefixcmp(refname, \"refs/remotes/\"))\n-\t\treturn 0;\n-\tslash = strchr(refname + 13, '/');\n-\tif (!slash || strcmp(slash + 1, cb->name))\n+\tstruct refspec query;\n+\tmemset(&query, 0, sizeof(struct refspec));\n+\tquery.src = cb->src_ref;\n+\tif (remote_find_tracking(remote, &query) ||\n+\t    get_sha1(query.dst, cb->dst_sha1))\n \t\treturn 0;\n-\tif (cb->remote) {\n+\tif (cb->dst_ref) {\n \t\tcb->unique = 0;\n \t\treturn 0;\n \t}\n-\tcb->remote = xstrdup(refname);\n+\tcb->dst_ref = xstrdup(query.dst);\n \treturn 0;\n }\n \n-static const char *unique_tracking_name(const char *name)\n+static const char *unique_tracking_name(const char *name, unsigned char *sha1)\n {\n-\tstruct tracking_name_data cb_data = { NULL, NULL, 1 };\n-\tcb_data.name = name;\n-\tfor_each_ref(check_tracking_name, &cb_data);\n+\tstruct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };\n+\tchar src_ref[PATH_MAX];\n+\tsnprintf(src_ref, PATH_MAX, \"refs/heads/%s\", name);\n+\tcb_data.src_ref = src_ref;\n+\tcb_data.dst_sha1 = sha1;\n+\tfor_each_remote(check_tracking_name, &cb_data);\n \tif (cb_data.unique)\n-\t\treturn cb_data.remote;\n-\tfree(cb_data.remote);\n+\t\treturn cb_data.dst_ref;\n+\tfree(cb_data.dst_ref);\n \treturn NULL;\n }\n \n@@ -916,8 +918,8 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\tif (dwim_new_local_branch_ok &&\n \t\t    !check_filename(NULL, arg) &&\n \t\t    argc == 1) {\n-\t\t\tconst char *remote = unique_tracking_name(arg);\n-\t\t\tif (!remote || get_sha1(remote, rev))\n+\t\t\tconst char *remote = unique_tracking_name(arg, rev);\n+\t\t\tif (!remote)\n \t\t\t\treturn argcount;\n \t\t\t*new_branch = arg;\n \t\t\targ = remote;\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex 36bf52f..fc6edc9 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -95,15 +95,15 @@ test_expect_success 'setup more remotes with unconventional refspecs' '\n \tgit fetch repo_d\n '\n \n-test_expect_failure 'checkout of branch from multiple remotes fails #2' '\n+test_expect_success 'checkout of branch from multiple remotes fails #2' '\n \ttest_must_fail git checkout bar\n '\n \n-test_expect_failure 'checkout of branch from multiple remotes fails #3' '\n+test_expect_success 'checkout of branch from multiple remotes fails #3' '\n \ttest_must_fail git checkout baz\n '\n \n-test_expect_failure 'checkout of branch from a single remote succeeds #3' '\n+test_expect_success 'checkout of branch from a single remote succeeds #3' '\n \tgit checkout spam &&\n \ttest_tracking_branch spam repo_c refs/remotes/extra_dir/repo_c/extra_dir/spam\n '\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"214815","messageId":"1366352442-501-5-git-send-email-johan@herland.net","threadId":"33542","inReplyTo":"1366352442-501-1-git-send-email-johan@herland.net","subject":"[RFD/PATCH 4/5] branch.c: Look up refspecs to validate tracking branches","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-19T06:20:41Z","receivedAt":"2013-04-19T06:20:41Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"The current code for validating tracking branches (e.g. the argument to the\n-t/--track option) hardcodes refs/heads/* and refs/remotes/* as the potential\nlocations for tracking branches. This works well with the conventional\nrefspecs created by \"git clone\" or \"git remote add\", but does not work if the\nuser tweaks the refspecs to place remote-tracking branches outside\nrefs/remotes/*.\n\nThis patch adds explicit checking of the refspecs for each remote to determine\nwhether a candidate tracking branch is indeed a valid remote-tracking branch,\neven if placed outside refs/heads/* and refs/remotes/*.\n\nThis new check is added as a fallback after checking for match against\nrefs/heads/* and refs/remotes/*, so the code will not be run in the common\ncase.\n\nThis patch also fixes the last remaining test failure in t2024-checkout-dwim.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n branch.c                 | 18 +++++++++++++++++-\n t/t2024-checkout-dwim.sh |  2 +-\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 6ae6a4c..c9f9dec 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -197,6 +197,21 @@ int validate_new_branchname(const char *name, struct strbuf *ref,\n \treturn 1;\n }\n \n+static int check_tracking_branch(struct remote *remote, void *cb_data)\n+{\n+\tchar *tracking_branch = cb_data;\n+\tstruct refspec query;\n+\tmemset(&query, 0, sizeof(struct refspec));\n+\tquery.dst = tracking_branch;\n+\treturn !(remote_find_tracking(remote, &query) ||\n+\t\t prefixcmp(query.src, \"refs/heads/\"));\n+}\n+\n+static int validate_remote_tracking_branch(char *ref)\n+{\n+\treturn !for_each_remote(check_tracking_branch, ref);\n+}\n+\n static const char upstream_not_branch[] =\n N_(\"Cannot setup tracking information; starting point '%s' is not a branch.\");\n static const char upstream_missing[] =\n@@ -259,7 +274,8 @@ void create_branch(const char *head,\n \tcase 1:\n \t\t/* Unique completion -- good, only if it is a real branch */\n \t\tif (prefixcmp(real_ref, \"refs/heads/\") &&\n-\t\t    prefixcmp(real_ref, \"refs/remotes/\")) {\n+\t\t    prefixcmp(real_ref, \"refs/remotes/\") &&\n+\t\t    validate_remote_tracking_branch(real_ref)) {\n \t\t\tif (explicit_tracking)\n \t\t\t\tdie(_(upstream_not_branch), start_name);\n \t\t\telse\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex fc6edc9..a8f0a90 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -108,7 +108,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #3' '\n \ttest_tracking_branch spam repo_c refs/remotes/extra_dir/repo_c/extra_dir/spam\n '\n \n-test_expect_failure 'checkout of branch from a single remote succeeds #4' '\n+test_expect_success 'checkout of branch from a single remote succeeds #4' '\n \tgit checkout eggs &&\n \ttest_tracking_branch eggs repo_d refs/repo_d/eggs\n '\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"214814","messageId":"1366352442-501-6-git-send-email-johan@herland.net","threadId":"33542","inReplyTo":"1366352442-501-1-git-send-email-johan@herland.net","subject":"[RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-19T06:20:42Z","receivedAt":"2013-04-19T06:20:42Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"The previous patch adds validation of upstream remote-tracking branches by\nparsing the configured refspecs, and making sure that the candidate upstream\n(if not already matching refs/heads/* or refs/remotes/*) is indeed a\nremote-tracking branch according to some remote's refspec. For a\ndefault/conventional setup, this check would automatically also cover\neverything within refs/remotes/*, meaning that the preceding check for\nrefs/remotes/* is redundant (although quicker than the validation against\nrefspecs). One could also argue that not everything inside refs/remotes/*\nshould be automatically acceptable as an upstream, if one were to keep\nother (non-branch) type of remote-tracking refs there.\n\nThis patch removes the simple check for refs/remotes/*, to make sure that\n_only_ validated remote-tracking branches (in addition to local branches)\nare allowed as upstreams.\n\nHowever, this means that for unconventional setups that place refs within\nrefs/remotes/* without configuring a corresponding refspec, those refs will\nno longer be usable as upstreams. This breaks a few existing tests, which\nare marked as test_expect_failure by this patch, to make them easy to find.\n---\n branch.c                         | 1 -\n t/t3200-branch.sh                | 2 +-\n t/t7201-co.sh                    | 2 +-\n t/t9114-git-svn-dcommit-merge.sh | 8 ++++----\n 4 files changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex c9f9dec..beaf11d 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -274,7 +274,6 @@ void create_branch(const char *head,\n \tcase 1:\n \t\t/* Unique completion -- good, only if it is a real branch */\n \t\tif (prefixcmp(real_ref, \"refs/heads/\") &&\n-\t\t    prefixcmp(real_ref, \"refs/remotes/\") &&\n \t\t    validate_remote_tracking_branch(real_ref)) {\n \t\t\tif (explicit_tracking)\n \t\t\t\tdie(_(upstream_not_branch), start_name);\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex d969f0e..41d293d 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -317,7 +317,7 @@ test_expect_success 'test tracking setup (non-wildcard, matching)' '\n \ttest $(git config branch.my4.merge) = refs/heads/master\n '\n \n-test_expect_success 'test tracking setup (non-wildcard, not matching)' '\n+test_expect_failure 'test tracking setup (non-wildcard, not matching)' '\n \tgit config remote.local.url . &&\n \tgit config remote.local.fetch refs/heads/s:refs/remotes/local/s &&\n \t(git show-ref -q refs/remotes/local/master || git fetch local) &&\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex be9672e..7267ee2 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -429,7 +429,7 @@ test_expect_success 'detach a symbolic link HEAD' '\n     test \"z$(git rev-parse --verify refs/heads/master)\" = \"z$here\"\n '\n \n-test_expect_success \\\n+test_expect_failure \\\n     'checkout with --track fakes a sensible -b <name>' '\n     git update-ref refs/remotes/origin/koala/bear renamer &&\n \ndiff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh\nindex 3077851..ef274fd 100755\n--- a/t/t9114-git-svn-dcommit-merge.sh\n+++ b/t/t9114-git-svn-dcommit-merge.sh\n@@ -45,7 +45,7 @@ test_expect_success 'setup svn repository' '\n \t)\n \t'\n \n-test_expect_success 'setup git mirror and merge' '\n+test_expect_failure 'setup git mirror and merge' '\n \tgit svn init \"$svnrepo\" -t tags -T trunk -b branches &&\n \tgit svn fetch &&\n \tgit checkout --track -b svn remotes/trunk &&\n@@ -67,7 +67,7 @@ test_expect_success 'setup git mirror and merge' '\n \n test_debug 'gitk --all & sleep 1'\n \n-test_expect_success 'verify pre-merge ancestry' \"\n+test_expect_failure 'verify pre-merge ancestry' \"\n \ttest x\\`git rev-parse --verify refs/heads/svn^2\\` = \\\n \t     x\\`git rev-parse --verify refs/heads/merge\\` &&\n \tgit cat-file commit refs/heads/svn^ | grep '^friend$'\n@@ -79,7 +79,7 @@ test_expect_success 'git svn dcommit merges' \"\n \n test_debug 'gitk --all & sleep 1'\n \n-test_expect_success 'verify post-merge ancestry' \"\n+test_expect_failure 'verify post-merge ancestry' \"\n \ttest x\\`git rev-parse --verify refs/heads/svn\\` = \\\n \t     x\\`git rev-parse --verify refs/remotes/trunk \\` &&\n \ttest x\\`git rev-parse --verify refs/heads/svn^2\\` = \\\n@@ -87,7 +87,7 @@ test_expect_success 'verify post-merge ancestry' \"\n \tgit cat-file commit refs/heads/svn^ | grep '^friend$'\n \t\"\n \n-test_expect_success 'verify merge commit message' \"\n+test_expect_failure 'verify merge commit message' \"\n \tgit rev-list --pretty=raw -1 refs/heads/svn | \\\n \t  grep \\\"    Merge branch 'merge' into svn\\\"\n \t\"\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"214879","messageId":"7vzjwuz4ov.fsf@alter.siamese.dyndns.org","threadId":"33542","inReplyTo":"1366352442-501-4-git-send-email-johan@herland.net","subject":"Re: [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T19:44:00Z","receivedAt":"2013-04-19T19:44:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> The DWIM mode of checkout allows you to run \"git checkout foo\" when there is\n> no existing local ref or path called \"foo\", and there is exactly one remote\n> with a remote-tracking branch called \"foo\". Git will then automatically\n> create a new local branch called \"foo\" using the remote-tracking \"foo\" as\n> its starting point and configured upstream.\n>\n> However, the current code hardcodes the assumption that all remote-tracking\n> branches are located at refs/remotes/$remote/*, and that \"git checkout foo\"\n> must find exactly one ref matching \"refs/remotes/*/foo\" to succeed.\n> This approach fails if a user has customized the refspec of a given remote to\n> place remote-tracking branches elsewhere.\n>\n> The better way to find a tracking branch is to use the fetch refspecs for the\n> configured remotes to deduce the available candidate remote-tracking branches\n> corresponding to a remote branch of the requested name, and if exactly one of\n> these exists (and points to a valid SHA1), then that is the remote-tracking\n> branch we will use.\n>\n> For example, in the case of \"git checkout foo\", we map \"refs/heads/foo\"\n> through each remote's refspec to find the available candidate remote-tracking\n> branches, and if exactly one of these candidates exist in our local repo, then\n> we have found the upstream for the new local branch \"foo\".\n\nOnce you introduce a concrete \"foo\" as a name in the example, it\nwould be far easier to understand if you spelled all the other\nassumptions in the example out.\n\nI am _guessing_ that you mean a case like this:\n\n\t[remote \"origin\"]\n\t\tfetch = refs/heads/*:refs/remotes/origin/*\n\t[remote \"xyzzy\"]\n\t\tfetch = refs/heads/*:refs/remotes/xyzzy/nitfol/*\n\t[remote \"frotz\"]\n\t\tfetch = refs/heads/*:refs/remotes/frotz/nitfol/*\n\nand refs/remotes/origin/foo or refs/remotes/frotz/nitfol/foo do not\nexist but refs/remotes/xyzzy/nitfol/foo does.  And the user says\n\"git checkout foo\".  Instead of finding a remote ref that matches\n\"refs/remotes/*/foo\" pattern (and assuming the part that matched *\nis the name of the remote), you can iterate the RHS of the refspecs\nto see if there is a unique match.\n\nThen the new branch can unambiguously find that its upstream is\nxyzzy's foo.\n\nI think it makes sense to update the semantics like that.\n\nWouldn't the traditional case (i.e. without \"nitfol/\" in the\nxyzzy/frotz remotes above) be a mere special case for this new\nlogic?  You mentioned there is a regression caught by existing tests\nif you go this route, but I do not see how that happens.\n"},{"id":"214881","messageId":"7vvc7iz4cz.fsf@alter.siamese.dyndns.org","threadId":"33542","inReplyTo":"1366352442-501-6-git-send-email-johan@herland.net","subject":"Re: [RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T19:51:08Z","receivedAt":"2013-04-19T19:51:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> The previous patch adds validation of upstream remote-tracking branches by\n> parsing the configured refspecs, and making sure that the candidate upstream\n> (if not already matching refs/heads/* or refs/remotes/*) is indeed a\n> remote-tracking branch according to some remote's refspec. For a\n> default/conventional setup, this check would automatically also cover\n> everything within refs/remotes/*, meaning that the preceding check for\n> refs/remotes/* is redundant (although quicker than the validation against\n> refspecs). One could also argue that not everything inside refs/remotes/*\n> should be automatically acceptable as an upstream, if one were to keep\n> other (non-branch) type of remote-tracking refs there.\n>\n> This patch removes the simple check for refs/remotes/*, to make sure that\n> _only_ validated remote-tracking branches (in addition to local branches)\n> are allowed as upstreams.\n>\n> However, this means that for unconventional setups that place refs within\n> refs/remotes/* without configuring a corresponding refspec, those refs will\n> no longer be usable as upstreams. This breaks a few existing tests, which\n> are marked as test_expect_failure by this patch, to make them easy to find.\n\nI think these tests are too loosely written, making assumptions on\nwhat the back-then-current implementation actually does.\n\nIf refs/remotes/origin/koala/bear is a remote tracking branch for\nkoala/bear branch taken from the origin, we will have a refspec that\nstores to refs/remotes/origin/koala/bear for _some_ remote (and in\nthe most normal case it would be \"origin\").\n\nInstead of expecting failures, I think it is a better change to make\nthe test case more realistic by adding remote.origin.refspec as well.\n\n> diff --git a/t/t7201-co.sh b/t/t7201-co.sh\n> index be9672e..7267ee2 100755\n> --- a/t/t7201-co.sh\n> +++ b/t/t7201-co.sh\n> @@ -429,7 +429,7 @@ test_expect_success 'detach a symbolic link HEAD' '\n>      test \"z$(git rev-parse --verify refs/heads/master)\" = \"z$here\"\n>  '\n>  \n> -test_expect_success \\\n> +test_expect_failure \\\n>      'checkout with --track fakes a sensible -b <name>' '\n>      git update-ref refs/remotes/origin/koala/bear renamer &&\n"},{"id":"214905","messageId":"CALKQrgcWdUxauU2rAGSVggE-QjBW2PMXCdr6xg4habqyA6rj=g@mail.gmail.com","threadId":"33542","inReplyTo":"7vzjwuz4ov.fsf@alter.siamese.dyndns.org","subject":"Re: [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-04-20T08:05:55Z","receivedAt":"2013-04-20T08:05:55Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Fri, Apr 19, 2013 at 9:44 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> I am _guessing_ that you mean a case like this:\n>\n>         [remote \"origin\"]\n>                 fetch = refs/heads/*:refs/remotes/origin/*\n>         [remote \"xyzzy\"]\n>                 fetch = refs/heads/*:refs/remotes/xyzzy/nitfol/*\n>         [remote \"frotz\"]\n>                 fetch = refs/heads/*:refs/remotes/frotz/nitfol/*\n>\n> and refs/remotes/origin/foo or refs/remotes/frotz/nitfol/foo do not\n> exist but refs/remotes/xyzzy/nitfol/foo does.  And the user says\n> \"git checkout foo\".  Instead of finding a remote ref that matches\n> \"refs/remotes/*/foo\" pattern (and assuming the part that matched *\n> is the name of the remote), you can iterate the RHS of the refspecs\n> to see if there is a unique match.\n>\n> Then the new branch can unambiguously find that its upstream is\n> xyzzy's foo.\n\nCorrect. I will try to phrase the problem better in the next iteration.\n\n> I think it makes sense to update the semantics like that.\n>\n> Wouldn't the traditional case (i.e. without \"nitfol/\" in the\n> xyzzy/frotz remotes above) be a mere special case for this new\n> logic?\n\nYes.\n\n> You mentioned there is a regression caught by existing tests\n> if you go this route, but I do not see how that happens.\n\nIt's technically a regression since it breaks existing tests, but as you\nobserve in your reply to patch #5, it is really the test that relies on\ncurrent implementation details.\n\n\n...Johan\n\n\n--\nJohan Herland, <johan@herland.net>\nwww.herland.net\n"}]}