{"thread":{"id":"37577","subject":"[PATCH v4 1/3] t1503: use test_must_be_empty","startedAt":"2014-09-16T03:24:08Z","lastAt":"2014-09-16T18:26:06Z","messageCount":4,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":3},"messages":[{"id":"249502","messageId":"1410837850-5169-1-git-send-email-davvid@gmail.com","threadId":"37577","inReplyTo":null,"subject":"[PATCH v4 1/3] t1503: use test_must_be_empty","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-16T03:24:08Z","receivedAt":"2014-09-16T03:24:08Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Use `test_must_be_be_empty <file>` instead of `test -z \"$(cat <file>)\"`.\n\nSuggested-by: Fabian Ruch <bafain@gmail.com>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis patch should probably be applied on top of the\nda/rev-parse-verify-quiet which is currently in pu\nand contains the rev-parse documentation patch.\n\nThis patch has been rebased to be 1/3.\n\n t/t1503-rev-parse-verify.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 813cc1b..d1f93b3 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -72,15 +72,15 @@ test_expect_success 'fails with any bad rev or many good revs' '\n \n test_expect_success 'fails silently when using -q' '\n \ttest_must_fail git rev-parse --verify --quiet 2>error &&\n-\ttest -z \"$(cat error)\" &&\n+\ttest_must_be_empty error &&\n \ttest_must_fail git rev-parse -q --verify foo 2>error &&\n-\ttest -z \"$(cat error)\" &&\n+\ttest_must_be_empty error &&\n \ttest_must_fail git rev-parse --verify -q HEAD bar 2>error &&\n-\ttest -z \"$(cat error)\" &&\n+\ttest_must_be_empty error &&\n \ttest_must_fail git rev-parse --quiet --verify baz HEAD 2>error &&\n-\ttest -z \"$(cat error)\" &&\n+\ttest_must_be_empty error &&\n \ttest_must_fail git rev-parse -q --verify $HASH2 HEAD 2>error &&\n-\ttest -z \"$(cat error)\"\n+\ttest_must_be_empty error\n '\n \n test_expect_success 'no stdout output on error' '\n-- \n2.1.0.30.g05c535b.dirty\n"},{"id":"249503","messageId":"1410837850-5169-2-git-send-email-davvid@gmail.com","threadId":"37577","inReplyTo":"1410837850-5169-1-git-send-email-davvid@gmail.com","subject":"[PATCH v4 2/3] refs: make rev-parse --quiet actually quiet","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-16T03:24:09Z","receivedAt":"2014-09-16T03:24:09Z","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 patch now has the t1503 test case squashed into it.\nIt was previously a separate patch, but it makes more sense\nfor them to go together.\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                 |  7 ++++---\n t/t1503-rev-parse-verify.sh |  9 +++++++++\n 6 files changed, 29 insertions(+), 10 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..447e339 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(1);\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..d292e31 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@@ -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@@ -801,7 +802,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..4fe9f0e 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -83,6 +83,15 @@ 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 \"reflog 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 'no stdout output on error' '\n \ttest -z \"$(git rev-parse --verify)\" &&\n \ttest -z \"$(git rev-parse --verify foo)\" &&\n-- \n2.1.0.30.g05c535b.dirty\n"},{"id":"249504","messageId":"1410837850-5169-3-git-send-email-davvid@gmail.com","threadId":"37577","inReplyTo":"1410837850-5169-2-git-send-email-davvid@gmail.com","subject":"[PATCH v4 3/3] stash: prefer --quiet over shell redirection of the standard error stream","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-16T03:24:10Z","receivedAt":"2014-09-16T03:24:10Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Use `git rev-parse --verify --quiet` instead of redirecting\nstderr to /dev/null.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis patch is unchanged from when it was first written,\nbut is now correct thanks to the preceding patch.\n\n git-stash.sh | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 9c1ba8e..7ece0f1 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -50,7 +50,7 @@ clear_stash () {\n \tthen\n \t\tdie \"$(gettext \"git stash clear with parameters is unimplemented\")\"\n \tfi\n-\tif current=$(git rev-parse --verify $ref_stash 2>/dev/null)\n+\tif current=$(git rev-parse --verify --quiet $ref_stash)\n \tthen\n \t\tgit update-ref -d $ref_stash $current\n \tfi\n@@ -292,7 +292,7 @@ save_stash () {\n }\n \n have_stash () {\n-\tgit rev-parse --verify $ref_stash >/dev/null 2>&1\n+\tgit rev-parse --verify --quiet $ref_stash >/dev/null\n }\n \n list_stash () {\n@@ -392,12 +392,12 @@ parse_flags_and_rev()\n \t\t;;\n \tesac\n \n-\tREV=$(git rev-parse --quiet --symbolic --verify \"$1\" 2>/dev/null) || {\n+\tREV=$(git rev-parse --symbolic --verify --quiet \"$1\") || {\n \t\treference=\"$1\"\n \t\tdie \"$(eval_gettext \"\\$reference is not valid reference\")\"\n \t}\n \n-\ti_commit=$(git rev-parse --quiet --verify \"$REV^2\" 2>/dev/null) &&\n+\ti_commit=$(git rev-parse --verify --quiet \"$REV^2\") &&\n \tset -- $(git rev-parse \"$REV\" \"$REV^1\" \"$REV:\" \"$REV^1:\" \"$REV^2:\" 2>/dev/null) &&\n \ts=$1 &&\n \tw_commit=$1 &&\n@@ -409,7 +409,7 @@ parse_flags_and_rev()\n \ttest \"$ref_stash\" = \"$(git rev-parse --symbolic-full-name \"${REV%@*}\")\" &&\n \tIS_STASH_REF=t\n \n-\tu_commit=$(git rev-parse --quiet --verify \"$REV^3\" 2>/dev/null) &&\n+\tu_commit=$(git rev-parse --verify --quiet \"$REV^3\") &&\n \tu_tree=$(git rev-parse \"$REV^3:\" 2>/dev/null)\n }\n \n@@ -531,7 +531,8 @@ drop_stash () {\n \t\tdie \"$(eval_gettext \"\\${REV}: Could not drop stash entry\")\"\n \n \t# clear_stash if we just dropped the last stash entry\n-\tgit rev-parse --verify \"$ref_stash@{0}\" >/dev/null 2>&1 || clear_stash\n+\tgit rev-parse --verify --quiet \"$ref_stash@{0}\" >/dev/null ||\n+\tclear_stash\n }\n \n apply_to_branch () {\n-- \n2.1.0.30.g05c535b.dirty\n"},{"id":"249512","messageId":"xmqqmw9z2zi9.fsf@gitster.dls.corp.google.com","threadId":"37577","inReplyTo":"1410837850-5169-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v4 2/3] refs: make rev-parse --quiet actually quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-16T18:26:06Z","receivedAt":"2014-09-16T18:26:06Z","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> This patch now has the t1503 test case squashed into it.\n> It was previously a separate patch, but it makes more sense\n> for them to go together.\n\nYes.\n\n> diff --git a/refs.c b/refs.c\n> index 2ce5d69..447e339 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(1);\n\nDo we want 1 or 128 just like die()?\n\n> +\t\telse\n> +\t\t\tdie(\"Log for %s is empty.\", refname);\n> +\t}\n\nGiven that I see this behaviour:\n\n    $ git rev-parse da/rev-parse-verify-quiet@{1}\n    2892dfeec3f98f7e65a2746d271471d2c3c4af57\n    $ git rev-parse da/rev-parse-verify-quiet@{10}\n    fatal: Log for 'da/rev-parse-verify-quiet' only has 4 entries\n\nand I do not see a change in this patch to touch \"only has %d\nentries\" (found in sha1_name.c), I have to suspect that this change\nis not sufficient to say \"rev-parse --quiet will now be quiet\".\n\n    $ git rev-parse --quiet --verify da/rev-parse-verify-quiet@{10.years.ago}\n\nmay want to return the oldest-known value as it has always done\nwithout giving a warning.\n\nThere probably are other cases that relate to reflogs, e.g.\n\n    $ git rev-parse --quiet --verify @{-99999}\n\nto ask the tip of the branch you were on 99999 \"git checkout\"s ago.\nI think the last one does not have to be in the same commit as this\nfix for \"empty\" and \"you do not have that many\" cases, though.\n\nThanks.\n"}]}