Re: [PATCH] submodule merge: update conflict error message

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 7, 2022, 00:48 UTC
Message-ID: <xmqqh74x1eol.fsf@gitster.g>
In-Reply-To: <20220606235449.2890858-1-calvinwan@google.com>

Calvin Wan <calvinwan@google.com> writes:

Show 13 quoted lines

> When attempting to do a non-fast-forward merge in a project with
> conflicts in the submodules, the merge fails and git prints the
> following error:
>
> Failed to merge submodule <submodules>
> CONFLICT (submodule):Merge conflict in <submodule>
> Automatic merge failed; fix conflicts and then commit the result.
>
> Git is left in a conflicted state, which requires the user to:
>  1. abort the merge
>  2. merge submodules
>  3. merge superproject
> These steps are non-obvious for newer submodule users to figure out

Hmph.  Is 1. necessary?

IOW, based on the information we already have (we may not be surfacing, which can be corrected), wouldn't it be easier to instead (A) go to submodule and make a merge and then (B) come back to the superproject, "git add <submodule" to record the result of submodule merge, and say "git commit" to conclude?

The thing I am worried most about is that you may be throwing away information that would help the user by aborting the superproject merge. Before doing so, you have stage #2 and stage #3 of the submodule commit, so which commits in the submodule you need to merge in (A) above should be fairly clear. If you abort the merge first, how does the user know which commits in the submodule the user needs to merge?

> The error message is based off of what would happen when `merge
> --recurse-submodules` is eventually supported

OK.

Show 7 quoted lines

> Failed to merge submodule <submodule>
> CONFLICT (submodule): Merge conflict in <submodule>
> Automatic merge failed; recursive merging with submodules is currently
> not supported. To manually merge, the following steps are recommended:
>  - abort the current merge
>  - merge submodules individually
>  - merge superproject

Again, I am not sure about the recommendation. The message saying "currently not supported" I think is a good idea.

Show 5 quoted lines

> I considered automatically aborting the merge if git detects the merge
> failed because of a submodule conflict, however, doing so causes a
> significant amount of tests in `t7610-mergetool.sh` (and some other test
> scripts as well) to fail, suggesting users have come to expect this
> state and have their workarounds with `git mergetool`

With or without test failures, my gut feeling sais that it cannot be a good idea to automatically abort the merge, without first grabbing some information out of the conflicted state.

Thanks.