All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Middelschulte, Leif" <Leif.Middelschulte@klsmartin.com>
To: "hvoigt@hvoigt.net" <hvoigt@hvoigt.net>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	"sbeller@google.com" <sbeller@google.com>,
	"jacob.keller@gmail.com" <jacob.keller@gmail.com>
Subject: Re: git merge banch w/ different submodule revision
Date: Fri, 4 May 2018 08:29:32 +0000	[thread overview]
Message-ID: <1525422571.2175.52.camel@klsmartin.com> (raw)
In-Reply-To: <20180503164226.GB23564@book.hvoigt.net>

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

  reply	other threads:[~2018-05-04  8:29 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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 publicly 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

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

  git send-email \
    --in-reply-to=1525422571.2175.52.camel@klsmartin.com \
    --to=leif.middelschulte@klsmartin.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=jacob.keller@gmail.com \
    --cc=sbeller@google.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.