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@xxxxxxxxx> wrote: > On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller <sbeller@xxxxxxxxxx> 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
