{"thread":{"id":"38922","subject":"[PATCH 1/2] git-p4: Make rename test case runnable under dash","startedAt":"2015-03-27T01:04:27Z","lastAt":"2015-03-28T16:12:59Z","messageCount":10,"participants":["Vitor Antunes","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"258578","messageId":"1427418269-3263-1-git-send-email-vitor.hda@gmail.com","threadId":"38922","inReplyTo":null,"subject":"[PATCH 0/2] git-p4: Small updates to test cases","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-27T01:04:27Z","receivedAt":"2015-03-27T01:04:27Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"This patch set includes two small fixes to the rename test case. The fix to\nsupport dash should be trivial, but in the fix to the copy detection test case\nit isn't obvious to me what changed in diff-tree to result in a different file\nbeing detected as the origin of a copy.\n\nVitor Antunes (2):\n git-p4: Make rename test case runnable under dash\n git-p4: Fix copy detection test\n\n t/t9814-git-p4-rename.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\n-- \n1.7.10.4\n"},{"id":"258577","messageId":"1427418269-3263-2-git-send-email-vitor.hda@gmail.com","threadId":"38922","inReplyTo":"1427418269-3263-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 1/2] git-p4: Make rename test case runnable under dash","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-27T01:04:28Z","receivedAt":"2015-03-27T01:04:28Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"\nSigned-off-by: Vitor Antunes \n---\n t/t9814-git-p4-rename.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex 95f4421..24008ff 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -178,9 +178,9 @@ test_expect_success 'detect copies' '\n \t\ttest -n \"$level\" && test \"$level\" -gt 0 && test \"$level\" -lt 98 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n \t\tcase \"$src\" in\n-\t\tfile10 | file11) : ;; # happy\n+\t\tfile10 | file11) true ;; # happy\n \t\t*) false ;; # not\n-\t\t&&\n+\t\tesac &&\n \t\tgit config git-p4.detectCopies $(($level + 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file12 &&\n@@ -195,9 +195,9 @@ test_expect_success 'detect copies' '\n \t\ttest -n \"$level\" && test \"$level\" -gt 2 && test \"$level\" -lt 100 &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n \t\tcase \"$src\" in\n-\t\tfile10 | file11 | file12) : ;; # happy\n+\t\tfile10 | file11 | file12) true ;; # happy\n \t\t*) false ;; # not\n-\t\t&&\n+\t\tesac &&\n \t\tgit config git-p4.detectCopies $(($level - 2)) &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file13 &&\n-- \n1.7.10.4\n"},{"id":"258579","messageId":"1427418269-3263-3-git-send-email-vitor.hda@gmail.com","threadId":"38922","inReplyTo":"1427418269-3263-1-git-send-email-vitor.hda@gmail.com","subject":"[PATCH 2/2] git-p4: Fix copy detection test","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-27T01:04:29Z","receivedAt":"2015-03-27T01:04:29Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"File file11 is copied from file2 and diff-tree correctly reports this file as\nits the source, but the test expression was checking for file10 instead (which\nwas a file that also originated from file2). It is possible that the diff-tree\nalgorithm was updated in recent versions, which resulted in this mismatch in\nbehavior.\n\nSigned-off-by: Vitor Antunes \n---\n t/t9814-git-p4-rename.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\nindex 24008ff..018f01d 100755\n--- a/t/t9814-git-p4-rename.sh\n+++ b/t/t9814-git-p4-rename.sh\n@@ -156,14 +156,14 @@ test_expect_success 'detect copies' '\n \t\tgit diff-tree -r -C HEAD &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file10 &&\n-\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n+\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n \n \t\tcp file2 file11 &&\n \t\tgit add file11 &&\n \t\tgit commit -a -m \"Copy file2 to file11\" &&\n \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n-\t\ttest \"$src\" = file10 &&\n+\t\ttest \"$src\" = file2 &&\n \t\tgit config git-p4.detectCopiesHarder true &&\n \t\tgit p4 submit &&\n \t\tp4 filelog //depot/file11 &&\n-- \n1.7.10.4\n"},{"id":"258580","messageId":"xmqqwq23w7qx.fsf@gitster.dls.corp.google.com","threadId":"38922","inReplyTo":"1427418269-3263-1-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH 0/2] git-p4: Small updates to test cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-27T01:26:30Z","receivedAt":"2015-03-27T01:26:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes writes:\n\n> This patch set includes two small fixes to the rename test case. The fix to\n> support dash should be trivial, but in the fix to the copy detection test case\n> it isn't obvious to me what changed in diff-tree to result in a different file\n> being detected as the origin of a copy.\n\nThanks.\n\nAs to 1/2 the lack of esac is clearly a bug---any self respecting\nPOSIX shell should have executed it without complaining. But\nchanging from ':' to true should not be necessary---after all, the\ncolon is a more traditional way to spell true to Bourne shells, and\nwe use it in many places already. Can you try reverting all the\n\"colon to true\" bits, keeping only the \"add missing esac\" part, and\nrun your tests again?\n"},{"id":"258581","messageId":"xmqqpp7vw6uy.fsf@gitster.dls.corp.google.com","threadId":"38922","inReplyTo":"xmqqwq23w7qx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] git-p4: Small updates to test cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-27T01:45:41Z","receivedAt":"2015-03-27T01:45:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> As to 1/2 the lack of esac is clearly a bug---any self respecting\n> POSIX shell should have executed it without complaining. But\n\ns/should/shouldn't/; sorry for a noise.\n"},{"id":"258582","messageId":"CAOpHH-WZXFodc3UAhdwJ6Rj1LiS-Duq2MEoN4iEtadNCT9mq5A@mail.gmail.com","threadId":"38922","inReplyTo":"xmqqwq23w7qx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] git-p4: Small updates to test cases","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-27T01:54:00Z","receivedAt":"2015-03-27T01:54:00Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"On Fri, 27 Mar 2015 at 01:26 Junio C Hamano wrote:\n\n>As to 1/2 the lack of esac is clearly a bug---any self respecting\n>POSIX shell should have executed it without complaining. But\n>changing from ':' to true should not be necessary---after all, the\n>colon is a more traditional way to spell true to Bourne shells, and\n>we use it in many places already. Can you try reverting all the\n>\"colon to true\" bits, keeping only the \"add missing esac\" part, and\n>run your tests again?\n\nI confirm that it still works with ':' instead of true; could swear I tested\nthat at the time... Anyway, I'll re-submit this patch with this fixed\ntomorrow.\n\nThanks for taking the time to review the patch.\n\nOne more thing: was there any change in way diff-tree detects copies?\n"},{"id":"258611","messageId":"xmqq619mw04r.fsf@gitster.dls.corp.google.com","threadId":"38922","inReplyTo":"1427418269-3263-3-git-send-email-vitor.hda@gmail.com","subject":"Re: [PATCH 2/2] git-p4: Fix copy detection test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-27T22:23:16Z","receivedAt":"2015-03-27T22:23:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes writes:\n\n> File file11 is copied from file2 and diff-tree correctly reports this file as\n> its the source, but the test expression was checking for file10 instead (which\n> was a file that also originated from file2). It is possible that the diff-tree\n> algorithm was updated in recent versions, which resulted in this mismatch in\n> behavior.\n>\n> Signed-off-by: Vitor Antunes \n\nPete, these tests blame to your 9b6513ac (git p4 test: split up big\nt9800 test, 2012-06-27). I presume that you tested the result of\nthis splitting, but do you happen to know if we did something to\ncause the test to break recently?\n\n> ---\n> t/t9814-git-p4-rename.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh\n> index 24008ff..018f01d 100755\n> --- a/t/t9814-git-p4-rename.sh\n> +++ b/t/t9814-git-p4-rename.sh\n> @@ -156,14 +156,14 @@ test_expect_success 'detect copies' '\n> \t\tgit diff-tree -r -C HEAD &&\n> \t\tgit p4 submit &&\n> \t\tp4 filelog //depot/file10 &&\n> -\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file\" &&\n> +\t\tp4 filelog //depot/file10 | grep -q \"branch from //depot/file2\" &&\n> \n> \t\tcp file2 file11 &&\n> \t\tgit add file11 &&\n> \t\tgit commit -a -m \"Copy file2 to file11\" &&\n> \t\tgit diff-tree -r -C --find-copies-harder HEAD &&\n> \t\tsrc=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&\n> -\t\ttest \"$src\" = file10 &&\n> +\t\ttest \"$src\" = file2 &&\n> \t\tgit config git-p4.detectCopiesHarder true &&\n> \t\tgit p4 submit &&\n> \t\tp4 filelog //depot/file11 &&\n"},{"id":"258623","messageId":"20150327235902.25ebf380@pt-vhugo","threadId":"38922","inReplyTo":"xmqq619mw04r.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] git-p4: Fix copy detection test","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-27T23:59:02Z","receivedAt":"2015-03-27T23:59:02Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Junio C Hamano wrote:\n\n>Vitor Antunes writes:\n>\n>> File file11 is copied from file2 and diff-tree correctly reports this file as\n>> its the source, but the test expression was checking for file10 instead (which\n>> was a file that also originated from file2). It is possible that the diff-tree\n>> algorithm was updated in recent versions, which resulted in this mismatch in\n>> behavior.\n>>\n>> Signed-off-by: Vitor Antunes \n>\n>Pete, these tests blame to your 9b6513ac (git p4 test: split up big\n>t9800 test, 2012-06-27). I presume that you tested the result of\n>this splitting, but do you happen to know if we did something to\n>cause the test to break recently?\n\nI also worked on these tests at that time and they were passing before and\nafter the reorganization. I'll prepare a bisect script and will try to find the\ncommit that started making this test fail.\n"},{"id":"258628","messageId":"20150328003636.07930f83@pt-vhugo","threadId":"38922","inReplyTo":"20150327235902.25ebf380@pt-vhugo","subject":"Re: [PATCH 2/2] git-p4: Fix copy detection test","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2015-03-28T00:36:36Z","receivedAt":"2015-03-28T00:36:36Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Vitor Antunes wrote:\n\n>Junio C Hamano wrote:\n>>Pete, these tests blame to your 9b6513ac (git p4 test: split up big\n>>t9800 test, 2012-06-27). I presume that you tested the result of\n>>this splitting, but do you happen to know if we did something to\n>>cause the test to break recently?\n>\n>I also worked on these tests at that time and they were passing before and\n>after the reorganization. I'll prepare a bisect script and will try to find the\n>commit that started making this test fail.\n\nAccording to bisect, this is the first commit that makes the test fail:\n\n7c85f8acb2282e3ed108c46b59fd5daa78bf17db\n\nDoes this make sense to you?\n"},{"id":"258637","messageId":"xmqq619lyub8.fsf@gitster.dls.corp.google.com","threadId":"38922","inReplyTo":"20150328003636.07930f83@pt-vhugo","subject":"Re: [PATCH 2/2] git-p4: Fix copy detection test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-28T16:12:59Z","receivedAt":"2015-03-28T16:12:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vitor Antunes writes:\n\n> Vitor Antunes wrote:\n>\n>>Junio C Hamano wrote:\n>>>Pete, these tests blame to your 9b6513ac (git p4 test: split up big\n>>>t9800 test, 2012-06-27). I presume that you tested the result of\n>>>this splitting, but do you happen to know if we did something to\n>>>cause the test to break recently?\n>>\n>>I also worked on these tests at that time and they were passing before and\n>>after the reorganization. I'll prepare a bisect script and will try to find the\n>>commit that started making this test fail.\n>\n> According to bisect, this is the first commit that makes the test fail:\n>\n> 7c85f8acb2282e3ed108c46b59fd5daa78bf17db\n>\n> Does this make sense to you?\n\nYeah, as the blamed commit changes the way the hashtable is used\nrecord and choose the rename source candidates, it is not surprising\nif it changes how two or more candidates with the same rename score\nare tie-broken.\n\nThanks.\n"}]}