Re: [PATCH 4/6] am --abort: revert changes introduced by failed 3way merge

2015-06-08 Thread Junio C Hamano
Paul Tan  writes:

> Even when a merge conflict occurs with am --3way, the index will be
> modified with the results of any successfully merged files. These
> changes to the index will not be reverted with a
> "git read-tree --reset -u HEAD ORIG_HEAD", as git read-tree will not be
> aware of how the current index differs from HEAD or ORIG_HEAD.
>
> To fix this, we first reset any conflicting entries in the index. The
> resulting index will contain the results of successfully merged files
> introduced by the failed merge. We write this index to a tree, and then
> use git read-tree to fast-forward this "index tree" back to ORIG_HEAD,
> thus undoing all the changes from the failed merge.
>
> When we are on an unborn branch, HEAD and ORIG_HEAD will not point to
> valid trees. In this case, use an empty tree.
>
> Signed-off-by: Paul Tan 
> ---
>  git-am.sh   |  6 +-
>  t/t4151-am-abort.sh | 23 +++
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 67f4f25..e4fe3ed 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -519,7 +519,11 @@ then
>   git rerere clear
>   if safe_to_abort
>   then
> - git read-tree --reset -u HEAD ORIG_HEAD
> + head_tree=$(git rev-parse --verify -q HEAD || echo 
> $empty_tree) &&
> + git read-tree --reset -u $head_tree $head_tree &&
> + index_tree=$(git write-tree) &&
> + orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo 
> $empty_tree) &&
> + git read-tree -m -u $index_tree $orig_head
>   git reset ORIG_HEAD

What would the last "reset" do when ORIG_HEAD is invalid?  In other
words, does it make sense to worry about the case where ORIG_HEAD
does not exist?  At which point in the flow does it get written using
what information?

>   fi
>   rm -fr "$dotest"
> diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
> index ea4a49e..2366042 100755
> --- a/t/t4151-am-abort.sh
> +++ b/t/t4151-am-abort.sh
> @@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' '
>   test 4 = "$(cat otherfile-4)" &&
>   git am --skip &&
>   test_cmp_rev initial HEAD &&
> + test -z "$(git ls-files -u)" &&
> + test_path_is_missing otherfile-4
> +'
> +
> +test_expect_success 'am -3 --abort removes otherfile-4' '
> + git reset --hard initial &&
> + test_must_fail git am -3 0003-*.patch &&
> + test 3 -eq $(git ls-files -u | wc -l) &&
> + test 4 = "$(cat otherfile-4)" &&
> + git am --abort &&
> + test_cmp_rev initial HEAD &&
>   test -z $(git ls-files -u) &&
>   test_path_is_missing otherfile-4
>  '
> @@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn 
> branch' '
>   test_path_is_missing tmpfile
>  '
>  
> +test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '
> + git checkout -f --orphan orphan &&
> + git reset &&
> + rm -f otherfile-4 file-1 &&
> + test_must_fail git am -3 0003-*.patch &&
> + test 2 -eq $(git ls-files -u | wc -l) &&
> + test 4 = "$(cat otherfile-4)" &&
> + git am --abort &&
> + test -z "$(git ls-files -u)" &&
> + test_path_is_missing otherfile-4
> +'
> +
>  test_done
--
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 4/6] am --abort: revert changes introduced by failed 3way merge

2015-06-06 Thread Paul Tan
Even when a merge conflict occurs with am --3way, the index will be
modified with the results of any successfully merged files. These
changes to the index will not be reverted with a
"git read-tree --reset -u HEAD ORIG_HEAD", as git read-tree will not be
aware of how the current index differs from HEAD or ORIG_HEAD.

To fix this, we first reset any conflicting entries in the index. The
resulting index will contain the results of successfully merged files
introduced by the failed merge. We write this index to a tree, and then
use git read-tree to fast-forward this "index tree" back to ORIG_HEAD,
thus undoing all the changes from the failed merge.

When we are on an unborn branch, HEAD and ORIG_HEAD will not point to
valid trees. In this case, use an empty tree.

Signed-off-by: Paul Tan 
---
 git-am.sh   |  6 +-
 t/t4151-am-abort.sh | 23 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 67f4f25..e4fe3ed 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -519,7 +519,11 @@ then
git rerere clear
if safe_to_abort
then
-   git read-tree --reset -u HEAD ORIG_HEAD
+   head_tree=$(git rev-parse --verify -q HEAD || echo 
$empty_tree) &&
+   git read-tree --reset -u $head_tree $head_tree &&
+   index_tree=$(git write-tree) &&
+   orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo 
$empty_tree) &&
+   git read-tree -m -u $index_tree $orig_head
git reset ORIG_HEAD
fi
rm -fr "$dotest"
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index ea4a49e..2366042 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' '
test 4 = "$(cat otherfile-4)" &&
git am --skip &&
test_cmp_rev initial HEAD &&
+   test -z "$(git ls-files -u)" &&
+   test_path_is_missing otherfile-4
+'
+
+test_expect_success 'am -3 --abort removes otherfile-4' '
+   git reset --hard initial &&
+   test_must_fail git am -3 0003-*.patch &&
+   test 3 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)" &&
+   git am --abort &&
+   test_cmp_rev initial HEAD &&
test -z $(git ls-files -u) &&
test_path_is_missing otherfile-4
 '
@@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn 
branch' '
test_path_is_missing tmpfile
 '
 
+test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '
+   git checkout -f --orphan orphan &&
+   git reset &&
+   rm -f otherfile-4 file-1 &&
+   test_must_fail git am -3 0003-*.patch &&
+   test 2 -eq $(git ls-files -u | wc -l) &&
+   test 4 = "$(cat otherfile-4)" &&
+   git am --abort &&
+   test -z "$(git ls-files -u)" &&
+   test_path_is_missing otherfile-4
+'
+
 test_done
-- 
2.1.4

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