[PATCH 0/5] provide better error messages for @{upstream}

[PATCH 0/5] provide better error messages for @{upstream}

[Zbigniew Jędrzejewski-Szmek]

Hi,

this is a small patch series to provide better error messages when
'@{u}' in various forms is used. Patches are really tiny, and 2-5/5
could probably be squashed together, but this way the individual
changes should be more readable.

The idea is to provide as much information as possible for the user
when @{u} cannot be resolved successfully. The following cases are
distinguished:
- branch@{u} on a branch with
  * no upstream,
  * on a branch with a configured upstream, but when a branch is not
    in remote.<remote>.fetch
- branch@{u} when branch 'branch' does not exist
- @{u} without the branch name (current branch name is used)

Zbigniew Jędrzejewski-Szmek (5):
  t1507: add additional tests for @{upstream}
  Provide branch name in error message when using @{u}
  Provide better message for barnhc_wiht_tpyo@{u}
  Be more specific if upstream branch is not fetched
  i18n: mark @{upstream} error messages for translation

 sha1_name.c                   | 20 ++++++++---
 t/t1507-rev-parse-upstream.sh | 82 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 95 insertions(+), 7 deletions(-)

-- 
1.7.10.344.g387ed

[PATCH 1/5] t1507: add additional tests for @{upstream}

[Zbigniew Jędrzejewski-Szmek]

- test branch@{u} with . as remote
- check error message for branch@{u} on a branch with
  * no upstream,
  * on a branch with a configured upstream, but when a branch is not
    in remote.<remote>.fetch
- check error message for branch@{u} when branch 'branch' does not
  exist
- check error message for @{u} without the branch name

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 t/t1507-rev-parse-upstream.sh | 82 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 3 deletions(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index a455551..1342915 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -15,10 +15,18 @@ test_expect_success 'setup' '
 	test_commit 3 &&
 	(cd clone &&
 	 test_commit 4 &&
-	 git branch --track my-side origin/side)
-
+	 git branch --track my-side origin/side &&
+	 git branch --track local-master master &&
+	 git remote add -t master master-only .. &&
+	 git fetch master-only &&
+	 git branch bad-upstream &&
+	 git config branch.bad-upstream.remote master-only &&
+	 git config branch.bad-upstream.merge refs/heads/side
+	)
 '
 
+sq="'"
+
 full_name () {
 	(cd clone &&
 	 git rev-parse --symbolic-full-name "$@")
@@ -29,6 +37,11 @@ commit_subject () {
 	 git show -s --pretty=format:%s "$@")
 }
 
+error_message () {
+	(cd clone &&
+	 test_must_fail git rev-parse --verify "$@")
+}
+
 test_expect_success '@{upstream} resolves to correct full name' '
 	test refs/remotes/origin/master = "$(full_name @{upstream})"
 '
