Re: [PATCH 1/2] sha1_name: fix error message for @{u}
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:23
Ramkumar Ramachandra [off-list ref] writes:
Currently, when no (valid) upstream is configured for a branch, we get
an error like:
$ git show @{u}
error: No upstream configured for branch 'upstream-error'
error: No upstream configured for branch 'upstream-error'
fatal: ambiguous argument '@{u}': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
The "error: " line actually appears twice, and the rest of the error
message is useless. In sha1_name.c:interpret_branch_name(), there is
really no point in processing further if @{u} couldn't be resolved, and
we might as well die() instead of returning an error(). After making
this change, you get:
$ git show @{u}
fatal: No upstream configured for branch 'upstream-error'
Also tweak a few tests in t1507 to expect this output.
Does a failure in interpret-branch-name that issue these error
messages always followed by die() in the caller? I know you looked
at the cases you noticed as an end-user (like the above "git show @{u}"
example), but if some codepaths did this:
if (interpret-branch-name()) {
you do not seem to have upstream defined,
so I will helpfully do something else that
you probably have meant.
}
this patch will break that codepath you did not look.
I do not offhand know if there is such a codepath, so if you did a
code audit and know this patch is regression-free, please say that
in the log message. "I ran all the tests and they passed" is not
good enough.
Other than that, the idea sounds OK.
quoted hunk
Signed-off-by: Ramkumar Ramachandra <redacted> --- sha1_name.c | 13 +++++++------ t/t1507-rev-parse-upstream.sh | 15 +++++---------- 2 files changed, 12 insertions(+), 16 deletions(-)diff --git a/sha1_name.c b/sha1_name.c index 3820f28..416a673 100644 --- a/sha1_name.c +++ b/sha1_name.c@@ -1033,14 +1033,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")); + die(_("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); - if (!upstream->merge) - return error(_("No upstream configured for branch '%s'"), - upstream->name); - return error( + die(_("No such branch: '%s'"), cp); + if (!upstream->merge) { + die(_("No upstream configured for branch '%s'"), + upstream->name); + } + die( _("Upstream branch '%s' not stored as a remote-tracking branch"), upstream->merge[0]->src); }diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh index b27a720..2a19e79 100755 --- a/t/t1507-rev-parse-upstream.sh +++ b/t/t1507-rev-parse-upstream.sh@@ -129,8 +129,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 configured for branch ${sq}non-tracking${sq} - fatal: Needed a single revision + fatal: No upstream configured for branch ${sq}non-tracking${sq} EOF error_message non-tracking@{u} 2>actual && test_i18ncmp expect actual@@ -138,8 +137,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 configured for branch ${sq}master${sq} - fatal: Needed a single revision + fatal: No upstream configured for branch ${sq}master${sq} EOF test_must_fail git rev-parse --verify @{u} 2>actual && test_i18ncmp expect actual@@ -147,8 +145,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 such branch: ${sq}no-such-branch${sq} - fatal: Needed a single revision + fatal: No such branch: ${sq}no-such-branch${sq} EOF error_message no-such-branch@{u} 2>actual && test_i18ncmp expect actual@@ -156,8 +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: HEAD does not point to a branch - fatal: Needed a single revision + fatal: HEAD does not point to a branch EOF git checkout HEAD^0 && test_must_fail git rev-parse --verify @{u} 2>actual &&@@ -166,8 +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: Upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch - fatal: Needed a single revision + fatal: Upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch EOF error_message bad-upstream@{u} 2>actual && test_i18ncmp expect actual