patch, 6 partsam --skip/--abort: improve index/worktree cleanup

> 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 <pyokagan@gmail.com>
> ---
>  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

>
> To be consistent with the behavior of git am --abort when it is not run
> from an unborn branch, we empty the index, and then destroy the branch
> pointed to by HEAD if there is no ORIG_HEAD.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>  git-am.sh           |  9 ++++++++-
>  t/t4151-am-abort.sh | 17 +++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index e4fe3ed..95f90a0 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -524,7 +524,14 @@ then
>  			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
> +			if git rev-parse --verify -q ORIG_HEAD >/dev/null 2>&1
> +			then
> +				git reset ORIG_HEAD
> +			else
> +				git read-tree $empty_tree
> +				curr_branch=$(git symbolic-ref HEAD 2>/dev/null) &&
> +				git update-ref -d $curr_branch
> +			fi
>  		fi
>  		rm -fr "$dotest"
>  		exit ;;
> diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
> index 2366042..b2a7fc5 100755
> --- a/t/t4151-am-abort.sh
> +++ b/t/t4151-am-abort.sh
> @@ -14,6 +14,7 @@ test_expect_success setup '
>  	git add file-1 file-2 &&
>  	git commit -m initial &&
>  	git tag initial &&
> +	git format-patch --stdout --root initial >initial.patch &&
>  	for i in 2 3 4 5 6
>  	do
>  		echo $i >>file-1 &&
> @@ -125,4 +126,20 @@ test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '
>  	test_path_is_missing otherfile-4
>  '
>  
> +test_expect_success 'am -3 --abort on unborn branch removes applied commits' '
> +	git checkout -f --orphan orphan &&
> +	git reset &&
> +	rm -f otherfile-4 otherfile-2 file-1 file-2 &&
> +	test_must_fail git am -3 initial.patch 0003-*.patch &&
> +	test 3 -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_path_is_missing file-1 &&
> +	test_path_is_missing file-2 &&
> +	test 0 -eq $(git log --oneline 2>/dev/null | wc -l) &&
> +	test refs/heads/orphan = "$(git symbolic-ref HEAD)"
> +'
> +
>  test_done

> Paul Tan <pyokagan@gmail.com> writes:
>
>> When git-am is first run on an unborn branch, no ORIG_HEAD is created.
>> As such, any applied commits will remain even after a git am --abort.
>
> I think this answered my question on 4/6; that may indicate that
> these two are either done in a wrong order or perhaps should be a
> single change.

> Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
> and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits
> were made since the last git-am failure. This check was implemented in
> safe_to_abort(), which checked to see if HEAD's hash matched the
> abort-safety file.
>
> However, this check was skipped if the abort-safety file was empty,
> which can happen if git-am failed while on an unborn branch.

> As such, if
> any commits were made since then, they would be discarded. Fix this by
> carrying on the abort safety check even if the abort-safety file is
> empty.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>  git-am.sh           |  2 +-
>  t/t4151-am-abort.sh | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 95f90a0..4324bb1 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -87,7 +87,7 @@ safe_to_abort () {
>  		return 1
>  	fi
>  
> -	if ! test -s "$dotest/abort-safety"
> +	if ! test -f "$dotest/abort-safety"
>  	then
>  		return 0
>  	fi
> diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
> index b2a7fc5..833e7b2 100755
> --- a/t/t4151-am-abort.sh
> +++ b/t/t4151-am-abort.sh
> @@ -142,4 +142,15 @@ test_expect_success 'am -3 --abort on unborn branch removes applied commits' '
>  	test refs/heads/orphan = "$(git symbolic-ref HEAD)"
>  '
>  
> +test_expect_success 'am --abort on unborn branch will keep local commits intact' '
> +	git checkout -f --orphan orphan &&
> +	git reset &&
> +	test_must_fail git am 0004-*.patch &&
> +	test_commit unrelated2 &&
> +	git rev-parse HEAD >expect &&
> +	git am --abort &&
> +	git rev-parse HEAD >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done

> Paul Tan <pyokagan@gmail.com> writes:
>
>> Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
>> and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits
>> were made since the last git-am failure. This check was implemented in
>> safe_to_abort(), which checked to see if HEAD's hash matched the
>> abort-safety file.
>>
>> However, this check was skipped if the abort-safety file was empty,
>> which can happen if git-am failed while on an unborn branch.
>
> Shouldn't we then be checking that the HEAD is still unborn if this
> file is found and says that there was no history in the beginning,
> in order to give the "am on top of unborn" workflow the same degree
> of safety?

> Currently git-am attempts to clean up the index/worktree when skipping or
> aborting, but does not do it very well:
>
> * While it discards conflicted index entries, it does not remove any other
>   modifications made to the index due to a previous threeway merge.
>
> * It expects HEAD/ORIG_HEAD to exist, and thus does not clean up the index when
>   on an unborn branch.
>
> This patch series addresses the above two general problems by making the calls
> to git-read-tree aware of the differences between our current index and
> HEAD/ORIG_HEAD, and by explictly defining what happens when we are on an unborn
> branch.
>
>
> Paul Tan (6):
>   am --skip: revert changes introduced by failed 3way merge
>   am -3: support 3way merge on unborn branch
>   am --skip: support skipping while on unborn branch
>   am --abort: revert changes introduced by failed 3way merge
>   am --abort: support aborting to unborn branch
>   am --abort: keep unrelated commits on unborn branch

>
>  git-am.sh           | 31 ++++++++++++++------
>  t/t4151-am-abort.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 104 insertions(+), 8 deletions(-)
>
> --
> 2.1.4
>