Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-30 Thread Christoph Bonitz 
On Thu, Jul 24, 2014 at 8:45 PM, Johannes Sixt j...@kdbg.org wrote:
 Am 23.07.2014 23:28, schrieb Christoph Bonitz:
 - test $src = file10 || test $src = file11 
 + test $src = file2 || test $src = file10 || test $src = file11 

 You can't test for alternatives in this way. It's already wrong in the
 original line, which is from 795fcb0e (avoid test cond -a/-o cond),
 and breaks the  chain.

Thank you, I'm sorry I missed that!
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-30 Thread Christoph Bonitz 
On Fri, Jul 25, 2014 at 12:05 AM, Junio C Hamano gits...@pobox.com wrote:
 Johannes Sixt j...@kdbg.org writes:
 I see a few other no-nos in the context of the changes, in particular,
 pipelines where git is not the last command; these would not catch
 failures in the git commands. But a fix for that is certainly outside
 the scope of this patch.

 Yuck.  Thanks for spotting.

 Perhaps we should apply a preliminary clean-up before doing anything
 else, perhaps?  The change in 9814 is a post 2.0 regression.

Apart from your change and the word wrap adjustments suggested by
Pete, would the following also make sense, to fix the other flaw
Johannes pointed out? With regards to failing, git diff-tree should be
idempotent. I think those are the two occurrences in this file:

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 1fc1f5f..7815f9a 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -176,6 +176,7 @@ test_expect_success 'detect copies' '
git diff-tree -r -C --find-copies-harder HEAD 
level=$(git diff-tree -r -C --find-copies-harder HEAD
| sed 1d | cut -f1 | cut -d  -f5 | sed s/C
test -n $level  test $level -gt 0  test
$level -lt 98 
+   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 = file11 
git config git-p4.detectCopies $(($level + 2)) 
@@ -190,6 +191,7 @@ test_expect_success 'detect copies' '
git diff-tree -r -C --find-copies-harder HEAD 
level=$(git diff-tree -r -C --find-copies-harder HEAD
| sed 1d | cut -f1 | cut -d  -f5 | sed s/C
test -n $level  test $level -gt 2  test
$level -lt 100 
+   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 = file11 || test
$src = file12 
git config git-p4.detectCopies $(($level - 2)) 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-30 Thread Junio C Hamano 
Christoph Bonitz ml.christophbon...@gmail.com writes:

 Apart from your change and the word wrap adjustments suggested by
 Pete, would the following also make sense, to fix the other flaw
 Johannes pointed out? With regards to failing, git diff-tree should be
 idempotent. I think those are the two occurrences in this file:

As a band-aid, that might be OK, but I think these pipelines are
unnecessarily and overly wasteful in the first place.

All the sed 1d you see here is only because the upstream uses
the one-tree form diff-tree options $commit; by comparing two,
i.e. diff-tree options $commit^ $commit, they can be dropped.

All the cut -f2 is to grab the pathname; we have --name-only
these days.

I.e.

  src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) 

should become

src=$(git diff-tree --name-only -r -C --find-copies-harder HEAD^ HEAD) 

I would think.

Extracting C[0-9]* manually with sed is bad, and expecting that the
score is within certain range is even worse, because there is no
formal guarantee that the definition of similarity indices will not
improve in the future.  --diff-filter=C to limit the output to only
copied paths, without looking at the similarity index, would be more
appropriate, e.g. 

git diff-tree --name-only --diff-filter=C -r -C HEAD^ HEAD

or something along those lines.

Otherwise, run them outside $(), keep the result in a temporary
file, and process the temporary file with the pipeline.  That has an
added benefit that lets you inspect the file when something goes
wrong.  I.e.

git diff-tree ... diff-tree-out 
level=$( sed 1d diff-tree-out | cut -f1 | ... )
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-24 Thread Johannes Sixt 
Am 23.07.2014 23:28, schrieb Christoph Bonitz:
 The scenario in the rename test makes unnecessary assumptions about
 which file git file-tree will detect as a source for a copy-operations.
 Furthermore, copy detection is not tested by checking the resulting
 perforce revision history via p4 filelog, but via git diff-tree.
 
 This patch makes the test more robust by accepting each of the possible
 sources, and more rigorous by doing so via p4 filelog.
 ---

Please sign off your patch.

  t/t9814-git-p4-rename.sh | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
 index 1fc1f5f..4068510 100755
 --- a/t/t9814-git-p4-rename.sh
 +++ b/t/t9814-git-p4-rename.sh
 @@ -156,18 +156,16 @@ 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 
   git config git-p4.detectCopiesHarder true 
   git p4 submit 
   p4 filelog //depot/file11 
 - p4 filelog //depot/file11 | grep -q branch from //depot/file 
 + p4 filelog //depot/file11 | grep -q -E branch from //depot/file(2|10) 
 
   cp file2 file12 
   echo some text file12 
 @@ -177,7 +175,7 @@ test_expect_success 'detect copies' '
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | 
 cut -d  -f5 | sed s/C0*//) 
   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) 
 - test $src = file10 || test $src = file11 
 + test $src = file2 || test $src = file10 || test $src = file11 

