[PATCH 0/5] branch: Fix --track on a remote-tracking non-branch

[PATCH 0/5] branch: Fix --track on a remote-tracking non-branch

[Johan Herland]

Hi,

Per Cederqvist alerted me to a change in v1.8.3.2 that broke his
build/test infrastructure. Specifically, before 41c21f2 (branch.c:
Validate tracking branches with refspecs instead of refs/remotes/*)
Git allowed a local branch to --track anything within refs/remotes/*,
and in 41c21f2, I changed the rules to require a configured remote
with a matching fetch refspec when setting up the upstream configuration
variables (so that there was no ambiguity on how to set
branch.<name>.remote and branch.<name>.merge).

So far so good.

However, in addition to requiring a matching remote/refspec, I also
(for reasons that are still unclear to me) added a requirement that
the resulting remote ref name (to be stored into branch.<name>.merge)
must start with "refs/heads/" (see the last line of
branch.c:check_tracking_branch()).

Although it is typically the case that an upstream branch is a proper
(refs/heads/*) branch in the remote repo (which explains why we have
not noticed this until now), I think it is _wrong_ of Git to _require_
this when configuring the upstream.

Per's setup that triggered this series is described in more detail in
patch #4/5 (which introduces a testcase illustrating the breakage),
and the actual fix (which simply removes the extra refs/heads/*
requirement on the remote ref) is in patch #5/5.

The two first patches are unrelated trivial fixes that I encountered
while working on this, and patch #3 is a small documentation update
suggested by Per.

...Johan


Johan Herland (4):
  t2024: Fix inconsequential typos
  t3200: Minor fix when preparing for tracking failure
  Refer to branch.<name>.remote/merge when documenting --track
  t3200: Add test demonstrating minor regression in 41c21f2

Per Cederqvist (1):
  branch.c: Relax unnecessary requirement on upstream's remote ref name

 Documentation/git-branch.txt |  6 ++++--
 branch.c                     |  3 +--
 t/t2024-checkout-dwim.sh     |  4 ++--
 t/t3200-branch.sh            | 37 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 43 insertions(+), 7 deletions(-)

--
1.8.3.GIT

[PATCH 1/5] t2024: Fix inconsequential typos

[Johan Herland]

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

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index dee55e4..6c78fba 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -113,9 +113,9 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 		cd repo_d &&
 		test_commit d_master &&
 		git checkout -b baz &&
-		test_commit f_baz
+		test_commit d_baz
 		git checkout -b eggs &&
-		test_commit c_eggs
+		test_commit d_eggs
 	) &&
 	git remote add repo_c repo_c &&
 	git config remote.repo_c.fetch \
-- 
1.8.3.GIT

[PATCH 2/5] t3200: Minor fix when preparing for tracking failure

[Johan Herland]

We're testing that trying to --track a ref that is not covered by any remote
refspec should fail. For that, we want to have refs/remotes/local/master
present, but we also want the remote.local.fetch refspec to NOT match
refs/remotes/local/master (so that the tracking setup will fail, as intended).
However, when doing "git fetch local" to ensure the existence of
refs/remotes/local/master, we must not already have changed remote.local.fetch
so as to cause refs/remotes/local/master not to be fetched. Therefore, set
remote.local.fetch to refs/heads/*:refs/remotes/local/* BEFORE we fetch, and
then reset it to refs/heads/s:refs/remotes/local/s AFTER we have fetched
(but before we test --track).

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

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 44ec6a4..8f6ab8e 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -319,8 +319,9 @@ test_expect_success 'test tracking setup (non-wildcard, matching)' '
 
 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 config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
+	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
 	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
-- 
1.8.3.GIT

[PATCH 3/5] Refer to branch.<name>.remote/merge when documenting --track

[Johan Herland]

Make it easier for readers to find the actual config variables that
implement the "upstream" relationship.

Suggested-by: Per Cederqvist <cederp@opera.com>
Signed-off-by: Johan Herland <johan@herland.net>
---

In a private email exchange, Per noted that it was hard for someone reading
the git-branch docs to grasp what really happens when you use --track to
establish an "upstream" relationship. This adds a couple of references to
the config variables involved, and will hopefully make the upstream
relationship a little less "magic".

...Johan

 Documentation/git-branch.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b7cb625..311b336 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -48,7 +48,8 @@ working tree to it; use "git checkout <newbranch>" to switch to the
 new branch.

 When a local branch is started off a remote-tracking branch, Git sets up the
-branch so that 'git pull' will appropriately merge from
+branch (specifically the `branch.<name>.remote` and `branch.<name>.merge`
+configuration entries) so that 'git pull' will appropriately merge from
 the remote-tracking branch. This behavior may be changed via the global
 `branch.autosetupmerge` configuration flag. That setting can be
 overridden by using the `--track` and `--no-track` options, and
@@ -156,7 +157,8 @@ This option is only applicable in non-verbose mode.

 -t::
 --track::
-	When creating a new branch, set up configuration to mark the
+	When creating a new branch, set up `branch.<name>.remote` and
+	`branch.<name>.merge` configuration entries to mark the
 	start-point branch as "upstream" from the new branch. This
 	configuration will tell git to show the relationship between the
 	two branches in `git status` and `git branch -v`. Furthermore,
--
1.8.3.GIT

[PATCH 4/5] t3200: Add test demonstrating minor regression in 41c21f2

[Johan Herland]

In 41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*), we changed the rules for what is considered a valid tracking
branch (a.k.a. upstream branch). We now use the configured remotes and their
refspecs to determine whether a proposed tracking branch is in fact within
the domain of a remote, and we then use that information to deduce the
upstream configuration (branch.<name>.remote and branch.<name>.merge).

However, with that change, we also check that - in addition to a matching
refspec - the result of mapping the tracking branch through that refspec
(i.e. the corresponding ref name in the remote repo) happens to start with
"refs/heads/". In other words, we require that a tracking branch refers to
a _branch_ in the remote repo.

Now, consider that you are e.g. setting up an automated building/testing
infrastructure for a group of similar "source" repositories. The build/test
infrastructure consists of a central scheduler, and a number of build/test
"slave" machines that perform the actual build/test work. The scheduler
monitors the group of similar repos for changes (e.g. with a periodic
"git fetch"), and triggers builds/tests to be run on one or more slaves.
Graphically the changes flow between the repos like this:

  Source #1 -------v          ----> Slave #1
                             /
  Source #2 -----> Scheduler -----> Slave #2
                             \
  Source #3 -------^          ----> Slave #3

        ...                           ...

The scheduler maintains a single Git repo with each of the source repos set
up as distinct remotes. The slaves also need access to all the changes from
all of the source repos, so they pull from the scheduler repo, but using the
following custom refspec:

  remote.origin.fetch = "+refs/remotes/*:refs/remotes/*"

This makes all of the scheduler's remote-tracking branches automatically
available as identical remote-tracking branches in each of the slaves.

Now, consider what happens if a slave tries to create a local branch with
one of the remote-tracking branches as upstream:

  git branch local_branch --track refs/remotes/source-1/some_branch

Git now looks at the configured remotes (in this case there is only "origin",
pointing to the scheduler's repo) and sees refs/remotes/source-1/some_branch
matching origin's refspec. Mapping through that refspec we find that the
corresponding remote ref name is "refs/remotes/source-1/some_branch".
However, since this remote ref name does not start with "refs/heads/", we
discard it as a suitable upstream, and the whole command fails.

This patch adds a testcase demonstrating this failure by creating two
source repos ("a" and "b") that are forwarded through a scheduler ("c")
to a slave repo ("d"), that then tries create a local branch with an
upstream. See the next patch in this series for the exciting conclusion
to this story...

Reported-by: Per Cederqvist <cederp@opera.com>
Signed-off-by: Johan Herland <johan@herland.net>
---

I was not sure where to place this, so I tacked it onto the end of t3200.

Also, I wouldn't mind dropping the whole build/test infrastructure story
from the commit message if the rationale for this kind of setup (and this
patch) can be expressed more concisely.

...Johan

 t/t3200-branch.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 8f6ab8e..4031693 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -871,4 +871,38 @@ test_expect_success '--merged catches invalid object names' '
 	test_must_fail git branch --merged 0000000000000000000000000000000000000000
 '

+test_expect_failure 'tracking with unexpected .fetch refspec' '
+	git init a &&
+	(
+		cd a &&
+		test_commit a
+	) &&
+	git init b &&
+	(
+		cd b &&
+		test_commit b
+	) &&
+	git init c &&
+	(
+		cd c &&
+		test_commit c &&
+		git remote add a ../a &&
+		git remote add b ../b &&
+		git fetch --all
+	) &&
+	git init d &&
+	(
+		cd d &&
+		git remote add c ../c &&
+		git config remote.c.fetch "+refs/remotes/*:refs/remotes/*" &&
+		git fetch c &&
+		git branch --track local/a/master remotes/a/master &&
+		test "$(git config branch.local/a/master.remote)" = "c" &&
+		test "$(git config branch.local/a/master.merge)" = "refs/remotes/a/master" &&
+		git rev-parse --verify a >expect &&
+		git rev-parse --verify local/a/master >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
--
1.8.3.GIT

[PATCH 5/5] branch.c: Relax unnecessary requirement on upstream's remote ref name

[Johan Herland]

From: Per Cederqvist <cederp@opera.com>

When creating an upstream relationship, we use the configured remotes and
their refspecs to determine the upstream configuration settings
branch.<name>.remote and branch.<name>.merge. However, if the matching
refspec does not have refs/heads/<something> on the remote side, we end
up rejecting the match, and failing the upstream configuration.

It could be argued that when we set up an branch's upstream, we want that
upstream to also be a proper branch in the remote repo. Although this is
typically the common case, there are cases (as demonstrated by the previous
patch in this series) where this requirement prevents a useful upstream
relationship from being formed. Furthermore:

 - We have fundamentally no say in how the remote repo have organized its
   branches. The remote repo may put branches (or branch-like constructs
   that are insteresting for downstreams to track) outside refs/heads/*.

 - The user may intentionally want to track a non-branch from a remote
   repo, by using a branch and configured upstream in the local repo.

Relaxing the checking to only require a matching remote/refspec allows the
testcase introduced in the previous patch to succeed, and has no negative
effect on the rest of the test suite.

This patch fixes a behavior (arguably a regression) first introduced in
41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*) on 2013-04-21 (released in >= v1.8.3.2).

Signed-off-by: Johan Herland <johan@herland.net>
---

FTR, Per made the original fix, and I supplied the commit message.

...Johan

 branch.c          | 3 +--
 t/t3200-branch.sh | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/branch.c b/branch.c
index c5c6984..2d15c19 100644
--- a/branch.c
+++ b/branch.c
@@ -203,8 +203,7 @@ static int check_tracking_branch(struct remote *remote, void *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/"));
+	return !remote_find_tracking(remote, &query);
 }

 static int validate_remote_tracking_branch(char *ref)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 4031693..f010303 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -871,7 +871,7 @@ test_expect_success '--merged catches invalid object names' '
 	test_must_fail git branch --merged 0000000000000000000000000000000000000000
 '

-test_expect_failure 'tracking with unexpected .fetch refspec' '
+test_expect_success 'tracking with unexpected .fetch refspec' '
 	git init a &&
 	(
 		cd a &&
--
1.8.3.GIT

Re: [PATCH 0/5] branch: Fix --track on a remote-tracking non-branch

[Junio C Hamano]

Johan Herland <johan@herland.net> writes:

> However, in addition to requiring a matching remote/refspec, I also
> (for reasons that are still unclear to me) added a requirement that
> the resulting remote ref name (to be stored into branch.<name>.merge)
> must start with "refs/heads/" (see the last line of
> branch.c:check_tracking_branch()).
>
> Although it is typically the case that an upstream branch is a proper
> (refs/heads/*) branch in the remote repo (which explains why we have
> not noticed this until now), I think it is _wrong_ of Git to _require_
> this when configuring the upstream.

Yeah, I agree.

Re: [PATCH 1/5] t2024: Fix inconsequential typos

[Junio C Hamano]

Johan Herland <johan@herland.net> writes:

> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>  t/t2024-checkout-dwim.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
> index dee55e4..6c78fba 100755
> --- a/t/t2024-checkout-dwim.sh
> +++ b/t/t2024-checkout-dwim.sh
> @@ -113,9 +113,9 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
>  		cd repo_d &&
>  		test_commit d_master &&
>  		git checkout -b baz &&
> -		test_commit f_baz
> +		test_commit d_baz

Not limited to this hunk but there seems to be a breakage in the &&
chain here.

>  		git checkout -b eggs &&
> -		test_commit c_eggs
> +		test_commit d_eggs
>  	) &&
>  	git remote add repo_c repo_c &&
>  	git config remote.repo_c.fetch \

Re: [PATCH 1/5] t2024: Fix inconsequential typos

[Johan Herland]

On Fri, Sep 6, 2013 at 7:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johan Herland <johan@herland.net> writes:
>> diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
>> index dee55e4..6c78fba 100755
>> --- a/t/t2024-checkout-dwim.sh
>> +++ b/t/t2024-checkout-dwim.sh
>> @@ -113,9 +113,9 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
>>               cd repo_d &&
>>               test_commit d_master &&
>>               git checkout -b baz &&
>> -             test_commit f_baz
>> +             test_commit d_baz
>
> Not limited to this hunk but there seems to be a breakage in the &&
> chain here.

Thanks, found 2 instances in the file (both in that test). Will be
fixed in the next iteration.

...Johan

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

[PATCHv2 0/5] branch: Fix --track on a remote-tracking non-branch

[Johan Herland]

Hi,

Here is the second iteration of this series. Only one change from the
first iteration: The first patch now also fixes some missing &&-chaining
noticed by Junio in t2024.

...Johan


Johan Herland (4):
  t2024: Fix &&-chaining and a couple of typos
  t3200: Minor fix when preparing for tracking failure
  Refer to branch.<name>.remote/merge when documenting --track
  t3200: Add test demonstrating minor regression in 41c21f2

Per Cederqvist (1):
  branch.c: Relax unnecessary requirement on upstream's remote ref name

 Documentation/git-branch.txt |  6 ++++--
 branch.c                     |  3 +--
 t/t2024-checkout-dwim.sh     |  6 +++---
 t/t3200-branch.sh            | 37 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 44 insertions(+), 8 deletions(-)

-- 
1.8.3.GIT

[PATCHv2 1/5] t2024: Fix &&-chaining and a couple of typos

[Johan Herland]

Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t2024-checkout-dwim.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index dee55e4..094b92e 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -104,7 +104,7 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 		cd repo_c &&
 		test_commit c_master &&
 		git checkout -b bar &&
-		test_commit c_bar
+		test_commit c_bar &&
 		git checkout -b spam &&
 		test_commit c_spam
 	) &&
@@ -113,9 +113,9 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 		cd repo_d &&
 		test_commit d_master &&
 		git checkout -b baz &&
-		test_commit f_baz
+		test_commit d_baz &&
 		git checkout -b eggs &&
-		test_commit c_eggs
+		test_commit d_eggs
 	) &&
 	git remote add repo_c repo_c &&
 	git config remote.repo_c.fetch \
-- 
1.8.3.GIT

[PATCHv2 2/5] t3200: Minor fix when preparing for tracking failure

[Johan Herland]

We're testing that trying to --track a ref that is not covered by any remote
refspec should fail. For that, we want to have refs/remotes/local/master
present, but we also want the remote.local.fetch refspec to NOT match
refs/remotes/local/master (so that the tracking setup will fail, as intended).
However, when doing "git fetch local" to ensure the existence of
refs/remotes/local/master, we must not already have changed remote.local.fetch
so as to cause refs/remotes/local/master not to be fetched. Therefore, set
remote.local.fetch to refs/heads/*:refs/remotes/local/* BEFORE we fetch, and
then reset it to refs/heads/s:refs/remotes/local/s AFTER we have fetched
(but before we test --track).

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

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 44ec6a4..8f6ab8e 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -319,8 +319,9 @@ test_expect_success 'test tracking setup (non-wildcard, matching)' '
 
 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 config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
+	git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
 	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
-- 
1.8.3.GIT

[PATCHv2 3/5] Refer to branch.<name>.remote/merge when documenting --track

[Johan Herland]

Make it easier for readers to find the actual config variables that
implement the "upstream" relationship.

Suggested-by: Per Cederqvist <cederp@opera.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-branch.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b7cb625..311b336 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -48,7 +48,8 @@ working tree to it; use "git checkout <newbranch>" to switch to the
 new branch.
 
 When a local branch is started off a remote-tracking branch, Git sets up the
-branch so that 'git pull' will appropriately merge from
+branch (specifically the `branch.<name>.remote` and `branch.<name>.merge`
+configuration entries) so that 'git pull' will appropriately merge from
 the remote-tracking branch. This behavior may be changed via the global
 `branch.autosetupmerge` configuration flag. That setting can be
 overridden by using the `--track` and `--no-track` options, and
@@ -156,7 +157,8 @@ This option is only applicable in non-verbose mode.
 
 -t::
 --track::
-	When creating a new branch, set up configuration to mark the
+	When creating a new branch, set up `branch.<name>.remote` and
+	`branch.<name>.merge` configuration entries to mark the
 	start-point branch as "upstream" from the new branch. This
 	configuration will tell git to show the relationship between the
 	two branches in `git status` and `git branch -v`. Furthermore,
-- 
1.8.3.GIT

[PATCHv2 4/5] t3200: Add test demonstrating minor regression in 41c21f2

[Johan Herland]

In 41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*), we changed the rules for what is considered a valid tracking
branch (a.k.a. upstream branch). We now use the configured remotes and their
refspecs to determine whether a proposed tracking branch is in fact within
the domain of a remote, and we then use that information to deduce the
upstream configuration (branch.<name>.remote and branch.<name>.merge).

However, with that change, we also check that - in addition to a matching
refspec - the result of mapping the tracking branch through that refspec
(i.e. the corresponding ref name in the remote repo) happens to start with
"refs/heads/". In other words, we require that a tracking branch refers to
a _branch_ in the remote repo.

Now, consider that you are e.g. setting up an automated building/testing
infrastructure for a group of similar "source" repositories. The build/test
infrastructure consists of a central scheduler, and a number of build/test
"slave" machines that perform the actual build/test work. The scheduler
monitors the group of similar repos for changes (e.g. with a periodic
"git fetch"), and triggers builds/tests to be run on one or more slaves.
Graphically the changes flow between the repos like this:

  Source #1 -------v          ----> Slave #1
                             /
  Source #2 -----> Scheduler -----> Slave #2
                             \
  Source #3 -------^          ----> Slave #3

        ...                           ...

The scheduler maintains a single Git repo with each of the source repos set
up as distinct remotes. The slaves also need access to all the changes from
all of the source repos, so they pull from the scheduler repo, but using the
following custom refspec:

  remote.origin.fetch = "+refs/remotes/*:refs/remotes/*"

This makes all of the scheduler's remote-tracking branches automatically
available as identical remote-tracking branches in each of the slaves.

Now, consider what happens if a slave tries to create a local branch with
one of the remote-tracking branches as upstream:

  git branch local_branch --track refs/remotes/source-1/some_branch

Git now looks at the configured remotes (in this case there is only "origin",
pointing to the scheduler's repo) and sees refs/remotes/source-1/some_branch
matching origin's refspec. Mapping through that refspec we find that the
corresponding remote ref name is "refs/remotes/source-1/some_branch".
However, since this remote ref name does not start with "refs/heads/", we
discard it as a suitable upstream, and the whole command fails.

This patch adds a testcase demonstrating this failure by creating two
source repos ("a" and "b") that are forwarded through a scheduler ("c")
to a slave repo ("d"), that then tries create a local branch with an
upstream. See the next patch in this series for the exciting conclusion
to this story...

Reported-by: Per Cederqvist <cederp@opera.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3200-branch.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 8f6ab8e..4031693 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -871,4 +871,38 @@ test_expect_success '--merged catches invalid object names' '
 	test_must_fail git branch --merged 0000000000000000000000000000000000000000
 '
 
+test_expect_failure 'tracking with unexpected .fetch refspec' '
+	git init a &&
+	(
+		cd a &&
+		test_commit a
+	) &&
+	git init b &&
+	(
+		cd b &&
+		test_commit b
+	) &&
+	git init c &&
+	(
+		cd c &&
+		test_commit c &&
+		git remote add a ../a &&
+		git remote add b ../b &&
+		git fetch --all
+	) &&
+	git init d &&
+	(
+		cd d &&
+		git remote add c ../c &&
+		git config remote.c.fetch "+refs/remotes/*:refs/remotes/*" &&
+		git fetch c &&
+		git branch --track local/a/master remotes/a/master &&
+		test "$(git config branch.local/a/master.remote)" = "c" &&
+		test "$(git config branch.local/a/master.merge)" = "refs/remotes/a/master" &&
+		git rev-parse --verify a >expect &&
+		git rev-parse --verify local/a/master >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
1.8.3.GIT

[PATCHv2 5/5] branch.c: Relax unnecessary requirement on upstream's remote ref name

[Johan Herland]

From: Per Cederqvist <cederp@opera.com>

When creating an upstream relationship, we use the configured remotes and
their refspecs to determine the upstream configuration settings
branch.<name>.remote and branch.<name>.merge. However, if the matching
refspec does not have refs/heads/<something> on the remote side, we end
up rejecting the match, and failing the upstream configuration.

It could be argued that when we set up an branch's upstream, we want that
upstream to also be a proper branch in the remote repo. Although this is
typically the common case, there are cases (as demonstrated by the previous
patch in this series) where this requirement prevents a useful upstream
relationship from being formed. Furthermore:

 - We have fundamentally no say in how the remote repo have organized its
   branches. The remote repo may put branches (or branch-like constructs
   that are insteresting for downstreams to track) outside refs/heads/*.

 - The user may intentionally want to track a non-branch from a remote
   repo, by using a branch and configured upstream in the local repo.

Relaxing the checking to only require a matching remote/refspec allows the
testcase introduced in the previous patch to succeed, and has no negative
effect on the rest of the test suite.

This patch fixes a behavior (arguably a regression) first introduced in
41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*) on 2013-04-21 (released in >= v1.8.3.2).

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

diff --git a/branch.c b/branch.c
index c5c6984..2d15c19 100644
--- a/branch.c
+++ b/branch.c
@@ -203,8 +203,7 @@ static int check_tracking_branch(struct remote *remote, void *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/"));
+	return !remote_find_tracking(remote, &query);
 }
 
 static int validate_remote_tracking_branch(char *ref)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 4031693..f010303 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -871,7 +871,7 @@ test_expect_success '--merged catches invalid object names' '
 	test_must_fail git branch --merged 0000000000000000000000000000000000000000
 '
 
-test_expect_failure 'tracking with unexpected .fetch refspec' '
+test_expect_success 'tracking with unexpected .fetch refspec' '
 	git init a &&
 	(
 		cd a &&
-- 
1.8.3.GIT