Thread (16 messages) 16 messages, 5 authors, 2018-05-07

Re: git merge banch w/ different submodule revision

From: Heiko Voigt <hidden>
Date: 2018-05-04 10:26:44

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:
quoted
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).
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help