You can't test for alternatives in this way. It's already wrong in the
original line, which is from 795fcb0e (avoid test cond -a/-o cond),
and breaks the  chain. You need braces like this:

  { test $src = file2 || test $src = file10 || test $src = file11; } 

or you wrap it up in a case statement.

   git config git-p4.detectCopies $(($level + 2)) 
   git p4 submit 
   p4 filelog //depot/file12 
 @@ -190,12 +188,10 @@ test_expect_success 'detect copies' '
   git diff-tree -r -C --find-copies-harder HEAD 
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | 
 cut -d  -f5 | sed s/C0*//) 
   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) 
 - test $src = file10 || test $src = file11 || test $src = file12 
   git config git-p4.detectCopies $(($level - 2)) 
   git p4 submit 
   p4 filelog //depot/file13 
 - p4 filelog //depot/file13 | grep -q branch from //depot/file
 + p4 filelog //depot/file13 | grep -q -E branch from 
 //depot/file(2|10|11|12)
   )
  '
 

I see a few other no-nos in the context of the changes, in particular,
pipelines where git is not the last command; these would not catch
failures in the git commands. But a fix for that is certainly outside
the scope of this patch.

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-24 Thread Junio C Hamano 
Johannes Sixt j...@kdbg.org writes:

 @@ -177,7 +175,7 @@ test_expect_success 'detect copies' '
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f1 | 
 cut -d  -f5 | sed s/C0*//) 
   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) 
 - test $src = file10 || test $src = file11 
 + test $src = file2 || test $src = file10 || test $src = file11 

 You can't test for alternatives in this way. It's already wrong in the
 original line, which is from 795fcb0e (avoid test cond -a/-o cond),
 and breaks the  chain. You need braces like this:

   { test $src = file2 || test $src = file10 || test $src = file11; } 

 or you wrap it up in a case statement.
 ...
   )
  '
 

 I see a few other no-nos in the context of the changes, in particular,
 pipelines where git is not the last command; these would not catch
 failures in the git commands. But a fix for that is certainly outside
 the scope of this patch.

Yuck.  Thanks for spotting.

Perhaps we should apply a preliminary clean-up before doing anything
else, perhaps?  The change in 9814 is a post 2.0 regression.



 t/t9814-git-p4-rename.sh | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 1fc1f5f..95f4421 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -177,7 +177,10 @@ test_expect_success 'detect copies' '
level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d 
| cut -f1 | cut -d  -f5 | sed s/C0*//) 
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) 
-   test $src = file10 || test $src = file11 
+   case $src in
+   file10 | file11) : ;; # happy
+   *) false ;; # not
+   
git config git-p4.detectCopies $(($level + 2)) 
git p4 submit 
p4 filelog //depot/file12 
@@ -191,7 +194,10 @@ test_expect_success 'detect copies' '
level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d 
| cut -f1 | cut -d  -f5 | sed s/C0*//) 
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) 
-   test $src = file10 || test $src = file11 || test $src = 
file12 
+   case $src in
+   file10 | file11 | file12) : ;; # happy
+   *) false ;; # not
+   
git config git-p4.detectCopies $(($level - 2)) 
git p4 submit 
p4 filelog //depot/file13 


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-23 Thread Christoph Bonitz 
On Mon, Jul 7, 2014 at 11:14 PM, Junio C Hamano gits...@pobox.com wrote:

 Choosing any of these as the copy source is fine is a sensible way
 to fix the problem with this test.  Would it be a better solution to
 avoid having multiple/ambiguous copy source candidates in the first
 place, by the way?

I agree that this would be the best solution, although I feel more
confident making a less invasive change first.

