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

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

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

Hi,

I usually sign my commits and merges. But when a merge has collisions I can't 
continue and sign in one step, because --continue and -S seem to be mutually 
exclusive. My way out is:

git merge --continue
git commit --amend --no-edit -S

... but this could be a bit simpler. Or maybe --continue could remember if I 
have passed "-S" to the original "git merge" command.

Regards,

Eike
-- 
Rolf Eike Beer, emlix GmbH, https://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 313 bytes --]

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

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 28 2022, Rolf Eike Beer wrote:

> [[PGP Signed Part:Undecided]]
> Hi,
>
> I usually sign my commits and merges. But when a merge has collisions I can't 
> continue and sign in one step, because --continue and -S seem to be mutually 
> exclusive. My way out is:
>
> git merge --continue
> git commit --amend --no-edit -S
>
> ... but this could be a bit simpler. Or maybe --continue could remember if I 
> have passed "-S" to the original "git merge" command.
>
> Regards,

You can just drop the use of "merge --continue" entirely and use
"commit" instead.

Caveats related to this were recently discussed on-list:
https://lore.kernel.org/git/CALRdAfcyfesNqfLhhe2GW_5V9s2hf++i6mZS1Lw5hqQYTca85w@mail.gmail.com/

As noted there I think this makes for confusing UI, but you should able to just:

    git commit -S

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> You can just drop the use of "merge --continue" entirely and use
> "commit" instead.
>
> Caveats related to this were recently discussed on-list:
> https://lore.kernel.org/git/CALRdAfcyfesNqfLhhe2GW_5V9s2hf++i6mZS1Lw5hqQYTca85w@mail.gmail.com/

Ah, that one.  We need to close the #leftoverbits on the topic.
Here is a starter.

----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: merge: 'git merge --continue' is merely 'git commit'

Among the commands with "--continue", "merge --continue" came much
later, and it did not even need to exist.  The other commands with
"--continue", e.g. "rebase", deal with multi-step operations, and it
is worth to have a way to say "I am finished with this step, let's
CONTINUE WITH THE REST".  But in "merge", there is no remaining
thing to do after you are done with the conflict you saw.

In hindsight, we probably should have resisted the urge to add
"merge --continue", just for the sake of misguided "consistency"
perceived on non-existent similarity with other commands that truly
need "--continue".  What is called "merge --continue" should have
been called "merge --finish", if we needed to add something back
then.

The way to finish a conflicted merge has always been to run "git
commit" before "merge --continue" was added, and it still is not
just accepted but is the right way to finish a conflicted merge.

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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-merge.txt | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

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

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

[Ævar Arnfjörð Bjarmason]

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