* git merge banch w/ different submodule revision @ 2018-04-26 10:49 Middelschulte, Leif 2018-04-26 17:56 ` Stefan Beller 2018-04-27 0:19 ` Elijah Newren 0 siblings, 2 replies; 16+ messages in thread From: Middelschulte, Leif @ 2018-04-26 10:49 UTC (permalink / raw) To: git@vger.kernel.org Hi, we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git. Assume the following setup: - Repository `S` is sourced by repository `p` as submodule `s` - Repository `p` has two branches: `feature_x` and `develop` - The revisions sourced via the submodule have a linear history * 1c1d38f (feature_x) update submodule revision to b17e9d9 | * 3290e69 (HEAD -> develop) update submodule revision to 0598394 |/ * cd5e1a5 initial submodule revision Problem case: Merge either branch into the other Expected behavior: Merge conflict. Actual behavior: Auto merge without conflicts. Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history 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) Thanks in advance, Leif ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-26 10:49 git merge banch w/ different submodule revision Middelschulte, Leif @ 2018-04-26 17:56 ` Stefan Beller 2018-04-26 21:46 ` Jacob Keller 2018-04-27 0:19 ` Elijah Newren 1 sibling, 1 reply; 16+ messages in thread From: Stefan Beller @ 2018-04-26 17:56 UTC (permalink / raw) To: Middelschulte, Leif; +Cc: git@vger.kernel.org On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif <Leif.Middelschulte@klsmartin.com> wrote: > Hi, > > we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git. > > Assume the following setup: > > - Repository `S` is sourced by repository `p` as submodule `s` > - Repository `p` has two branches: `feature_x` and `develop` > - The revisions sourced via the submodule have a linear history > > > * 1c1d38f (feature_x) update submodule revision to b17e9d9 > | * 3290e69 (HEAD -> develop) update submodule revision to 0598394 > |/ > * cd5e1a5 initial submodule revision > > > Problem case: Merge either branch into the other > > Expected behavior: Merge conflict. > > Actual behavior: Auto merge without conflicts. > > Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history > > Did I get something wrong about how git resolves merges? We often treating a submodule as a file from the superproject, but not always. And in case of a merge, git seems to be a bit smarter than treating it as a textfile with two different lines. See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07) to explain the situation you encounter. (specifically merge_submodule at the end of the diff) > Shouldn't git be like: "hey, you're trying to merge two different contents for the same line" (the submodule's revision) As we have a history in the submodule we can do more than that and resolve the conflict. For two lines, you usually need manual intervention (which line to pick, or craft a complete new line out of parts of each line?), whereas for submodule commits you can reason about their dependencies due to their history and not just look at the textual conflict. Stefan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-26 17:56 ` Stefan Beller @ 2018-04-26 21:46 ` Jacob Keller 2018-04-26 22:19 ` Stefan Beller 2018-04-27 0:02 ` Elijah Newren 0 siblings, 2 replies; 16+ messages in thread From: Jacob Keller @ 2018-04-26 21:46 UTC (permalink / raw) To: Stefan Beller; +Cc: Middelschulte, Leif, git@vger.kernel.org On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller <sbeller@google.com> wrote: > We often treating a submodule as a file from the superproject, but not always. > And in case of a merge, git seems to be a bit smarter than treating it > as a textfile > with two different lines. Sure, but a submodule is checked out "at a commit", so if two branches of history are merged, and they conflict over which place the submodule is at.... shouldn't that produce a conflict?? I mean, how is the merge algorithm supposed to know which is right? The patch you linked appears to be able to resolve it to the one which contains both commits.. but that may not actually be true since you can rewind submodules since they're *pointers* to commits, not commits themselves. I'm not against that as a possible strategy to merge submodules, but it seems like not necessarily something you would always want... Thanks, Jake ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-26 21:46 ` Jacob Keller @ 2018-04-26 22:19 ` Stefan Beller 2018-04-30 17:02 ` Heiko Voigt 2018-04-27 0:02 ` Elijah Newren 1 sibling, 1 reply; 16+ messages in thread From: Stefan Beller @ 2018-04-26 22:19 UTC (permalink / raw) To: Jacob Keller, Heiko Voigt; +Cc: Middelschulte, Leif, git@vger.kernel.org Stefan wrote: > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07) > to explain the situation you encounter. (specifically merge_submodule > at the end of the diff) +cc Heiko, author of that commit. On Thu, Apr 26, 2018 at 2:46 PM, Jacob Keller <jacob.keller@gmail.com> wrote: > On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller <sbeller@google.com> wrote: >> We often treating a submodule as a file from the superproject, but not always. >> And in case of a merge, git seems to be a bit smarter than treating it >> as a textfile with two different lines. > > Sure, but a submodule is checked out "at a commit", so if two branches > of history are merged, and they conflict over which place the > submodule is at.... shouldn't that produce a conflict?? Stepping back a little bit: When two branches developed a file differently, they can be merged iff they do not change the same lines (plus a little bit of margin of 1 extra line) That is the builtin merge-driver for "plain text files" and seems to be accepted widely as "good enough" or "that is how git merges". What if this text file happens to be the .gitmodules file and the changed lines happen to be 2 options in there (Say one option was the path, as one branch renamed the submodule, and the other option is submodule.branch) ? Then we could do better as we know the structure of the file. We would not need the extra buffer line as a cautious step, but instead could parse both sides of the merge and merge each config in-memory and then write out a .gitmodules file. I think David Turner proposed a custom merge driver for .gitmodules a couple month ago. Another example is the merge code respecting renames on one side (even for directories) and edits in the other side. Technically the rename of a file is a "delete of all lines in this path", which could also argued to just conflict with the edit on the other side. With these examples given, I think it is legit to treat submodule changes not as "two lines of text differ at the same place, mark it as conflict", but we are allowed to be smarter about it. > I mean, how is the merge algorithm supposed to know which is right? Good question. As said above, the merge algorithm for text files is just correct for "plain text files". In source code, I can give an example which merges fine, but doesn't compile after merging: One side changes a function signature and the other side adds a call to the function (still using the old signature). Here you can see that our merge algorithm is wrong. It sucks. The solution is a custom merge driver for C code (or whatever language you happen to use). For submodules, the given commit made the assumption that progressing in history of a submodule is never bad, i.e. there are no reverts and no bugs introduced, only perfect features are added by new submodule commits. (I don't know which assumptions were actually made, I made this up). Maybe we need to revisit that decision? > The patch you linked appears to be able to resolve it to the one which > contains both commits.. but that may not actually be true since you > can rewind submodules since they're *pointers* to commits, not commits > themselves. Right, and that is the problem, as the pointer is a small thing, which doesn't allow for the dumb text merging strategy that is used in files. So we could always err out and have the user make a decision. Or we could provide a basic merge driver for submodules (which was implemented in that commit). If you use a different workflow this doesn't work for you, so obviously you want a different custom merge driver for submodules? > I'm not against that as a possible strategy to merge submodules, but > it seems like not necessarily something you would always want... I agree that it is reasonable to want different things, just like wanting a merge driver that works better with C code. (side note: I am rebasing a large series currently and one of the frequent conflicts were different #includes at the top of a file. You could totally automate merging that :/) Thanks, Stefan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-26 22:19 ` Stefan Beller @ 2018-04-30 17:02 ` Heiko Voigt 2018-05-02 7:30 ` Middelschulte, Leif 0 siblings, 1 reply; 16+ messages in thread From: Heiko Voigt @ 2018-04-30 17:02 UTC (permalink / raw) To: Stefan Beller; +Cc: Jacob Keller, Middelschulte, Leif, git@vger.kernel.org On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote: > Stefan wrote: > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07) > > to explain the situation you encounter. (specifically merge_submodule > > at the end of the diff) > > +cc Heiko, author of that commit. In that commit we tried to be very careful about. I do not understand the situation in which the current strategy would be wrong by default. We only merge if the following applies: * The changes in the superproject on both sides point forward in the submodule. * One side is contained in the other. Contained from the submodule perspective. Sides from the superproject merge perspective. So in case of the mentioned rewind of a submodule: Only one side of the 3-way merge would point forward and the merge would fail. I can imagine, that in case of a temporary revert of a commit in the submodule that you would not want that merged into some other branch. But that would be the same without submodules. If you merge a temporary revert from another branch you will not get any conflict. So maybe someone can explain the use case in which one would get the results that seem wrong? Cheers Heiko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-30 17:02 ` Heiko Voigt @ 2018-05-02 7:30 ` Middelschulte, Leif 2018-05-03 16:42 ` Heiko Voigt 0 siblings, 1 reply; 16+ messages in thread From: Middelschulte, Leif @ 2018-05-02 7:30 UTC (permalink / raw) To: hvoigt@hvoigt.net, sbeller@google.com Cc: git@vger.kernel.org, jacob.keller@gmail.com Am Montag, den 30.04.2018, 19:02 +0200 schrieb Heiko Voigt: > On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote: > > Stefan wrote: > > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd > > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07) > > > to explain the situation you encounter. (specifically merge_submodule > > > at the end of the diff) > > > > +cc Heiko, author of that commit. > > In that commit we tried to be very careful about. I do not understand > the situation in which the current strategy would be wrong by default. > > We only merge if the following applies: > > * The changes in the superproject on both sides point forward in the > submodule. > > * One side is contained in the other. Contained from the submodule > perspective. Sides from the superproject merge perspective. > > So in case of the mentioned rewind of a submodule: Only one side of the > 3-way merge would point forward and the merge would fail. > > I can imagine, that in case of a temporary revert of a commit in the > submodule that you would not want that merged into some other branch. > But that would be the same without submodules. If you merge a temporary > revert from another branch you will not get any conflict. > > So maybe someone can explain the use case in which one would get the > results that seem wrong? In an ideal world, where there are no regressions between revisions, a fast-forward is appropriate. However, we might have regressions within submodules. So the usecase is the following: Environment: - We have a base library L that is developed by some team (Team B). - Another team (Team A) developes a product P based on those libraries using git-flow. Case: The problem occurs, when a developer (D) of Team A tries to have a feature that he developed on a branch accepted by a core developer of P: If a core developer of P advanced the reference of L within P (linear history), he might deem the work D insufficient. Not because of the actual work by D, but regressions that snuck into L. The core developer will not be informed about the missmatching revisions of L. So it would be nice if there was some kind of switch or at least some trigger. Cheers, Leif > > Cheers Heiko > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-05-02 7:30 ` Middelschulte, Leif @ 2018-05-03 16:42 ` Heiko Voigt 2018-05-04 8:29 ` Middelschulte, Leif 0 siblings, 1 reply; 16+ messages in thread From: Heiko Voigt @ 2018-05-03 16:42 UTC (permalink / raw) To: Middelschulte, Leif Cc: sbeller@google.com, git@vger.kernel.org, jacob.keller@gmail.com Hi, On Wed, May 02, 2018 at 07:30:25AM +0000, Middelschulte, Leif wrote: > Am Montag, den 30.04.2018, 19:02 +0200 schrieb Heiko Voigt: > > On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote: > > > Stefan wrote: > > > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd > > > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07) > > > > to explain the situation you encounter. (specifically merge_submodule > > > > at the end of the diff) > > > > > > +cc Heiko, author of that commit. > > > > In that commit we tried to be very careful about. I do not understand > > the situation in which the current strategy would be wrong by default. > > > > We only merge if the following applies: > > > > * The changes in the superproject on both sides point forward in the > > submodule. > > > > * One side is contained in the other. Contained from the submodule > > perspective. Sides from the superproject merge perspective. > > > > So in case of the mentioned rewind of a submodule: Only one side of the > > 3-way merge would point forward and the merge would fail. > > > > I can imagine, that in case of a temporary revert of a commit in the > > submodule that you would not want that merged into some other branch. > > But that would be the same without submodules. If you merge a temporary > > revert from another branch you will not get any conflict. > > > > So maybe someone can explain the use case in which one would get the > > results that seem wrong? > In an ideal world, where there are no regressions between revisions, a > fast-forward is appropriate. However, we might have regressions within > submodules. > > So the usecase is the following: > > Environment: > - We have a base library L that is developed by some team (Team B). > - Another team (Team A) developes a product P based on those libraries using git-flow. > > Case: > The problem occurs, when a developer (D) of Team A tries to have a feature > that he developed on a branch accepted by a core developer of P: > If a core developer of P advanced the reference of L within P (linear history), he might > deem the work D insufficient. Not because of the actual work by D, but regressions > that snuck into L. The core developer will not be informed about the missmatching > revisions of L. > > So it would be nice if there was some kind of switch or at least some trigger. I still do not understand how the current behaviour is mismatching with users expectations. Let's assume that you directly tracked the files of L in your product repository P, without any submodule boundary. How would the behavior be different? Would it be? If D started on an older revision and gets merged into a newer revision, there can always be regressions even without submodules. Why would the core developer need to be informed about mismatching revisions if he himself advanced the submodule? It seems to me that you do not want to mix integration testing and testing of the feature itself. How about just testing/reviewing on the branch then? You would still get the submodule revision D was working on and then in a later stage check if integration with everything else works. Cheers Heiko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-05-03 16:42 ` Heiko Voigt @ 2018-05-04 8:29 ` Middelschulte, Leif 2018-05-04 10:18 ` Heiko Voigt 0 siblings, 1 reply; 16+ messages in thread From: Middelschulte, Leif @ 2018-05-04 8:29 UTC (permalink / raw) To: hvoigt@hvoigt.net Cc: git@vger.kernel.org, sbeller@google.com, jacob.keller@gmail.com Hi, Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt: > Hi, > > On Wed, May 02, 2018 at 07:30:25AM +0000, Middelschulte, Leif wrote: > > Am Montag, den 30.04.2018, 19:02 +0200 schrieb Heiko Voigt: > > > On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote: > > > > Stefan wrote: > > > > > See https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd > > > > > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 2010-07-07) > > > > > to explain the situation you encounter. (specifically merge_submodule > > > > > at the end of the diff) > > > > > > > > +cc Heiko, author of that commit. > > > > > > In that commit we tried to be very careful about. I do not understand > > > the situation in which the current strategy would be wrong by default. > > > > > > We only merge if the following applies: > > > > > > * The changes in the superproject on both sides point forward in the > > > submodule. > > > > > > * One side is contained in the other. Contained from the submodule > > > perspective. Sides from the superproject merge perspective. > > > > > > So in case of the mentioned rewind of a submodule: Only one side of the > > > 3-way merge would point forward and the merge would fail. > > > > > > I can imagine, that in case of a temporary revert of a commit in the > > > submodule that you would not want that merged into some other branch. > > > But that would be the same without submodules. If you merge a temporary > > > revert from another branch you will not get any conflict. > > > > > > So maybe someone can explain the use case in which one would get the > > > results that seem wrong? > > > > In an ideal world, where there are no regressions between revisions, a > > fast-forward is appropriate. However, we might have regressions within > > submodules. > > > > So the usecase is the following: > > > > Environment: > > - We have a base library L that is developed by some team (Team B). > > - Another team (Team A) developes a product P based on those libraries using git-flow. > > > > Case: > > The problem occurs, when a developer (D) of Team A tries to have a feature > > that he developed on a branch accepted by a core developer of P: > > If a core developer of P advanced the reference of L within P (linear history), he might > > deem the work D insufficient. Not because of the actual work by D, but regressions > > that snuck into L. The core developer will not be informed about the missmatching > > revisions of L. > > > > So it would be nice if there was some kind of switch or at least some trigger. > > I still do not understand how the current behaviour is mismatching with > users expectations. Let's assume that you directly tracked the files of > L in your product repository P, without any submodule boundary. How > would the behavior be different? Would it be? If D started on an older > revision and gets merged into a newer revision, there can always be > regressions even without submodules. > > Why would the core developer need to be informed about mismatching > revisions if he himself advanced the submodule? In that case you'd be right. I should have picked my example more wisely. Assume right here that not a core developer, but another developer advanced the submodule (also via feature branch + merge). > > It seems to me that you do not want to mix integration testing and > testing of the feature itself. That's on point. That's why it would be nice if git *at least* warned about the different revisions wrt submodules. But, I guess, I learned something about submodules: I used to think of submodules as means to pin down a specific revision like: `ver == x`. Now I'm learning that submodules are treated as `ver >= x` during a merge. > How about just testing/reviewing on the > branch then? You would still get the submodule revision D was working on > and then in a later stage check if integration with everything else > works. Sure. But if the behavior deviates after a merge the merging developer is currently not aware that it *might* have to do with different submodule revisions used, not the "actual" code merged. Like not even "beware: the (feature) branch you've merged used an 'older' revision of X" > > Cheers Heiko Cheers, Leif ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-05-04 8:29 ` Middelschulte, Leif @ 2018-05-04 10:18 ` Heiko Voigt 2018-05-04 14:43 ` Elijah Newren 0 siblings, 1 reply; 16+ messages in thread From: Heiko Voigt @ 2018-05-04 10:18 UTC (permalink / raw) To: Middelschulte, Leif Cc: git@vger.kernel.org, sbeller@google.com, jacob.keller@gmail.com Hi, On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote: > Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt: > > I still do not understand how the current behaviour is mismatching with > > users expectations. Let's assume that you directly tracked the files of > > L in your product repository P, without any submodule boundary. How > > would the behavior be different? Would it be? If D started on an older > > revision and gets merged into a newer revision, there can always be > > regressions even without submodules. > > > > Why would the core developer need to be informed about mismatching > > revisions if he himself advanced the submodule? > In that case you'd be right. I should have picked my example more wisely. > Assume right here that not a core developer, but another developer advanced > the submodule (also via feature branch + merge). > > > > It seems to me that you do not want to mix integration testing and > > testing of the feature itself. > That's on point. That's why it would be nice if git *at least* warned > about the different revisions wrt submodules. > > But, I guess, I learned something about submodules: > I used to think of submodules as means to pin down a specific revision like: `ver == x`. > Now I'm learning that submodules are treated as `ver >= x` during a merge. Well a submodule version is pinned down as long a you do not change it and commit it. The same as files and the goal is to make submodules behave as close to normal files as possible. And git "warns" about changed submodules by displaying them in the diff. Actually the use case you are describing is not even involving a real merge for submodules. It is just changing the pointer to another revision. > > How about just testing/reviewing on the > > branch then? You would still get the submodule revision D was working on > > and then in a later stage check if integration with everything else > > works. > Sure. But if the behavior deviates after a merge the merging developer is currently not > aware that it *might* have to do with different submodule revisions used, not the "actual" code merged. > > Like not even "beware: the (feature) branch you've merged used an 'older' revision of X" The submodule is part of the "actual" code and should be reviewed the same. Maybe you want to set the diff.submodule option to 'diff' ? Then git shows the actual diff of the changed contents in the submodule and it would be more obvious how the code changed. At the moment it seems to me that you want submodules to behave differently than we handle normal files/directories which is the opposite direction we have been trying to get git into. My feeling though is that this should be covered by the review process instead of a failing merge. Another option would be that you could write a hook that warns reviewers that they are merging a submodule update. Cheers Heiko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-05-04 10:18 ` Heiko Voigt @ 2018-05-04 14:43 ` Elijah Newren 2018-05-07 14:23 ` Middelschulte, Leif 0 siblings, 1 reply; 16+ messages in thread From: Elijah Newren @ 2018-05-04 14:43 UTC (permalink / raw) To: Heiko Voigt Cc: Middelschulte, Leif, git@vger.kernel.org, sbeller@google.com, jacob.keller@gmail.com On Fri, May 4, 2018 at 3:18 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote: > Hi, > > On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote: >> Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt: <snip> >> > It seems to me that you do not want to mix integration testing and >> > testing of the feature itself. >> That's on point. That's why it would be nice if git *at least* warned >> about the different revisions wrt submodules. There's a good point here... > Well a submodule version is pinned down as long a you do not change it > and commit it. The same as files and the goal is to make submodules > behave as close to normal files as possible. And git "warns" about > changed submodules by displaying them in the diff. Actually, submodules do behave differently than normal files in an important way, which we may be able to fix and may help Leif here: When merging two regular files that have been modified on both sides of history, git always prints a message, "Auto-merging $FILE". We could omit that and depend on the user to check the diffstat or run diff afterwards or something, but we don't just rely on that; we also warn them with a simple message that we are doing something to resolve this both-sides-changed-this-path (namely employing the well known three-way-file-merge algorithm to come up with something). Inside merge_submodule(), the equivalent would be printing a message whenever we decide that one branch is a fast-forward of the other ("Case #1", as it's called in the code), yet currently it prints nothing. Perhaps it should. Leif, would you like to try your hand at creating a patch for this? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-05-04 14:43 ` Elijah Newren @ 2018-05-07 14:23 ` Middelschulte, Leif 0 siblings, 0 replies; 16+ messages in thread From: Middelschulte, Leif @ 2018-05-07 14:23 UTC (permalink / raw) To: newren@gmail.com, hvoigt@hvoigt.net Cc: git@vger.kernel.org, sbeller@google.com, jacob.keller@gmail.com Hi, Am Freitag, den 04.05.2018, 07:43 -0700 schrieb Elijah Newren: > On Fri, May 4, 2018 at 3:18 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote: > > Hi, > > > > On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote: > > > Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt: > > <snip> > > > > It seems to me that you do not want to mix integration testing and > > > > testing of the feature itself. > > > > > > That's on point. That's why it would be nice if git *at least* warned > > > about the different revisions wrt submodules. > > There's a good point here... > > > Well a submodule version is pinned down as long a you do not change it > > and commit it. The same as files and the goal is to make submodules > > behave as close to normal files as possible. And git "warns" about > > changed submodules by displaying them in the diff. > > Actually, submodules do behave differently than normal files in an > important way, which we may be able to fix and may help Leif here: > > When merging two regular files that have been modified on both sides > of history, git always prints a message, "Auto-merging $FILE". We > could omit that and depend on the user to check the diffstat or run > diff afterwards or something, but we don't just rely on that; we also > warn them with a simple message that we are doing something to resolve > this both-sides-changed-this-path (namely employing the well known > three-way-file-merge algorithm to come up with something). > > Inside merge_submodule(), the equivalent would be printing a message > whenever we decide that one branch is a fast-forward of the other > ("Case #1", as it's called in the code), yet currently it prints > nothing. Perhaps it should. > > > Leif, would you like to try your hand at creating a patch for this? Thanks for the feedback and the advice/direction. I'll try to work on it this week and send patches to the ML for review. Cheers, Leif ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-26 21:46 ` Jacob Keller 2018-04-26 22:19 ` Stefan Beller @ 2018-04-27 0:02 ` Elijah Newren 1 sibling, 0 replies; 16+ messages in thread From: Elijah Newren @ 2018-04-27 0:02 UTC (permalink / raw) To: Jacob Keller; +Cc: Stefan Beller, Middelschulte, Leif, git@vger.kernel.org On Thu, Apr 26, 2018 at 2:46 PM, Jacob Keller <jacob.keller@gmail.com> wrote: > On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller <sbeller@google.com> wrote: >> We often treating a submodule as a file from the superproject, but not always. >> And in case of a merge, git seems to be a bit smarter than treating it >> as a textfile >> with two different lines. > > Sure, but a submodule is checked out "at a commit", so if two branches > of history are merged, and they conflict over which place the > submodule is at.... shouldn't that produce a conflict?? By "which place a submodule is at", do you mean the commit it points to, or the path at which the submodule is found within the parent repository? Continuing on it sounds like you meant the former, but I was unsure if you were asking mutliple different questions here. > I mean, how is the merge algorithm supposed to know which is right? > The patch you linked appears to be able to resolve it to the one which > contains both commits.. but that may not actually be true since you > can rewind submodules since they're *pointers* to commits, not commits > themselves. Only if both commits also contain the base; see lines 328 to 332 of that patch. So, if the submodules are rewound, that algorithm would leave them as conflicted. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-26 10:49 git merge banch w/ different submodule revision Middelschulte, Leif 2018-04-26 17:56 ` Stefan Beller @ 2018-04-27 0:19 ` Elijah Newren 2018-04-27 10:37 ` Middelschulte, Leif 1 sibling, 1 reply; 16+ messages in thread From: Elijah Newren @ 2018-04-27 0:19 UTC (permalink / raw) To: Middelschulte, Leif; +Cc: git@vger.kernel.org On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif <Leif.Middelschulte@klsmartin.com> wrote: > Hi, > > we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git. > > Assume the following setup: > > - Repository `S` is sourced by repository `p` as submodule `s` > - Repository `p` has two branches: `feature_x` and `develop` > - The revisions sourced via the submodule have a linear history > > > * 1c1d38f (feature_x) update submodule revision to b17e9d9 > | * 3290e69 (HEAD -> develop) update submodule revision to 0598394 > |/ > * cd5e1a5 initial submodule revision > > > Problem case: Merge either branch into the other > > Expected behavior: Merge conflict. > > Actual behavior: Auto merge without conflicts. > > Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history > > 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) Hard to say without saying what commit was referenced for the submodule in the merge-bases for the two repositories you have. In the basic case.. If branch A and branch B have different commits checked out in the submodule, say: A: deadbeef B: ba5eba11 then it's not clear whether there's a conflict or not. The merge-base (the common point of history) matters. So, for example if the original version (which I'll refer to as 'O") had: O: deadbeef then you would say, "Oh, branch A made no change to this submodule but B did. So let's go with what B has." Conversely, of O had ba5eba11, then you'd go the other way. But, there is some further smarts in that if either A or B point at commits that contain the other in their history and both contain the commit that O points at, then you can just do a fast-forward update to the newest. You didn't tell us how the merge-base (cd5e1a5 from the diagram you gave) differed in your example here between the two repositories. In fact, the non-linear case could have several merge-bases, in which case they all become potentially relevant (as does their merge-bases since at that point you'll trigger the recursive portion of merge-recursive). Giving us that info might help us point out what happened, though if either the fast-forward logic comes into play or the recursive logic gets in the mix, then we may need you to provide a testcase (or access to the repo in question) in order to explain it and/or determine if you've found a bug. Does that help? Elijah ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-27 0:19 ` Elijah Newren @ 2018-04-27 10:37 ` Middelschulte, Leif 2018-04-28 0:24 ` Elijah Newren 0 siblings, 1 reply; 16+ messages in thread From: Middelschulte, Leif @ 2018-04-27 10:37 UTC (permalink / raw) To: newren@gmail.com; +Cc: git@vger.kernel.org Hi, firstofall: thank all of you for your feedback. Am Donnerstag, den 26.04.2018, 17:19 -0700 schrieb Elijah Newren: > On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif > <Leif.Middelschulte@klsmartin.com> wrote: > > Hi, > > > > we're using git-flow as a basic development workflow. However, doing so revealed unexpected merge-behavior by git. > > > > Assume the following setup: > > > > - Repository `S` is sourced by repository `p` as submodule `s` > > - Repository `p` has two branches: `feature_x` and `develop` > > - The revisions sourced via the submodule have a linear history > > > > > > * 1c1d38f (feature_x) update submodule revision to b17e9d9 > > > * 3290e69 (HEAD -> develop) update submodule revision to 0598394 > > > / > > > > * cd5e1a5 initial submodule revision > > > > > > Problem case: Merge either branch into the other > > > > Expected behavior: Merge conflict. > > > > Actual behavior: Auto merge without conflicts. > > > > Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history > > > > 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) > > Hard to say without saying what commit was referenced for the > submodule in the merge-bases for the two repositories you have. In > the basic case.. > > If branch A and branch B have different commits checked out in the > submodule, say: > A: deadbeef > B: ba5eba11 > > then it's not clear whether there's a conflict or not. The merge-base > (the common point of history) matters. So, for example if the > original version (which I'll refer to as 'O") had: > O: deadbeef > > then you would say, "Oh, branch A made no change to this submodule but > B did. So let's go with what B has." Conversely, of O had ba5eba11, > then you'd go the other way. > > But, there is some further smarts in that if either A or B point at > commits that contain the other in their history and both contain the > commit that O points at, then you can just do a fast-forward update to > the newest. > > > You didn't tell us how the merge-base (cd5e1a5 from the diagram you > gave) differed in your example here between the two repositories. In > fact, the non-linear case could have several merge-bases, in which > case they all become potentially relevant (as does their merge-bases > since at that point you'll trigger the recursive portion of > merge-recursive). Giving us that info might help us point out what > happened, though if either the fast-forward logic comes into play or > the recursive logic gets in the mix, then we may need you to provide a > testcase (or access to the repo in question) in order to explain it > and/or determine if you've found a bug. I placed two reositories here: https://gitlab.com/foss-contributions/git-examples/network/develop The access should be public w/o login. If you prefer the examples to be placed somewhere else, let me know. > > Does that help? I guess it's somehow understandable that it tries to be more smart about things wrt submodules. However, I believe that there should be some kind of choice here. Not giving *any* notice, makes testing feature-branches hell. I hope the provided example exhibits the challenge. BR, Leif > > Elijah > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-27 10:37 ` Middelschulte, Leif @ 2018-04-28 0:24 ` Elijah Newren 2018-04-28 7:22 ` Jacob Keller 0 siblings, 1 reply; 16+ messages in thread From: Elijah Newren @ 2018-04-28 0:24 UTC (permalink / raw) To: Middelschulte, Leif; +Cc: git@vger.kernel.org Hi, On Fri, Apr 27, 2018 at 3:37 AM, Middelschulte, Leif <Leif.Middelschulte@klsmartin.com> wrote: > Am Donnerstag, den 26.04.2018, 17:19 -0700 schrieb Elijah Newren: >> On Thu, Apr 26, 2018 at 3:49 AM, Middelschulte, Leif >> <Leif.Middelschulte@klsmartin.com> wrote: <snip> >> > Problem case: Merge either branch into the other >> > >> > Expected behavior: Merge conflict. >> > >> > Actual behavior: Auto merge without conflicts. >> > >> > Note 1: A merge conflict does occur, if the sourced revisions do *not* have a linear history Let me just note that I don't actually use submodules myself, and rarely run across them, so as far as users expect submodules should behave I may have to defer to others. But it was particularly this sentence of yours that caught my attention and got me to respond. I may have misunderstood which repository had the non-linear history, but... <snip> >> But, there is some further smarts in that if either A or B point at >> commits that contain the other in their history and both contain the >> commit that O points at, then you can just do a fast-forward update to >> the newest. This particular paragraph, is relevant to your example; more details below. >> You didn't tell us how the merge-base (cd5e1a5 from the diagram you >> gave) differed in your example here between the two repositories. In >> fact, the non-linear case could have several merge-bases, in which >> case they all become potentially relevant (as does their merge-bases >> since at that point you'll trigger the recursive portion of >> merge-recursive). Giving us that info might help us point out what >> happened, though if either the fast-forward logic comes into play or >> the recursive logic gets in the mix, then we may need you to provide a >> testcase (or access to the repo in question) in order to explain it >> and/or determine if you've found a bug. > > I placed two reositories here: https://gitlab.com/foss-contributions/git-examples/network/develop > The access should be public w/o login. > > If you prefer the examples to be placed somewhere else, let me know. So the only thing I see here is a single repository, which contains a submodule with linear history. (unless I was grabbing it wrong; I just tried `git clone --recurse-submodules https://gitlab.com/foss-contributions/git-examples`) Do you also have an example with non-linear history demonstrating your claim that it behaves differently, for comparison? Anyway, in this case you had both branches updating the submodule to something newer (to a fast-forward update of what it previously was), but one side advanced it further than the other side did (in particular, to what turned out to be a fast-forward update of what the other branch used). That means the whole fast-forwarding logic of commit 68d03e4a6e44 ("Implement automatic fast-forward merge for submodules", 2010-07-07)) came into play. I would expect that a different example involving non-linear history would behave the same, if both sides update the submodule in a fashion that is just fast-forwarding and one commit contains the other in its history. I'm curious if you have a counter example. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git merge banch w/ different submodule revision 2018-04-28 0:24 ` Elijah Newren @ 2018-04-28 7:22 ` Jacob Keller 0 siblings, 0 replies; 16+ messages in thread From: Jacob Keller @ 2018-04-28 7:22 UTC (permalink / raw) To: Elijah Newren; +Cc: Middelschulte, Leif, git@vger.kernel.org On Fri, Apr 27, 2018 at 5:24 PM, Elijah Newren <newren@gmail.com> wrote: > I would expect that a different example involving non-linear history > would behave the same, if both sides update the submodule in a fashion > that is just fast-forwarding and one commit contains the other in its > history. I'm curious if you have a counter example. My interpretation of the counter example was that the two submodule updates were not linear (i.e. one did not contain the other after updating). I could be wrong, so more clarification from Lief would be helpful in illuminating the problem. Thanks, Jake ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-05-07 14:23 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-26 10:49 git merge banch w/ different submodule revision Middelschulte, Leif 2018-04-26 17:56 ` Stefan Beller 2018-04-26 21:46 ` Jacob Keller 2018-04-26 22:19 ` Stefan Beller 2018-04-30 17:02 ` Heiko Voigt 2018-05-02 7:30 ` Middelschulte, Leif 2018-05-03 16:42 ` Heiko Voigt 2018-05-04 8:29 ` Middelschulte, Leif 2018-05-04 10:18 ` Heiko Voigt 2018-05-04 14:43 ` Elijah Newren 2018-05-07 14:23 ` Middelschulte, Leif 2018-04-27 0:02 ` Elijah Newren 2018-04-27 0:19 ` Elijah Newren 2018-04-27 10:37 ` Middelschulte, Leif 2018-04-28 0:24 ` Elijah Newren 2018-04-28 7:22 ` Jacob Keller
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).