{"thread":{"id":"44496","subject":"[PATCH] rev-parse: fix parent shorthands with --symbolic","startedAt":"2016-11-16T17:18:07Z","lastAt":"2016-11-16T17:18:07Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"306064","messageId":"20161116084624.6bfeiekwhdt7iscr@sigill.intra.peff.net","threadId":"44496","inReplyTo":null,"subject":"[PATCH] rev-parse: fix parent shorthands with --symbolic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-11-16T08:46:26Z","receivedAt":"2016-11-16T17:18:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The try_parent_shorthands() function shows each parent via\nshow_rev(). We pass the correct parent sha1, but our \"name\"\nparameter still points at the original refname. So asking\nfor a regular rev-parse works fine (it prints the sha1s),\nbut asking for the symbolic name gives nonsense like:\n\n    $ git rev-parse --symbolic HEAD^-1\n    HEAD\n    ^HEAD\n\nwhich is always an empty set of commits. Asking for \"^!\" is\nlikewise broken, with the added bonus that its prints ^HEAD\nfor _each_ parent. And \"^@\" just prints HEAD repeatedly.\n\nArguably it would be correct to just pass NULL as the name\nhere, and always get the parent expressed as a sha1. The\n\"--symbolic\" documentaton claims only \"as close to the\noriginal input as possible\", and we certainly fallback to\nsha1s where necessary. But it's pretty easy to generate a\nsymbolic name on the fly from the original.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n\nI noticed this because tig uses \"--symbolic\", and \"tig 1234abcd^-1\" does\nnot work without this patch.\n\nI thought at first it might be a regression in the upcoming v2.11 from\n8779351dd (revision: new rev^-n shorthand for rev^n..rev, 2016-09-27).\nBut nope, \"^!\" has been broken at least as far back as v1.6.6.3 (I\ndidn't check further).\n\nSo assuming it's too late in the -rc cycle for a non-regression fix,\nthis is probably post-v2.11 maint material.\n\n builtin/rev-parse.c          |  7 ++++++-\n t/t6101-rev-parse-parents.sh | 18 ++++++++++++++++++\n 2 files changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex cfb0f1510..ff13e59e1 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -342,11 +342,16 @@ static int try_parent_shorthands(const char *arg)\n \tfor (parents = commit->parents, parent_number = 1;\n \t     parents;\n \t     parents = parents->next, parent_number++) {\n+\t\tchar *name = NULL;\n+\n \t\tif (exclude_parent && parent_number != exclude_parent)\n \t\t\tcontinue;\n \n+\t\tif (symbolic)\n+\t\t\tname = xstrfmt(\"%s^%d\", arg, parent_number);\n \t\tshow_rev(include_parents ? NORMAL : REVERSED,\n-\t\t\t parents->item->object.oid.hash, arg);\n+\t\t\t parents->item->object.oid.hash, name);\n+\t\tfree(name);\n \t}\n \n \t*dotdot = '^';\ndiff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh\nindex 64a9850e3..8c617981a 100755\n--- a/t/t6101-rev-parse-parents.sh\n+++ b/t/t6101-rev-parse-parents.sh\n@@ -83,12 +83,24 @@ test_expect_success 'final^1^@ = final^1^1 final^1^2' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'symbolic final^1^@ = final^1^1 final^1^2' '\n+\tgit rev-parse --symbolic final^1^1 final^1^2 >expect &&\n+\tgit rev-parse --symbolic final^1^@ >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' '\n \tgit rev-parse final^1 ^final^1^1 ^final^1^2 >expect &&\n \tgit rev-parse final^1^! >actual &&\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'symbolic final^1^! = final^1 ^final^1^1 ^final^1^2' '\n+\tgit rev-parse --symbolic final^1 ^final^1^1 ^final^1^2 >expect &&\n+\tgit rev-parse --symbolic final^1^! >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'large graft octopus' '\n \ttest_cmp_rev_output b31 \"git rev-parse --verify b1^30\"\n '\n@@ -143,6 +155,12 @@ test_expect_success 'rev-parse merge^-2 = merge^2..merge' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'symbolic merge^-1 = merge^1..merge' '\n+\tgit rev-parse --symbolic merge^1..merge >expect &&\n+\tgit rev-parse --symbolic merge^-1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'rev-parse merge^-0 (invalid parent)' '\n \ttest_must_fail git rev-parse merge^-0\n '\n-- \n2.11.0.rc1.280.gf9bb6f9\n"}]}