{"thread":{"id":"44954","subject":"[PATCH v3 3/5] show-ref: Move --quiet handling into show_one","startedAt":"2017-01-23T18:01:12Z","lastAt":"2017-01-24T02:49:02Z","messageCount":8,"participants":["Vladimir Panteleev","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":5},"messages":[{"id":"309981","messageId":"20170123180059.4288-4-git@thecybershadow.net","threadId":"44954","inReplyTo":"20170123180059.4288-1-git@thecybershadow.net","subject":"[PATCH v3 3/5] show-ref: Move --quiet handling into show_one","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-23T18:00:57Z","receivedAt":"2017-01-23T18:01:12Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"Do the same with --quiet as was done with -d, to remove the need to\nperform this check at show_one's call site from the --verify branch.\n\nSigned-off-by: Vladimir Panteleev <git@thecybershadow.net>\n---\n builtin/show-ref.c | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex a72a626b1..ab8e0dc41 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -22,6 +22,9 @@ static void show_one(const char *refname, const struct object_id *oid)\n \tconst char *hex;\n \tstruct object_id peeled;\n \n+\tif (quiet)\n+\t\treturn;\n+\n \thex = find_unique_abbrev(oid->hash, abbrev);\n \tif (hash_only)\n \t\tprintf(\"%s\\n\", hex);\n@@ -82,9 +85,6 @@ static int show_ref(const char *refname, const struct object_id *oid,\n \t\tdie(\"git show-ref: bad ref %s (%s)\", refname,\n \t\t    oid_to_hex(oid));\n \n-\tif (quiet)\n-\t\treturn 0;\n-\n \tshow_one(refname, oid);\n \n \treturn 0;\n@@ -205,8 +205,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)\n \n \t\t\tif ((starts_with(*pattern, \"refs/\") || !strcmp(*pattern, \"HEAD\")) &&\n \t\t\t    !read_ref(*pattern, oid.hash)) {\n-\t\t\t\tif (!quiet)\n-\t\t\t\t\tshow_one(*pattern, &oid);\n+\t\t\t\tshow_one(*pattern, &oid);\n \t\t\t}\n \t\t\telse if (!quiet)\n \t\t\t\tdie(\"'%s' - not a valid ref\", *pattern);\n-- \n2.11.0\n\n"},{"id":"309982","messageId":"20170123180059.4288-6-git@thecybershadow.net","threadId":"44954","inReplyTo":"20170123180059.4288-1-git@thecybershadow.net","subject":"[PATCH v3 5/5] show-ref: Remove dead `if (verify)' check","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-23T18:00:59Z","receivedAt":"2017-01-23T18:01:15Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"As show_ref is only ever called on the path where --verify is not\nspecified, `verify' can never possibly be true here.\n\nSigned-off-by: Vladimir Panteleev <git@thecybershadow.net>\n---\n builtin/show-ref.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 107d05fe0..2dfcb5634 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -73,9 +73,6 @@ static int show_ref(const char *refname, const struct object_id *oid,\n \t\t\t\tcontinue;\n \t\t\tif (len == reflen)\n \t\t\t\tgoto match;\n-\t\t\t/* \"--verify\" requires an exact match */\n-\t\t\tif (verify)\n-\t\t\t\tcontinue;\n \t\t\tif (refname[reflen - len - 1] == '/')\n \t\t\t\tgoto match;\n \t\t}\n-- \n2.11.0\n\n"},{"id":"309983","messageId":"20170123180059.4288-2-git@thecybershadow.net","threadId":"44954","inReplyTo":"20170123180059.4288-1-git@thecybershadow.net","subject":"[PATCH v3 1/5] show-ref: Accept HEAD with --verify","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-23T18:00:55Z","receivedAt":"2017-01-23T18:01:19Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"Previously, when --verify was specified, show-ref would use a separate\ncode path which did not handle HEAD and treated it as an invalid\nref. Thus, \"git show-ref --verify HEAD\" (where \"--verify\" is used\nbecause the user is not interested in seeing refs/remotes/origin/HEAD)\ndid not work as expected.\n\nInstead of insisting that the input begins with \"refs/\", allow \"HEAD\"\nas well in the codepath that handles \"--verify\", so that all valid\nfull refnames including HEAD are passed to the same output machinery.\n\nSigned-off-by: Vladimir Panteleev <git@thecybershadow.net>\n---\n builtin/show-ref.c  |  2 +-\n t/t1403-show-ref.sh | 11 +++++++++++\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 6d4e66900..0e53e3da4 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -202,7 +202,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)\n \t\twhile (*pattern) {\n \t\t\tstruct object_id oid;\n \n-\t\t\tif (starts_with(*pattern, \"refs/\") &&\n+\t\t\tif ((starts_with(*pattern, \"refs/\") || !strcmp(*pattern, \"HEAD\")) &&\n \t\t\t    !read_ref(*pattern, oid.hash)) {\n \t\t\t\tif (!quiet)\n \t\t\t\t\tshow_one(*pattern, &oid);\ndiff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh\nindex 7e10bcfe3..5932beada 100755\n--- a/t/t1403-show-ref.sh\n+++ b/t/t1403-show-ref.sh\n@@ -164,4 +164,15 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'show-ref --verify HEAD' '\n+\techo $(git rev-parse HEAD) HEAD >expect &&\n+\tgit show-ref --verify HEAD >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t>expect &&\n+\n+\tgit show-ref --verify -q HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"309984","messageId":"20170123180059.4288-1-git@thecybershadow.net","threadId":"44954","inReplyTo":null,"subject":"[PATCH v3 0/5] show-ref: Allow -d, --head to work with --verify","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-23T18:00:54Z","receivedAt":"2017-01-23T18:01:22Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"Third iteration, according to Junio's comments. This time we keep\nshow_ref and show_one separate, accept HEAD with --verify even without\n--head, and add tests for dangling ref validation with --verify.\n\n"},{"id":"309985","messageId":"20170123180059.4288-3-git@thecybershadow.net","threadId":"44954","inReplyTo":"20170123180059.4288-1-git@thecybershadow.net","subject":"[PATCH v3 2/5] show-ref: Allow -d to work with --verify","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-23T18:00:56Z","receivedAt":"2017-01-23T18:01:23Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"Move handling of -d into show_one, so that it takes effect when\n--verify is present as well as when it is absent. This is useful when\nthe user wishes to avoid the costly iteration of refs.\n\nSigned-off-by: Vladimir Panteleev <git@thecybershadow.net>\n---\n builtin/show-ref.c  | 23 ++++++++++++-----------\n t/t1403-show-ref.sh |  9 +++++++++\n 2 files changed, 21 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 0e53e3da4..a72a626b1 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -19,19 +19,27 @@ static const char *exclude_existing_arg;\n \n static void show_one(const char *refname, const struct object_id *oid)\n {\n-\tconst char *hex = find_unique_abbrev(oid->hash, abbrev);\n+\tconst char *hex;\n+\tstruct object_id peeled;\n+\n+\thex = find_unique_abbrev(oid->hash, abbrev);\n \tif (hash_only)\n \t\tprintf(\"%s\\n\", hex);\n \telse\n \t\tprintf(\"%s %s\\n\", hex, refname);\n+\n+\tif (!deref_tags)\n+\t\treturn;\n+\n+\tif (!peel_ref(refname, peeled.hash)) {\n+\t\thex = find_unique_abbrev(peeled.hash, abbrev);\n+\t\tprintf(\"%s %s^{}\\n\", hex, refname);\n+\t}\n }\n \n static int show_ref(const char *refname, const struct object_id *oid,\n \t\t    int flag, void *cbdata)\n {\n-\tconst char *hex;\n-\tstruct object_id peeled;\n-\n \tif (show_head && !strcmp(refname, \"HEAD\"))\n \t\tgoto match;\n \n@@ -79,13 +87,6 @@ static int show_ref(const char *refname, const struct object_id *oid,\n \n \tshow_one(refname, oid);\n \n-\tif (!deref_tags)\n-\t\treturn 0;\n-\n-\tif (!peel_ref(refname, peeled.hash)) {\n-\t\thex = find_unique_abbrev(peeled.hash, abbrev);\n-\t\tprintf(\"%s %s^{}\\n\", hex, refname);\n-\t}\n \treturn 0;\n }\n \ndiff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh\nindex 5932beada..c6872bd96 100755\n--- a/t/t1403-show-ref.sh\n+++ b/t/t1403-show-ref.sh\n@@ -97,6 +97,9 @@ test_expect_success 'show-ref -d' '\n \tgit show-ref -d refs/tags/A refs/tags/C >actual &&\n \ttest_cmp expect actual &&\n \n+\tgit show-ref --verify -d refs/tags/A refs/tags/C >actual &&\n+\ttest_cmp expect actual &&\n+\n \techo $(git rev-parse refs/heads/master) refs/heads/master >expect &&\n \tgit show-ref -d master >actual &&\n \ttest_cmp expect actual &&\n@@ -116,6 +119,12 @@ test_expect_success 'show-ref -d' '\n \ttest_cmp expect actual &&\n \n \ttest_must_fail git show-ref -d --verify heads/master >actual &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_must_fail git show-ref --verify -d A C >actual &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_must_fail git show-ref --verify -d tags/A tags/C >actual &&\n \ttest_cmp expect actual\n \n '\n-- \n2.11.0\n\n"},{"id":"309986","messageId":"20170123180059.4288-5-git@thecybershadow.net","threadId":"44954","inReplyTo":"20170123180059.4288-1-git@thecybershadow.net","subject":"[PATCH v3 4/5] show-ref: Detect dangling refs under --verify as well","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-23T18:00:58Z","receivedAt":"2017-01-23T18:01:26Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"Move detection of dangling refs into show_one, so that they are\ndetected when --verify is present as well as when it is absent.\n\nSigned-off-by: Vladimir Panteleev <git@thecybershadow.net>\n---\n builtin/show-ref.c  | 16 ++++++++--------\n t/t1403-show-ref.sh | 22 ++++++++++++++++++++++\n 2 files changed, 30 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex ab8e0dc41..107d05fe0 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -22,6 +22,14 @@ static void show_one(const char *refname, const struct object_id *oid)\n \tconst char *hex;\n \tstruct object_id peeled;\n \n+\t/* This changes the semantics slightly that even under quiet we\n+\t * detect and return error if the repository is corrupt and\n+\t * ref points at a nonexistent object.\n+\t */\n+\tif (!has_sha1_file(oid->hash))\n+\t\tdie(\"git show-ref: bad ref %s (%s)\", refname,\n+\t\t    oid_to_hex(oid));\n+\n \tif (quiet)\n \t\treturn;\n \n@@ -77,14 +85,6 @@ static int show_ref(const char *refname, const struct object_id *oid,\n match:\n \tfound_match++;\n \n-\t/* This changes the semantics slightly that even under quiet we\n-\t * detect and return error if the repository is corrupt and\n-\t * ref points at a nonexistent object.\n-\t */\n-\tif (!has_sha1_file(oid->hash))\n-\t\tdie(\"git show-ref: bad ref %s (%s)\", refname,\n-\t\t    oid_to_hex(oid));\n-\n \tshow_one(refname, oid);\n \n \treturn 0;\ndiff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh\nindex c6872bd96..30354fd26 100755\n--- a/t/t1403-show-ref.sh\n+++ b/t/t1403-show-ref.sh\n@@ -184,4 +184,26 @@ test_expect_success 'show-ref --verify HEAD' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'show-ref --verify with dangling ref' '\n+\tsha1_file() {\n+\t\techo \"$*\" | sed \"s#..#.git/objects/&/#\"\n+\t} &&\n+\n+\tremove_object() {\n+\t\tfile=$(sha1_file \"$*\") &&\n+\t\ttest -e \"$file\" &&\n+\t\trm -f \"$file\"\n+\t} &&\n+\n+\ttest_when_finished \"rm -rf dangling\" &&\n+\t(\n+\t\tgit init dangling &&\n+\t\tcd dangling &&\n+\t\ttest_commit dangling &&\n+\t\tsha=$(git rev-parse refs/tags/dangling) &&\n+\t\tremove_object $sha &&\n+\t\ttest_must_fail git show-ref --verify refs/tags/dangling\n+\t)\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"310007","messageId":"xmqqk29lr1ar.fsf@gitster.mtv.corp.google.com","threadId":"44954","inReplyTo":"20170123180059.4288-1-git@thecybershadow.net","subject":"Re: [PATCH v3 0/5] show-ref: Allow -d, --head to work with --verify","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-23T20:03:56Z","receivedAt":"2017-01-23T20:04:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vladimir Panteleev <git@thecybershadow.net> writes:\n\n> Third iteration, according to Junio's comments. This time we keep\n> show_ref and show_one separate, accept HEAD with --verify even without\n> --head, and add tests for dangling ref validation with --verify.\n\nI am no longer a neutral judge but to me the resulting code looks a\nlot more naturally structured than the original.\n\nThanks.  Will queue.\n"},{"id":"310073","messageId":"xmqqefztp3zd.fsf@gitster.mtv.corp.google.com","threadId":"44954","inReplyTo":"20170123180059.4288-5-git@thecybershadow.net","subject":"Re: [PATCH v3 4/5] show-ref: Detect dangling refs under --verify as well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-24T02:48:54Z","receivedAt":"2017-01-24T02:49:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vladimir Panteleev <git@thecybershadow.net> writes:\n\n> Move detection of dangling refs into show_one, so that they are\n> detected when --verify is present as well as when it is absent.\n>\n> Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>\n> ---\n>  builtin/show-ref.c  | 16 ++++++++--------\n>  t/t1403-show-ref.sh | 22 ++++++++++++++++++++++\n>  2 files changed, 30 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/show-ref.c b/builtin/show-ref.c\n> index ab8e0dc41..107d05fe0 100644\n> --- a/builtin/show-ref.c\n> +++ b/builtin/show-ref.c\n> @@ -22,6 +22,14 @@ static void show_one(const char *refname, const struct object_id *oid)\n>  \tconst char *hex;\n>  \tstruct object_id peeled;\n>  \n> +\t/* This changes the semantics slightly that even under quiet we\n> +\t * detect and return error if the repository is corrupt and\n> +\t * ref points at a nonexistent object.\n> +\t */\n\nThis is my fault from more than 10 years ago, but I think the\ncomment shouldn't have been here (or at its original location).  It\ntalks about the behaviour change relative to the previous version\nwhen the comment was added, i.e. cf0adba788 (\"Store peeled refs in\npacked-refs file.\", 2006-11-19).\n\nI'll remove it after the series settles.\n"}]}