{"thread":{"id":"44926","subject":"[PATCH v2 4/4] show-ref: Inline show_one","startedAt":"2017-01-21T01:08:42Z","lastAt":"2017-01-22T22:55:27Z","messageCount":7,"participants":["Vladimir Panteleev","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"309878","messageId":"20170121010821.25046-5-git@thecybershadow.net","threadId":"44926","inReplyTo":"20170121010821.25046-1-git@thecybershadow.net","subject":"[PATCH v2 4/4] show-ref: Inline show_one","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-21T01:08:21Z","receivedAt":"2017-01-21T01:08:42Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"As the small function is now only called from a single place, there is\nno point in keeping it as a separate function any longer.\n\nSigned-off-by: Vladimir Panteleev \n---\n builtin/show-ref.c | 15 +++++----------\n 1 file changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 3cf344d47..ec44164d8 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -17,15 +17,6 @@ static int deref_tags, show_head, tags_only, heads_only, found_match, verify,\n static const char **pattern;\n 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-\tif (hash_only)\n-\t\tprintf(\"%s\\n\", hex);\n-\telse\n-\t\tprintf(\"%s %s\\n\", hex, refname);\n-}\n-\n static int show_ref(const char *refname, const struct object_id *oid,\n \t\t int flag, void *cbdata)\n {\n@@ -74,7 +65,11 @@ static int show_ref(const char *refname, const struct object_id *oid,\n \tif (quiet)\n \t\treturn 0;\n \n-\tshow_one(refname, oid);\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 0;\n-- \n2.11.0\n\n"},{"id":"309879","messageId":"20170121010821.25046-4-git@thecybershadow.net","threadId":"44926","inReplyTo":"20170121010821.25046-1-git@thecybershadow.net","subject":"[PATCH v2 3/4] show-ref: Optimize show_ref a bit","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-21T01:08:20Z","receivedAt":"2017-01-21T01:08:46Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"The inner `if (verify)' check was not being used before the preceding\ncommit, as show_ref was never being called from a code path where\nverify was non-zero.\n\nAdding a `!verify' check to the outer if skips an unnecessary string\ncomparison when verify is non-zero, and show_ref is already called\nwith a reference exactly matching pattern.\n\nSigned-off-by: Vladimir Panteleev \n---\n builtin/show-ref.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex bcdc1a95e..3cf344d47 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -43,7 +43,7 @@ static int show_ref(const char *refname, const struct object_id *oid,\n \t\tif (!match)\n \t\t\treturn 0;\n \t}\n-\tif (pattern) {\n+\tif (pattern && !verify) {\n \t\tint reflen = strlen(refname);\n \t\tconst char **p = pattern, *m;\n \t\twhile ((m = *p++) != NULL) {\n@@ -54,9 +54,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":"309880","messageId":"20170121010821.25046-2-git@thecybershadow.net","threadId":"44926","inReplyTo":"20170121010821.25046-1-git@thecybershadow.net","subject":"[PATCH v2 1/4] show-ref: Allow --head to work with --verify","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-21T01:08:18Z","receivedAt":"2017-01-21T01:08:50Z","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 ignored --head. Thus, \"git show-ref HEAD\" used with\n\"--verify\" (because the user is not interested in seeing\nrefs/remotes/origin/HEAD), and used with \"--head\" (because the user\ndoes not want HEAD to be filtered out), i.e. \"git show-ref --head\n--verify HEAD\", did not work as expected.\n\nInstead of insisting that the input begins with \"refs/\", allow \"HEAD\"\nwhen \"--head\" is given in the codepath that handles \"--verify\", so\nthat all valid full refnames including HEAD are passed to the same\noutput machinery.\n\nSigned-off-by: Vladimir Panteleev \n---\n builtin/show-ref.c | 3 ++-\n t/t1403-show-ref.sh | 14 ++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 6d4e66900..945a483e3 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -202,7 +202,8 @@ 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/\") ||\n+\t\t\t (show_head && !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..2fb5dc879 100755\n--- a/t/t1403-show-ref.sh\n+++ b/t/t1403-show-ref.sh\n@@ -164,4 +164,18 @@ 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 HEAD >actual &&\n+\ttest_cmp expect actual &&\n+\n+\t>expect &&\n+\n+\tgit show-ref --verify --head -q HEAD >actual &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_must_fail git show-ref --verify --head -q A >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"309881","messageId":"20170121010821.25046-3-git@thecybershadow.net","threadId":"44926","inReplyTo":"20170121010821.25046-1-git@thecybershadow.net","subject":"[PATCH v2 2/4] show-ref: Allow -d to work with --verify","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-21T01:08:19Z","receivedAt":"2017-01-21T01:08:54Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"Use the same output machinery when --verify is absent or present,\nwhich allows tag dereferencing (-d) to work with --verify. This is\nuseful when the user wishes to avoid the costly iteration of refs.\n\nSigned-off-by: Vladimir Panteleev \n---\n builtin/show-ref.c | 3 +--\n t/t1403-show-ref.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 945a483e3..bcdc1a95e 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -205,8 +205,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)\n \t\t\tif ((starts_with(*pattern, \"refs/\") ||\n \t\t\t (show_head && !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_ref(*pattern, &oid, 0, NULL);\n \t\t\t}\n \t\t\telse if (!quiet)\n \t\t\t\tdie(\"'%s' - not a valid ref\", *pattern);\ndiff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh\nindex 2fb5dc879..5c540e67f 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":"309882","messageId":"20170121010821.25046-1-git@thecybershadow.net","threadId":"44926","inReplyTo":null,"subject":"[PATCH v2] show-ref: Allow -d, --head to work with --verify","fromName":"Vladimir Panteleev","fromEmail":"git@thecybershadow.net","sentAt":"2017-01-21T01:08:17Z","receivedAt":"2017-01-21T01:08:55Z","isPatch":true,"sender":{"key":"git@thecybershadow.net","avatar":"https://avatars.githubusercontent.com/u/160894?v=4"},"body":"This is the second take for \"[PATCH] show-ref: Allow --head to work\nwith --verify\". Thanks to Junio for his extensive feedback.\n\n"},{"id":"309950","messageId":"xmqqa8aivhik.fsf@gitster.mtv.corp.google.com","threadId":"44926","inReplyTo":"20170121010821.25046-4-git@thecybershadow.net","subject":"Re: [PATCH v2 3/4] show-ref: Optimize show_ref a bit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-22T22:47:47Z","receivedAt":"2017-01-22T22:47:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vladimir Panteleev writes:\n\n> The inner `if (verify)' check was not being used before the preceding\n> commit, as show_ref was never being called from a code path where\n> verify was non-zero.\n>\n> Adding a `!verify' check to the outer if skips an unnecessary string\n> comparison when verify is non-zero, and show_ref is already called\n> with a reference exactly matching pattern.\n>\n> Signed-off-by: Vladimir Panteleev \n> ---\n> builtin/show-ref.c | 5 +----\n> 1 file changed, 1 insertion(+), 4 deletions(-)\n>\n> diff --git a/builtin/show-ref.c b/builtin/show-ref.c\n> index bcdc1a95e..3cf344d47 100644\n> --- a/builtin/show-ref.c\n> +++ b/builtin/show-ref.c\n> @@ -43,7 +43,7 @@ static int show_ref(const char *refname, const struct object_id *oid,\n> \t\tif (!match)\n> \t\t\treturn 0;\n> \t}\n> -\tif (pattern) {\n> +\tif (pattern && !verify) {\n> \t\tint reflen = strlen(refname);\n> \t\tconst char **p = pattern, *m;\n> \t\twhile ((m = *p++) != NULL) {\n> @@ -54,9 +54,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\nHaving to do this change probably is an indication that the division\nof labour between show_ref() and show_one() up to this step needs to\nbe rethought.\n\nConceptually, \"git show-ref\" works in two ways:\n\n * When in --verify mode, the end user gives which refnames to\n consider showing.\n\n * Otherwise the end user gives pattern and the command infers which\n refnames to consider showing using the pattern.\n\nAnd for the refnames that are considered for showing, we may do\nvarious things, like -d to deref and --quiet to be silent. We want\nthis actual \"output\" step to be the same between two modes of\noperation.\n\nSo a better division of labour would be:\n\n * Make show_ref() about \"using pattern, enumerate what refs to\n show\" and call show_one().\n\n * Update show_one() and teach _it_ to handle quiet and deref_tags.\n\n * From cmd_show_ref(), in --verify mode, make sure the ref exists\n and call show_one(), because we do not do the \"using pattern,\n enumerate what refs to show\" at all.\n\nAnd from that point of view, I think 4/4 is going in the wrong\ndirection.\n\nI also think that \"git show-ref --verify HEAD\" should work; it is OK\nthat the command accepts \"--head\" in that case, but when in --verify\nmode, the end user gives which refnames to consider showing, and\nHEAD given from the command line is a signal enough that that\npsuedo-ref is what the end user wants to be shown. \"--head\" is\nabout not filtering in \"enumerate the ones that match the given\npatterns\" mode, and it feels unnecessary to require it in \"--verify\"\nmode.\n\nThanks.\n"},{"id":"309951","messageId":"xmqq60l6vh61.fsf@gitster.mtv.corp.google.com","threadId":"44926","inReplyTo":"xmqqa8aivhik.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 3/4] show-ref: Optimize show_ref a bit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-22T22:55:18Z","receivedAt":"2017-01-22T22:55:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Having to do this change probably is an indication that the division\n> of labour between show_ref() and show_one() up to this step needs to\n> be rethought.\n>\n> Conceptually, \"git show-ref\" works in two ways:\n>\n> * When in --verify mode, the end user gives which refnames to\n> consider showing.\n>\n> * Otherwise the end user gives pattern and the command infers which\n> refnames to consider showing using the pattern.\n>\n> And for the refnames that are considered for showing, we may do\n> various things, like -d to deref and --quiet to be silent. We want\n> this actual \"output\" step to be the same between two modes of\n> operation.\n\n... also \"error out if the named object did not exist\" can be part\nof this, which means ...\n\n> So a better division of labour would be:\n>\n> * Make show_ref() about \"using pattern, enumerate what refs to\n> show\" and call show_one().\n>\n> * Update show_one() and teach _it_ to handle quiet and deref_tags.\n\n... \"if (!has_sha1_file(oid->hash)) die()\" in show_ref() would\nprobably want to be part of this update.\n"}]}