@@ -78,7 +91,6 @@ test_expect_success 'checkout -b new my-side@{u} forks from the same' '
 
 test_expect_success 'merge my-side@{u} records the correct name' '
 (
-	sq="'\''" &&
 	cd clone || exit
 	git checkout master || exit
 	git branch -D new ;# can fail but is ok
@@ -107,6 +119,70 @@ test_expect_success 'checkout other@{u}' '
 	test_cmp expect actual
 '
 
+test_expect_success 'branch@{u} works when tracking a local branch' '
+	test refs/heads/master = "$(full_name local-master@{u})"
+'
+
+test_expect_success 'branch@{u} error message when no upstream' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}non-tracking${sq}
+	fatal: Needed a single revision
+	EOF
+	(cd clone &&
+	 test_must_fail git rev-parse --verify non-tracking@{u}) 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '@{u} error message when no upstream' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}${sq}
+	fatal: Needed a single revision
+	EOF
+	test_must_fail git rev-parse --verify @{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'branch@{u} error message with misspelt branch' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}no-such-branch${sq}
+	fatal: Needed a single revision
+	EOF
+	error_message no-such-branch@{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '@{u} error message when not on a branch' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}${sq}
+	fatal: Needed a single revision
+	EOF
+	git checkout HEAD^0 &&
+	test_must_fail git rev-parse --verify @{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'branch@{u} error message if upstream branch not fetched' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}bad-upstream${sq}
+	fatal: Needed a single revision
+	EOF
+	error_message bad-upstream@{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pull works when tracking a local branch' '
+(
+	cd clone &&
+	git checkout local-master &&
+	git pull
+)
+'
+
+# makes sense if the previous one succeeded
+test_expect_success '@{u} works when tracking a local branch' '
+	test refs/heads/master = "$(full_name @{u})"
+'
+
 cat >expect <<EOF
 commit 8f489d01d0cc65c3b0f09504ec50b5ed02a70bd5
 Reflog: master@{0} (C O Mitter <committer@example.com>)
-- 
1.7.10.344.g387ed

Re: [PATCH 1/5] t1507: add additional tests for @{upstream}

[Junio C Hamano]

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

> +error_message () {
> +	(cd clone &&
> +	 test_must_fail git rev-parse --verify "$@")
> +}
> ...
> +test_expect_success 'branch@{u} error message when no upstream' '
> +	cat >expect <<-EOF &&
> +	error: No upstream branch found for ${sq}non-tracking${sq}
> +	fatal: Needed a single revision
> +	EOF
> +	(cd clone &&
> +	 test_must_fail git rev-parse --verify non-tracking@{u}) 2>actual &&

Why not use "error_message" as other new tests?

> +	test_cmp expect actual

Should we worry about test_i18ncmp here (and all the other test_cmp this
patch introduces)?

> +'
> +
> +test_expect_success '@{u} error message when no upstream' '
> +	cat >expect <<-EOF &&
> +	error: No upstream branch found for ${sq}${sq}
> +	fatal: Needed a single revision
> +	EOF
> +	test_must_fail git rev-parse --verify @{u} 2>actual &&
> +	test_cmp expect actual
> +'

We may want to update the error message for "@{u}" when the current one is
not tracked, instead of saying ''.  Perhaps

	error: No upstream branch found for the current branch.

or something?

Likewise for the detached HEAD case.

> +test_expect_success 'branch@{u} error message if upstream branch not fetched' '
> +	cat >expect <<-EOF &&
> +	error: No upstream branch found for ${sq}bad-upstream${sq}
> +	fatal: Needed a single revision
> +	EOF
> +	error_message bad-upstream@{u} 2>actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'pull works when tracking a local branch' '
> +(
> +	cd clone &&
> +	git checkout local-master &&
> +	git pull
> +)
> +'
> +
> +# makes sense if the previous one succeeded

Can't you make this not to depend on the result of the previous test
(Which one?  The immediately previous one is done in "clone" directory,
so that is not it)?

> +test_expect_success '@{u} works when tracking a local branch' '
> +	test refs/heads/master = "$(full_name @{u})"
> +'
> +
>  cat >expect <<EOF
>  commit 8f489d01d0cc65c3b0f09504ec50b5ed02a70bd5
>  Reflog: master@{0} (C O Mitter <committer@example.com>)

Re: [PATCH 1/5] t1507: add additional tests for @{upstream}

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
>
>> +error_message () {
>> +	(cd clone &&
>> +	 test_must_fail git rev-parse --verify "$@")
>> +}
>> ...
>> +test_expect_success 'branch@{u} error message when no upstream' '
>> +	cat >expect <<-EOF &&
>> +	error: No upstream branch found for ${sq}non-tracking${sq}
>> +	fatal: Needed a single revision
>> +	EOF
>> +	(cd clone &&
>> +	 test_must_fail git rev-parse --verify non-tracking@{u}) 2>actual &&
>
> Why not use "error_message" as other new tests?

I think the remainder of the message should be ignored.  Will comment on
individual steps.

Thanks.

Re: [PATCH 1/5] t1507: add additional tests for @{upstream}

[Zbigniew Jędrzejewski-Szmek]

On 04/11/2012 07:57 PM, Junio C Hamano wrote:
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>>
>>> +error_message () {
>>> +	(cd clone&&
>>> +	 test_must_fail git rev-parse --verify "$@")
>>> +}
>>> ...
>>> +test_expect_success 'branch@{u} error message when no upstream' '
>>> +	cat>expect<<-EOF&&
>>> +	error: No upstream branch found for ${sq}non-tracking${sq}
>>> +	fatal: Needed a single revision
>>> +	EOF
>>> +	(cd clone&&
>>> +	 test_must_fail git rev-parse --verify non-tracking@{u}) 2>actual&&
>>
>> Why not use "error_message" as other new tests?
There _was_ some reason, in some earlier version of the patch, but now it
is gone. Will use error_message.

> I think the remainder of the message should be ignored.  Will comment on
> individual steps.
>
> Thanks.
I'll try to improve the description in first patch to not confuse people :)

Thanks,
Zbyszek

Re: [PATCH 1/5] t1507: add additional tests for @{upstream}

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> We may want to update the error message for "@{u}" when the current one is
> not tracked, instead of saying ''.  Perhaps
>
> 	error: No upstream branch found for the current branch.
>
> or something?
>
> Likewise for the detached HEAD case.

This is indeed the point of the patch serie, and I like how it first
shows how bad the error messages can be, and then illustrate the fix
with patch hunks in the newly added testcases in further patches.

But the commit message for this patch could probably be improved: we
usually do not give a list of _what_ is done, since the code already
says that, but we insist on _why_ it is done.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 1/5] t1507: add additional tests for @{upstream}

[Zbigniew Jędrzejewski-Szmek]

On 04/11/2012 07:59 PM, Matthieu Moy wrote:
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> We may want to update the error message for "@{u}" when the current one is
>> not tracked, instead of saying ''.  Perhaps
>>
>> 	error: No upstream branch found for the current branch.
>>
>> or something?
>>
>> Likewise for the detached HEAD case.
>
> This is indeed the point of the patch serie, and I like how it first
> shows how bad the error messages can be, and then illustrate the fix
> with patch hunks in the newly added testcases in further patches.
>
> But the commit message for this patch could probably be improved: we
> usually do not give a list of _what_ is done, since the code already
> says that, but we insist on _why_ it is done.
Yeah, I need to provide a better motivation/description in reroll.

