{"thread":{"id":"44663","subject":"[PATCH] describe: add tests for unusual graphs","startedAt":"2016-12-09T13:21:53Z","lastAt":"2016-12-10T06:47:59Z","messageCount":3,"participants":["Quinn Grier","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"307351","messageId":"20161209131141.3096-1-quinn@quinngrier.com","threadId":"44663","inReplyTo":null,"subject":"[PATCH] describe: add tests for unusual graphs","fromName":"Quinn Grier","fromEmail":"quinn@quinngrier.com","sentAt":"2016-12-09T13:11:41Z","receivedAt":"2016-12-09T13:21:53Z","isPatch":true,"sender":{"key":"quinn@quinngrier.com","avatar":null},"body":"git describe may give incorrect results if there are backdated commits\nor multiple roots. This commit adds two test_expect_failure tests that\ndemonstrate these problems.\n\nSigned-off-by: Quinn Grier <quinn@quinngrier.com>\n---\n t/t6120-describe.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 48 insertions(+)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 85f2694..ca82837 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -206,4 +206,52 @@ test_expect_success 'describe --contains with the exact tags' '\n \ttest_cmp expect actual\n '\n \n+#\n+# A---B*--D master\n+#  \\     /\n+#   .---C topic\n+#\n+\n+test_expect_failure 'backdated commit' '(\n+\ttest_tick &&\n+\tb=$GIT_COMMITTER_DATE && test_tick &&\n+\ttest_create_repo backdated-commit &&\n+\tcd backdated-commit &&\n+\tgit commit --allow-empty -m A && test_tick &&\n+\tGIT_COMMITTER_DATE=$b git commit --allow-empty -m B && test_tick &&\n+\tgit checkout -b topic :/A &&\n+\tgit commit --allow-empty -m C && test_tick &&\n+\tgit checkout master &&\n+\tgit merge -m D topic && test_tick &&\n+\tgit tag -m B B :/B && test_tick &&\n+\tgit describe :/D >tmp &&\n+\tsed s/-g.\\*// tmp >actual &&\n+\techo B-2 >expected &&\n+\ttest_cmp expected actual\n+)'\n+\n+#\n+# A---B*--D master\n+#        /\n+#       C* other\n+#\n+\n+test_expect_failure 'multiple roots' '(\n+\ttest_tick &&\n+\ttest_create_repo multiple-roots &&\n+\tcd multiple-roots &&\n+\tgit commit --allow-empty -m A && test_tick &&\n+\tgit commit --allow-empty -m B && test_tick &&\n+\tgit checkout --orphan other &&\n+\tgit commit --allow-empty -m C && test_tick &&\n+\tgit checkout master &&\n+\tgit merge --allow-unrelated-histories -m D other && test_tick &&\n+\tgit tag -m B B :/B && test_tick &&\n+\tgit tag -m C C :/C && test_tick &&\n+\tgit describe :/D >tmp &&\n+\tsed s/-g.\\*// tmp >actual &&\n+\techo B-2 >expected &&\n+\ttest_cmp expected actual\n+)'\n+\n test_done\n-- \n2.8.3\n\n"},{"id":"307404","messageId":"xmqq60msr92w.fsf@gitster.mtv.corp.google.com","threadId":"44663","inReplyTo":"20161209131141.3096-1-quinn@quinngrier.com","subject":"Re: [PATCH] describe: add tests for unusual graphs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-09T23:12:07Z","receivedAt":"2016-12-09T23:12:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Quinn Grier <quinn@quinngrier.com> writes:\n\n> git describe may give incorrect results if there are backdated commits\n> or multiple roots. This commit adds two test_expect_failure tests that\n> demonstrate these problems.\n\nI am not sure if this is a good patch to take.  test_expect_failure\nis to demonstrate an incorrect behaviour that we wish to correct\nlater, but I do not think these demonstrate incorrect behaviours to\nbegin with.\n\nFor example, the latter one seems to expect that by asking to\ndescribe D in this picture\n\n> +#\n> +# A---B*--D master\n> +#        /\n> +#       C* other\n> +#\n\nyou seem to expect the description is based on B.  \n\nIt is not at all clear why it is incorrect if the description were\nmade based on C.  If D were described relative to A, ignoring B,\nthen I understand why that result is incorrect and I would agree\nthat describing D in terms of B is more correct.  But I do not think\nthat is what the test is trying to demonstrate.\n\nBut it is hard to guess only from looking at the test and the\nproposed log message, because it does not say what makes you think\nthe behaviour you saw was incorrect.\n"},{"id":"307415","messageId":"dfc5172d-e87c-2c2f-dc7f-a6f7ccf0028e@quinngrier.com","threadId":"44663","inReplyTo":"xmqq60msr92w.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] describe: add tests for unusual graphs","fromName":"Quinn Grier","fromEmail":"quinn@quinngrier.com","sentAt":"2016-12-10T06:47:50Z","receivedAt":"2016-12-10T06:47:59Z","isPatch":true,"sender":{"key":"quinn@quinngrier.com","avatar":null},"body":"On 2016-12-09 17:12, Junio C Hamano wrote:\n> Quinn Grier <quinn@quinngrier.com> writes:\n> \n>> git describe may give incorrect results if there are backdated commits\n>> or multiple roots. This commit adds two test_expect_failure tests that\n>> demonstrate these problems.\n> \n> I am not sure if this is a good patch to take.  test_expect_failure\n> is to demonstrate an incorrect behaviour that we wish to correct\n> later, but I do not think these demonstrate incorrect behaviours to\n> begin with.\n> \n> For example, the latter one seems to expect that by asking to\n> describe D in this picture\n> \n>> +#\n>> +# A---B*--D master\n>> +#        /\n>> +#       C* other\n>> +#\n> \n> you seem to expect the description is based on B.  \n> \n> It is not at all clear why it is incorrect if the description were\n> made based on C.  If D were described relative to A, ignoring B,\n> then I understand why that result is incorrect and I would agree\n> that describing D in terms of B is more correct.  But I do not think\n> that is what the test is trying to demonstrate.\n> \n> But it is hard to guess only from looking at the test and the\n> proposed log message, because it does not say what makes you think\n> the behaviour you saw was incorrect.\n> \n\nI thought the behavior was incorrect because of the following paragraph\nfrom the documentation for git describe:\n\n      If multiple tags were found during the walk then the tag\n      which has the fewest commits different from the input\n      commit-ish will be selected and output. Here fewest commits\n      different is defined as the number of commits which would be\n      shown by git log tag..input will be the smallest number of\n      commits possible.\n"}]}