# [PATCH 1/2] git-p4: Make rename test case runnable under dash 10 messages from 2015-03-27 to 2015-03-28. Participants: Vitor Antunes, Junio C Hamano. Thread: https://gitlist.dev/t/38922 ## Vitor Antunes, 2015-03-27 01:04 Subject: [PATCH 0/2] git-p4: Small updates to test cases Message-ID: <1427418269-3263-1-git-send-email-vitor.hda@gmail.com> URL: https://gitlist.dev/e/1427418269-3263-1-git-send-email-vitor.hda%40gmail.com ``` This patch set includes two small fixes to the rename test case. The fix to support dash should be trivial, but in the fix to the copy detection test case it isn't obvious to me what changed in diff-tree to result in a different file being detected as the origin of a copy. Vitor Antunes (2): git-p4: Make rename test case runnable under dash git-p4: Fix copy detection test t/t9814-git-p4-rename.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) -- 1.7.10.4 ``` ## Vitor Antunes, 2015-03-27 01:04 Subject: [PATCH 1/2] git-p4: Make rename test case runnable under dash Message-ID: <1427418269-3263-2-git-send-email-vitor.hda@gmail.com> URL: https://gitlist.dev/e/1427418269-3263-2-git-send-email-vitor.hda%40gmail.com In-Reply-To: <1427418269-3263-1-git-send-email-vitor.hda@gmail.com> ``` Signed-off-by: Vitor Antunes --- t/t9814-git-p4-rename.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh index 95f4421..24008ff 100755 --- a/t/t9814-git-p4-rename.sh +++ b/t/t9814-git-p4-rename.sh @@ -178,9 +178,9 @@ test_expect_success 'detect copies' ' test -n "$level" && test "$level" -gt 0 && test "$level" -lt 98 && src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) && case "$src" in - file10 | file11) : ;; # happy + file10 | file11) true ;; # happy *) false ;; # not - && + esac && git config git-p4.detectCopies $(($level + 2)) && git p4 submit && p4 filelog //depot/file12 && @@ -195,9 +195,9 @@ test_expect_success 'detect copies' ' test -n "$level" && test "$level" -gt 2 && test "$level" -lt 100 && src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) && case "$src" in - file10 | file11 | file12) : ;; # happy + file10 | file11 | file12) true ;; # happy *) false ;; # not - && + esac && git config git-p4.detectCopies $(($level - 2)) && git p4 submit && p4 filelog //depot/file13 && -- 1.7.10.4 ``` ## Vitor Antunes, 2015-03-27 01:04 Subject: [PATCH 2/2] git-p4: Fix copy detection test Message-ID: <1427418269-3263-3-git-send-email-vitor.hda@gmail.com> URL: https://gitlist.dev/e/1427418269-3263-3-git-send-email-vitor.hda%40gmail.com In-Reply-To: <1427418269-3263-1-git-send-email-vitor.hda@gmail.com> ``` File file11 is copied from file2 and diff-tree correctly reports this file as its the source, but the test expression was checking for file10 instead (which was a file that also originated from file2). It is possible that the diff-tree algorithm was updated in recent versions, which resulted in this mismatch in behavior. Signed-off-by: Vitor Antunes --- t/t9814-git-p4-rename.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh index 24008ff..018f01d 100755 --- a/t/t9814-git-p4-rename.sh +++ b/t/t9814-git-p4-rename.sh @@ -156,14 +156,14 @@ test_expect_success 'detect copies' ' git diff-tree -r -C HEAD && git p4 submit && p4 filelog //depot/file10 && - p4 filelog //depot/file10 | grep -q "branch from //depot/file" && + p4 filelog //depot/file10 | grep -q "branch from //depot/file2" && cp file2 file11 && git add file11 && git commit -a -m "Copy file2 to file11" && git diff-tree -r -C --find-copies-harder HEAD && src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) && - test "$src" = file10 && + test "$src" = file2 && git config git-p4.detectCopiesHarder true && git p4 submit && p4 filelog //depot/file11 && -- 1.7.10.4 ``` ## Junio C Hamano, 2015-03-27 01:26 Subject: Re: [PATCH 0/2] git-p4: Small updates to test cases Message-ID: URL: https://gitlist.dev/e/xmqqwq23w7qx.fsf%40gitster.dls.corp.google.com In-Reply-To: <1427418269-3263-1-git-send-email-vitor.hda@gmail.com> ``` Vitor Antunes writes: > This patch set includes two small fixes to the rename test case. The fix to > support dash should be trivial, but in the fix to the copy detection test case > it isn't obvious to me what changed in diff-tree to result in a different file > being detected as the origin of a copy. Thanks. As to 1/2 the lack of esac is clearly a bug---any self respecting POSIX shell should have executed it without complaining. But changing from ':' to true should not be necessary---after all, the colon is a more traditional way to spell true to Bourne shells, and we use it in many places already. Can you try reverting all the "colon to true" bits, keeping only the "add missing esac" part, and run your tests again? ``` ## Junio C Hamano, 2015-03-27 01:45 Subject: Re: [PATCH 0/2] git-p4: Small updates to test cases Message-ID: URL: https://gitlist.dev/e/xmqqpp7vw6uy.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > As to 1/2 the lack of esac is clearly a bug---any self respecting > POSIX shell should have executed it without complaining. But s/should/shouldn't/; sorry for a noise. ``` ## Vitor Antunes, 2015-03-27 01:54 Subject: Re: [PATCH 0/2] git-p4: Small updates to test cases Message-ID: URL: https://gitlist.dev/e/CAOpHH-WZXFodc3UAhdwJ6Rj1LiS-Duq2MEoN4iEtadNCT9mq5A%40mail.gmail.com In-Reply-To: ``` On Fri, 27 Mar 2015 at 01:26 Junio C Hamano wrote: >As to 1/2 the lack of esac is clearly a bug---any self respecting >POSIX shell should have executed it without complaining. But >changing from ':' to true should not be necessary---after all, the >colon is a more traditional way to spell true to Bourne shells, and >we use it in many places already. Can you try reverting all the >"colon to true" bits, keeping only the "add missing esac" part, and >run your tests again? I confirm that it still works with ':' instead of true; could swear I tested that at the time... Anyway, I'll re-submit this patch with this fixed tomorrow. Thanks for taking the time to review the patch. One more thing: was there any change in way diff-tree detects copies? ``` ## Junio C Hamano, 2015-03-27 22:23 Subject: Re: [PATCH 2/2] git-p4: Fix copy detection test Message-ID: URL: https://gitlist.dev/e/xmqq619mw04r.fsf%40gitster.dls.corp.google.com In-Reply-To: <1427418269-3263-3-git-send-email-vitor.hda@gmail.com> ``` Vitor Antunes writes: > File file11 is copied from file2 and diff-tree correctly reports this file as > its the source, but the test expression was checking for file10 instead (which > was a file that also originated from file2). It is possible that the diff-tree > algorithm was updated in recent versions, which resulted in this mismatch in > behavior. > > Signed-off-by: Vitor Antunes Pete, these tests blame to your 9b6513ac (git p4 test: split up big t9800 test, 2012-06-27). I presume that you tested the result of this splitting, but do you happen to know if we did something to cause the test to break recently? > --- > t/t9814-git-p4-rename.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh > index 24008ff..018f01d 100755 > --- a/t/t9814-git-p4-rename.sh > +++ b/t/t9814-git-p4-rename.sh > @@ -156,14 +156,14 @@ test_expect_success 'detect copies' ' > git diff-tree -r -C HEAD && > git p4 submit && > p4 filelog //depot/file10 && > - p4 filelog //depot/file10 | grep -q "branch from //depot/file" && > + p4 filelog //depot/file10 | grep -q "branch from //depot/file2" && > > cp file2 file11 && > git add file11 && > git commit -a -m "Copy file2 to file11" && > git diff-tree -r -C --find-copies-harder HEAD && > src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) && > - test "$src" = file10 && > + test "$src" = file2 && > git config git-p4.detectCopiesHarder true && > git p4 submit && > p4 filelog //depot/file11 && ``` ## Vitor Antunes, 2015-03-27 23:59 Subject: Re: [PATCH 2/2] git-p4: Fix copy detection test Message-ID: <20150327235902.25ebf380@pt-vhugo> URL: https://gitlist.dev/e/20150327235902.25ebf380%40pt-vhugo In-Reply-To: ``` Junio C Hamano wrote: >Vitor Antunes writes: > >> File file11 is copied from file2 and diff-tree correctly reports this file as >> its the source, but the test expression was checking for file10 instead (which >> was a file that also originated from file2). It is possible that the diff-tree >> algorithm was updated in recent versions, which resulted in this mismatch in >> behavior. >> >> Signed-off-by: Vitor Antunes > >Pete, these tests blame to your 9b6513ac (git p4 test: split up big >t9800 test, 2012-06-27). I presume that you tested the result of >this splitting, but do you happen to know if we did something to >cause the test to break recently? I also worked on these tests at that time and they were passing before and after the reorganization. I'll prepare a bisect script and will try to find the commit that started making this test fail. ``` ## Vitor Antunes, 2015-03-28 00:36 Subject: Re: [PATCH 2/2] git-p4: Fix copy detection test Message-ID: <20150328003636.07930f83@pt-vhugo> URL: https://gitlist.dev/e/20150328003636.07930f83%40pt-vhugo In-Reply-To: <20150327235902.25ebf380@pt-vhugo> ``` Vitor Antunes wrote: >Junio C Hamano wrote: >>Pete, these tests blame to your 9b6513ac (git p4 test: split up big >>t9800 test, 2012-06-27). I presume that you tested the result of >>this splitting, but do you happen to know if we did something to >>cause the test to break recently? > >I also worked on these tests at that time and they were passing before and >after the reorganization. I'll prepare a bisect script and will try to find the >commit that started making this test fail. According to bisect, this is the first commit that makes the test fail: 7c85f8acb2282e3ed108c46b59fd5daa78bf17db Does this make sense to you? ``` ## Junio C Hamano, 2015-03-28 16:12 Subject: Re: [PATCH 2/2] git-p4: Fix copy detection test Message-ID: URL: https://gitlist.dev/e/xmqq619lyub8.fsf%40gitster.dls.corp.google.com In-Reply-To: <20150328003636.07930f83@pt-vhugo> ``` Vitor Antunes writes: > Vitor Antunes wrote: > >>Junio C Hamano wrote: >>>Pete, these tests blame to your 9b6513ac (git p4 test: split up big >>>t9800 test, 2012-06-27). I presume that you tested the result of >>>this splitting, but do you happen to know if we did something to >>>cause the test to break recently? >> >>I also worked on these tests at that time and they were passing before and >>after the reorganization. I'll prepare a bisect script and will try to find the >>commit that started making this test fail. > > According to bisect, this is the first commit that makes the test fail: > > 7c85f8acb2282e3ed108c46b59fd5daa78bf17db > > Does this make sense to you? Yeah, as the blamed commit changes the way the hashtable is used record and choose the rename source candidates, it is not surprising if it changes how two or more candidates with the same rename score are tie-broken. Thanks. ```