Thanks,
Christoph
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-23 Thread Christoph Bonitz 
The scenario in the rename test makes unnecessary assumptions about
which file git file-tree will detect as a source for a copy-operations.
Furthermore, copy detection is not tested by checking the resulting
perforce revision history via p4 filelog, but via git diff-tree.

This patch makes the test more robust by accepting each of the possible
sources, and more rigorous by doing so via p4 filelog.
---
 t/t9814-git-p4-rename.sh | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 1fc1f5f..4068510 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -156,18 +156,16 @@ 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 
  git config git-p4.detectCopiesHarder true 
  git p4 submit 
  p4 filelog //depot/file11 
- p4 filelog //depot/file11 | grep -q branch from //depot/file 
+ p4 filelog //depot/file11 | grep -q -E branch from //depot/file(2|10) 

  cp file2 file12 
  echo some text file12 
@@ -177,7 +175,7 @@ test_expect_success 'detect copies' '
  level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut
-f1 | cut -d  -f5 | sed s/C0*//) 
  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) 
- test $src = file10 || test $src = file11 
+ test $src = file2 || test $src = file10 || test $src = file11 
  git config git-p4.detectCopies $(($level + 2)) 
  git p4 submit 
  p4 filelog //depot/file12 
@@ -190,12 +188,10 @@ test_expect_success 'detect copies' '
  git diff-tree -r -C --find-copies-harder HEAD 
  level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut
-f1 | cut -d  -f5 | sed s/C0*//) 
  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) 
- test $src = file10 || test $src = file11 || test $src = file12 
  git config git-p4.detectCopies $(($level - 2)) 
  git p4 submit 
  p4 filelog //depot/file13 
- p4 filelog //depot/file13 | grep -q branch from //depot/file
+ p4 filelog //depot/file13 | grep -q -E branch from //depot/file(2|10|11|12)
  )
 '

-- 
2.0.1

On Mon, Jul 7, 2014 at 3:10 AM, Pete Wyckoff p...@padd.com wrote:
 ml.christophbon...@gmail.com wrote on Sun, 06 Jul 2014 16:32 +0200:
 I'm trying to get the git p4 tests to pass on my machine (OS X
 Mavericks) from master before making some changes. I'm experiencing a
 test failure in detect copies of the rename test.

 The test creates file2 with some content, creates a few copies (each
 with a commit), then does the following (no git write operations
 omitted):
 echo file2 file2 
 cp file2 file10 
 git add file2 file10 
 git commit -a -m Modify and copy file2 to file10 
 ... (some non-write-operations) ...
 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 

 This is where it fails on my machine. The git diff-tree output is this
 :100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
 22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
 so git diff-tree sees file2 as the copy source, not file10. In my
 opinion, the diff-tree result is legitimate (at that point, file2,
 file10 and file11 are identical). Later in the tests, after making
 more copies of file2, the conditions are more flexible, e.g.
 test $src = file10 || test $src = file11 || test $src = file12 

 IMO, the test discounts the legitimate possibility of diff-tree
 detecting file2 as source, making unnecessary assumptions about
 implementation details. Is this correct, or do I misunderstand the
 workings of diff-tree?

 I'd be grateful for advice, both on whether this is a bug, and if so,
 which branch to base a patch on.

 I think your analysis is correct.  And I agree that later tests
 have noticed this ambiguity and added multiple comparisons like
 you quote.

 I'm not sure how to robustify this.  At least doing the multiple
 comparisons should make the tests work again.  The goal of this
 series of tests is to make sure that copy detection is working,
 not to verify that the correct copy choice was made.  That should
 be in other (non-p4) tests.

 Do send patches based on Junio's master.  I can ack, and they'll
 show up in a future git release.

 Thanks!

 -- Pete
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at

Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-23 Thread Pete Wyckoff 
ml.christophbon...@gmail.com wrote on Wed, 23 Jul 2014 23:28 +0200:
 The scenario in the rename test makes unnecessary assumptions about
 which file git file-tree will detect as a source for a copy-operations.
 Furthermore, copy detection is not tested by checking the resulting
 perforce revision history via p4 filelog, but via git diff-tree.
 
 This patch makes the test more robust by accepting each of the possible
 sources, and more rigorous by doing so via p4 filelog.

I see what you're doing here, and it all looks good.  The
diff-tree checks were mainly debugging, and if p4 filelog
shows the right branch from, that means it works.

You might be able to firm up the branch from lines for file8
and file9 too, but those aren't likely to fail.