Thanks for looking at this,
Zbyszek

[PATCH 2/5] Provide branch name in error message when using @{u}

[Zbigniew Jędrzejewski-Szmek]

When using @{u} or @{upstream} it is common to omit the branch name,
implying current branch. If the upstream is not configured, the error
message was "No upstream branch found for ''".

When resolving '@{u}', branch_get() is called, which almost always
returns a description of a branch. This allows us to use a branch name
in the error message, even if the user said something like '@{u}'.

The only case when branch_get() returns NULL is when HEAD points to so
something which is not a branch. Of course this also means that no
upstream is configured, but it is better to directly say that HEAD
does not point to a branch.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   | 12 ++++++++----
 t/t1507-rev-parse-upstream.sh |  4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 03ffc2c..c2fe1aa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -856,10 +856,14 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	len = cp + tmp_len - name;
 	cp = xstrndup(name, cp - name);
 	upstream = branch_get(*cp ? cp : NULL);
-	if (!upstream
-	    || !upstream->merge
-	    || !upstream->merge[0]->dst)
-		return error("No upstream branch found for '%s'", cp);
+	/*
+	 * Upstream can be NULL only if cp refers to HEAD and HEAD
+	 * points to something different than a branch.
+	 */
+	if (!upstream)
+		return error("HEAD does not point to a branch");
+	if (!upstream->merge || !upstream->merge[0]->dst)
+		return error("No upstream branch found for '%s'", upstream->name);
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
 	strbuf_reset(buf);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 1342915..a00b689 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -135,7 +135,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
 
 test_expect_success '@{u} error message when no upstream' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}${sq}
+	error: No upstream branch found for ${sq}master${sq}
 	fatal: Needed a single revision
 	EOF
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
@@ -153,7 +153,7 @@ test_expect_success 'branch@{u} error message with misspelt branch' '
 
 test_expect_success '@{u} error message when not on a branch' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}${sq}
+	error: HEAD does not point to a branch
 	fatal: Needed a single revision
 	EOF
 	git checkout HEAD^0 &&
-- 
1.7.10.344.g387ed

Re: [PATCH 2/5] Provide branch name in error message when using @{u}

[Junio C Hamano]

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

> diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
> index 1342915..a00b689 100755
> --- a/t/t1507-rev-parse-upstream.sh
> +++ b/t/t1507-rev-parse-upstream.sh
> @@ -135,7 +135,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
>  
>  test_expect_success '@{u} error message when no upstream' '
>  	cat >expect <<-EOF &&
> -	error: No upstream branch found for ${sq}${sq}
> +	error: No upstream branch found for ${sq}master${sq}
>  	fatal: Needed a single revision
>  	EOF
>  	test_must_fail git rev-parse --verify @{u} 2>actual &&

I am not sure if saying "... for 'master'" is better or "... for the
current branch" is better.  Using different wording reflects the fact that
the user gave "@{u}" and not "master@{u}".

But I do not care too deeply.  Either way, it is a vast improvement over
the current "... for ''" output.

And the "detached" case is definitely better.

> @@ -153,7 +153,7 @@ test_expect_success 'branch@{u} error message with misspelt branch' '
>  
>  test_expect_success '@{u} error message when not on a branch' '
>  	cat >expect <<-EOF &&
> -	error: No upstream branch found for ${sq}${sq}
> +	error: HEAD does not point to a branch
>  	fatal: Needed a single revision
>  	EOF

Thanks.

Re: [PATCH 2/5] Provide branch name in error message when using @{u}

[Zbigniew Jędrzejewski-Szmek]

On 04/11/2012 08:00 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>
>> diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
>> index 1342915..a00b689 100755
>> --- a/t/t1507-rev-parse-upstream.sh
>> +++ b/t/t1507-rev-parse-upstream.sh
>> @@ -135,7 +135,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
>>
>>   test_expect_success '@{u} error message when no upstream' '
>>   	cat>expect<<-EOF&&
>> -	error: No upstream branch found for ${sq}${sq}
>> +	error: No upstream branch found for ${sq}master${sq}
>>   	fatal: Needed a single revision
>>   	EOF
>>   	test_must_fail git rev-parse --verify @{u} 2>actual&&
>
> I am not sure if saying "... for 'master'" is better or "... for the
> current branch" is better.  Using different wording reflects the fact that
> the user gave "@{u}" and not "master@{u}".
Hi,

I think that explicitly providing the name of the branch is useless when
the user has a properly configured git prompt which always shows the 
current branch. But not everybody does that, and for such people 
providing the name in the error message could be useful.

> But I do not care too deeply.
I don't either. I'll wait to see if other people chime in.

 > Either way, it is a vast improvement over
 > the current "... for ''" output.
> And the "detached" case is definitely better.
Thanks!

Thank you for the review. I'll send a reroll taking into account your 
and Matthieu's comments in a day or two if nobody else comments.

Zbyszek

[PATCH 3/5] Provide better message for barnhc_wiht_tpyo@{u}

[Zbigniew Jędrzejewski-Szmek]

