git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Calvin Wan <calvinwan@google.com>
Cc: git@vger.kernel.org, chooglen@google.com
Subject: Re: [PATCH] submodule merge: update conflict error message
Date: Mon, 06 Jun 2022 17:48:42 -0700	[thread overview]
Message-ID: <xmqqh74x1eol.fsf@gitster.g> (raw)
In-Reply-To: <20220606235449.2890858-1-calvinwan@google.com> (Calvin Wan's message of "Mon, 6 Jun 2022 23:54:49 +0000")

Calvin Wan <calvinwan@google.com> writes:

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

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

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

  reply	other threads:[~2022-06-07  0:48 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-06 23:54 [PATCH] submodule merge: update conflict error message Calvin Wan
2022-06-07  0:48 ` Junio C Hamano [this message]
2022-06-08 17:19   ` Calvin Wan
2022-06-08 17:34     ` Glen Choo
2022-06-08 18:01       ` Calvin Wan
2022-06-08 19:13         ` Junio C Hamano
2022-06-10 23:11 ` [PATCH v2] " Calvin Wan
2022-06-11  4:53   ` Elijah Newren
2022-06-11 17:08     ` Philippe Blain
2022-06-12  6:56       ` Elijah Newren
2022-06-13  2:03       ` Calvin Wan
2022-06-12 23:30     ` Junio C Hamano
2022-06-13  1:54     ` Calvin Wan
2022-06-29 22:40   ` [PATCH v3] " Calvin Wan
2022-06-30  2:40     ` Elijah Newren
2022-06-30 19:48       ` Calvin Wan
2022-07-01  4:27         ` Elijah Newren
2022-06-30 20:35     ` Glen Choo
2022-06-30 20:45       ` Glen Choo
2022-06-30 21:08       ` Calvin Wan
2022-07-12 23:19     ` [PATCH v4] " Calvin Wan
2022-07-13 18:11       ` Junio C Hamano
2022-07-17  2:46         ` Elijah Newren
2022-07-15 12:57       ` Johannes Schindelin
2022-07-16  6:22         ` Junio C Hamano
2022-07-17  2:44         ` Elijah Newren
2022-07-18 17:03         ` Calvin Wan
2022-07-18 21:43       ` [PATCH v5] " Calvin Wan
2022-07-19  6:39         ` Junio C Hamano
2022-07-19 19:30           ` Calvin Wan
2022-07-19 20:16             ` Junio C Hamano
2022-07-19  7:13         ` Junio C Hamano
2022-07-19 19:07           ` Calvin Wan
2022-07-19 20:30             ` Junio C Hamano
2022-07-25  6:05         ` Ævar Arnfjörð Bjarmason
2022-07-25 12:11           ` Ævar Arnfjörð Bjarmason
2022-07-25 22:03             ` Calvin Wan
2022-07-25 12:31         ` Ævar Arnfjörð Bjarmason
2022-07-25 21:27           ` Calvin Wan
2022-07-26 21:00         ` [PATCH v6] " Calvin Wan
2022-07-27  1:13           ` Elijah Newren
2022-07-27 22:00             ` Calvin Wan
2022-07-28  0:41               ` Elijah Newren
2022-07-28 19:06                 ` Calvin Wan
2022-07-27  9:20           ` Ævar Arnfjörð Bjarmason
2022-07-28 21:12           ` [PATCH v7] " Calvin Wan
2022-07-28 23:22             ` Ævar Arnfjörð Bjarmason
2022-07-29  0:24             ` Elijah Newren
2022-08-01 22:24               ` Calvin Wan
2022-08-01 12:06             ` Ævar Arnfjörð Bjarmason
2022-08-02  0:50             ` Junio C Hamano
2022-08-02 21:03               ` Calvin Wan
2022-08-02 21:11                 ` Junio C Hamano
2022-08-02 21:55                   ` Calvin Wan
2022-08-02 22:22                     ` Junio C Hamano
2022-08-04 19:51             ` [PATCH v8] " Calvin Wan
2022-08-16 15:58               ` Junio C Hamano
2022-08-16 18:58                 ` Junio C Hamano
2022-08-16 19:34                   ` Calvin Wan
2022-08-16 19:39                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh74x1eol.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=calvinwan@google.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).