{"thread":{"id":"48363","subject":"git merge banch w/ different submodule revision","startedAt":"2018-04-26T10:59:38Z","lastAt":"2018-05-07T14:23:29Z","messageCount":16,"participants":["Middelschulte, Leif","Stefan Beller","Jacob Keller","Elijah Newren","Heiko Voigt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"345892","messageId":"1524739599.20251.17.camel@klsmartin.com","threadId":"48363","inReplyTo":null,"subject":"git merge banch w/ different submodule revision","fromName":"Middelschulte, Leif","fromEmail":"leif.middelschulte@klsmartin.com","sentAt":"2018-04-26T10:49:42Z","receivedAt":"2018-04-26T10:59:38Z","isPatch":false,"sender":{"key":"leif.middelschulte@klsmartin.com","avatar":null},"body":"Hi,\n\nwe're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git.\n\nAssume the following setup:\n\n- Repository `S` is sourced by repository `p` as submodule `s`\n- Repository `p` has two branches: `feature_x` and `develop`\n- The revisions sourced via the submodule have a linear history\n\n\n* 1c1d38f (feature_x) update submodule revision to b17e9d9\n| * 3290e69 (HEAD -> develop) update submodule revision to 0598394\n|/ \n* cd5e1a5 initial submodule revision\n\n\nProblem case: Merge either branch into the other\n\nExpected behavior: Merge conflict.\n\nActual behavior: Auto merge without conflicts.\n\nNote 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history\n\nDid I get something wrong about how git resolves merges? Shouldn't git be like: \"hey, you're trying to merge two different contents for the same line\" (the submodule's revision)\n\nThanks in advance,\n\nLeif"},{"id":"345909","messageId":"CAGZ79kZA_R-5bA6mPdoHkVW-C21pNn_0x6FayhuuXqnOTrmjWw@mail.gmail.com","threadId":"48363","inReplyTo":"1524739599.20251.17.camel@klsmartin.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-26T17:56:56Z","receivedAt":"2018-04-26T17:57:01Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif\n wrote:\n> Hi,\n>\n> we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git.\n>\n> Assume the following setup:\n>\n> - Repository `S` is sourced by repository `p` as submodule `s`\n> - Repository `p` has two branches: `feature_x` and `develop`\n> - The revisions sourced via the submodule have a linear history\n>\n>\n> * 1c1d38f (feature_x) update submodule revision to b17e9d9\n> | * 3290e69 (HEAD -> develop) update submodule revision to 0598394\n> |/\n> * cd5e1a5 initial submodule revision\n>\n>\n> Problem case: Merge either branch into the other\n>\n> Expected behavior: Merge conflict.\n>\n> Actual behavior: Auto merge without conflicts.\n>\n> Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history\n>\n> Did I get something wrong about how git resolves merges?\n\nWe often treating a submodule as a file from the superproject, but not always.\nAnd in case of a merge, git seems to be a bit smarter than treating it\nas a textfile\nwith two different lines.\n\nSee https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd\n(68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07)\nto explain the situation you encounter. (specifically merge_submodule\nat the end of the diff)\n\n> Shouldn't git be like: \"hey, you're trying to merge two different contents for the same line\" (the submodule's revision)\n\nAs we have a history in the submodule we can do more than that and\nresolve the conflict.\n\nFor two lines, you usually need manual intervention (which line to\npick, or craft a complete\nnew line out of parts of each line?), whereas for submodule commits\nyou can reason\nabout their dependencies due to their history and not just look at the\ntextual conflict.\n\nStefan\n"},{"id":"345917","messageId":"CA+P7+xrUwq0G2YySC3SLKqyihhPnFPCiQnQpoVVa89+=W9O9+w@mail.gmail.com","threadId":"48363","inReplyTo":"CAGZ79kZA_R-5bA6mPdoHkVW-C21pNn_0x6FayhuuXqnOTrmjWw@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-04-26T21:46:32Z","receivedAt":"2018-04-26T21:47:09Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller wrote:\n> We often treating a submodule as a file from the superproject, but not always.\n> And in case of a merge, git seems to be a bit smarter than treating it\n> as a textfile\n> with two different lines.\n\nSure, but a submodule is checked out \"at a commit\", so if two branches\nof history are merged, and they conflict over which place the\nsubmodule is at.... shouldn't that produce a conflict??\n\nI mean, how is the merge algorithm supposed to know which is right?\nThe patch you linked appears to be able to resolve it to the one which\ncontains both commits.. but that may not actually be true since you\ncan rewind submodules since they're *pointers* to commits, not commits\nthemselves.\n\nI'm not against that as a possible strategy to merge submodules, but\nit seems like not necessarily something you would always want...\n\nThanks,\nJake\n"},{"id":"345919","messageId":"CAGZ79kaub2k-q-Mcj3H5o6ekyZ8ZZzG7+r5sHt5Ne25Nc3_nPQ@mail.gmail.com","threadId":"48363","inReplyTo":"CA+P7+xrUwq0G2YySC3SLKqyihhPnFPCiQnQpoVVa89+=W9O9+w@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-26T22:19:36Z","receivedAt":"2018-04-26T22:19:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Stefan wrote:\n> See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd\n> (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07)\n> to explain the situation you encounter. (specifically merge_submodule\n> at the end of the diff)\n\n+cc Heiko, author of that commit.\n\nOn Thu, Apr 26, 2018 at 2:46 PM, Jacob Keller wrote:\n> On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller wrote:\n>> We often treating a submodule as a file from the superproject, but not always.\n>> And in case of a merge, git seems to be a bit smarter than treating it\n>> as a textfile with two different lines.\n>\n> Sure, but a submodule is checked out \"at a commit\", so if two branches\n> of history are merged, and they conflict over which place the\n> submodule is at.... shouldn't that produce a conflict??\n\nStepping back a little bit:\n\nWhen two branches developed a file differently, they can be merged\niff they do not change the same lines (plus a little bit of margin of 1\nextra line)\n\nThat is the builtin merge-driver for \"plain text files\" and seems to be accepted\nwidely as \"good enough\" or \"that is how git merges\".\n\nWhat if this text file happens to be the .gitmodules file and the changed lines\nhappen to be 2 options in there (Say one option was the path, as one branch\nrenamed the submodule, and the other option is submodule.branch) ?\n\nThen we could do better as we know the structure of the file. We would not\nneed the extra buffer line as a cautious step, but instead could parse both\nsides of the merge and merge each config in-memory and then write out\na .gitmodules file. I think David Turner proposed a custom merge driver\nfor .gitmodules a couple month ago.\n\nAnother example is the merge code respecting renames on one side\n(even for directories) and edits in the other side. Technically the rename\nof a file is a \"delete of all lines in this path\", which could also argued to\njust conflict with the edit on the other side.\n\nWith these examples given, I think it is legit to treat submodule changes\nnot as \"two lines of text differ at the same place, mark it as conflict\",\nbut we are allowed to be smarter about it.\n\n> I mean, how is the merge algorithm supposed to know which is right?\n\nGood question. As said above, the merge algorithm for text files is just\ncorrect for \"plain text files\". In source code, I can give an example\nwhich merges fine, but doesn't compile after merging: One side changes\na function signature and the other side adds a call to the function (still using\nthe old signature).\n\nHere you can see that our merge algorithm is wrong. It sucks.\nThe solution is a custom merge driver for C code (or whatever\nlanguage you happen to use).\n\nFor submodules, the given commit made the assumption that\nprogressing in history of a submodule is never bad, i.e. there are\nno reverts and no bugs introduced, only perfect features are added\nby new submodule commits. (I don't know which assumptions were\nactually made, I made this up).\n\nMaybe we need to revisit that decision?\n\n> The patch you linked appears to be able to resolve it to the one which\n> contains both commits.. but that may not actually be true since you\n> can rewind submodules since they're *pointers* to commits, not commits\n> themselves.\n\nRight, and that is the problem, as the pointer is a small thing, which\ndoesn't allow for the dumb text merging strategy that is used in files.\n\nSo we could always err out and have the user make a decision.\nOr we could provide a basic merge driver for submodules (which\nwas implemented in that commit).\n\nIf you use a different workflow this doesn't work for you, so\nobviously you want a different custom merge driver for\nsubmodules?\n\n> I'm not against that as a possible strategy to merge submodules, but\n> it seems like not necessarily something you would always want...\n\nI agree that it is reasonable to want different things, just like\nwanting a merge driver that works better with C code.\n(side note: I am rebasing a large series currently and one of the\nfrequent conflicts were different #includes at the top of a file.\nYou could totally automate merging that :/)\n\nThanks,\nStefan\n"},{"id":"345927","messageId":"CABPp-BG0zjcCuO1q7ek5LH0oh+nruKhB7etqb9ZhtbjSCqpMVg@mail.gmail.com","threadId":"48363","inReplyTo":"CA+P7+xrUwq0G2YySC3SLKqyihhPnFPCiQnQpoVVa89+=W9O9+w@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-04-27T00:02:31Z","receivedAt":"2018-04-27T00:02:37Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Apr 26, 2018 at 2:46 PM, Jacob Keller wrote:\n> On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller wrote:\n>> We often treating a submodule as a file from the superproject, but not always.\n>> And in case of a merge, git seems to be a bit smarter than treating it\n>> as a textfile\n>> with two different lines.\n>\n> Sure, but a submodule is checked out \"at a commit\", so if two branches\n> of history are merged, and they conflict over which place the\n> submodule is at.... shouldn't that produce a conflict??\n\nBy \"which place a submodule is at\", do you mean the commit it points\nto, or the path at which the submodule is found within the parent\nrepository? Continuing on it sounds like you meant the former, but I\nwas unsure if you were asking mutliple different questions here.\n\n> I mean, how is the merge algorithm supposed to know which is right?\n> The patch you linked appears to be able to resolve it to the one which\n> contains both commits.. but that may not actually be true since you\n> can rewind submodules since they're *pointers* to commits, not commits\n> themselves.\n\nOnly if both commits also contain the base; see lines 328 to 332 of\nthat patch. So, if the submodules are rewound, that algorithm would\nleave them as conflicted.\n"},{"id":"345928","messageId":"CABPp-BE5jRG8JdDfH1XG-Btz9jJxfwf_oyNni8Ci1j+J3icbVQ@mail.gmail.com","threadId":"48363","inReplyTo":"1524739599.20251.17.camel@klsmartin.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-04-27T00:19:20Z","receivedAt":"2018-04-27T00:19:28Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif\n wrote:\n> Hi,\n>\n> we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git.\n>\n> Assume the following setup:\n>\n> - Repository `S` is sourced by repository `p` as submodule `s`\n> - Repository `p` has two branches: `feature_x` and `develop`\n> - The revisions sourced via the submodule have a linear history\n>\n>\n> * 1c1d38f (feature_x) update submodule revision to b17e9d9\n> | * 3290e69 (HEAD -> develop) update submodule revision to 0598394\n> |/\n> * cd5e1a5 initial submodule revision\n>\n>\n> Problem case: Merge either branch into the other\n>\n> Expected behavior: Merge conflict.\n>\n> Actual behavior: Auto merge without conflicts.\n>\n> Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history\n>\n> Did I get something wrong about how git resolves merges? Shouldn't git be like: \"hey, you're trying to merge two different contents for the same line\" (the submodule's revision)\n\nHard to say without saying what commit was referenced for the\nsubmodule in the merge-bases for the two repositories you have. In\nthe basic case..\n\nIf branch A and branch B have different commits checked out in the\nsubmodule, say:\n A: deadbeef\n B: ba5eba11\n\nthen it's not clear whether there's a conflict or not. The merge-base\n(the common point of history) matters. So, for example if the\noriginal version (which I'll refer to as 'O\") had:\n O: deadbeef\n\nthen you would say, \"Oh, branch A made no change to this submodule but\nB did. So let's go with what B has.\" Conversely, of O had ba5eba11,\nthen you'd go the other way.\n\nBut, there is some further smarts in that if either A or B point at\ncommits that contain the other in their history and both contain the\ncommit that O points at, then you can just do a fast-forward update to\nthe newest.\n\n\nYou didn't tell us how the merge-base (cd5e1a5 from the diagram you\ngave) differed in your example here between the two repositories. In\nfact, the non-linear case could have several merge-bases, in which\ncase they all become potentially relevant (as does their merge-bases\nsince at that point you'll trigger the recursive portion of\nmerge-recursive). Giving us that info might help us point out what\nhappened, though if either the fast-forward logic comes into play or\nthe recursive logic gets in the mix, then we may need you to provide a\ntestcase (or access to the repo in question) in order to explain it\nand/or determine if you've found a bug.\n\nDoes that help?\n\nElijah\n"},{"id":"345946","messageId":"1524825269.2227.5.camel@klsmartin.com","threadId":"48363","inReplyTo":"CABPp-BE5jRG8JdDfH1XG-Btz9jJxfwf_oyNni8Ci1j+J3icbVQ@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Middelschulte, Leif","fromEmail":"leif.middelschulte@klsmartin.com","sentAt":"2018-04-27T10:37:38Z","receivedAt":"2018-04-27T10:37:49Z","isPatch":false,"sender":{"key":"leif.middelschulte@klsmartin.com","avatar":null},"body":"Hi,\n\nfirstofall: thank all of you for your feedback.\n\nAm Donnerstag, den 26.04.2018, 17:19 -0700 schrieb Elijah Newren:\n> On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif\n> wrote:\n> > Hi,\n> > \n> > we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git.\n> > \n> > Assume the following setup:\n> > \n> > - Repository `S` is sourced by repository `p` as submodule `s`\n> > - Repository `p` has two branches: `feature_x` and `develop`\n> > - The revisions sourced via the submodule have a linear history\n> > \n> > \n> > * 1c1d38f (feature_x) update submodule revision to b17e9d9\n> > > * 3290e69 (HEAD -> develop) update submodule revision to 0598394\n> > > /\n> > \n> > * cd5e1a5 initial submodule revision\n> > \n> > \n> > Problem case: Merge either branch into the other\n> > \n> > Expected behavior: Merge conflict.\n> > \n> > Actual behavior: Auto merge without conflicts.\n> > \n> > Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history\n> > \n> > Did I get something wrong about how git resolves merges? Shouldn't git be like: \"hey, you're trying to merge two different contents for the same line\" (the submodule's revision)\n> \n> Hard to say without saying what commit was referenced for the\n> submodule in the merge-bases for the two repositories you have. In\n> the basic case..\n> \n> If branch A and branch B have different commits checked out in the\n> submodule, say:\n> A: deadbeef\n> B: ba5eba11\n> \n> then it's not clear whether there's a conflict or not. The merge-base\n> (the common point of history) matters. So, for example if the\n> original version (which I'll refer to as 'O\") had:\n> O: deadbeef\n> \n> then you would say, \"Oh, branch A made no change to this submodule but\n> B did. So let's go with what B has.\" Conversely, of O had ba5eba11,\n> then you'd go the other way.\n> \n> But, there is some further smarts in that if either A or B point at\n> commits that contain the other in their history and both contain the\n> commit that O points at, then you can just do a fast-forward update to\n> the newest.\n> \n> \n> You didn't tell us how the merge-base (cd5e1a5 from the diagram you\n> gave) differed in your example here between the two repositories. In\n> fact, the non-linear case could have several merge-bases, in which\n> case they all become potentially relevant (as does their merge-bases\n> since at that point you'll trigger the recursive portion of\n> merge-recursive). Giving us that info might help us point out what\n> happened, though if either the fast-forward logic comes into play or\n> the recursive logic gets in the mix, then we may need you to provide a\n> testcase (or access to the repo in question) in order to explain it\n> and/or determine if you've found a bug.\n\nI placed two reositories here: https://gitlab.com/foss-contributions/git-examples/network/develop\nThe access should be public w/o login.\n\nIf you prefer the examples to be placed somewhere else, let me know.\n\n> \n> Does that help?\n\nI guess it's somehow understandable that it tries to be more smart about things wrt submodules.\n\nHowever, I believe that there should be some kind of choice here. Not giving *any* notice, makes testing feature-branches hell.\n\nI hope the provided example exhibits the challenge.\n\n\nBR,\n\nLeif\n> \n> Elijah\n> "},{"id":"346017","messageId":"CABPp-BGX-hQYdqfNQZ42313VVhKd7GzgUJqvgwOj=0TEO5UQpQ@mail.gmail.com","threadId":"48363","inReplyTo":"1524825269.2227.5.camel@klsmartin.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-04-28T00:24:41Z","receivedAt":"2018-04-28T00:24:47Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Fri, Apr 27, 2018 at 3:37 AM, Middelschulte, Leif\n wrote:\n> Am Donnerstag, den 26.04.2018, 17:19 -0700 schrieb Elijah Newren:\n>> On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif\n>> wrote:\n\n>> > Problem case: Merge either branch into the other\n>> >\n>> > Expected behavior: Merge conflict.\n>> >\n>> > Actual behavior: Auto merge without conflicts.\n>> >\n>> > Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history\n\nLet me just note that I don't actually use submodules myself, and\nrarely run across them, so as far as users expect submodules should\nbehave I may have to defer to others. But it was particularly this\nsentence of yours that caught my attention and got me to respond. I\nmay have misunderstood which repository had the non-linear history,\nbut...\n\n\n>> But, there is some further smarts in that if either A or B point at\n>> commits that contain the other in their history and both contain the\n>> commit that O points at, then you can just do a fast-forward update to\n>> the newest.\n\nThis particular paragraph, is relevant to your example; more details below.\n\n>> You didn't tell us how the merge-base (cd5e1a5 from the diagram you\n>> gave) differed in your example here between the two repositories. In\n>> fact, the non-linear case could have several merge-bases, in which\n>> case they all become potentially relevant (as does their merge-bases\n>> since at that point you'll trigger the recursive portion of\n>> merge-recursive). Giving us that info might help us point out what\n>> happened, though if either the fast-forward logic comes into play or\n>> the recursive logic gets in the mix, then we may need you to provide a\n>> testcase (or access to the repo in question) in order to explain it\n>> and/or determine if you've found a bug.\n>\n> I placed two reositories here: https://gitlab.com/foss-contributions/git-examples/network/develop\n> The access should be public w/o login.\n>\n> If you prefer the examples to be placed somewhere else, let me know.\n\nSo the only thing I see here is a single repository, which contains a\nsubmodule with linear history. (unless I was grabbing it wrong; I\njust tried `git clone --recurse-submodules\nhttps://gitlab.com/foss-contributions/git-examples`) Do you also have\nan example with non-linear history demonstrating your claim that it\nbehaves differently, for comparison?\n\n\nAnyway, in this case you had both branches updating the submodule to\nsomething newer (to a fast-forward update of what it previously was),\nbut one side advanced it further than the other side did (in\nparticular, to what turned out to be a fast-forward update of what the\nother branch used). That means the whole fast-forwarding logic of\ncommit 68d03e4a6e44 (\"Implement automatic fast-forward merge for\nsubmodules\", 2010-07-07)) came into play.\n\nI would expect that a different example involving non-linear history\nwould behave the same, if both sides update the submodule in a fashion\nthat is just fast-forwarding and one commit contains the other in its\nhistory. I'm curious if you have a counter example.\n"},{"id":"346024","messageId":"CA+P7+xrK85JMJW4bMhJVBbbcdgs=6MQKvoZqTvKdB4yBNVc7Ag@mail.gmail.com","threadId":"48363","inReplyTo":"CABPp-BGX-hQYdqfNQZ42313VVhKd7GzgUJqvgwOj=0TEO5UQpQ@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-04-28T07:22:00Z","receivedAt":"2018-04-28T07:22:25Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Apr 27, 2018 at 5:24 PM, Elijah Newren wrote:\n> I would expect that a different example involving non-linear history\n> would behave the same, if both sides update the submodule in a fashion\n> that is just fast-forwarding and one commit contains the other in its\n> history. I'm curious if you have a counter example.\n\nMy interpretation of the counter example was that the two submodule\nupdates were not linear (i.e. one did not contain the other after\nupdating).\n\nI could be wrong, so more clarification from Lief would be helpful in\nilluminating the problem.\n\nThanks,\nJake\n"},{"id":"346181","messageId":"20180430170229.GA775@book.hvoigt.net","threadId":"48363","inReplyTo":"CAGZ79kaub2k-q-Mcj3H5o6ekyZ8ZZzG7+r5sHt5Ne25Nc3_nPQ@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2018-04-30T17:02:29Z","receivedAt":"2018-04-30T17:34:18Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote:\n> Stefan wrote:\n> > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd\n> > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07)\n> > to explain the situation you encounter. (specifically merge_submodule\n> > at the end of the diff)\n> \n> +cc Heiko, author of that commit.\n\nIn that commit we tried to be very careful about. I do not understand\nthe situation in which the current strategy would be wrong by default.\n\nWe only merge if the following applies:\n\n * The changes in the superproject on both sides point forward in the\n submodule.\n\n * One side is contained in the other. Contained from the submodule\n perspective. Sides from the superproject merge perspective.\n\nSo in case of the mentioned rewind of a submodule: Only one side of the\n3-way merge would point forward and the merge would fail.\n\nI can imagine, that in case of a temporary revert of a commit in the\nsubmodule that you would not want that merged into some other branch.\nBut that would be the same without submodules. If you merge a temporary\nrevert from another branch you will not get any conflict.\n\nSo maybe someone can explain the use case in which one would get the\nresults that seem wrong?\n\nCheers Heiko\n"},{"id":"346433","messageId":"1525246025.2176.12.camel@klsmartin.com","threadId":"48363","inReplyTo":"20180430170229.GA775@book.hvoigt.net","subject":"Re: git merge banch w/ different submodule revision","fromName":"Middelschulte, Leif","fromEmail":"leif.middelschulte@klsmartin.com","sentAt":"2018-05-02T07:30:25Z","receivedAt":"2018-05-02T07:30:34Z","isPatch":false,"sender":{"key":"leif.middelschulte@klsmartin.com","avatar":null},"body":"Am Montag, den 30.04.2018, 19:02 +0200 schrieb Heiko Voigt:\n> On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote:\n> > Stefan wrote:\n> > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd\n> > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07)\n> > > to explain the situation you encounter. (specifically merge_submodule\n> > > at the end of the diff)\n> > \n> > +cc Heiko, author of that commit.\n> \n> In that commit we tried to be very careful about. I do not understand\n> the situation in which the current strategy would be wrong by default.\n> \n> We only merge if the following applies:\n> \n> * The changes in the superproject on both sides point forward in the\n> submodule.\n> \n> * One side is contained in the other. Contained from the submodule\n> perspective. Sides from the superproject merge perspective.\n> \n> So in case of the mentioned rewind of a submodule: Only one side of the\n> 3-way merge would point forward and the merge would fail.\n> \n> I can imagine, that in case of a temporary revert of a commit in the\n> submodule that you would not want that merged into some other branch.\n> But that would be the same without submodules. If you merge a temporary\n> revert from another branch you will not get any conflict.\n> \n> So maybe someone can explain the use case in which one would get the\n> results that seem wrong?\nIn an ideal world, where there are no regressions between revisions, a\nfast-forward is appropriate. However, we might have regressions within\nsubmodules.\n\nSo the usecase is the following:\n\nEnvironment:\n- We have a base library L that is developed by some team (Team B).\n- Another team (Team A) developes a product P based on those libraries using git-flow.\n\nCase:\nThe problem occurs, when a developer (D) of Team A tries to have a feature\nthat he developed on a branch accepted by a core developer of P:\nIf a core developer of P advanced the reference of L within P (linear history), he might\ndeem the work D insufficient. Not because of the actual work by D, but regressions\nthat snuck into L. The core developer will not be informed about the missmatching\nrevisions of L.\n\nSo it would be nice if there was some kind of switch or at least some trigger.\n\nCheers,\n\nLeif\n\n\n> \n> Cheers Heiko\n> "},{"id":"346545","messageId":"20180503164226.GB23564@book.hvoigt.net","threadId":"48363","inReplyTo":"1525246025.2176.12.camel@klsmartin.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2018-05-03T16:42:26Z","receivedAt":"2018-05-03T16:50:15Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Wed, May 02, 2018 at 07:30:25AM +0000, Middelschulte, Leif wrote:\n> Am Montag, den 30.04.2018, 19:02 +0200 schrieb Heiko Voigt:\n> > On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote:\n> > > Stefan wrote:\n> > > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd\n> > > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07)\n> > > > to explain the situation you encounter. (specifically merge_submodule\n> > > > at the end of the diff)\n> > > \n> > > +cc Heiko, author of that commit.\n> > \n> > In that commit we tried to be very careful about. I do not understand\n> > the situation in which the current strategy would be wrong by default.\n> > \n> > We only merge if the following applies:\n> > \n> > * The changes in the superproject on both sides point forward in the\n> > submodule.\n> > \n> > * One side is contained in the other. Contained from the submodule\n> > perspective. Sides from the superproject merge perspective.\n> > \n> > So in case of the mentioned rewind of a submodule: Only one side of the\n> > 3-way merge would point forward and the merge would fail.\n> > \n> > I can imagine, that in case of a temporary revert of a commit in the\n> > submodule that you would not want that merged into some other branch.\n> > But that would be the same without submodules. If you merge a temporary\n> > revert from another branch you will not get any conflict.\n> > \n> > So maybe someone can explain the use case in which one would get the\n> > results that seem wrong?\n> In an ideal world, where there are no regressions between revisions, a\n> fast-forward is appropriate. However, we might have regressions within\n> submodules.\n> \n> So the usecase is the following:\n> \n> Environment:\n> - We have a base library L that is developed by some team (Team B).\n> - Another team (Team A) developes a product P based on those libraries using git-flow.\n> \n> Case:\n> The problem occurs, when a developer (D) of Team A tries to have a feature\n> that he developed on a branch accepted by a core developer of P:\n> If a core developer of P advanced the reference of L within P (linear history), he might\n> deem the work D insufficient. Not because of the actual work by D, but regressions\n> that snuck into L. The core developer will not be informed about the missmatching\n> revisions of L.\n> \n> So it would be nice if there was some kind of switch or at least some trigger.\n\nI still do not understand how the current behaviour is mismatching with\nusers expectations. Let's assume that you directly tracked the files of\nL in your product repository P, without any submodule boundary. How\nwould the behavior be different? Would it be? If D started on an older\nrevision and gets merged into a newer revision, there can always be\nregressions even without submodules.\n\nWhy would the core developer need to be informed about mismatching\nrevisions if he himself advanced the submodule?\n\nIt seems to me that you do not want to mix integration testing and\ntesting of the feature itself. How about just testing/reviewing on the\nbranch then? You would still get the submodule revision D was working on\nand then in a later stage check if integration with everything else\nworks.\n\nCheers Heiko\n"},{"id":"346640","messageId":"1525422571.2175.52.camel@klsmartin.com","threadId":"48363","inReplyTo":"20180503164226.GB23564@book.hvoigt.net","subject":"Re: git merge banch w/ different submodule revision","fromName":"Middelschulte, Leif","fromEmail":"leif.middelschulte@klsmartin.com","sentAt":"2018-05-04T08:29:32Z","receivedAt":"2018-05-04T08:29:47Z","isPatch":false,"sender":{"key":"leif.middelschulte@klsmartin.com","avatar":null},"body":"Hi,\nAm Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt:\n> Hi,\n> \n> On Wed, May 02, 2018 at 07:30:25AM +0000, Middelschulte, Leif wrote:\n> > Am Montag, den 30.04.2018, 19:02 +0200 schrieb Heiko Voigt:\n> > > On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote:\n> > > > Stefan wrote:\n> > > > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd\n> > > > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07)\n> > > > > to explain the situation you encounter. (specifically merge_submodule\n> > > > > at the end of the diff)\n> > > > \n> > > > +cc Heiko, author of that commit.\n> > > \n> > > In that commit we tried to be very careful about. I do not understand\n> > > the situation in which the current strategy would be wrong by default.\n> > > \n> > > We only merge if the following applies:\n> > > \n> > > * The changes in the superproject on both sides point forward in the\n> > > submodule.\n> > > \n> > > * One side is contained in the other. Contained from the submodule\n> > > perspective. Sides from the superproject merge perspective.\n> > > \n> > > So in case of the mentioned rewind of a submodule: Only one side of the\n> > > 3-way merge would point forward and the merge would fail.\n> > > \n> > > I can imagine, that in case of a temporary revert of a commit in the\n> > > submodule that you would not want that merged into some other branch.\n> > > But that would be the same without submodules. If you merge a temporary\n> > > revert from another branch you will not get any conflict.\n> > > \n> > > So maybe someone can explain the use case in which one would get the\n> > > results that seem wrong?\n> > \n> > In an ideal world, where there are no regressions between revisions, a\n> > fast-forward is appropriate. However, we might have regressions within\n> > submodules.\n> > \n> > So the usecase is the following:\n> > \n> > Environment:\n> > - We have a base library L that is developed by some team (Team B).\n> > - Another team (Team A) developes a product P based on those libraries using git-flow.\n> > \n> > Case:\n> > The problem occurs, when a developer (D) of Team A tries to have a feature\n> > that he developed on a branch accepted by a core developer of P:\n> > If a core developer of P advanced the reference of L within P (linear history), he might\n> > deem the work D insufficient. Not because of the actual work by D, but regressions\n> > that snuck into L. The core developer will not be informed about the missmatching\n> > revisions of L.\n> > \n> > So it would be nice if there was some kind of switch or at least some trigger.\n> \n> I still do not understand how the current behaviour is mismatching with\n> users expectations. Let's assume that you directly tracked the files of\n> L in your product repository P, without any submodule boundary. How\n> would the behavior be different? Would it be? If D started on an older\n> revision and gets merged into a newer revision, there can always be\n> regressions even without submodules.\n> \n> Why would the core developer need to be informed about mismatching\n> revisions if he himself advanced the submodule?\nIn that case you'd be right. I should have picked my example more wisely.\nAssume right here that not a core developer, but another developer advanced\nthe submodule (also via feature branch + merge).\n> \n> It seems to me that you do not want to mix integration testing and\n> testing of the feature itself. \nThat's on point. That's why it would be nice if git *at least* warned about the different revisions wrt submodules.\n\nBut, I guess, I learned something about submodules:\nI used to think of submodules as means to pin down a specific revision like: `ver == x`.\nNow I'm learning that submodules are treated as `ver >= x` during a merge.\n\n> How about just testing/reviewing on the\n> branch then? You would still get the submodule revision D was working on\n> and then in a later stage check if integration with everything else\n> works.\nSure. But if the behavior deviates after a merge the merging developer is currently not\naware that it *might* have to do with different submodule revisions used, not the \"actual\" code merged.\n\nLike not even \"beware: the (feature) branch you've merged used an 'older' revision of X\"\n\n> \n> Cheers Heiko\n\nCheers,\n\nLeif"},{"id":"346643","messageId":"20180504101854.GA29828@book.hvoigt.net","threadId":"48363","inReplyTo":"1525422571.2175.52.camel@klsmartin.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2018-05-04T10:18:54Z","receivedAt":"2018-05-04T10:26:44Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote:\n> Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt:\n> > I still do not understand how the current behaviour is mismatching with\n> > users expectations. Let's assume that you directly tracked the files of\n> > L in your product repository P, without any submodule boundary. How\n> > would the behavior be different? Would it be? If D started on an older\n> > revision and gets merged into a newer revision, there can always be\n> > regressions even without submodules.\n> > \n> > Why would the core developer need to be informed about mismatching\n> > revisions if he himself advanced the submodule?\n> In that case you'd be right. I should have picked my example more wisely.\n> Assume right here that not a core developer, but another developer advanced\n> the submodule (also via feature branch + merge).\n> > \n> > It seems to me that you do not want to mix integration testing and\n> > testing of the feature itself. \n> That's on point. That's why it would be nice if git *at least* warned\n> about the different revisions wrt submodules.\n> \n> But, I guess, I learned something about submodules:\n> I used to think of submodules as means to pin down a specific revision like: `ver == x`.\n> Now I'm learning that submodules are treated as `ver >= x` during a merge.\n\nWell a submodule version is pinned down as long a you do not change it\nand commit it. The same as files and the goal is to make submodules\nbehave as close to normal files as possible. And git \"warns\" about\nchanged submodules by displaying them in the diff.\n\nActually the use case you are describing is not even involving a real\nmerge for submodules. It is just changing the pointer to another\nrevision.\n\n> > How about just testing/reviewing on the\n> > branch then? You would still get the submodule revision D was working on\n> > and then in a later stage check if integration with everything else\n> > works.\n> Sure. But if the behavior deviates after a merge the merging developer is currently not\n> aware that it *might* have to do with different submodule revisions used, not the \"actual\" code merged.\n> \n> Like not even \"beware: the (feature) branch you've merged used an 'older' revision of X\"\n\nThe submodule is part of the \"actual\" code and should be reviewed the\nsame. Maybe you want to set the diff.submodule option to 'diff' ? Then\ngit shows the actual diff of the changed contents in the submodule and\nit would be more obvious how the code changed.\n\nAt the moment it seems to me that you want submodules to behave\ndifferently than we handle normal files/directories which is the\nopposite direction we have been trying to get git into. My feeling\nthough is that this should be covered by the review process instead of a\nfailing merge. Another option would be that you could write a hook that\nwarns reviewers that they are merging a submodule update.\n\nCheers Heiko\n"},{"id":"346650","messageId":"CABPp-BGaibCPWuCnaX5Af=sv-2zvyhNcupT+-PkxHDfJBg_Vbw@mail.gmail.com","threadId":"48363","inReplyTo":"20180504101854.GA29828@book.hvoigt.net","subject":"Re: git merge banch w/ different submodule revision","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-04T14:43:00Z","receivedAt":"2018-05-04T14:43:06Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 4, 2018 at 3:18 AM, Heiko Voigt wrote:\n> Hi,\n>\n> On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote:\n>> Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt:\n\n>> > It seems to me that you do not want to mix integration testing and\n>> > testing of the feature itself.\n>> That's on point. That's why it would be nice if git *at least* warned\n>> about the different revisions wrt submodules.\n\nThere's a good point here...\n\n> Well a submodule version is pinned down as long a you do not change it\n> and commit it. The same as files and the goal is to make submodules\n> behave as close to normal files as possible. And git \"warns\" about\n> changed submodules by displaying them in the diff.\n\nActually, submodules do behave differently than normal files in an\nimportant way, which we may be able to fix and may help Leif here:\n\nWhen merging two regular files that have been modified on both sides\nof history, git always prints a message, \"Auto-merging $FILE\". We\ncould omit that and depend on the user to check the diffstat or run\ndiff afterwards or something, but we don't just rely on that; we also\nwarn them with a simple message that we are doing something to resolve\nthis both-sides-changed-this-path (namely employing the well known\nthree-way-file-merge algorithm to come up with something).\n\nInside merge_submodule(), the equivalent would be printing a message\nwhenever we decide that one branch is a fast-forward of the other\n(\"Case #1\", as it's called in the code), yet currently it prints\nnothing. Perhaps it should.\n\n\nLeif, would you like to try your hand at creating a patch for this?\n"},{"id":"346870","messageId":"1525702992.2177.3.camel@klsmartin.com","threadId":"48363","inReplyTo":"CABPp-BGaibCPWuCnaX5Af=sv-2zvyhNcupT+-PkxHDfJBg_Vbw@mail.gmail.com","subject":"Re: git merge banch w/ different submodule revision","fromName":"Middelschulte, Leif","fromEmail":"leif.middelschulte@klsmartin.com","sentAt":"2018-05-07T14:23:16Z","receivedAt":"2018-05-07T14:23:29Z","isPatch":false,"sender":{"key":"leif.middelschulte@klsmartin.com","avatar":null},"body":"Hi,\n\nAm Freitag, den 04.05.2018, 07:43 -0700 schrieb Elijah Newren:\n> On Fri, May 4, 2018 at 3:18 AM, Heiko Voigt wrote:\n> > Hi,\n> > \n> > On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote:\n> > > Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt:\n> \n> \n> > > > It seems to me that you do not want to mix integration testing and\n> > > > testing of the feature itself.\n> > > \n> > > That's on point. That's why it would be nice if git *at least* warned\n> > > about the different revisions wrt submodules.\n> \n> There's a good point here...\n> \n> > Well a submodule version is pinned down as long a you do not change it\n> > and commit it. The same as files and the goal is to make submodules\n> > behave as close to normal files as possible. And git \"warns\" about\n> > changed submodules by displaying them in the diff.\n> \n> Actually, submodules do behave differently than normal files in an\n> important way, which we may be able to fix and may help Leif here:\n> \n> When merging two regular files that have been modified on both sides\n> of history, git always prints a message, \"Auto-merging $FILE\". We\n> could omit that and depend on the user to check the diffstat or run\n> diff afterwards or something, but we don't just rely on that; we also\n> warn them with a simple message that we are doing something to resolve\n> this both-sides-changed-this-path (namely employing the well known\n> three-way-file-merge algorithm to come up with something).\n> \n> Inside merge_submodule(), the equivalent would be printing a message\n> whenever we decide that one branch is a fast-forward of the other\n> (\"Case #1\", as it's called in the code), yet currently it prints\n> nothing. Perhaps it should.\n> \n> \n> Leif, would you like to try your hand at creating a patch for this?\nThanks for the feedback and the advice/direction.\n\nI'll try to work on it this week and send patches to the ML for review.\n\nCheers,\n\nLeif"}]}