Instead of just saying that no upstream exists for such branch,
which is true but not very helpful, check that there's no
refs/heads/barnhc_wiht_tpyo and tell it to the user.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   | 5 ++++-
 t/t1507-rev-parse-upstream.sh | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index c2fe1aa..e2d576a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -862,8 +862,11 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	 */
 	if (!upstream)
 		return error("HEAD does not point to a branch");
-	if (!upstream->merge || !upstream->merge[0]->dst)
+	if (!upstream->merge || !upstream->merge[0]->dst) {
+		if (!ref_exists(upstream->refname))
+			return error("No such branch: '%s'", cp);
 		return error("No upstream branch found for '%s'", upstream->name);
+	}
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
 	strbuf_reset(buf);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index a00b689..05c4bc4 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -144,7 +144,7 @@ test_expect_success '@{u} error message when no upstream' '
 
 test_expect_success 'branch@{u} error message with misspelt branch' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}no-such-branch${sq}
+	error: No such branch: ${sq}no-such-branch${sq}
 	fatal: Needed a single revision
 	EOF
 	error_message no-such-branch@{u} 2>actual &&
-- 
1.7.10.344.g387ed

[PATCH 4/5] Be more specific if upstream branch is not fetched

[Zbigniew Jędrzejewski-Szmek]

If the branch configured as upstream was missing from
remote.<remote>.fetch, git said "Upstream branch not found".
We can be more helpful, and separate the cases when upstream
is not configured, and when it is configured, but specific
branch is not fetched.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   | 6 +++++-
 t/t1507-rev-parse-upstream.sh | 6 +++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index e2d576a..5b1b0f9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -865,7 +865,11 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	if (!upstream->merge || !upstream->merge[0]->dst) {
 		if (!ref_exists(upstream->refname))
 			return error("No such branch: '%s'", cp);
-		return error("No upstream branch found for '%s'", upstream->name);
+		if (!upstream->merge)
+			return error("No upstream configured for branch '%s'",
+				     upstream->name);
+		return error("Upstream branch '%s' not fetched from remote '%s'",
+			     upstream->merge[0]->src, upstream->remote_name);
 	}
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 05c4bc4..112025f 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -125,7 +125,7 @@ test_expect_success 'branch@{u} works when tracking a local branch' '
 
 test_expect_success 'branch@{u} error message when no upstream' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}non-tracking${sq}
+	error: No upstream configured for branch ${sq}non-tracking${sq}
 	fatal: Needed a single revision
 	EOF
 	(cd clone &&
@@ -135,7 +135,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
 
 test_expect_success '@{u} error message when no upstream' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}master${sq}
+	error: No upstream configured for branch ${sq}master${sq}
 	fatal: Needed a single revision
 	EOF
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
@@ -163,7 +163,7 @@ test_expect_success '@{u} error message when not on a branch' '
 
 test_expect_success 'branch@{u} error message if upstream branch not fetched' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}bad-upstream${sq}
+	error: Upstream branch ${sq}refs/heads/side${sq} not fetched from remote ${sq}master-only${sq}
 	fatal: Needed a single revision
 	EOF
 	error_message bad-upstream@{u} 2>actual &&
-- 
1.7.10.344.g387ed

Re: [PATCH 4/5] Be more specific if upstream branch is not fetched

[Jeff King]

On Wed, Apr 11, 2012 at 06:17:14PM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> If the branch configured as upstream was missing from
> remote.<remote>.fetch, git said "Upstream branch not found".
> We can be more helpful, and separate the cases when upstream
> is not configured, and when it is configured, but specific
> branch is not fetched.

I very much like the direction of this series, but I found this one a
little confusing. If you have upstream config, but the configured merge
branch is not part of the remote's refspecs, what does it mean? You
would be able to "git pull", but you would not have a remote tracking
branch representing what the remote has. So this message:

> -		return error("No upstream branch found for '%s'", upstream->name);
> +		if (!upstream->merge)
> +			return error("No upstream configured for branch '%s'",
> +				     upstream->name);
> +		return error("Upstream branch '%s' not fetched from remote '%s'",
> +			     upstream->merge[0]->src, upstream->remote_name);

doesn't seem right to me. The upstream branch can be fetched just fine;
it is simply that we do not maintain a tracking branch for it.

Having worked it out in my head, I think that is maybe even what you
meant, but reading the message the first time left me very confused.
I'm not sure what a better wording would be, though. I was thinking
something like:

  Upstream branch '%s' is not stored as a remote-tracking branch.

or something, but I know we have had trouble with the term "tracking
branch" in the past. Maybe there is a less loaded term.

-Peff

Re: [PATCH 4/5] Be more specific if upstream branch is not fetched

[Zbigniew Jędrzejewski-Szmek]

