git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Jacob Keller <jacob.keller@gmail.com>, Heiko Voigt <hvoigt@hvoigt.net>
Cc: "Middelschulte, Leif" <Leif.Middelschulte@klsmartin.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: git merge banch w/ different submodule revision
Date: Thu, 26 Apr 2018 15:19:36 -0700
Message-ID: <CAGZ79kaub2k-q-Mcj3H5o6ekyZ8ZZzG7+r5sHt5Ne25Nc3_nPQ@mail.gmail.com> (raw)
In-Reply-To: <CA+P7+xrUwq0G2YySC3SLKqyihhPnFPCiQnQpoVVa89+=W9O9+w@mail.gmail.com>

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

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-26 10:49 Middelschulte, Leif
2018-04-26 17:56 ` Stefan Beller
2018-04-26 21:46   ` Jacob Keller
2018-04-26 22:19     ` Stefan Beller [this message]
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

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGZ79kaub2k-q-Mcj3H5o6ekyZ8ZZzG7+r5sHt5Ne25Nc3_nPQ@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=Leif.Middelschulte@klsmartin.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=jacob.keller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox