git merge banch w/ different submodule revision

git merge banch w/ different submodule revision

[Middelschulte, Leif]

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

Re: git merge banch w/ different submodule revision

[Stefan Beller]

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

Re: git merge banch w/ different submodule revision

[Jacob Keller]

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

Re: git merge banch w/ different submodule revision

[Stefan Beller]

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

Re: git merge banch w/ different submodule revision

[Elijah Newren]

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.

Re: git merge banch w/ different submodule revision

[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.

Does that help?

Elijah

Re: git merge banch w/ different submodule revision

[Middelschulte, Leif]

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
> 

Re: git merge banch w/ different submodule revision

[Elijah Newren]

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.

Re: git merge banch w/ different submodule revision

[Jacob Keller]

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

Re: git merge banch w/ different submodule revision

[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?

Cheers Heiko

Re: git merge banch w/ different submodule revision

[Middelschulte, Leif]

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
> 

Re: git merge banch w/ different submodule revision

[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?

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

Re: git merge banch w/ different submodule revision

[Middelschulte, Leif]

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

Re: git merge banch w/ different submodule revision

[Heiko Voigt]

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

Re: git merge banch w/ different submodule revision

[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?

Re: git merge banch w/ different submodule revision

[Middelschulte, Leif]

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