On 04/12/2012 07:30 AM, Jeff King wrote:
> On Wed, Apr 11, 2012 at 06:17:14PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> 
>> If the branch configured as upstream was missing from
>> remote.<remote>.fetch, git said "Upstream branch not found".
>> We can be more helpful, and separate the cases when upstream
>> is not configured, and when it is configured, but specific
>> branch is not fetched.
> 
> I very much like the direction of this series, but I found this one a
> little confusing. If you have upstream config, but the configured merge
> branch is not part of the remote's refspecs, what does it mean? You
> would be able to "git pull", but you would not have a remote tracking
> branch representing what the remote has. So this message:
> 
>> -		return error("No upstream branch found for '%s'", upstream->name);
>> +		if (!upstream->merge)
>> +			return error("No upstream configured for branch '%s'",
>> +				     upstream->name);
>> +		return error("Upstream branch '%s' not fetched from remote '%s'",
>> +			     upstream->merge[0]->src, upstream->remote_name);
> 
> doesn't seem right to me. The upstream branch can be fetched just fine;
> it is simply that we do not maintain a tracking branch for it.
Hi,

maybe I'm missing something, but I think that git will not fetch branches
(or commits) which are not part of remote's refspecs. The fact that we do
not have a remote-tracking branch would be secondary.

% git init repo6
Initialized empty Git repository in /var/tmp/repo6/.git/
% cd repo6
% git commit -m init --allow-empty
[master (root-commit) 46c16de] init
% git checkout -b side
% git commit -m side --allow-empty
[side 6a0f0e9] side
% git clone . clone
Cloning into 'clone'...
done.
% cd clone
% git config remote.origin.fetch refs/heads/master:refs/remotes/master
% git fetch -v
From /var/tmp/repo6/.
 * [new branch]      master     -> master
% git show 6a0f0e9
fatal: ambiguous argument '6a0f0e9': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions

The scenario leading to this error message would be:
1. user clones
2. user sets remote's refspec to avoid fetching too much stuff
3. user creates a side branch
4. user edits .git/config by hand to create [branch "side"]
based on [branch "master]. (I think that this is a pretty common thing to do.)
5. side@{u} is not fetched

A second scenario:

Actually, the fact that we have a remote tracking branch is ignored, if it is
missing from the remote's refspec. (Such situation will arise if the remote's
refspec is set after the remote branch was fetched.)

% git branch -a
  master
* side
  remotes/origin/HEAD -> origin/master
  remotes/origin/master
  remotes/origin/side
% git show side@{u}
error: Upstream branch 'refs/heads/origin/side' not fetched from remote 'origin'
error: Upstream branch 'refs/heads/origin/side' not fetched from remote 'origin'
fatal: ambiguous argument 'side@{u}': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
% git config remote.origin.fetch
refs/heads/master:refs/remotes/master
% tail -3 .git/config
[branch "side"]
        remote = origin
        merge = refs/heads/origin/side

So I think that the proposed error message is OK.

Zbyszek

> Having worked it out in my head, I think that is maybe even what you
> meant, but reading the message the first time left me very confused.
> I'm not sure what a better wording would be, though. I was thinking
> something like:
> 
>    Upstream branch '%s' is not stored as a remote-tracking branch.
> 
> or something, but I know we have had trouble with the term "tracking
> branch" in the past. Maybe there is a less loaded term.
> 
> -Peff
> 

Re: [PATCH 4/5] Be more specific if upstream branch is not fetched

[Junio C Hamano]

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

> maybe I'm missing something, but I think that git will not fetch branches
> (or commits) which are not part of remote's refspecs.

A remote's refspec does not have to "store" a tracking branch.  The branch
is still fetched, but not stored anywhere locally.

> % cd clone
> % git config remote.origin.fetch refs/heads/master:refs/remotes/master

In other words, the fetch refspec could be without colon.  With

	[remote "origin"]
		url = ...
        	fetch = refs/heads/master
	[branch "master"]
        	remote = origin
                merge = refs/heads/master

you can still say "git pull" without any other argument while on your
"master" and it will do the right thing.  Actually, you do not even have
to have remote.origin.fetch at all (branch.master.merge will be added to
the set of refs to be fetched).

In such a case "master@{upstream}" does not (and should not) resolve to
anything, and the reason is not because it is not "fetched", but as Peff
said, because it is not "stored".

Re: [PATCH 4/5] Be more specific if upstream branch is not fetched

[Jeff King]

On Thu, Apr 12, 2012 at 08:15:52AM -0700, Junio C Hamano wrote:

> In other words, the fetch refspec could be without colon.  With
> 
> 	[remote "origin"]
> 		url = ...
>         	fetch = refs/heads/master
> 	[branch "master"]
>         	remote = origin
>                 merge = refs/heads/master
> 
> you can still say "git pull" without any other argument while on your
> "master" and it will do the right thing.  Actually, you do not even have
> to have remote.origin.fetch at all (branch.master.merge will be added to
> the set of refs to be fetched).
> 
> In such a case "master@{upstream}" does not (and should not) resolve to
> anything, and the reason is not because it is not "fetched", but as Peff
> said, because it is not "stored".

Exactly. That was the distinction I was trying to make, but you
explained it much better.

-Peff

[PATCH v2 0/5] provide better error messages for @{upstream}

[Zbigniew Jędrzejewski-Szmek]

Hi,

v2:

- Better commit descriptions (hopefully)
- error_message used in tests in one more place
- Error message changed from
  "Upstream branch '%s' not fetched from remote '%s'"
  to
  "Upstream branch '%s' not stored as a remote-tracking branch".

