[PATCH/RFC 1/3] t3030-merge-recursive: Test known breakage with empty work tree

2014-01-24 Thread Brad King
Add test cases that use 'merge-recursive' plumbing with a temporary
index and empty work tree.  Populate the index using 'read-tree' and
'update-index --ignore-missing --refresh' to prepare for merge without
actually checking all files out to disk.  Verify that each merge
produces its expected tree while displaying no error diagnostics.

This approach can be used to compute tree merges while checking out only
conflicting files to disk (which is useful for server-side scripts).
Prior to commit 5b448b85 (merge-recursive: When we detect we can skip an
update, actually skip it, 2011-08-11) this worked cleanly in all cases.
Since that commit, merge-recursive displays a diagnostic such as

 error: addinfo_cache failed for path 'e'

when our side has a rename (to 'e').  The diagnostic does not
influence the return code and the merge appears to succeed, but it
causes this test case to fail.

Signed-off-by: Brad King brad.k...@kitware.com
---
 t/t3030-merge-recursive.sh | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 2f96100..b6d3ed0 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -257,6 +257,7 @@ test_expect_success 'setup 8' '
git add e 
test_tick 
git commit -m rename a-e 
+   c7=$(git rev-parse --verify HEAD) 
git checkout rename-ln 
git mv a e 
test_ln_s_add e a 
@@ -517,6 +518,52 @@ test_expect_success 'reset and bind merge' '
 
 '
 
+test_expect_failure 'merge-recursive w/ empty work tree - ours has rename' '
+   (
+GIT_WORK_TREE=$PWD/ours-has-rename-work 
+export GIT_WORK_TREE 
+GIT_INDEX_FILE=$PWD/ours-has-rename-index 
+export GIT_INDEX_FILE 
+mkdir $GIT_WORK_TREE 
+git read-tree -i -m $c7 
+git update-index --ignore-missing --refresh 
+git merge-recursive $c0 -- $c7 $c3 
+git ls-files -s actual-files
+   ) 2actual-err 
+   expected-err 
+   cat expected-files -EOF 
+   100644 $o3 0b/c
+   100644 $o0 0c
+   100644 $o0 0d/e
+   100644 $o0 0e
+   EOF
+   test_cmp expected-files actual-files 
+   test_cmp expected-err actual-err
+'
+
+test_expect_success 'merge-recursive w/ empty work tree - theirs has rename' '
+   (
+GIT_WORK_TREE=$PWD/theirs-has-rename-work 
+export GIT_WORK_TREE 
+GIT_INDEX_FILE=$PWD/theirs-has-rename-index 
+export GIT_INDEX_FILE 
+mkdir $GIT_WORK_TREE 
+git read-tree -i -m $c3 
+git update-index --ignore-missing --refresh 
+git merge-recursive $c0 -- $c3 $c7 
+git ls-files -s actual-files
+   ) 2actual-err 
+   expected-err 
+   cat expected-files -EOF 
+   100644 $o3 0b/c
+   100644 $o0 0c
+   100644 $o0 0d/e
+   100644 $o0 0e
+   EOF
+   test_cmp expected-files actual-files 
+   test_cmp expected-err actual-err
+'
+
 test_expect_success 'merge removes empty directories' '
 
git reset --hard master 
-- 
1.8.5.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/RFC 1/3] t3030-merge-recursive: Test known breakage with empty work tree

2014-01-24 Thread Jonathan Nieder
Hi,

Brad King wrote:

 Add test cases that use 'merge-recursive' plumbing with a temporary
 index and empty work tree.  Populate the index using 'read-tree' and
 'update-index --ignore-missing --refresh' to prepare for merge without
 actually checking all files out to disk.  Verify that each merge
 produces its expected tree while displaying no error diagnostics.

Following my usual review practice of lazy reading for the sake of
readers in the future who might be in a hurry, it's not clear what
problem these tests are solving or trying to detect.  Could you start
with a quick summary of the symptoms and when it came up?

The commit message doesn't need to paraphrase the actual code, since
anyone curious about the details can always look at the code.  It's
more important to explain the motivation and intended effect so people
can understand what went wrong if something ends up being broken by a
later patch.

 This approach can be used to compute tree merges while checking out only
 conflicting files to disk (which is useful for server-side scripts).
 Prior to commit 5b448b85 (merge-recursive: When we detect we can skip an
 update, actually skip it, 2011-08-11) this worked cleanly in all cases.

Do you mean something like the following?

Sometimes when working with a large repository it can be useful to
try out a merge and only check out conflicting files to disk (for
example as a speed optimization on a server).  Until v1.7.7-rc1~28^2~20
(merge-recursive: When we detect we can skip an update, actually
skip it, 2011-08-11), it was possible to do so with the following
idiom:

... summary of commands here ...

Nowadays, that still works and the exit status is the same,
but merge-recursive produces a diagnostic if our side renamed
a file:

error: addinfo_cache failed for path 'dst'

Add a test to document this regression.

[...]
 +++ b/t/t3030-merge-recursive.sh
[...]
 @@ -517,6 +518,52 @@ test_expect_success 'reset and bind merge' '
  
  '
  
 +test_expect_failure 'merge-recursive w/ empty work tree - ours has rename' '
 + (
 +  GIT_WORK_TREE=$PWD/ours-has-rename-work 

Elsewhere in the test, commands in a subshell are indented by another
tab, so these new tests should probably follow suit.  As a side
effect, that makes the indentation easier to see.

Hope that helps,
Jonathan
--
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/RFC 1/3] t3030-merge-recursive: Test known breakage with empty work tree

2014-01-24 Thread Brad King
On 01/24/2014 11:51 AM, Jonathan Nieder wrote:
 a quick summary of the symptoms and when it came up?

You're suggested commit message correctly explains it:

 Do you mean something like the following?
 
   Sometimes when working with a large repository it can be useful to
   try out a merge and only check out conflicting files to disk (for
   example as a speed optimization on a server).  Until v1.7.7-rc1~28^2~20
   (merge-recursive: When we detect we can skip an update, actually
   skip it, 2011-08-11), it was possible to do so with the following
   idiom:
 
   ... summary of commands here ...
 
   Nowadays, that still works and the exit status is the same,
   but merge-recursive produces a diagnostic if our side renamed
   a file:
 
   error: addinfo_cache failed for path 'dst'
 
   Add a test to document this regression.

Yes, thanks.

 Elsewhere in the test, commands in a subshell are indented by another
 tab, so these new tests should probably follow suit.

Great.  I'll fold both of the above into the next revision of the series.

Thanks,
-Brad

--
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