Re: git merge banch w/ different submodule revision

2018-05-07 Thread 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 <hvo...@hvoigt.net> wrote:
> > Hi,
> > 
> > On Fri, May 04, 2018 at 08:29:32AM +, Middelschulte, Leif wrote:
> > > Am Donnerstag, den 03.05.2018, 18:42 +0200 schrieb Heiko Voigt:
> 
> 
> > > > 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

Re: git merge banch w/ different submodule revision

2018-05-04 Thread 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 +, 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

2018-05-02 Thread 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

2018-04-27 Thread 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.middelschu...@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
> 

git merge banch w/ different submodule revision

2018-04-26 Thread 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