Re: git merge banch w/ different submodule revision

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: May 4, 2018, 10:18 UTC
Message-ID: <20180504101854.GA29828@book.hvoigt.net>
In-Reply-To: <1525422571.2175.52.camel@klsmartin.com>

Hi,

On Fri, May 04, 2018 at 08:29:32AM +0000, Middelschulte, Leif wrote:

Show 22 quoted lines

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

Show 8 quoted lines

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