{"thread":{"id":"33915","subject":"[PATCH v3 0/2] Replacement for rr/die-on-missing-upstream","startedAt":"2013-05-24T07:42:02Z","lastAt":"2013-05-24T17:50:50Z","messageCount":7,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"218345","messageId":"1369381324-10206-1-git-send-email-artagnon@gmail.com","threadId":"33915","inReplyTo":null,"subject":"[PATCH v3 0/2] Replacement for rr/die-on-missing-upstream","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-24T07:42:02Z","receivedAt":"2013-05-24T07:42:02Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"[1/2] clarifies the commit message to say that a grep didn't find\n\"@{u}\" or \"@{upstream}\" hard-coded by any callers.  Thanks to Junio.\n\n[2/2] fixes a small grammar error in the commit message.  Thanks to\nEric Sunshine.\n\nRamkumar Ramachandra (2):\n  sha1_name: fix error message for @{u}\n  sha1_name: fix error message for @{<N>}, @{<date>}\n\n sha1_name.c                   | 21 +++++++++++++++------\n t/t1507-rev-parse-upstream.sh | 15 +++++----------\n 2 files changed, 20 insertions(+), 16 deletions(-)\n\n-- \n1.8.3.rc3.17.gd95ec6c.dirty\n"},{"id":"218346","messageId":"1369381324-10206-2-git-send-email-artagnon@gmail.com","threadId":"33915","inReplyTo":"1369381324-10206-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/2] sha1_name: fix error message for @{u}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-24T07:42:03Z","receivedAt":"2013-05-24T07:42:03Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, when no (valid) upstream is configured for a branch, you get\nan error like:\n\n  $ git show @{u}\n  error: No upstream configured for branch 'upstream-error'\n  error: No upstream configured for branch 'upstream-error'\n  fatal: ambiguous argument '@{u}': unknown revision or path not in the working tree.\n  Use '--' to separate paths from revisions, like this:\n  'git <command> [<revision>...] -- [<file>...]'\n\nThe \"error: \" line actually appears twice, and the rest of the error\nmessage is useless.  In sha1_name.c:interpret_branch_name(), there is\nreally no point in processing further if @{u} couldn't be resolved, and\nwe might as well die() instead of returning an error().  After making\nthis change, you get:\n\n  $ git show @{u}\n  fatal: No upstream configured for branch 'upstream-error'\n\nAlso tweak a few tests in t1507 to expect this output.\n\nTo justify that this change is safe, consider that all callers of\ninterpret_branch_name() have to fall in two categories:\n\n1. Direct end-user facing applications like [rev-parse, show] calling in\n   with end-user data (in which case the data can contain \"@{u}\").\n   Failing immediately is the right thing to do: the only difference is\n   that the die() happens in interpret_branch_name() instead of\n   die_verify_filename(), and this is desirable.\n\n2. Callers calling in with programmatic data, and expecting the function\n   to return and not die().  In this case, why would anyone ever\n   construct a string containing \"@{u}\" programmatically in the first\n   place?  A grep reveals that no part of the code hard-codes either\n   \"@{u}\" or \"@{upstream}\".  So, these callers will never hit the\n   codepath touched by the patch.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n sha1_name.c                   | 11 +++++------\n t/t1507-rev-parse-upstream.sh | 15 +++++----------\n 2 files changed, 10 insertions(+), 16 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 3820f28..61f5a34 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1033,15 +1033,14 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n \t * points to something different than a branch.\n \t */\n \tif (!upstream)\n-\t\treturn error(_(\"HEAD does not point to a branch\"));\n+\t\tdie(_(\"HEAD does not point to a branch\"));\n \tif (!upstream->merge || !upstream->merge[0]->dst) {\n \t\tif (!ref_exists(upstream->refname))\n-\t\t\treturn error(_(\"No such branch: '%s'\"), cp);\n+\t\t\tdie(_(\"No such branch: '%s'\"), cp);\n \t\tif (!upstream->merge)\n-\t\t\treturn error(_(\"No upstream configured for branch '%s'\"),\n-\t\t\t\t     upstream->name);\n-\t\treturn error(\n-\t\t\t_(\"Upstream branch '%s' not stored as a remote-tracking branch\"),\n+\t\t\tdie(_(\"No upstream configured for branch '%s'\"),\n+\t\t\t\tupstream->name);\n+\t\tdie(_(\"Upstream branch '%s' not stored as a remote-tracking branch\"),\n \t\t\tupstream->merge[0]->src);\n \t}\n \tfree(cp);\ndiff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh\nindex b27a720..2a19e79 100755\n--- a/t/t1507-rev-parse-upstream.sh\n+++ b/t/t1507-rev-parse-upstream.sh\n@@ -129,8 +129,7 @@ test_expect_success 'branch@{u} works when tracking a local branch' '\n \n test_expect_success 'branch@{u} error message when no upstream' '\n \tcat >expect <<-EOF &&\n-\terror: No upstream configured for branch ${sq}non-tracking${sq}\n-\tfatal: Needed a single revision\n+\tfatal: No upstream configured for branch ${sq}non-tracking${sq}\n \tEOF\n \terror_message non-tracking@{u} 2>actual &&\n \ttest_i18ncmp expect actual\n@@ -138,8 +137,7 @@ test_expect_success 'branch@{u} error message when no upstream' '\n \n test_expect_success '@{u} error message when no upstream' '\n \tcat >expect <<-EOF &&\n-\terror: No upstream configured for branch ${sq}master${sq}\n-\tfatal: Needed a single revision\n+\tfatal: No upstream configured for branch ${sq}master${sq}\n \tEOF\n \ttest_must_fail git rev-parse --verify @{u} 2>actual &&\n \ttest_i18ncmp expect actual\n@@ -147,8 +145,7 @@ test_expect_success '@{u} error message when no upstream' '\n \n test_expect_success 'branch@{u} error message with misspelt branch' '\n \tcat >expect <<-EOF &&\n-\terror: No such branch: ${sq}no-such-branch${sq}\n-\tfatal: Needed a single revision\n+\tfatal: No such branch: ${sq}no-such-branch${sq}\n \tEOF\n \terror_message no-such-branch@{u} 2>actual &&\n \ttest_i18ncmp expect actual\n@@ -156,8 +153,7 @@ test_expect_success 'branch@{u} error message with misspelt branch' '\n \n test_expect_success '@{u} error message when not on a branch' '\n \tcat >expect <<-EOF &&\n-\terror: HEAD does not point to a branch\n-\tfatal: Needed a single revision\n+\tfatal: HEAD does not point to a branch\n \tEOF\n \tgit checkout HEAD^0 &&\n \ttest_must_fail git rev-parse --verify @{u} 2>actual &&\n@@ -166,8 +162,7 @@ test_expect_success '@{u} error message when not on a branch' '\n \n test_expect_success 'branch@{u} error message if upstream branch not fetched' '\n \tcat >expect <<-EOF &&\n-\terror: Upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch\n-\tfatal: Needed a single revision\n+\tfatal: Upstream branch ${sq}refs/heads/side${sq} not stored as a remote-tracking branch\n \tEOF\n \terror_message bad-upstream@{u} 2>actual &&\n \ttest_i18ncmp expect actual\n-- \n1.8.3.rc3.17.gd95ec6c.dirty\n"},{"id":"218347","messageId":"1369381324-10206-3-git-send-email-artagnon@gmail.com","threadId":"33915","inReplyTo":"1369381324-10206-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/2] sha1_name: fix error message for @{<N>}, @{<date>}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-24T07:42:04Z","receivedAt":"2013-05-24T07:42:04Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, when we try to resolve @{<N>} or @{<date>} when the reflog\ndoesn't go back far enough, we get errors like:\n\n  # on branch master\n  $ git show @{10000}\n  fatal: Log for '' only has 7 entries.\n\n  $ git show @{10000.days.ago}\n  warning: Log for '' only goes back to Tue, 21 May 2013 14:14:45 +0530.\n  ...\n\n  # detached HEAD case\n  $ git show @{10000}\n  fatal: Log for '' only has 2005 entries.\n\n  $ git show master@{10000}\n  fatal: Log for 'master' only has 7 entries.\n\nThe empty string '' is ugly, inconsistent, and fails to convey\ninformation about whose logs we are inspecting.  Change this so that we\nget:\n\n  # on branch master\n  $ git show @{10000}\n  fatal: Log for 'master' only has 7 entries.\n\n  $ git show @{10000.days.ago}\n  warning: Log for 'master' only goes back to Tue, 21 May 2013 14:14:45 +0530.\n  ...\n\n  # detached HEAD case\n  $ git show @{10000}\n  fatal: Log for 'HEAD' only has 2005 entries.\n\n  $ git show master@{10000}\n  fatal: Log for 'master' only has 7 entries.\n\nSimple, consistent, and informative; suitable for output even from\nplumbing commands like rev-parse.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n sha1_name.c | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 61f5a34..6928cc7 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -517,6 +517,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \t\t}\n \t\tif (read_ref_at(real_ref, at_time, nth, sha1, NULL,\n \t\t\t\t&co_time, &co_tz, &co_cnt)) {\n+\t\t\tif (!len) {\n+\t\t\t\tif (!prefixcmp(real_ref, \"refs/heads/\")) {\n+\t\t\t\t\tstr = real_ref + 11;\n+\t\t\t\t\tlen = strlen(real_ref + 11);\n+\t\t\t\t} else {\n+\t\t\t\t\t/* detached HEAD */\n+\t\t\t\t\tstr = \"HEAD\";\n+\t\t\t\t\tlen = 4;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tif (at_time)\n \t\t\t\twarning(\"Log for '%.*s' only goes \"\n \t\t\t\t\t\"back to %s.\", len, str,\n-- \n1.8.3.rc3.17.gd95ec6c.dirty\n"},{"id":"218395","messageId":"7vd2sgb9r8.fsf@alter.siamese.dyndns.org","threadId":"33915","inReplyTo":"1369381324-10206-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/2] sha1_name: fix error message for @{<N>}, @{<date>}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-24T16:52:43Z","receivedAt":"2013-05-24T16:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Currently, when we try to resolve @{<N>} or @{<date>} when the reflog\n> doesn't go back far enough, we get errors like:\n>\n>   # on branch master\n>   $ git show @{10000}\n>   fatal: Log for '' only has 7 entries.\n>\n>   $ git show @{10000.days.ago}\n>   warning: Log for '' only goes back to Tue, 21 May 2013 14:14:45 +0530.\n>   ...\n>\n>   # detached HEAD case\n>   $ git show @{10000}\n>   fatal: Log for '' only has 2005 entries.\n>\n>   $ git show master@{10000}\n>   fatal: Log for 'master' only has 7 entries.\n>\n> The empty string '' is ugly, inconsistent, and fails to convey\n> information about whose logs we are inspecting.  Change this so that we\n\nWhat is this meant to update?  I recall rewriting this part on\npurpose.\n\nEven though it appears unusual and invites confusion, it is very\nconsistent to say '' when the user asks for @... as we say 'master'\nwhen the user asks for master@...; \"inconsistent\" is a very bad\nlabel for it.\n"},{"id":"218398","messageId":"CALkWK0moMOBUZqqSe521qfa7q8xe0Z5sN10ZRQpPaTu8EuxuDw@mail.gmail.com","threadId":"33915","inReplyTo":"7vd2sgb9r8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] sha1_name: fix error message for @{<N>}, @{<date>}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-24T17:24:35Z","receivedAt":"2013-05-24T17:24:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> What is this meant to update?  I recall rewriting this part on\n> purpose.\n\nI was being verbose to show that I handle the detached HEAD case too,\nwhich I missed last time.\n\n> Even though it appears unusual and invites confusion, it is very\n> consistent to say '' when the user asks for @... as we say 'master'\n> when the user asks for master@...; \"inconsistent\" is a very bad\n> label for it.\n\nThe inconsistency refers to these two:\n\n>>   # on branch master\n>>   $ git show @{10000}\n>>   fatal: Log for '' only has 7 entries.\n\n>>   # detached HEAD case\n>>   $ git show @{10000}\n>>   fatal: Log for '' only has 2005 entries.\n\nIs the commit message unclear?\n"},{"id":"218401","messageId":"7vsj1c9spm.fsf@alter.siamese.dyndns.org","threadId":"33915","inReplyTo":"CALkWK0moMOBUZqqSe521qfa7q8xe0Z5sN10ZRQpPaTu8EuxuDw@mail.gmail.com","subject":"Re: [PATCH 2/2] sha1_name: fix error message for @{<N>}, @{<date>}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-24T17:46:13Z","receivedAt":"2013-05-24T17:46:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>> What is this meant to update?  I recall rewriting this part on\n>> purpose.\n>\n> I was being verbose to show that I handle the detached HEAD case too,\n> which I missed last time.\n>\n>> Even though it appears unusual and invites confusion, it is very\n>> consistent to say '' when the user asks for @... as we say 'master'\n>> when the user asks for master@...; \"inconsistent\" is a very bad\n>> label for it.\n>\n> The inconsistency refers to these two:\n>\n>>>   # on branch master\n>>>   $ git show @{10000}\n>>>   fatal: Log for '' only has 7 entries.\n>\n>>>   # detached HEAD case\n>>>   $ git show @{10000}\n>>>   fatal: Log for '' only has 2005 entries.\n>\n> Is the commit message unclear?\n\nVery unclear.  You said\n\n    The empty string '' is ugly, inconsistent, and fails to convey\n    information...\n\nI was reacting to \"inconsistent\" by pointing out that showing an\nempty string is very consistent.\n\nI was _not_ saying that \"'' must be kept because it is consistent\",\nof course; that is a consistency that is not useful.  Which means\ninconsistency that does not exist is a reason to rewrite it, nor the\nrewrite is to gain consistency.  It is to make the result more useful.\n\nThat is why I rewrote it like so:\n\n    The empty string '' is confusing and does not convey information\n    about whose logs we are inspecting.  Change this so that...\n"},{"id":"218402","messageId":"CALkWK0=YiOoZPBYY4M5PwHGD2r-cTHZi4BLQwbizXkHtpG80ww@mail.gmail.com","threadId":"33915","inReplyTo":"7vsj1c9spm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] sha1_name: fix error message for @{<N>}, @{<date>}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-24T17:50:50Z","receivedAt":"2013-05-24T17:50:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> That is why I rewrote it like so:\n>\n>     The empty string '' is confusing and does not convey information\n>     about whose logs we are inspecting.  Change this so that...\n\nAh, I didn't notice the rewrite in pu.  Thanks.\n"}]}