{"thread":{"id":"37592","subject":"[PATCH v2] refs: make rev-parse --quiet actually quiet","startedAt":"2014-09-19T03:45:37Z","lastAt":"2014-09-19T17:52:50Z","messageCount":2,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"249651","messageId":"1411098337-35406-1-git-send-email-davvid@gmail.com","threadId":"37592","inReplyTo":null,"subject":"[PATCH v2] refs: make rev-parse --quiet actually quiet","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-19T03:45:37Z","receivedAt":"2014-09-19T03:45:37Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"When a reflog is deleted, e.g. when \"git stash\" clears its stashes,\n\"git rev-parse --verify --quiet\" dies:\n\n\tfatal: Log for refs/stash is empty.\n\nThe reason is that the get_sha1() code path does not allow us\nto suppress this message.\n\nPass the flags bitfield through get_sha1_with_context() so that\nread_ref_at() can suppress the message.\n\nUse get_sha1_with_context1() instead of get_sha1() in rev-parse\nso that the --quiet flag is honored.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis is a replacement patch for \"refs: make rev-parse --quiet actually quiet\"\nfrom da/rev-parse-verify-quiet in pu.\n\nThe test message now mentions \"reflogs that do not go far back enough in time\"\nand the conditional above the warning() has been adjusted.\n\nThis also includes the warning patch squashed in.\nI can split them apart if desired, but this seems nice and coherent.\n\n builtin/rev-parse.c         |  5 ++++-\n builtin/show-branch.c       |  5 +++--\n refs.c                      | 10 +++++++---\n refs.h                      |  3 ++-\n sha1_name.c                 | 24 +++++++++++++++---------\n t/t1503-rev-parse-verify.sh | 27 +++++++++++++++++++++++++++\n 6 files changed, 58 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex c911b45..35d3c43 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -508,7 +508,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \tint has_dashdash = 0;\n \tint output_prefix = 0;\n \tunsigned char sha1[20];\n+\tunsigned int flags = 0;\n \tconst char *name = NULL;\n+\tstruct object_context unused;\n \n \tif (argc > 1 && !strcmp(\"--parseopt\", argv[1]))\n \t\treturn cmd_parseopt(argc - 1, argv + 1, prefix);\n@@ -596,6 +598,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--quiet\") || !strcmp(arg, \"-q\")) {\n \t\t\t\tquiet = 1;\n+\t\t\t\tflags |= GET_SHA1_QUIETLY;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--short\") ||\n@@ -818,7 +821,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tname++;\n \t\t\ttype = REVERSED;\n \t\t}\n-\t\tif (!get_sha1(name, sha1)) {\n+\t\tif (!get_sha1_with_context(name, flags, sha1, &unused)) {\n \t\t\tif (verify)\n \t\t\t\trevs_count++;\n \t\t\telse\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 298c95e..46498e1 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -723,6 +723,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\tchar nth_desc[256];\n \t\tchar *ref;\n \t\tint base = 0;\n+\t\tunsigned int flags = 0;\n \n \t\tif (ac == 0) {\n \t\t\tstatic const char *fake_av[2];\n@@ -749,7 +750,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\t/* Ah, that is a date spec... */\n \t\t\t\tunsigned long at;\n \t\t\t\tat = approxidate(reflog_base);\n-\t\t\t\tread_ref_at(ref, at, -1, sha1, NULL,\n+\t\t\t\tread_ref_at(ref, flags, at, -1, sha1, NULL,\n \t\t\t\t\t    NULL, NULL, &base);\n \t\t\t}\n \t\t}\n@@ -760,7 +761,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\tunsigned long timestamp;\n \t\t\tint tz;\n \n-\t\t\tif (read_ref_at(ref, 0, base+i, sha1, &logmsg,\n+\t\t\tif (read_ref_at(ref, flags, 0, base+i, sha1, &logmsg,\n \t\t\t\t\t&timestamp, &tz, NULL)) {\n \t\t\t\treflog = i;\n \t\t\t\tbreak;\ndiff --git a/refs.c b/refs.c\nindex 2ce5d69..9e405f9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3108,7 +3108,7 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,\n \treturn 1;\n }\n \n-int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n+int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time, int cnt,\n \t\tunsigned char *sha1, char **msg,\n \t\tunsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)\n {\n@@ -3126,8 +3126,12 @@ int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n \n \tfor_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb);\n \n-\tif (!cb.reccnt)\n-\t\tdie(\"Log for %s is empty.\", refname);\n+\tif (!cb.reccnt) {\n+\t\tif (flags & GET_SHA1_QUIETLY)\n+\t\t\texit(128);\n+\t\telse\n+\t\t\tdie(\"Log for %s is empty.\", refname);\n+\t}\n \tif (cb.found_it)\n \t\treturn 0;\n \ndiff --git a/refs.h b/refs.h\nindex 68c5770..0ca6059 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -206,7 +206,8 @@ extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, cons\n int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n /** Reads log for the value of ref during at_time. **/\n-extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n+extern int read_ref_at(const char *refname, unsigned int flags,\n+\t\t       unsigned long at_time, int cnt,\n \t\t       unsigned char *sha1, char **msg,\n \t\t       unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);\n \ndiff --git a/sha1_name.c b/sha1_name.c\nindex 7098b10..5b004f5 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -432,7 +432,8 @@ static inline int upstream_mark(const char *string, int len)\n static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);\n static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf);\n \n-static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n+static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n+\t\t\t  unsigned int flags)\n {\n \tstatic const char *warn_msg = \"refname '%.*s' is ambiguous.\";\n \tstatic const char *object_name_msg = N_(\n@@ -511,7 +512,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \tif (!refs_found)\n \t\treturn -1;\n \n-\tif (warn_ambiguous_refs &&\n+\tif (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) &&\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@@ -545,7 +546,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t}\n-\t\tif (read_ref_at(real_ref, at_time, nth, sha1, NULL,\n+\t\tif (read_ref_at(real_ref, flags, 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 (starts_with(real_ref, \"refs/heads/\")) {\n@@ -557,11 +558,16 @@ 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}\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}\n@@ -801,7 +807,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l\n \tif (!ret)\n \t\treturn 0;\n \n-\tret = get_sha1_basic(name, len, sha1);\n+\tret = get_sha1_basic(name, len, sha1, lookup_flags);\n \tif (!ret)\n \t\treturn 0;\n \ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex d1f93b3..823fe1d 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -83,6 +83,33 @@ test_expect_success 'fails silently when using -q' '\n \ttest_must_be_empty error\n '\n \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 \"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 'succeeds silently with -q and reflogs that do not go far back enough in time' '\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.247.g18aa141.dirty\n"},{"id":"249673","messageId":"xmqqwq8zmr9p.fsf@gitster.dls.corp.google.com","threadId":"37592","inReplyTo":"1411098337-35406-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v2] refs: make rev-parse --quiet actually quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-19T17:52:50Z","receivedAt":"2014-09-19T17:52:50Z","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> When a reflog is deleted, e.g. when \"git stash\" clears its stashes,\n> \"git rev-parse --verify --quiet\" dies:\n>\n> \tfatal: Log for refs/stash is empty.\n>\n> The reason is that the get_sha1() code path does not allow us\n> to suppress this message.\n>\n> Pass the flags bitfield through get_sha1_with_context() so that\n> read_ref_at() can suppress the message.\n>\n> Use get_sha1_with_context1() instead of get_sha1() in rev-parse\n> so that the --quiet flag is honored.\n>\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n> This is a replacement patch for \"refs: make rev-parse --quiet actually quiet\"\n> from da/rev-parse-verify-quiet in pu.\n\nLooks good; thanks.\n"}]}