diff mbox series

[GSoC,v2] merge-strategies.adoc: detail submodule merge

Message ID 20250220151207.3248-1-lucasseikioshiro@gmail.com (mailing list archive)
State New
Headers show
Series [GSoC,v2] merge-strategies.adoc: detail submodule merge | expand

Commit Message

Lucas Seiki Oshiro Feb. 20, 2025, 3:12 p.m. UTC
Submodule merges are, in general, similar to other merges based on oid
three-way-merge. When a conflict happens, however, Git has two special
cases (introduced in 68d03e4a6e44) on handling the conflict before
yielding it to the user. From the merge-ort and merge-recursive sources:

- "Case #1: a is contained in b or vice versa": both strategies try to
perform a fast-forward in the submodules if the commit referred by the
conflicted submodule is descendant of another;

- "Case #2: There are one or more merges that contain a and b in the
submodule.  If there is only one, then present it as a suggestion to the
user, but leave it marked unmerged so the user needs to confirm the
resolution."

Add a small paragraph on merge-strategies.adoc describing this behavior.

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---

This v2 changes the documentation text to a clearer explanation (as
suggested in the v1 review), and changes its location to
merge-strategies.adoc instead of git-merge.adoc.

This content is duplicated as this works for both `ort` and `recursive`
strategies.

 Documentation/merge-strategies.adoc | 15 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

D. Ben Knoble Feb. 20, 2025, 6:46 p.m. UTC | #1
On Thu, Feb 20, 2025 at 10:14 AM Lucas Seiki Oshiro
<lucasseikioshiro@gmail.com> wrote:
>
> Submodule merges are, in general, similar to other merges based on oid
> three-way-merge. When a conflict happens, however, Git has two special
> cases (introduced in 68d03e4a6e44) on handling the conflict before
> yielding it to the user. From the merge-ort and merge-recursive sources:
>
> - "Case #1: a is contained in b or vice versa": both strategies try to
> perform a fast-forward in the submodules if the commit referred by the
> conflicted submodule is descendant of another;
>
> - "Case #2: There are one or more merges that contain a and b in the
> submodule.  If there is only one, then present it as a suggestion to the
> user, but leave it marked unmerged so the user needs to confirm the
> resolution."
>
> Add a small paragraph on merge-strategies.adoc describing this behavior.
>
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
>
> This v2 changes the documentation text to a clearer explanation (as
> suggested in the v1 review), and changes its location to
> merge-strategies.adoc instead of git-merge.adoc.

This version is clearer to me at least, thanks!

>
> This content is duplicated as this works for both `ort` and `recursive`
> strategies.
>
>  Documentation/merge-strategies.adoc | 15 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc
> index 5fc54ec060..a7fca249e2 100644
> --- a/Documentation/merge-strategies.adoc
> +++ b/Documentation/merge-strategies.adoc
> @@ -21,6 +21,13 @@ ort::
>         ("Ostensibly Recursive's Twin") and came from the fact that it
>         was written as a replacement for the previous default
>         algorithm, `recursive`.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +
>  The 'ort' strategy can take the following options:
>
> @@ -95,6 +102,13 @@ recursive::
>         renames.  It does not make use of detected copies.  This was
>         the default strategy for resolving two heads from Git v0.99.9k
>         until v2.33.0.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +

I'm not particularly a fan of duplicated documentation text: is there
a way to reuse one from the other or have one refer/link to the other?
Junio C Hamano Feb. 20, 2025, 7:12 p.m. UTC | #2
Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> Submodule merges are, in general, similar to other merges based on oid
> three-way-merge. When a conflict happens, however, Git has two special
> cases (introduced in 68d03e4a6e44) on handling the conflict before
> yielding it to the user. From the merge-ort and merge-recursive sources:
>
> - "Case #1: a is contained in b or vice versa": both strategies try to
> perform a fast-forward in the submodules if the commit referred by the
> conflicted submodule is descendant of another;
>
> - "Case #2: There are one or more merges that contain a and b in the
> submodule.  If there is only one, then present it as a suggestion to the
> user, but leave it marked unmerged so the user needs to confirm the
> resolution."
>
> Add a small paragraph on merge-strategies.adoc describing this behavior.
>
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
>
> This v2 changes the documentation text to a clearer explanation (as
> suggested in the v1 review), and changes its location to
> merge-strategies.adoc instead of git-merge.adoc.
>
> This content is duplicated as this works for both `ort` and `recursive`
> strategies.
>
>  Documentation/merge-strategies.adoc | 15 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc
> index 5fc54ec060..a7fca249e2 100644
> --- a/Documentation/merge-strategies.adoc
> +++ b/Documentation/merge-strategies.adoc
> @@ -21,6 +21,13 @@ ort::
>  	("Ostensibly Recursive's Twin") and came from the fact that it
>  	was written as a replacement for the previous default
>  	algorithm, `recursive`.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +
>  The 'ort' strategy can take the following options:

I am not going to comment on the text, but as to the formatting, I'd
point out that the existing

    +
    The 'ort' strategy can ...

construct, i.e. a line with only '+' on it, followed by left-aligned
block of text, is how AsciiDoc wants our second and subsequent
paragraphs for an enumerated list of explanations.  Here, the
existing "The 'ort' strategy can ..." is the second paragraph that
follows the paragraph that ends with "... previous default
algorithm, 'recursive'", which is the explanation for the item with
"ort::" heading.

So, you'd need to (1) change the empty line at the beginning of your
added text to have one '+' and nothing else on it, and (2) dedent
the rest of the text you added to abut the left edge of the page.
Ditto for the other hunk on "recursive::".

Thanks.
Jean-Noël Avila Feb. 21, 2025, 10:17 a.m. UTC | #3
Le 20/02/2025 à 16:12, Lucas Seiki Oshiro a écrit :
> Submodule merges are, in general, similar to other merges based on oid
> three-way-merge. When a conflict happens, however, Git has two special
> cases (introduced in 68d03e4a6e44) on handling the conflict before
> yielding it to the user. From the merge-ort and merge-recursive sources:
> 
> - "Case #1: a is contained in b or vice versa": both strategies try to
> perform a fast-forward in the submodules if the commit referred by the
> conflicted submodule is descendant of another;
> 
> - "Case #2: There are one or more merges that contain a and b in the
> submodule.  If there is only one, then present it as a suggestion to the
> user, but leave it marked unmerged so the user needs to confirm the
> resolution."
> 
> Add a small paragraph on merge-strategies.adoc describing this behavior.
> 
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
> 
> This v2 changes the documentation text to a clearer explanation (as
> suggested in the v1 review), and changes its location to
> merge-strategies.adoc instead of git-merge.adoc.
> 
> This content is duplicated as this works for both `ort` and `recursive`
> strategies.
> 
>  Documentation/merge-strategies.adoc | 15 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc
> index 5fc54ec060..a7fca249e2 100644
> --- a/Documentation/merge-strategies.adoc
> +++ b/Documentation/merge-strategies.adoc
> @@ -21,6 +21,13 @@ ort::
>  	("Ostensibly Recursive's Twin") and came from the fact that it
>  	was written as a replacement for the previous default
>  	algorithm, `recursive`.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +
>  The 'ort' strategy can take the following options:
>  
> @@ -95,6 +102,13 @@ recursive::
>  	renames.  It does not make use of detected copies.  This was
>  	the default strategy for resolving two heads from Git v0.99.9k
>  	until v2.33.0.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +
>  The 'recursive' strategy takes the same options as 'ort'.  However,
>  there are three additional options that 'ort' ignores (not documented


If both chunks are meant to be kept identical, I would recommend to
define an attribute (see
https://docs.asciidoctor.org/asciidoc/latest/attributes/custom-attributes/)
and use it at both sites.

Thanks.
Junio C Hamano Feb. 21, 2025, 6:07 p.m. UTC | #4
Jean-Noël Avila <jn.avila@free.fr> writes:

>> @@ -95,6 +102,13 @@ recursive::
>>  	renames.  It does not make use of detected copies.  This was
>>  	the default strategy for resolving two heads from Git v0.99.9k
>>  	until v2.33.0.
>> +
>> +        In the case where the path is a submodule, if the submodule commit
>> +        used on one side of the merge is a descendant of the submodule
>> +        commit used on the other side of the merge, Git attempts to
>> +        fast-forward to the descendant. Otherwise, Git will treat this case
>> +        as a conflict, suggesting as a resolution a submodule commit that
>> +        is descendant of the conflicting ones, if one exists.
>>  +
>>  The 'recursive' strategy takes the same options as 'ort'.  However,
>>  there are three additional options that 'ort' ignores (not documented
>
>
> If both chunks are meant to be kept identical, I would recommend to
> define an attribute (see
> https://docs.asciidoctor.org/asciidoc/latest/attributes/custom-attributes/)
> and use it at both sites.

Wouldn't it be a bit awkward to maintain a six-line paragraph as a
custom attribute, though [*1*]?  Would the resulting text become
like (without indentation) this?

  :submodule-merge: \
  In the case where the path is a submodule, if the submodule commit \
  used on one side of the merge is a descendant of the submodule \
  commit used on the other side of the merge, Git attempts to \
  fast-forward to the descendant. Otherwise, Git will treat this case \
  as a conflict, suggesting as a resolution a submodule commit that \
  is descendant of the conflicting ones, if one exists.

  recursive::
          ...
          the default strategy for resolving two heads from Git v0.99.9k
          until v2.33.0.
  +
  {submodule-merge}
  +
  The 'recursive strategy takes the same options as ...


Just as in C preprocessor macros in *.h files, I am reluctant to
force our people to edit long multi-line text while not forgetting
to lose the backslash for line continuation (or misplace existing
ones when wrapping lines).

And of course a 6-line paragraph is not large enough to put in a
separate file to be included.

Hmph...

Thanks.



[Reference]

*1* 
https://github.com/asciidoctor/asciidoctor/issues/1341#issuecomment-101841014
Elijah Newren Feb. 21, 2025, 9:56 p.m. UTC | #5
Hi,

On Thu, Feb 20, 2025 at 7:12 AM Lucas Seiki Oshiro
<lucasseikioshiro@gmail.com> wrote:
>
> Submodule merges are, in general, similar to other merges based on oid
> three-way-merge. When a conflict happens, however, Git has two special
> cases (introduced in 68d03e4a6e44) on handling the conflict before
> yielding it to the user. From the merge-ort and merge-recursive sources:
>
> - "Case #1: a is contained in b or vice versa": both strategies try to
> perform a fast-forward in the submodules if the commit referred by the
> conflicted submodule is descendant of another;
>
> - "Case #2: There are one or more merges that contain a and b in the
> submodule.  If there is only one, then present it as a suggestion to the
> user, but leave it marked unmerged so the user needs to confirm the
> resolution."
>
> Add a small paragraph on merge-strategies.adoc describing this behavior.
>
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
>
> This v2 changes the documentation text to a clearer explanation (as
> suggested in the v1 review), and changes its location to
> merge-strategies.adoc instead of git-merge.adoc.
>
> This content is duplicated as this works for both `ort` and `recursive`
> strategies.
>
>  Documentation/merge-strategies.adoc | 15 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc
> index 5fc54ec060..a7fca249e2 100644
> --- a/Documentation/merge-strategies.adoc
> +++ b/Documentation/merge-strategies.adoc
> @@ -21,6 +21,13 @@ ort::
>         ("Ostensibly Recursive's Twin") and came from the fact that it
>         was written as a replacement for the previous default
>         algorithm, `recursive`.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +
>  The 'ort' strategy can take the following options:
>
> @@ -95,6 +102,13 @@ recursive::
>         renames.  It does not make use of detected copies.  This was
>         the default strategy for resolving two heads from Git v0.99.9k
>         until v2.33.0.
> +
> +        In the case where the path is a submodule, if the submodule commit
> +        used on one side of the merge is a descendant of the submodule
> +        commit used on the other side of the merge, Git attempts to
> +        fast-forward to the descendant. Otherwise, Git will treat this case
> +        as a conflict, suggesting as a resolution a submodule commit that
> +        is descendant of the conflicting ones, if one exists.
>  +
>  The 'recursive' strategy takes the same options as 'ort'.  However,
>  there are three additional options that 'ort' ignores (not documented
> --
> 2.39.5 (Apple Git-154)

So, seeing it here, I note that these are meant a bit more as
high-level overviews of the algorithms.  I pushed you away from
including this in git-merge.adoc because while that manual page does
dive into merge resolution details, that manual page is specific to
merge.  The information here pertains to merge as well as cherry-pick,
rebase, revert, replay, merge-tree, etc.

We don't seem to have a place that is general for all
merge-machinery-using commands, and which also dives into details
about how merges are resolved.

I don't have a good solution.  I think it's probably fine to include
here in merge-strategies.adoc, even if it feels suboptimal and icky,
since any other current solution would be as well.  But I would be
interested in the opinions of other reviewers on this point and
whether they see a good solution (short of completely overhauling all
merge-related documentation for any merge-using-command, which might
be a viable strategy but shouldn't hold up a small patch like this).
diff mbox series

Patch

diff --git a/Documentation/merge-strategies.adoc b/Documentation/merge-strategies.adoc
index 5fc54ec060..a7fca249e2 100644
--- a/Documentation/merge-strategies.adoc
+++ b/Documentation/merge-strategies.adoc
@@ -21,6 +21,13 @@  ort::
 	("Ostensibly Recursive's Twin") and came from the fact that it
 	was written as a replacement for the previous default
 	algorithm, `recursive`.
+
+        In the case where the path is a submodule, if the submodule commit
+        used on one side of the merge is a descendant of the submodule
+        commit used on the other side of the merge, Git attempts to
+        fast-forward to the descendant. Otherwise, Git will treat this case
+        as a conflict, suggesting as a resolution a submodule commit that
+        is descendant of the conflicting ones, if one exists.
 +
 The 'ort' strategy can take the following options:
 
@@ -95,6 +102,13 @@  recursive::
 	renames.  It does not make use of detected copies.  This was
 	the default strategy for resolving two heads from Git v0.99.9k
 	until v2.33.0.
+
+        In the case where the path is a submodule, if the submodule commit
+        used on one side of the merge is a descendant of the submodule
+        commit used on the other side of the merge, Git attempts to
+        fast-forward to the descendant. Otherwise, Git will treat this case
+        as a conflict, suggesting as a resolution a submodule commit that
+        is descendant of the conflicting ones, if one exists.
 +
 The 'recursive' strategy takes the same options as 'ort'.  However,
 there are three additional options that 'ort' ignores (not documented