[I thought about using
 "No tracking brach for remote branch '%s'"
 or
 "No remote-tracking branch for upstream branch '%s'"
 or
 "Tracking branch not configured for upstream branch '%s'"
 but neither one is convincing. ]

Zbigniew Jędrzejewski-Szmek (5):
      t1507: add tests to document @{upstream} behaviour
      Provide branch name in error message when using @{u}
      Provide better message for barnhc_wiht_tpyo@{u}
      Be more specific if upstream branch is not tracked
      i18n: mark @{upstream} error messages for translation

 sha1_name.c                   |   20 ++++++++--
 t/t1507-rev-parse-upstream.sh |   81 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 94 insertions(+), 7 deletions(-)

-- 
1.7.10.344.g387ed

[PATCH v2 1/5] t1507: add tests to document @{upstream} behaviour

[Zbigniew Jędrzejewski-Szmek]

In preparation for future changes, add tests which show error messages
with @{upstream} in various conditions:

- test branch@{u} with . as remote
- check error message for branch@{u} on a branch with
  * no upstream,
  * on a branch with a configured upstream which doesn't have a
    remote-tracking branch
- check error message for branch@{u} when branch 'branch' does not
  exist
- check error message for @{u} without the branch name

Right now the messages are very similar, but various cases can and
will be distinguished.

Note: test_i18ncmp is not used, because currently error output is not
internationalized. test_cmp will be switched to test_i18ncmp in a later
patch, when error messages are internationalized.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 t/t1507-rev-parse-upstream.sh |   81 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index a455551..c4981ba 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -15,10 +15,18 @@ test_expect_success 'setup' '
 	test_commit 3 &&
 	(cd clone &&
 	 test_commit 4 &&
-	 git branch --track my-side origin/side)
-
+	 git branch --track my-side origin/side &&
+	 git branch --track local-master master &&
+	 git remote add -t master master-only .. &&
+	 git fetch master-only &&
+	 git branch bad-upstream &&
+	 git config branch.bad-upstream.remote master-only &&
+	 git config branch.bad-upstream.merge refs/heads/side
+	)
 '
 
+sq="'"
+
 full_name () {
 	(cd clone &&
 	 git rev-parse --symbolic-full-name "$@")
@@ -29,6 +37,11 @@ commit_subject () {
 	 git show -s --pretty=format:%s "$@")
 }
 
+error_message () {
+	(cd clone &&
+	 test_must_fail git rev-parse --verify "$@")
+}
+
 test_expect_success '@{upstream} resolves to correct full name' '
 	test refs/remotes/origin/master = "$(full_name @{upstream})"
 '
@@ -78,7 +91,6 @@ test_expect_success 'checkout -b new my-side@{u} forks from the same' '
 
 test_expect_success 'merge my-side@{u} records the correct name' '
 (
-	sq="'\''" &&
 	cd clone || exit
 	git checkout master || exit
 	git branch -D new ;# can fail but is ok
@@ -107,6 +119,69 @@ test_expect_success 'checkout other@{u}' '
 	test_cmp expect actual
 '
 
+test_expect_success 'branch@{u} works when tracking a local branch' '
+	test refs/heads/master = "$(full_name local-master@{u})"
+'
+
+test_expect_success 'branch@{u} error message when no upstream' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}non-tracking${sq}
+	fatal: Needed a single revision
+	EOF
+	error_message non-tracking@{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '@{u} error message when no upstream' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}${sq}
+	fatal: Needed a single revision
+	EOF
+	test_must_fail git rev-parse --verify @{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'branch@{u} error message with misspelt branch' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}no-such-branch${sq}
+	fatal: Needed a single revision
+	EOF
+	error_message no-such-branch@{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '@{u} error message when not on a branch' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}${sq}
+	fatal: Needed a single revision
+	EOF
+	git checkout HEAD^0 &&
+	test_must_fail git rev-parse --verify @{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'branch@{u} error message if upstream branch not fetched' '
+	cat >expect <<-EOF &&
+	error: No upstream branch found for ${sq}bad-upstream${sq}
+	fatal: Needed a single revision
+	EOF
+	error_message bad-upstream@{u} 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pull works when tracking a local branch' '
+(
+	cd clone &&
+	git checkout local-master &&
+	git pull
+)
+'
+
+# makes sense if the previous one succeeded
+test_expect_success '@{u} works when tracking a local branch' '
+	test refs/heads/master = "$(full_name @{u})"
+'
+
 cat >expect <<EOF
 commit 8f489d01d0cc65c3b0f09504ec50b5ed02a70bd5
 Reflog: master@{0} (C O Mitter <committer@example.com>)
-- 
1.7.10.226.gfe575

[PATCH v2 2/5] Provide branch name in error message when using @{u}

[Zbigniew Jędrzejewski-Szmek]

When using @{u} or @{upstream} it is common to omit the branch name,
implying current branch. If the upstream is not configured, the error
message was "No upstream branch found for ''".

When resolving '@{u}', branch_get() is called, which almost always
returns a description of a branch. This allows us to use a branch name
in the error message, even if the user said something like '@{u}'.

The only case when branch_get() returns NULL is when HEAD points to so
something which is not a branch. Of course this also means that no
upstream is configured, but it is better to directly say that HEAD
does not point to a branch.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
I kept the branch name in the error message instead of saying 'current branch'.

 sha1_name.c                   |   12 ++++++++----
 t/t1507-rev-parse-upstream.sh |    4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 03ffc2c..c2fe1aa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -856,10 +856,14 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	len = cp + tmp_len - name;
 	cp = xstrndup(name, cp - name);
 	upstream = branch_get(*cp ? cp : NULL);
-	if (!upstream
-	    || !upstream->merge
-	    || !upstream->merge[0]->dst)
-		return error("No upstream branch found for '%s'", cp);
+	/*
+	 * Upstream can be NULL only if cp refers to HEAD and HEAD
+	 * points to something different than a branch.
+	 */
+	if (!upstream)
+		return error("HEAD does not point to a branch");
+	if (!upstream->merge || !upstream->merge[0]->dst)
+		return error("No upstream branch found for '%s'", upstream->name);
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
 	strbuf_reset(buf);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index c4981ba..2f4f0d1 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -134,7 +134,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
 
 test_expect_success '@{u} error message when no upstream' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}${sq}
