Re: [PATCH 1/2] sha1_name: fix error message for @{u}
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 21, 2013, 16:42 UTC
- Message-ID
- <7vtxlwp9mf.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1369132915-25657-2-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 20 quoted lines
> 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.
Show 87 quoted lines
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> 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