Re: Please add support for "git merge --continue -S"

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Feb 28 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> [...]
> There is an argument that it makes it somehow "safer" to use "merge
> --continue" because the command fails when there is no interrupted
> merge going on, but what the user sees from "git commit" when there
> is and there is not interrupted merge are so different, there is not
> much "safety" benefit in practice.  We probably should deprecate and
> eventually remove "git merge --continue" eventually, but one step at
> a time.

If you run "git status" it'll look like you have a bunch of stuff
staged, and it's easy to miss if it's telling you you're in a merge
conflict or not (especially if it scrolls off the screen).

If we're just taking personal experience into account when deciding if
something improves safety I've made that mistake more than once & more
than twice. I.e. done a parents=1 commit when I thought I was in a merge
(maybe confused about what terminal I was in etc).

If someone with N number of commits in this project can make that
mistake, I daresay it helps some regular users too :)

> diff --git c/Documentation/git-merge.txt w/Documentation/git-merge.txt
> index 3125473cc1..95f252598e 100644
> --- c/Documentation/git-merge.txt
> +++ w/Documentation/git-merge.txt
> @@ -122,9 +122,9 @@ list.
>  	stash entry will be saved to the stash list.
>  
>  --continue::
> -	After a 'git merge' stops due to conflicts you can conclude the
> -	merge by running 'git merge --continue' (see "HOW TO RESOLVE
> -	CONFLICTS" section below).
> +	After a 'git merge' stops due to conflicts, you can conclude
> +	the merge with "git commit" (see "HOW TO RESOLVE CONFLICTS"
> +	section below).  'git merge --continue' is a synonym for it.

Saying it's a synonym isn't correct, and also now contradicts the last
paragraph of the DESCRIPTION section. Saying something like:

    'git merge --continue' will run 'git commit', after first checking
    whether a conflicted merge is underway.

Would be correct, consistent with DESCRIPTION, and basically paraphrases
what it says there.

>  <commit>...::
>  	Commits, usually other branch heads, to merge into our branch.
> @@ -326,10 +326,9 @@ After seeing a conflict, you can do two things:
>  
>   * Resolve the conflicts.  Git will mark the conflicts in
>     the working tree.  Edit the files into shape and
> -   'git add' them to the index.  Use 'git commit' or
> -   'git merge --continue' to seal the deal. The latter command
> -   checks whether there is a (interrupted) merge in progress
> -   before calling 'git commit'.
> +   'git add' them to the index.  Use 'git commit' (or
> +   'git merge --continue', which stops if there is no 
> +   interrupted merge in progress) to seal the deal.
>  
>  You can work through the conflict with a number of tools:

I think the former hunk with a minor edit is an improvement, i.e. let's
say 'git commit' works too.

But losing the description of the difference between the two here in the
more detailed section, whose job it is to explain the minor details,
makes the documentation worse IMO.

The "seal the deal" wording in the pre-image is a bit odd and
inconsistent with our general tone, that's worth changing.

But there should still be some variant of "'merge --continue', unlike
'commit' will abort if no merge is in progress".

Aside: After the last time this came up & looking at this again I looked
at some of the tests, and I'm entirely confused about what f8b863598c9
(builtin/merge: honor commit-msg hook for merges, 2017-09-07) is talking
about.

I.e. it's "just a synonym", but it seems to claim that "merge
--continue" somehow remembers --allow-unrelated-histories, but not
--no-verify (for which we have a TODO test).

If you're set on deprecating it, as opposed to us supporting "merge
--continue -S" or whatever, amending/removing that TODO test seem like a
good addition.

Also, after this came up the other day I came up with this WIP to have
the "reflog" reflect what command we ran, just as we do with rebase.

IMO that really makes the difference beween the two worth it. E.g. for
my git.git integration branch running 'git reflog' and seeing at a
glance wheher something was a conflicted merge or not (as long as I
consistently use "merge --continue", which I do) really aids
readability:

diff --git a/builtin/merge.c b/builtin/merge.c
index 74e53cf20a7..3cbb47c96f9 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1362,6 +1362,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (!file_exists(git_path_merge_head(the_repository)))
 			die(_("There is no merge in progress (MERGE_HEAD missing)."));
 
 		/* Invoke 'git commit' */
+		setenv("GIT_REFLOG_ACTION", "merge (continue)", 0);
 		ret = cmd_commit(nargc, nargv, prefix);
 		goto done;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index f0f6fda150b..f4a0b70a213 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -772,6 +772,8 @@ test_expect_success 'completed merge (git merge --continue) with --no-commit and
 	git stash show -p MERGE_AUTOSTASH >actual &&
 	test_cmp expect actual &&
 	git merge --continue 2>err &&
+	git reflog -1 >reflog &&
+	grep -F "merge (continue)" reflog &&
 	test_i18ngrep "Applied autostash." err &&
 	git show HEAD:file >merge-result &&
 	test_cmp result.1-5 merge-result &&