+	error: No upstream branch found for ${sq}master${sq}
 	fatal: Needed a single revision
 	EOF
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
@@ -152,7 +152,7 @@ test_expect_success 'branch@{u} error message with misspelt branch' '
 
 test_expect_success '@{u} error message when not on a branch' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}${sq}
+	error: HEAD does not point to a branch
 	fatal: Needed a single revision
 	EOF
 	git checkout HEAD^0 &&
-- 
1.7.10.226.gfe575

[PATCH v2 3/5] Provide better message for barnhc_wiht_tpyo@{u}

[Zbigniew Jędrzejewski-Szmek]

Instead of just saying that no upstream exists for such branch,
which is true but not very helpful, check that there's no
refs/heads/barnhc_wiht_tpyo and tell it to the user.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   |    5 ++++-
 t/t1507-rev-parse-upstream.sh |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index c2fe1aa..e2d576a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -862,8 +862,11 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	 */
 	if (!upstream)
 		return error("HEAD does not point to a branch");
-	if (!upstream->merge || !upstream->merge[0]->dst)
+	if (!upstream->merge || !upstream->merge[0]->dst) {
+		if (!ref_exists(upstream->refname))
+			return error("No such branch: '%s'", cp);
 		return error("No upstream branch found for '%s'", upstream->name);
+	}
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
 	strbuf_reset(buf);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2f4f0d1..2b8ba31 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -143,7 +143,7 @@ test_expect_success '@{u} error message when no upstream' '
 
 test_expect_success 'branch@{u} error message with misspelt branch' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}no-such-branch${sq}
+	error: No such branch: ${sq}no-such-branch${sq}
 	fatal: Needed a single revision
 	EOF
 	error_message no-such-branch@{u} 2>actual &&
-- 
1.7.10.226.gfe575

[PATCH v2 4/5] Be more specific if upstream branch is not tracked

[Zbigniew Jędrzejewski-Szmek]

If the branch configured as upstream didn't have a local tracking
branch, git said "Upstream branch not found". We can be more helpful,
and separate the cases when upstream is not configured, and when it is
configured, but the upstream branch is not tracked in a local branch.

The following configuration leads to the second scenario:

    [remote "origin"]
    	    url = ...
            fetch = refs/heads/master
    [branch "master"]
            remote = origin
            merge = refs/heads/master

'git pull' will work on master, but master@{upstream} is not defined.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   |    7 ++++++-
 t/t1507-rev-parse-upstream.sh |    6 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index e2d576a..361708b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -865,7 +865,12 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	if (!upstream->merge || !upstream->merge[0]->dst) {
 		if (!ref_exists(upstream->refname))
 			return error("No such branch: '%s'", cp);
-		return error("No upstream branch found for '%s'", upstream->name);
+		if (!upstream->merge)
+			return error("No upstream configured for branch '%s'",
+				     upstream->name);
+		return error(
+			"Upstream branch '%s' not stored as a remote-tracking branch",
+			upstream->merge[0]->src);
 	}
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2b8ba31..e742ce0 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -125,7 +125,7 @@ test_expect_success 'branch@{u} works when tracking a local branch' '
 
 test_expect_success 'branch@{u} error message when no upstream' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}non-tracking${sq}
+	error: No upstream configured for branch ${sq}non-tracking${sq}
 	fatal: Needed a single revision
 	EOF
 	error_message non-tracking@{u} 2>actual &&
@@ -134,7 +134,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
 
 test_expect_success '@{u} error message when no upstream' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}master${sq}
+	error: No upstream configured for branch ${sq}master${sq}
 	fatal: Needed a single revision
 	EOF
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
@@ -162,7 +162,7 @@ test_expect_success '@{u} error message when not on a branch' '
 
 test_expect_success 'branch@{u} error message if upstream branch not fetched' '
 	cat >expect <<-EOF &&
-	error: No upstream branch found for ${sq}bad-upstream${sq}
+	error: Upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch
 	fatal: Needed a single revision
 	EOF
 	error_message bad-upstream@{u} 2>actual &&
-- 
1.7.10.226.gfe575

