Subject: Re: [PATCH v2 2/2] Teach read-tree the -n|--dry-run option

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 May 2011 15:30:02 -0700

Jens Lehmann <[email-masked]> writes:

> Using this option tells read-tree to not update the index. That makes it
> possible to check if updating the index would be successful without
> changing anything. Using this option will silently ignore -u but still
> test if updating the work tree would succeed.

I'll reword the above to de-stress "the index" when I queue this patch, as
this is not about the index anymore.  Also I do not think you "silently
ignore" the "-u" option.  Don't you check what would happen when "-u" is
given and checking out the merge result would overwrite the file in the
working tree?  In that scenario, "-u" actively participates in the dry-run
check you implemented.

> diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh
> new file mode 100644
> index 0000000..435d19c
> --- /dev/null
> +++ b/t/lib-read-tree.sh
> @@ -0,0 +1,43 @@
> +#!/bin/sh
> +#
> +# Helper functions to check if read-tree would succeed/fail as expected with
> +# and without the dry-run option. They also test that the dry-run does not
> +# write the index and that together with -u it doesn't touch the work tree.
> +#
> ...
> +read_tree_u_must_succeed () {
> +    git ls-files -s >pre-dry-run &&
> +    git diff-files >pre-dry-run-wt &&
> +    git read-tree -n "$@" &&
> +    git ls-files -s >post-dry-run &&
> +    git diff-files >post-dry-run-wt &&
> +    test_cmp pre-dry-run post-dry-run &&
> +    test_cmp pre-dry-run-wt post-dry-run-wt &&

As the RHS of diff-files output that represent the working tree status
indicates "contents indeterminate" with 0{40}, this comparison is the same
as comparing what is in the index before and after the dry-run.

> ...
> +read_tree_u_must_fail () {
> +    git ls-files -s >pre-dry-run &&
> +    git diff-files >pre-dry-run-wt &&
> +    test_must_fail git read-tree -n "$@" &&
> +    git ls-files -s >post-dry-run &&
> +    git diff-files >post-dry-run-wt &&
> +    test_cmp pre-dry-run post-dry-run &&
> +    test_cmp pre-dry-run-wt post-dry-run-wt &&
> +    test_must_fail git read-tree "$@"
> +}

Same here.  A few additions of -p will trivially fix them, though.

diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh
index 435d19c..abc2c6f 100644
--- a/t/lib-read-tree.sh
+++ b/t/lib-read-tree.sh
@@ -22,10 +22,10 @@ read_tree_must_fail () {
 
 read_tree_u_must_succeed () {
     git ls-files -s >pre-dry-run &&
-    git diff-files >pre-dry-run-wt &&
+    git diff-files -p >pre-dry-run-wt &&
     git read-tree -n "$@" &&
     git ls-files -s >post-dry-run &&
-    git diff-files >post-dry-run-wt &&
+    git diff-files -p >post-dry-run-wt &&
     test_cmp pre-dry-run post-dry-run &&
     test_cmp pre-dry-run-wt post-dry-run-wt &&
     git read-tree "$@"
@@ -33,10 +33,10 @@ read_tree_u_must_succeed () {
 
 read_tree_u_must_fail () {
     git ls-files -s >pre-dry-run &&
-    git diff-files >pre-dry-run-wt &&
+    git diff-files -p >pre-dry-run-wt &&
     test_must_fail git read-tree -n "$@" &&
     git ls-files -s >post-dry-run &&
-    git diff-files >post-dry-run-wt &&
+    git diff-files -p >post-dry-run-wt &&
     test_cmp pre-dry-run post-dry-run &&
     test_cmp pre-dry-run-wt post-dry-run-wt &&
     test_must_fail git read-tree "$@"