patch, 4 parts[RFC] merge --autostash: apply autostash in more cases

> From: Philippe Blain <levraiphilippeblain@gmail.com>
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1715,7 +1715,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  	else {
>  		printf(_("Rewinding the tree to pristine...\n"));
>  		restore_state(&head_commit->object.oid, &stash);
> -		printf(_("Using the %s to prepare resolving by hand.\n"),
> +		printf(_("Using the %s strategy to prepare resolving by hand.\n"),

> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1560,6 +1560,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  					  &head_commit->object.oid,
>  					  &commit->object.oid,
>  					  overwrite_ignore)) {
> +			apply_autostash(git_path_merge_autostash(the_repository));
>  			ret = 1;
>  			goto done;
>  		}

> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 1cbc9715a81..216113d3537 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -122,6 +122,8 @@ test_expect_success 'setup' '
>  	c0=$(git rev-parse HEAD) &&
>  	cp file.1 file &&
>  	git add file &&
> +	cp file.1 other &&
> +	git add other &&
>  	test_tick &&
>  	git commit -m "commit 1" &&
>  	git tag c1 &&
> @@ -711,6 +713,15 @@ test_expect_success 'fast-forward merge with --autostash' '
>  	test_cmp result.1-5 file
>  '
>  
> +test_expect_success 'failed fast-forward merge with --autostash' '
> +	git reset --hard c0 &&
> +	git merge-file file file.orig file.5 &&
> +	cp file.5 other &&
> +	test_must_fail git merge --autostash c1 2>err &&
> +	test_i18ngrep "Applied autostash." err &&

> +	test_cmp file.5 file
> +'
> +
>  test_expect_success 'octopus merge with --autostash' '
>  	git reset --hard c1 &&
>  	git merge-file file file.orig file.3 &&

> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> Since 'git merge' learned '--autostash' in a03b55530a (merge: teach
> --autostash option, 2020-04-07), 'cmd_merge', in the fast-forward case,
> calls 'create_autostash' before calling 'checkout_fast_forward' if
> '--autostash' is given.
>
> However, if 'checkout_fast_forward' fails, the autostash is not applied
> to the working tree, nor saved in the stash list, since the code simply
> calls 'goto done'.
>
> Be more helpful to the user by applying the autostash in that case.
>
> An easy way to test a failing fast-forward is when we are merging a
> branch that has a tracked file that conflicts with an untracked file in
> the working tree.

>
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>  builtin/merge.c  |  1 +
>  t/t7600-merge.sh | 11 +++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 74797b6c7a6..788a6b0cd55 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1560,6 +1560,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  					  &head_commit->object.oid,
>  					  &commit->object.oid,
>  					  overwrite_ignore)) {
> +			apply_autostash(git_path_merge_autostash(the_repository));
>  			ret = 1;
>  			goto done;
>  		}
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 1cbc9715a81..216113d3537 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -122,6 +122,8 @@ test_expect_success 'setup' '
>  	c0=$(git rev-parse HEAD) &&
>  	cp file.1 file &&
>  	git add file &&
> +	cp file.1 other &&
> +	git add other &&
>  	test_tick &&
>  	git commit -m "commit 1" &&
>  	git tag c1 &&
> @@ -711,6 +713,15 @@ test_expect_success 'fast-forward merge with --autostash' '
>  	test_cmp result.1-5 file
>  '
>  
> +test_expect_success 'failed fast-forward merge with --autostash' '
> +	git reset --hard c0 &&
> +	git merge-file file file.orig file.5 &&
> +	cp file.5 other &&
> +	test_must_fail git merge --autostash c1 2>err &&
> +	test_i18ngrep "Applied autostash." err &&
> +	test_cmp file.5 file
> +'
> +
>  test_expect_success 'octopus merge with --autostash' '
>  	git reset --hard c1 &&
>  	git merge-file file file.orig file.3 &&

> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -154,7 +154,8 @@ endif::git-pull[]
>  --autostash::
>  --no-autostash::
>  	Automatically create a temporary stash entry before the operation
> -	begins, and apply it after the operation ends.  This means
> +	begins, record it in the special ref `MERGE_AUTOSTASH`
> +	and apply it after the operation ends.  This means
>  	that you can run the operation on a dirty worktree.  However, use
>  	with care: the final stash application after a successful
>  	merge might result in non-trivial conflicts.

> As explained in a03b55530a, and made more abvious by the tests added in
> that commit, the autostash is then applied if the merge succeeds, either
> directly or by committing (after conflict resolution or if '--no-commit'
> was given), or if the merge is aborted with 'git merge --abort'. In some
> other cases, like the user calling 'git reset --merge' or 'git merge
> --quit', the autostash is not applied, but saved in the stash list.

> However, there exists a scenario that creates an autostash but does not
> apply nor save it to the stash list: if the chosen merge strategy
> completely fails to handle the merge, i.e. 'try_merge_strategy' returns
> 2.
>
> Apply the autostash in that case also. An easy way to test that is to
> try to merge more than two commits but explicitely ask for the 'recursive'
> merge strategy.

> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>  builtin/merge.c  | 1 +
>  t/t7600-merge.sh | 8 ++++++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 788a6b0cd55..d44c14a21a3 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1709,6 +1709,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		else
>  			fprintf(stderr, _("Merge with strategy %s failed.\n"),
>  				use_strategies[0]->name);
> +		apply_autostash(git_path_merge_autostash(the_repository));
>  		ret = 2;
>  		goto done;
>  	} else if (best_strategy == wt_strategy)
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 216113d3537..2ef39d3088e 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -732,6 +732,14 @@ test_expect_success 'octopus merge with --autostash' '
>  	test_cmp result.1-3-5-9 file
>  '
>  
> +test_expect_success 'failed merge (exit 2) with --autostash' '
> +	git reset --hard c1 &&
> +	git merge-file file file.orig file.5 &&
> +	test_must_fail git merge -s recursive --autostash c2 c3 2>err &&
> +	test_i18ngrep "Applied autostash." err &&
> +	test_cmp result.1-5 file
> +'
> +
>  test_expect_success 'conflicted merge with --autostash, --abort restores stash' '
>  	git reset --hard c3 &&
>  	cp file.1 file &&

> This series aims to apply the stash created by 'git merge --autostash' in 2
> situations that were not covered by the code:
> 
>  1. If the merge is fast-forward but the fast-forward operation fails -
>     PATCH 3/4
>  2. If the merge strategy completely fails to handle the merge (exit code 2)
>     - PATCH 4/4
> 
> The first 2 commits are small improvements that I noticed while implementing
> the other two.
> 
> I'm marking it [RFC] because I'm not 100% sure that trying to apply the
> autostash in 3/4 and 4/4 is actually the best course of action (or if it
> would be better to call 'save_autostash' instead). That's because:
> 
> For 3/4 (fast-forward fails): I'm not sure if 'unpack_trees' (called by
> 'checkout_fast_forward') is guaranteed to fail atomically, or it might fail
> mid-way and leave the worktree unclean, in which case it might be better not
> to apply the autostash, but just save it instead (and tell the user). In the
> test case I'm adding, it does fail before starting to update the working
> tree, but I'm not sure if it's always the case.