git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 1/2] sha1_name: fix error message for @{u}
Date: Fri, 24 May 2013 13:12:03 +0530	[thread overview]
Message-ID: <1369381324-10206-2-git-send-email-artagnon@gmail.com> (raw)
In-Reply-To: <1369381324-10206-1-git-send-email-artagnon@gmail.com>

Currently, when no (valid) upstream is configured for a branch, you 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.

To justify that this change is safe, consider that all callers of
interpret_branch_name() have to fall in two categories:

1. Direct end-user facing applications like [rev-parse, show] calling in
   with end-user data (in which case the data can contain "@{u}").
   Failing immediately is the right thing to do: the only difference is
   that the die() happens in interpret_branch_name() instead of
   die_verify_filename(), and this is desirable.

2. Callers calling in with programmatic data, and expecting the function
   to return and not die().  In this case, why would anyone ever
   construct a string containing "@{u}" programmatically in the first
   place?  A grep reveals that no part of the code hard-codes either
   "@{u}" or "@{upstream}".  So, these callers will never hit the
   codepath touched by the patch.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c                   | 11 +++++------
 t/t1507-rev-parse-upstream.sh | 15 +++++----------
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..61f5a34 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1033,15 +1033,14 @@ 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);
+			die(_("No such branch: '%s'"), cp);
 		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"),
+			die(_("No upstream configured for branch '%s'"),
+				upstream->name);
+		die(_("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 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
-- 
1.8.3.rc3.17.gd95ec6c.dirty

  reply	other threads:[~2013-05-24  7:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-24  7:42 [PATCH v3 0/2] Replacement for rr/die-on-missing-upstream Ramkumar Ramachandra
2013-05-24  7:42 ` Ramkumar Ramachandra [this message]
2013-05-24  7:42 ` [PATCH 2/2] sha1_name: fix error message for @{<N>}, @{<date>} Ramkumar Ramachandra
2013-05-24 16:52   ` Junio C Hamano
2013-05-24 17:24     ` Ramkumar Ramachandra
2013-05-24 17:46       ` Junio C Hamano
2013-05-24 17:50         ` Ramkumar Ramachandra
  -- strict thread matches above, loose matches on Subject: below --
2013-05-22 10:39 [PATCH v2 0/2] Fix invalid revision error messages Ramkumar Ramachandra
2013-05-22 10:39 ` [PATCH 1/2] sha1_name: fix error message for @{u} Ramkumar Ramachandra
2013-05-22 17:35   ` Junio C Hamano
2013-05-23 11:03     ` Ramkumar Ramachandra
2013-05-21 10:41 [PATCH 0/2] Fix invalid revision error messages for 1.8.3 Ramkumar Ramachandra
2013-05-21 10:41 ` [PATCH 1/2] sha1_name: fix error message for @{u} Ramkumar Ramachandra
2013-05-21 16:42   ` Junio C Hamano
2013-05-21 17:56     ` Ramkumar Ramachandra
2013-05-21 18:02       ` Junio C Hamano
2013-05-21 18:04         ` Ramkumar Ramachandra
2013-05-21 18:09           ` Junio C Hamano
2013-05-21 19:19           ` Ramkumar Ramachandra
2013-05-21 20:08             ` Junio C Hamano
2013-05-21 20:14               ` Ramkumar Ramachandra
2013-05-21 20:33                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1369381324-10206-2-git-send-email-artagnon@gmail.com \
    --to=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).