{"thread":{"id":"37581","subject":"[PATCH 1/2] fixup! refs: make rev-parse --quiet actually quiet","startedAt":"2014-09-17T04:35:29Z","lastAt":"2014-09-19T03:47:17Z","messageCount":5,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"249586","messageId":"1410928530-21595-1-git-send-email-davvid@gmail.com","threadId":"37581","inReplyTo":null,"subject":"[PATCH 1/2] fixup! refs: make rev-parse --quiet actually quiet","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-17T04:35:29Z","receivedAt":"2014-09-17T04:35:29Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"---\nThis is a fixup for da/rev-parse-verify-quiet in pu\nWe now exit(128) and handle the \"Log for XXX only has DDD entries\" case.\n\n refs.c                      |  2 +-\n sha1_name.c                 |  3 +++\n t/t1503-rev-parse-verify.sh | 10 +++++++++-\n 3 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 447e339..9e405f9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3128,7 +3128,7 @@ int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time,\n \n \tif (!cb.reccnt) {\n \t\tif (flags & GET_SHA1_QUIETLY)\n-\t\t\texit(1);\n+\t\t\texit(128);\n \t\telse\n \t\t\tdie(\"Log for %s is empty.\", refname);\n \t}\ndiff --git a/sha1_name.c b/sha1_name.c\nindex d292e31..be9a9de 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -563,6 +563,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n \t\t\t\t\t\"back to %s.\", len, str,\n \t\t\t\t\tshow_date(co_time, co_tz, DATE_RFC2822));\n \t\t\telse {\n+\t\t\t\tif (flags & GET_SHA1_QUIETLY) {\n+\t\t\t\t\texit(128);\n+\t\t\t\t}\n \t\t\t\tdie(\"Log for '%.*s' only has %d entries.\",\n \t\t\t\t    len, str, co_cnt);\n \t\t\t}\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 4fe9f0e..7c3d830 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -86,12 +86,20 @@ test_expect_success 'fails silently when using -q' '\n test_expect_success 'fails silently when using -q with deleted reflogs' '\n \tref=$(git rev-parse HEAD) &&\n \t: >.git/logs/refs/test &&\n-\tgit update-ref -m \"reflog message for refs/test\" refs/test \"$ref\" &&\n+\tgit update-ref -m \"message for refs/test\" refs/test \"$ref\" &&\n \tgit reflog delete --updateref --rewrite refs/test@{0} &&\n \ttest_must_fail git rev-parse -q --verify refs/test@{0} >error 2>&1 &&\n \ttest_must_be_empty error\n '\n \n+test_expect_success 'fails silently when using -q with not enough reflogs' '\n+\tref=$(git rev-parse HEAD) &&\n+\t: >.git/logs/refs/test2 &&\n+\tgit update-ref -m \"message for refs/test2\" refs/test2 \"$ref\" &&\n+\ttest_must_fail git rev-parse -q --verify refs/test2@{999} >error 2>&1 &&\n+\ttest_must_be_empty error\n+'\n+\n test_expect_success 'no stdout output on error' '\n \ttest -z \"$(git rev-parse --verify)\" &&\n \ttest -z \"$(git rev-parse --verify foo)\" &&\n-- \n2.1.0.248.g6401287.dirty\n"},{"id":"249587","messageId":"1410928530-21595-2-git-send-email-davvid@gmail.com","threadId":"37581","inReplyTo":"1410928530-21595-1-git-send-email-davvid@gmail.com","subject":"[PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-17T04:35:30Z","receivedAt":"2014-09-17T04:35:30Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Make rev-parse --verify --quiet ref@{1.year.ago} when the reflog\ndoes not go back that far succeed silently with --quiet.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n sha1_name.c                 | 19 ++++++++++++-------\n t/t1503-rev-parse-verify.sh | 10 ++++++++++\n 2 files changed, 22 insertions(+), 7 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex be9a9de..5836f94 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -514,8 +514,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n \n \tif (warn_ambiguous_refs &&\n \t    (refs_found > 1 ||\n-\t     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))\n-\t\twarning(warn_msg, len, str);\n+\t     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {\n+\t\tif (!(flags & GET_SHA1_QUIETLY)) {\n+\t\t\twarning(warn_msg, len, str);\n+\t\t}\n+\t}\n \n \tif (reflog_len) {\n \t\tint nth, i;\n@@ -558,11 +561,13 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\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-\t\t\t\t\tshow_date(co_time, co_tz, DATE_RFC2822));\n-\t\t\telse {\n+\t\t\tif (at_time) {\n+\t\t\t\tif (!(flags & GET_SHA1_QUIETLY)) {\n+\t\t\t\t\twarning(\"Log for '%.*s' only goes \"\n+\t\t\t\t\t\t\"back to %s.\", len, str,\n+\t\t\t\t\t\tshow_date(co_time, co_tz, DATE_RFC2822));\n+\t\t\t\t}\n+\t\t\t} else {\n \t\t\t\tif (flags & GET_SHA1_QUIETLY) {\n \t\t\t\t\texit(128);\n \t\t\t\t}\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 7c3d830..e7dece0 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -100,6 +100,16 @@ test_expect_success 'fails silently when using -q with not enough reflogs' '\n \ttest_must_be_empty error\n '\n \n+test_expect_success 'succeeds silently when using -q with invalid dates' '\n+\tref=$(git rev-parse HEAD) &&\n+\t: >.git/logs/refs/test3 &&\n+\tgit update-ref -m \"message for refs/test3\" refs/test3 \"$ref\" &&\n+\tgit rev-parse -q --verify refs/test3@{1.year.ago} >actual 2>error &&\n+\ttest_must_be_empty error &&\n+\techo \"$ref\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'no stdout output on error' '\n \ttest -z \"$(git rev-parse --verify)\" &&\n \ttest -z \"$(git rev-parse --verify foo)\" &&\n-- \n2.1.0.248.g6401287.dirty\n"},{"id":"249636","messageId":"xmqqr3z8q5dw.fsf@gitster.dls.corp.google.com","threadId":"37581","inReplyTo":"1410928530-21595-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-18T16:07:23Z","receivedAt":"2014-09-18T16:07:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> Subject: Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist\n> ...\n> Make rev-parse --verify --quiet ref@{1.year.ago} when the reflog\n> does not go back that far succeed silently with --quiet.\n> ...\n> +test_expect_success 'succeeds silently when using -q with invalid dates' '\n\nThese phrases may invite confusion and misunderstanding.  These\ndates on which you do not know where the tip of the ref was are not\nnecessarily \"do not exist\", \"does not go back that far\", or \"invalid\".\n\nYou may have pruned old entries for a long running branch.  It is\njust you no longer have records that go that far enough.\n\nWe would be able to tell two cases apart if we really wanted to by\nlooking at the oldest reflog entry.  If it records a creation of the\nref, then the ref did not exist back then and the reflog does not go\nback that far.  It may not be a bad idea to turn the current warning\nto an error in that case.  And if the oldest entry knows where the\nref used to point at, then we have expired older entries.  We can\ncontinue giving the \"oldest known\" as before with a warning.\n\nBut that would be a separate issue.\n\n> +\tref=$(git rev-parse HEAD) &&\n> +\t: >.git/logs/refs/test3 &&\n> +\tgit update-ref -m \"message for refs/test3\" refs/test3 \"$ref\" &&\n> +\tgit rev-parse -q --verify refs/test3@{1.year.ago} >actual 2>error &&\n> +\ttest_must_be_empty error &&\n> +\techo \"$ref\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_expect_success 'no stdout output on error' '\n>  \ttest -z \"$(git rev-parse --verify)\" &&\n>  \ttest -z \"$(git rev-parse --verify foo)\" &&\n"},{"id":"249637","messageId":"xmqqmw9wq54z.fsf@gitster.dls.corp.google.com","threadId":"37581","inReplyTo":"1410928530-21595-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-18T16:12:44Z","receivedAt":"2014-09-18T16:12:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> @@ -514,8 +514,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n>  \n>  \tif (warn_ambiguous_refs &&\n>  \t    (refs_found > 1 ||\n> -\t     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))\n> -\t\twarning(warn_msg, len, str);\n> +\t     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {\n> +\t\tif (!(flags & GET_SHA1_QUIETLY)) {\n> +\t\t\twarning(warn_msg, len, str);\n> +\t\t}\n> +\t}\n\nHmph, wouldn't it be simpler to read and understand if it were done\nthis way instead?\n\n-\tif (warn_ambiguous_refs &&\n+\tif (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) &&\n \t    (refs_found > 1 || !get_short_sha1(...)))\n\t\twaqrning(...);\n"},{"id":"249652","messageId":"20140919034715.GA26728@gmail.com","threadId":"37581","inReplyTo":"xmqqmw9wq54z.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] rev-parse: honor --quiet when asking for reflog dates that do not exist","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-19T03:47:17Z","receivedAt":"2014-09-19T03:47:17Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Sep 18, 2014 at 09:12:44AM -0700, Junio C Hamano wrote:\n> David Aguilar <davvid@gmail.com> writes:\n> \n> > @@ -514,8 +514,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n> >  \n> >  \tif (warn_ambiguous_refs &&\n> >  \t    (refs_found > 1 ||\n> > -\t     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))\n> > -\t\twarning(warn_msg, len, str);\n> > +\t     !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {\n> > +\t\tif (!(flags & GET_SHA1_QUIETLY)) {\n> > +\t\t\twarning(warn_msg, len, str);\n> > +\t\t}\n> > +\t}\n> \n> Hmph, wouldn't it be simpler to read and understand if it were done\n> this way instead?\n> \n> -\tif (warn_ambiguous_refs &&\n> +\tif (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) &&\n>  \t    (refs_found > 1 || !get_short_sha1(...)))\n> \t\twaqrning(...);\n> \n\nYes, it would.\n\nI squashed this patch into the original \"make rev-parse --quiet actually quiet\"\npatch, along with your suggestions, and sent it as a replacement\npatch for the commit in pu.\n-- \nDavid\n"}]}