{"thread":{"id":"62966","subject":"[GSoC][RFC PATCH] git-merge.adoc: detail submodule merge","startedAt":"2025-02-17T23:28:56Z","lastAt":"2025-02-19T14:54:56Z","messageCount":5,"participants":["Lucas Seiki Oshiro","Elijah Newren","Lucas Oshiro"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512548","messageId":"20250217232847.8567-1-lucasseikioshiro@gmail.com","threadId":"62966","inReplyTo":null,"subject":"[GSoC][RFC PATCH] git-merge.adoc: detail submodule merge","fromName":"Lucas Seiki Oshiro","fromEmail":"lucasseikioshiro@gmail.com","sentAt":"2025-02-17T23:28:47Z","receivedAt":"2025-02-17T23:28:56Z","isPatch":true,"sender":{"key":"lucasseikioshiro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701580?v=4"},"body":"Submodule merges are, in general, similar to other merges based on oid\nthree-way-merge. When a conflict happens, however, Git has two special\ncases on handling the conflict before yielding it to the user. From the\nmerge-ort and merge-recursive sources:\n\n- \"Case #1: a is contained in b or vice versa\": both strategies try to\nperform a fast-forward in the submodules if the commit referred by the\nconflicted submodule is descendant of another;\n\n- \"Case #2: There are one or more merges that contain a and b in the\nsubmodule.  If there is only one, then present it as a suggestion to the\nuser, but leave it marked unmerged so the user needs to confirm the\nresolution.\"\n\nAdd a small paragraph on git-merge.adoc describing this behavior.\n\nSigned-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>\n---\n\nHi,\n\nThis is a \"scratch-my-own-itch\" documentation patch. Some years ago I was\nquestioned why some submodule merges on GitHub lead to conflicts while locally\nthey didn't. I only could find a answer for that reading the merge-ort source\ncode, then a wrote a blog post about that, which you check here:\nhttps://lucasoshiro.github.io/posts-en/2022-03-12-merge-submodule/\n\nThus, this patch adds to the official documentation what I found at the time.\nI wasn't certain if this should belong to the submodule or merge documentation,\nso, by now, I'm sending it as a merge documentation patch.\n\n Documentation/git-merge.adoc | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/Documentation/git-merge.adoc b/Documentation/git-merge.adoc\nindex 64281d6d44..7b12c0d648 100644\n--- a/Documentation/git-merge.adoc\n+++ b/Documentation/git-merge.adoc\n@@ -205,6 +205,13 @@ happens:\n    same and the index entries for them stay as they were,\n    i.e. matching `HEAD`.\n \n+In the case where the path is a submodule, if the commit referred by it in HEAD\n+is descendant of the one referred by it in MERGE_HEAD or vice-versa, Git\n+attempts to fast-forward to the descendant, when using `ort` or `recursive`\n+strategies. Otherwise, Git will treat this case as a conflict, suggesting as a\n+resolution a submodule commit that is descendant of the conflicting ones, if one\n+exists.\n+\n If you tried a merge which resulted in complex conflicts and\n want to start over, you can recover with `git merge --abort`.\n \n-- \n2.39.5 (Apple Git-154)\n\n"},{"id":"512549","messageId":"CABPp-BGpOi9ZqrPFwxCiRLFjozgnYKV=zbYVAA5c9Z9yDUTBGg@mail.gmail.com","threadId":"62966","inReplyTo":"20250217232847.8567-1-lucasseikioshiro@gmail.com","subject":"Re: [GSoC][RFC PATCH] git-merge.adoc: detail submodule merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-02-18T02:18:31Z","receivedAt":"2025-02-18T02:18:43Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Feb 17, 2025 at 3:29 PM Lucas Seiki Oshiro\n<lucasseikioshiro@gmail.com> wrote:\n>\n> Submodule merges are, in general, similar to other merges based on oid\n> three-way-merge. When a conflict happens, however, Git has two special\n> cases on handling the conflict before yielding it to the user. From the\n> merge-ort and merge-recursive sources:\n>\n> - \"Case #1: a is contained in b or vice versa\": both strategies try to\n> perform a fast-forward in the submodules if the commit referred by the\n> conflicted submodule is descendant of another;\n>\n> - \"Case #2: There are one or more merges that contain a and b in the\n> submodule.  If there is only one, then present it as a suggestion to the\n> user, but leave it marked unmerged so the user needs to confirm the\n> resolution.\"\n>\n> Add a small paragraph on git-merge.adoc describing this behavior.\n\nIt may be worth referencing the commit(s) that introduced the behavior\nfor other reviewers: commit 68d03e4a6e44 (Implement automatic\nfast-forward merge for submodules, 2010-07-07).\n\n> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>\n> ---\n>\n> Hi,\n>\n> This is a \"scratch-my-own-itch\" documentation patch. Some years ago I was\n> questioned why some submodule merges on GitHub lead to conflicts while locally\n> they didn't. I only could find a answer for that reading the merge-ort source\n> code, then a wrote a blog post about that, which you check here:\n> https://lucasoshiro.github.io/posts-en/2022-03-12-merge-submodule/\n>\n> Thus, this patch adds to the official documentation what I found at the time.\n> I wasn't certain if this should belong to the submodule or merge documentation,\n> so, by now, I'm sending it as a merge documentation patch.\n\nThe merge_submodule() function was moved years ago from the submodule\ncode to the merge code -- see 18cfc088661 (\"submodule.c: move\nsubmodule merging to merge-recursive.c\", 2018-05-15)\n-- so to me the documentation makes more sense to be associated with\nmerging than with submodules.\n\nHowever, a bigger problem I see is that documenting how the merge\nmachinery works is not really specific to `git merge` but is general\nto any command that uses the merge machinery.  Thus, it also applies\nfor cherry-pick, merge-tree, rebase, replay, and revert. But I don't\nknow where that kind of general how-merging-logic-behaves\ndocumentation should go...\n\n>  Documentation/git-merge.adoc | 7 +++++++\n>  1 file changed, 7 insertions(+)\n>\n> diff --git a/Documentation/git-merge.adoc b/Documentation/git-merge.adoc\n> index 64281d6d44..7b12c0d648 100644\n> --- a/Documentation/git-merge.adoc\n> +++ b/Documentation/git-merge.adoc\n> @@ -205,6 +205,13 @@ happens:\n>     same and the index entries for them stay as they were,\n>     i.e. matching `HEAD`.\n>\n> +In the case where the path is a submodule, if the commit referred by it in HEAD\n> +is descendant of the one referred by it in MERGE_HEAD or vice-versa, Git\n\n\"referred by it\" is hard for me to parse.  Maybe something like\n\n\"\"\"\nIn the case where the path is a submodule, if the HEAD version of the\nsubmodule is a descendant of the MERGE_HEAD version of the submodule,\nor vice-versa, Git...\n\"\"\"\n?\n\nAlso, the references to HEAD and MERGE_HEAD do tie this documentation\nrather directly to `git merge`; the basic idea is applicable to all\ncallers of the merge machinery, but none of the other callers use\nMERGE_HEAD (some use CHERRY_PICK_HEAD or REBASE_HEAD), and some do not\nassume HEAD points to one of the parents either (e.g. merge-tree and\nreplay).  So, if we want to move this somewhere more general, we'd\nneed to reword it a bit.\n\n> +attempts to fast-forward to the descendant, when using `ort` or `recursive`\n> +strategies.\n\nOh, maybe we could put this information in\nDocumentation/merge-strategies.txt?  Hmm....\n\n> Otherwise, Git will treat this case as a conflict, suggesting as a\n> +resolution a submodule commit that is descendant of the conflicting ones, if one\n> +exists.\n\nThanks for sending this in.  It's always helpful to get researched\ndocumentation improvements, even if I can't help but nitpick and\ncomplicate matters here and there....  ;-)\n"},{"id":"512633","messageId":"EE88CF05-9120-4CAB-A097-84CC5EA2FACE@gmail.com","threadId":"62966","inReplyTo":"CABPp-BGpOi9ZqrPFwxCiRLFjozgnYKV=zbYVAA5c9Z9yDUTBGg@mail.gmail.com","subject":"Re: [GSoC][RFC PATCH] git-merge.adoc: detail submodule merge","fromName":"Lucas Oshiro","fromEmail":"lucasseikioshiro@gmail.com","sentAt":"2025-02-18T18:54:57Z","receivedAt":"2025-02-18T18:55:10Z","isPatch":true,"sender":{"key":"lucasseikioshiro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701580?v=4"},"body":"\n> It may be worth referencing the commit(s) that introduced the behavior\n> for other reviewers: commit 68d03e4a6e44 (Implement automatic\n> fast-forward merge for submodules, 2010-07-07).\n\nOk! I'll inspect the codebase and reference it in a future v2.\n\n> \"referred by it\" is hard for me to parse.  Maybe something like\n> \n> \"\"\"\n> In the case where the path is a submodule, if the HEAD version of the\n> submodule is a descendant of the MERGE_HEAD version of the submodule,\n> or vice-versa, Git...\n> \"\"\"\n> ?\n\nPerfect! Actually, I find submodules a little abstract to be explained\nusing only words, but your sentence is very clear.\n\n> Also, the references to HEAD and MERGE_HEAD do tie this documentation\n> rather directly to `git merge`; the basic idea is applicable to all\n> callers of the merge machinery, but none of the other callers use\n> MERGE_HEAD (some use CHERRY_PICK_HEAD or REBASE_HEAD), and some do not\n> assume HEAD points to one of the parents either (e.g. merge-tree and\n> replay).  So, if we want to move this somewhere more general, we'd\n> need to reword it a bit.\n\nGiven your previous suggestion, what about:\n\n\"\"\"\nIn the case where the path is a submodule, if one of the versions of\nsubmodule is descendant of another, Git...\n\"\"\"\n\n?\n\n> Oh, maybe we could put this information in\n> Documentation/merge-strategies.txt?  Hmm....\n\nLooks like a good place to put this. My only concerns are:\n\n1. It would need to be documented in both `ort` and `recursive`. I don't\nthink it would be a big deal as most of the first paragraph of both\nstrategies are equal.\n\n2. Maybe it would detail too much on this specific case, while not\ncovering others (e.g. changing file permissions, symlinks, etc).\n\n> Thanks for sending this in.  It's always helpful to get researched\n> documentation improvements, even if I can't help but nitpick and\n> complicate matters here and there....  ;-)\n\nThank you! Given how deeply you understand the merge machinery any nitpick\nis immensely valuable!\n\n"},{"id":"512643","messageId":"CABPp-BHvutRL80QiTYx6gJbYiATOD32DrvXgQQtEy=u5oUZ9Zg@mail.gmail.com","threadId":"62966","inReplyTo":"EE88CF05-9120-4CAB-A097-84CC5EA2FACE@gmail.com","subject":"Re: [GSoC][RFC PATCH] git-merge.adoc: detail submodule merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-02-18T21:58:57Z","receivedAt":"2025-02-18T21:59:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Feb 18, 2025 at 10:55 AM Lucas Oshiro\n<lucasseikioshiro@gmail.com> wrote:\n>\n> > It may be worth referencing the commit(s) that introduced the behavior\n> > for other reviewers: commit 68d03e4a6e44 (Implement automatic\n> > fast-forward merge for submodules, 2010-07-07).\n>\n> Ok! I'll inspect the codebase and reference it in a future v2.\n>\n> > \"referred by it\" is hard for me to parse.  Maybe something like\n> >\n> > \"\"\"\n> > In the case where the path is a submodule, if the HEAD version of the\n> > submodule is a descendant of the MERGE_HEAD version of the submodule,\n> > or vice-versa, Git...\n> > \"\"\"\n> > ?\n>\n> Perfect! Actually, I find submodules a little abstract to be explained\n> using only words, but your sentence is very clear.\n>\n> > Also, the references to HEAD and MERGE_HEAD do tie this documentation\n> > rather directly to `git merge`; the basic idea is applicable to all\n> > callers of the merge machinery, but none of the other callers use\n> > MERGE_HEAD (some use CHERRY_PICK_HEAD or REBASE_HEAD), and some do not\n> > assume HEAD points to one of the parents either (e.g. merge-tree and\n> > replay).  So, if we want to move this somewhere more general, we'd\n> > need to reword it a bit.\n>\n> Given your previous suggestion, what about:\n>\n> \"\"\"\n> In the case where the path is a submodule, if one of the versions of\n> submodule is descendant of another, Git...\n> \"\"\"\n>\n> ?\n\nThat seems like the right direction, but I think \"descendant of\nanother\" is vague/confusing.  Perhaps\n\n\"\"\"\nIn the case where the path is a submodule, if the submodule commit used on\none side of the merge is a descendant of the submodule commit used on the\nother side of the merge, Git...\n\"\"\"\n\n?\n\n\n>\n> > Oh, maybe we could put this information in\n> > Documentation/merge-strategies.txt?  Hmm....\n>\n> Looks like a good place to put this. My only concerns are:\n>\n> 1. It would need to be documented in both `ort` and `recursive`. I don't\n> think it would be a big deal as most of the first paragraph of both\n> strategies are equal.\n\nYes, until `recursive` is deleted anyway.  (At which point we'll just\nremap `recursive` to mean `ort` and not have to have separate\ndocumentation for the two.)\n\n> 2. Maybe it would detail too much on this specific case, while not\n> covering others (e.g. changing file permissions, symlinks, etc).\n\nYeah, but we don't have a way to resolve differences for those kinds\nof changes when neither side matches the base version (unless\nsomething like -Xours or -Xtheirs is passed, but even then that\nbelongs under the -X documentation); submodules are somewhat special\nin that regard.\n\n> > Thanks for sending this in.  It's always helpful to get researched\n> > documentation improvements, even if I can't help but nitpick and\n> > complicate matters here and there....  ;-)\n>\n> Thank you! Given how deeply you understand the merge machinery any nitpick\n> is immensely valuable!\n"},{"id":"512712","messageId":"522E8293-5248-497C-B4EB-D9C6C8251D04@gmail.com","threadId":"62966","inReplyTo":"CABPp-BHvutRL80QiTYx6gJbYiATOD32DrvXgQQtEy=u5oUZ9Zg@mail.gmail.com","subject":"Re: [GSoC][RFC PATCH] git-merge.adoc: detail submodule merge","fromName":"Lucas Oshiro","fromEmail":"lucasseikioshiro@gmail.com","sentAt":"2025-02-19T14:54:40Z","receivedAt":"2025-02-19T14:54:56Z","isPatch":true,"sender":{"key":"lucasseikioshiro@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701580?v=4"},"body":"\n> \"\"\"\n> In the case where the path is a submodule, if the submodule commit used on\n> one side of the merge is a descendant of the submodule commit used on the\n> other side of the merge, Git...\n> \"\"\"\n\nLooks good to me!\n\n> Yes, until `recursive` is deleted anyway.  (At which point we'll just\n> remap `recursive` to mean `ort` and not have to have separate\n> documentation for the two.)\n\nOk. I'll write for both `ort` and `recursive`.\n\n> Yeah, but we don't have a way to resolve differences for those kinds\n> of changes when neither side matches the base version (unless\n> something like -Xours or -Xtheirs is passed, but even then that\n> belongs under the -X documentation); submodules are somewhat special\n> in that regard.\n\nThanks again! I'll send a v2 soon.\n\n"}]}