[PATCH v2 5/5] i18n: mark @{upstream} error messages for translation

[Zbigniew Jędrzejewski-Szmek]

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   |    8 ++++----
 t/t1507-rev-parse-upstream.sh |   10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 361708b..c633113 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -861,15 +861,15 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	 * points to something different than a branch.
 	 */
 	if (!upstream)
-		return error("HEAD does not point to a branch");
+		return error(_("HEAD does not point to a branch"));
 	if (!upstream->merge || !upstream->merge[0]->dst) {
 		if (!ref_exists(upstream->refname))
-			return error("No such branch: '%s'", cp);
+			return error(_("No such branch: '%s'"), cp);
 		if (!upstream->merge)
-			return error("No upstream configured for branch '%s'",
+			return error(_("No upstream configured for branch '%s'"),
 				     upstream->name);
 		return error(
-			"Upstream branch '%s' not stored as a remote-tracking branch",
+			_("Upstream branch '%s' not stored as a remote-tracking branch"),
 			upstream->merge[0]->src);
 	}
 	free(cp);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index e742ce0..d6e5761 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -129,7 +129,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
 	fatal: Needed a single revision
 	EOF
 	error_message non-tracking@{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success '@{u} error message when no upstream' '
@@ -138,7 +138,7 @@ test_expect_success '@{u} error message when no upstream' '
 	fatal: Needed a single revision
 	EOF
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success 'branch@{u} error message with misspelt branch' '
@@ -147,7 +147,7 @@ test_expect_success 'branch@{u} error message with misspelt branch' '
 	fatal: Needed a single revision
 	EOF
 	error_message no-such-branch@{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success '@{u} error message when not on a branch' '
@@ -157,7 +157,7 @@ test_expect_success '@{u} error message when not on a branch' '
 	EOF
 	git checkout HEAD^0 &&
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success 'branch@{u} error message if upstream branch not fetched' '
@@ -166,7 +166,7 @@ test_expect_success 'branch@{u} error message if upstream branch not fetched' '
 	fatal: Needed a single revision
 	EOF
 	error_message bad-upstream@{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success 'pull works when tracking a local branch' '
-- 
1.7.10.226.gfe575

Re: [PATCH v2 0/5] provide better error messages for @{upstream}

[Jeff King]

On Sat, Apr 14, 2012 at 09:54:30AM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> v2:
> 
> - Better commit descriptions (hopefully)
> - error_message used in tests in one more place
> - Error message changed from
>   "Upstream branch '%s' not fetched from remote '%s'"
>   to
>   "Upstream branch '%s' not stored as a remote-tracking branch".

Thanks, that addresses my complaint with the previous round.

-Peff

[PATCH 5/5] i18n: mark @{upstream} error messages for translation

[Zbigniew Jędrzejewski-Szmek]

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
 sha1_name.c                   | 11 ++++++-----
 t/t1507-rev-parse-upstream.sh | 10 +++++-----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 5b1b0f9..bd1769e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -861,15 +861,16 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 	 * points to something different than a branch.
 	 */
 	if (!upstream)
-		return error("HEAD does not point to a branch");
+		return error(_("HEAD does not point to a branch"));
 	if (!upstream->merge || !upstream->merge[0]->dst) {
 		if (!ref_exists(upstream->refname))
-			return error("No such branch: '%s'", cp);
+			return error(_("No such branch: '%s'"), cp);
 		if (!upstream->merge)
-			return error("No upstream configured for branch '%s'",
+			return error(_("No upstream configured for branch '%s'"),
 				     upstream->name);
-		return error("Upstream branch '%s' not fetched from remote '%s'",
-			     upstream->merge[0]->src, upstream->remote_name);
+		return error(
+			_("Upstream branch '%s' not fetched from remote '%s'"),
+			upstream->merge[0]->src, upstream->remote_name);
 	}
 	free(cp);
 	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 112025f..a827f98 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -130,7 +130,7 @@ test_expect_success 'branch@{u} error message when no upstream' '
 	EOF
 	(cd clone &&
 	 test_must_fail git rev-parse --verify non-tracking@{u}) 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success '@{u} error message when no upstream' '
@@ -139,7 +139,7 @@ test_expect_success '@{u} error message when no upstream' '
 	fatal: Needed a single revision
 	EOF
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success 'branch@{u} error message with misspelt branch' '
@@ -148,7 +148,7 @@ test_expect_success 'branch@{u} error message with misspelt branch' '
 	fatal: Needed a single revision
 	EOF
 	error_message no-such-branch@{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success '@{u} error message when not on a branch' '
@@ -158,7 +158,7 @@ test_expect_success '@{u} error message when not on a branch' '
 	EOF
 	git checkout HEAD^0 &&
 	test_must_fail git rev-parse --verify @{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success 'branch@{u} error message if upstream branch not fetched' '
@@ -167,7 +167,7 @@ test_expect_success 'branch@{u} error message if upstream branch not fetched' '
 	fatal: Needed a single revision
 	EOF
 	error_message bad-upstream@{u} 2>actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 '
 
 test_expect_success 'pull works when tracking a local branch' '
-- 
1.7.10.344.g387ed