Indeed, as noted in the other thread, it would be better to make
these not so flaky, but your change here fixes a problem, and
doesn't do any harm.  And gives you an opportunity to fix it more
later.  :)

Be sure to fix the word-wrapping you have on two of the lines
below.  And be careful not to top post.

Here's my ack for when you decide to send it back to the list,
cc junio.

Acked-by: Pete Wyckoff p...@padd.com

 ---
  t/t9814-git-p4-rename.sh | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
 index 1fc1f5f..4068510 100755
 --- a/t/t9814-git-p4-rename.sh
 +++ b/t/t9814-git-p4-rename.sh
 @@ -156,18 +156,16 @@ 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 
   git config git-p4.detectCopiesHarder true 
   git p4 submit 
   p4 filelog //depot/file11 
 - p4 filelog //depot/file11 | grep -q branch from //depot/file 
 + p4 filelog //depot/file11 | grep -q -E branch from //depot/file(2|10) 
 
   cp file2 file12 
   echo some text file12 
 @@ -177,7 +175,7 @@ test_expect_success 'detect copies' '
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut
 -f1 | cut -d  -f5 | sed s/C0*//) 
   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) 
 - test $src = file10 || test $src = file11 
 + test $src = file2 || test $src = file10 || test $src = file11 
   git config git-p4.detectCopies $(($level + 2)) 
   git p4 submit 
   p4 filelog //depot/file12 
 @@ -190,12 +188,10 @@ test_expect_success 'detect copies' '
   git diff-tree -r -C --find-copies-harder HEAD 
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut
 -f1 | cut -d  -f5 | sed s/C0*//) 
   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) 
 - test $src = file10 || test $src = file11 || test $src = file12 
   git config git-p4.detectCopies $(($level - 2)) 
   git p4 submit 
   p4 filelog //depot/file13 
 - p4 filelog //depot/file13 | grep -q branch from //depot/file
 + p4 filelog //depot/file13 | grep -q -E branch from 
 //depot/file(2|10|11|12)
   )
  '
 
 -- 
 2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-07 Thread Junio C Hamano 
Pete Wyckoff p...@padd.com writes:

 I'm not sure how to robustify this.  At least doing the multiple
 comparisons should make the tests work again.  The goal of this
 series of tests is to make sure that copy detection is working,
 not to verify that the correct copy choice was made.  That should
 be in other (non-p4) tests.

Choosing any of these as the copy source is fine is a sensible way
to fix the problem with this test.  Would it be a better solution to
avoid having multiple/ambiguous copy source candidates in the first
place, by the way?

 Do send patches based on Junio's master.  I can ack, and they'll
 show up in a future git release.

 Thanks!

   -- Pete
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

2014-07-06 Thread Pete Wyckoff 
ml.christophbon...@gmail.com wrote on Sun, 06 Jul 2014 16:32 +0200:
 I'm trying to get the git p4 tests to pass on my machine (OS X
 Mavericks) from master before making some changes. I'm experiencing a
 test failure in detect copies of the rename test.
 
 The test creates file2 with some content, creates a few copies (each
 with a commit), then does the following (no git write operations
 omitted):
 echo file2 file2 
 cp file2 file10 
 git add file2 file10 
 git commit -a -m Modify and copy file2 to file10 
 ... (some non-write-operations) ...
 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 
 
 This is where it fails on my machine. The git diff-tree output is this
 :100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
 22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
 so git diff-tree sees file2 as the copy source, not file10. In my
 opinion, the diff-tree result is legitimate (at that point, file2,
 file10 and file11 are identical). Later in the tests, after making
 more copies of file2, the conditions are more flexible, e.g.
 test $src = file10 || test $src = file11 || test $src = file12 
 
 IMO, the test discounts the legitimate possibility of diff-tree
 detecting file2 as source, making unnecessary assumptions about
 implementation details. Is this correct, or do I misunderstand the
 workings of diff-tree?
 
 I'd be grateful for advice, both on whether this is a bug, and if so,
 which branch to base a patch on.

I think your analysis is correct.  And I agree that later tests
have noticed this ambiguity and added multiple comparisons like
you quote.

I'm not sure how to robustify this.  At least doing the multiple
comparisons should make the tests work again.  The goal of this
series of tests is to make sure that copy detection is working,
not to verify that the correct copy choice was made.  That should
be in other (non-p4) tests.

Do send patches based on Junio's master.  I can ack, and they'll
show up in a future git release.

Thanks!

-- Pete
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html