* [RFD/PATCH 1/5] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'
2013-04-19 6:20 [RFD/PATCH 0/5] Improving the search for remote-tracking branches Johan Herland
@ 2013-04-19 6:20 ` Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 2/5] t2024: Show failure to use refspec when DWIMming remote branch names Johan Herland
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Johan Herland @ 2013-04-19 6:20 UTC (permalink / raw)
To: git; +Cc: johan
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFD/PATCH 2/5] t2024: Show failure to use refspec when DWIMming remote branch names
2013-04-19 6:20 [RFD/PATCH 0/5] Improving the search for remote-tracking branches Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 1/5] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>' Johan Herland
@ 2013-04-19 6:20 ` Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches Johan Herland
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Johan Herland @ 2013-04-19 6:20 UTC (permalink / raw)
To: git; +Cc: johan
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
2013-04-19 6:20 [RFD/PATCH 0/5] Improving the search for remote-tracking branches Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 1/5] t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>' Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 2/5] t2024: Show failure to use refspec when DWIMming remote branch names Johan Herland
@ 2013-04-19 6:20 ` Johan Herland
2013-04-19 19:44 ` Junio C Hamano
2013-04-19 6:20 ` [RFD/PATCH 4/5] branch.c: Look up refspecs to validate " Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream Johan Herland
4 siblings, 1 reply; 9+ messages in thread
From: Johan Herland @ 2013-04-19 6:20 UTC (permalink / raw)
To: git; +Cc: johan
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.
However, the current code hardcodes the assumption that all remote-tracking
branches are located at refs/remotes/$remote/*, and that "git checkout foo"
must find exactly one ref matching "refs/remotes/*/foo" to succeed.
This approach fails if a user has customized the refspec of a given remote to
place remote-tracking branches elsewhere.
The better way to find a tracking branch is to use the fetch refspecs for the
configured remotes to deduce the available candidate remote-tracking branches
corresponding to a remote branch of the requested name, and if exactly one of
these exists (and points to a valid SHA1), then that is the remote-tracking
branch we will use.
For example, in the case of "git checkout foo", we map "refs/heads/foo"
through each remote's refspec to find the available candidate remote-tracking
branches, and if exactly one of these candidates exist in our local repo, then
we have found the upstream for the new local branch "foo".
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 f8033f4..d6f9c01 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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
2013-04-19 6:20 ` [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches Johan Herland
@ 2013-04-19 19:44 ` Junio C Hamano
2013-04-20 8:05 ` Johan Herland
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2013-04-19 19:44 UTC (permalink / raw)
To: Johan Herland; +Cc: git
Johan Herland <johan@herland.net> writes:
> 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.
>
> However, the current code hardcodes the assumption that all remote-tracking
> branches are located at refs/remotes/$remote/*, and that "git checkout foo"
> must find exactly one ref matching "refs/remotes/*/foo" to succeed.
> This approach fails if a user has customized the refspec of a given remote to
> place remote-tracking branches elsewhere.
>
> The better way to find a tracking branch is to use the fetch refspecs for the
> configured remotes to deduce the available candidate remote-tracking branches
> corresponding to a remote branch of the requested name, and if exactly one of
> these exists (and points to a valid SHA1), then that is the remote-tracking
> branch we will use.
>
> For example, in the case of "git checkout foo", we map "refs/heads/foo"
> through each remote's refspec to find the available candidate remote-tracking
> branches, and if exactly one of these candidates exist in our local repo, then
> we have found the upstream for the new local branch "foo".
Once you introduce a concrete "foo" as a name in the example, it
would be far easier to understand if you spelled all the other
assumptions in the example out.
I am _guessing_ that you mean a case like this:
[remote "origin"]
fetch = refs/heads/*:refs/remotes/origin/*
[remote "xyzzy"]
fetch = refs/heads/*:refs/remotes/xyzzy/nitfol/*
[remote "frotz"]
fetch = refs/heads/*:refs/remotes/frotz/nitfol/*
and refs/remotes/origin/foo or refs/remotes/frotz/nitfol/foo do not
exist but refs/remotes/xyzzy/nitfol/foo does. And the user says
"git checkout foo". Instead of finding a remote ref that matches
"refs/remotes/*/foo" pattern (and assuming the part that matched *
is the name of the remote), you can iterate the RHS of the refspecs
to see if there is a unique match.
Then the new branch can unambiguously find that its upstream is
xyzzy's foo.
I think it makes sense to update the semantics like that.
Wouldn't the traditional case (i.e. without "nitfol/" in the
xyzzy/frotz remotes above) be a mere special case for this new
logic? You mentioned there is a regression caught by existing tests
if you go this route, but I do not see how that happens.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
2013-04-19 19:44 ` Junio C Hamano
@ 2013-04-20 8:05 ` Johan Herland
0 siblings, 0 replies; 9+ messages in thread
From: Johan Herland @ 2013-04-20 8:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git mailing list
On Fri, Apr 19, 2013 at 9:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I am _guessing_ that you mean a case like this:
>
> [remote "origin"]
> fetch = refs/heads/*:refs/remotes/origin/*
> [remote "xyzzy"]
> fetch = refs/heads/*:refs/remotes/xyzzy/nitfol/*
> [remote "frotz"]
> fetch = refs/heads/*:refs/remotes/frotz/nitfol/*
>
> and refs/remotes/origin/foo or refs/remotes/frotz/nitfol/foo do not
> exist but refs/remotes/xyzzy/nitfol/foo does. And the user says
> "git checkout foo". Instead of finding a remote ref that matches
> "refs/remotes/*/foo" pattern (and assuming the part that matched *
> is the name of the remote), you can iterate the RHS of the refspecs
> to see if there is a unique match.
>
> Then the new branch can unambiguously find that its upstream is
> xyzzy's foo.
Correct. I will try to phrase the problem better in the next iteration.
> I think it makes sense to update the semantics like that.
>
> Wouldn't the traditional case (i.e. without "nitfol/" in the
> xyzzy/frotz remotes above) be a mere special case for this new
> logic?
Yes.
> You mentioned there is a regression caught by existing tests
> if you go this route, but I do not see how that happens.
It's technically a regression since it breaks existing tests, but as you
observe in your reply to patch #5, it is really the test that relies on
current implementation details.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFD/PATCH 4/5] branch.c: Look up refspecs to validate tracking branches
2013-04-19 6:20 [RFD/PATCH 0/5] Improving the search for remote-tracking branches Johan Herland
` (2 preceding siblings ...)
2013-04-19 6:20 ` [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches Johan Herland
@ 2013-04-19 6:20 ` Johan Herland
2013-04-19 6:20 ` [RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream Johan Herland
4 siblings, 0 replies; 9+ messages in thread
From: Johan Herland @ 2013-04-19 6:20 UTC (permalink / raw)
To: git; +Cc: johan
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 well with the conventional
refspecs created by "git clone" or "git remote add", but does not work if the
user tweaks the refspecs to place remote-tracking branches outside
refs/remotes/*.
This patch adds explicit checking of the refspecs for each remote to determine
whether a candidate tracking branch is indeed a valid remote-tracking branch,
even if placed outside refs/heads/* and refs/remotes/*.
This new check is added as a fallback after checking for match against
refs/heads/* and refs/remotes/*, so the code will not be run in the common
case.
This patch also fixes the last remaining test failure in t2024-checkout-dwim.
Signed-off-by: Johan Herland <johan@herland.net>
---
branch.c | 18 +++++++++++++++++-
t/t2024-checkout-dwim.sh | 2 +-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/branch.c b/branch.c
index 6ae6a4c..c9f9dec 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,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
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
'
--
1.8.1.3.704.g33f7d4f
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream
2013-04-19 6:20 [RFD/PATCH 0/5] Improving the search for remote-tracking branches Johan Herland
` (3 preceding siblings ...)
2013-04-19 6:20 ` [RFD/PATCH 4/5] branch.c: Look up refspecs to validate " Johan Herland
@ 2013-04-19 6:20 ` Johan Herland
2013-04-19 19:51 ` Junio C Hamano
4 siblings, 1 reply; 9+ messages in thread
From: Johan Herland @ 2013-04-19 6:20 UTC (permalink / raw)
To: git; +Cc: johan
The previous patch adds validation of upstream remote-tracking branches by
parsing the configured refspecs, and making sure that the candidate upstream
(if not already matching refs/heads/* or refs/remotes/*) is indeed a
remote-tracking branch according to some remote's refspec. For a
default/conventional setup, this check would automatically also cover
everything within refs/remotes/*, meaning that the preceding check for
refs/remotes/* is redundant (although quicker than the validation against
refspecs). One could also argue that not everything inside refs/remotes/*
should be automatically acceptable as an upstream, if one were to keep
other (non-branch) type of remote-tracking refs there.
This patch removes the simple check for refs/remotes/*, to make sure that
_only_ validated remote-tracking branches (in addition to local branches)
are allowed as upstreams.
However, this means that for unconventional setups that place refs within
refs/remotes/* without configuring a corresponding refspec, those refs will
no longer be usable as upstreams. This breaks a few existing tests, which
are marked as test_expect_failure by this patch, to make them easy to find.
---
branch.c | 1 -
t/t3200-branch.sh | 2 +-
t/t7201-co.sh | 2 +-
t/t9114-git-svn-dcommit-merge.sh | 8 ++++----
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/branch.c b/branch.c
index c9f9dec..beaf11d 100644
--- a/branch.c
+++ b/branch.c
@@ -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);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index d969f0e..41d293d 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_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) &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index be9672e..7267ee2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -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 &&
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 3077851..ef274fd 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -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\"
"
--
1.8.1.3.704.g33f7d4f
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream
2013-04-19 6:20 ` [RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream Johan Herland
@ 2013-04-19 19:51 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2013-04-19 19:51 UTC (permalink / raw)
To: Johan Herland; +Cc: git
Johan Herland <johan@herland.net> writes:
> The previous patch adds validation of upstream remote-tracking branches by
> parsing the configured refspecs, and making sure that the candidate upstream
> (if not already matching refs/heads/* or refs/remotes/*) is indeed a
> remote-tracking branch according to some remote's refspec. For a
> default/conventional setup, this check would automatically also cover
> everything within refs/remotes/*, meaning that the preceding check for
> refs/remotes/* is redundant (although quicker than the validation against
> refspecs). One could also argue that not everything inside refs/remotes/*
> should be automatically acceptable as an upstream, if one were to keep
> other (non-branch) type of remote-tracking refs there.
>
> This patch removes the simple check for refs/remotes/*, to make sure that
> _only_ validated remote-tracking branches (in addition to local branches)
> are allowed as upstreams.
>
> However, this means that for unconventional setups that place refs within
> refs/remotes/* without configuring a corresponding refspec, those refs will
> no longer be usable as upstreams. This breaks a few existing tests, which
> are marked as test_expect_failure by this patch, to make them easy to find.
I think these tests are too loosely written, making assumptions on
what the back-then-current implementation actually does.
If refs/remotes/origin/koala/bear is a remote tracking branch for
koala/bear branch taken from the origin, we will have a refspec that
stores to refs/remotes/origin/koala/bear for _some_ remote (and in
the most normal case it would be "origin").
Instead of expecting failures, I think it is a better change to make
the test case more realistic by adding remote.origin.refspec as well.
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index be9672e..7267ee2 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -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 &&
^ permalink raw reply [flat|nested] 9+ messages in thread