Proposed config addition: submodules.denyNonFastForward

In my previous thread regarding submodules "Manasging submodules on large multi-user projects" I feel that some things were left largely unresolved. My options given were:

  * Use repo (and retrain users)
  * Use Avery's git-subtree(1) addition (and potentially retrain)

After discussing some of the pitfalls that I've had with submodule deployment with a kindred spirit in the #github channel, we came to the conclusion that submodules would be very usable if one could implement a "submodules.denyNonFastForward" configuration.

THE ERRORCASE: -------------- The majority of problems with submodules comes from PEBKAC, where users don't run `git submodule update` and/or the often execute `git commit -a` with nary a concern for whether it's a good idea or not. I concede that these are systemic organizational changes, but I find changing software to be well within the acheivable, whereas changing people I've discovered is near-impossible.

Problems occur in this scenario:

            -sub@A-  -sub@B-
[master] ------*--------*------->
                \        \
        [branch] `--------`---X (commit -a)

In the second merge from master to branch, if the user neglects to execute `git submodule update` and then executes the commit at 'X' they will inadvertantly change the index for the submodule to roll it back to "A" instead of keeping it at "B" where it should be.

THE POTENTIAL SOLUTION (and it's problems)
------------------------------------------
The implementation details behind submodules.denyNonFastForward are
certainly not simple. 
   * Is it a server-side (bare repo) config value or a client side config value? 
     (I'm assuming it fits best client side). 
   * What if you have a submodule and you explicitly need to roll it
     back to a previous version of the submodule?
   * Does this cross the line of separation between super- and
     sub-module? Currently ths super (from my understanding) does not 
     contain or maintain any "knowledge" of the sub-module, it merely 
     is aware of a particular commit to detach the head to.

With the weekend coming up, I have some available coding time, and certainly have no problems attempting to come up with a patch set to add this configuration value, but I do need some guidance as to whether it fits into Git conceptually and whether it's technically feasible.

Cheers

-R. Tyler Ballance Slide, Inc.

"R. Tyler Ballance" <tyler@slide.com> writes:

Perhaps it was obvious to _you_ and that was why you omitted mentioning it, but it is unclear when you envision this configuration would kick in.

Do you deny changing the commit bound at path "sub" from B to X if X is not a descendant of B (e.g. A in your example) when the user does "git add sub"? When the user makes such a change into a commit? When the user pushes a history that contains such a change to the remote repository?

I suspect that it is quite common for a library-ish project to have two branches, v1.0 and v2.0, that are _not_ strictly fast forwards. In this respect, git.git is not a norm but is an exception, where 'maint' is always a subset of 'master', causing v1.X.Y always be a subset of v1.X.(Y+1).

And you may want to bind such a project to your project as a submodule.

During the evolution of _your_ superproject, at some point you would decide to switch from using v1.0 line to v2.0 line of the submodule, and that won't be a fast forward. On the other hand, when you are in control of both submodule and superproject, it is reasonable to worry about the fast forwardness of your submodule like you depicted in your ERRORCASE scenario. Your project may use both kinds of submodules, which leads me to guess that this shouldn't be a single submodule.denyFastForward, but should be a per submodule variable, submodule.$name.denyFastForward, regardless of at what stage the check kicks in.

Also it is plausible that your submodule people are working actively and from time to time break their tip, and you as a superproject person may have to temporarily revert to an older and more stable commit from the submodule history until things stabilize on their end.

The submodule history may look like this:

 ---A---B---C---...---G

where A was a bit stale but proven one, B was a nice bleeding edge but later turned out to be broken, and the submodule people are working towards the goal of producing a good one G.

Your superproject's history may first bind B, hoping that submodule people did a great job. But then you discover it was not ready and you have to step back to A temporarily, in order to be able to continue in the meantime. That won't be a fast-forward (you listed this as one of your "problems").

You could fork the submodule history to queue a revert of B on top of it, to satisfy the fast-forward requirement you are introducing here, and then keep going.

          B' (revert of B, whose tree is the same as A)
         /       
 ---A---B---C---...---G

When submodule people got their act together, you would want to bind commit G as the submodule to your project. However, B' won't fast forward to G, so you will end up making another merge (checkout G and merge B' using ours strategy) in the submodule, again just to satisfy the fast forward requirement.

          B'------------G'
         /             /
 ---A---B---C---...---G

The implication of this is that your history will never converge with the one from submodule people. Unless you have a way to force them to take G' as their tip after they complete G, that is.

Which mekes me think that, even if you _normally_ want to require fast forward, it is essential for you to be able to override the check to rebind A in place of B, violating the usual fast forward check.

I think the first prototype should be done as a pre-commit hook script.

An escape hatch is essential, as I discussed above (not just to "roll B back to A" example, but also to "switch from v1.0 line to v2.0 line"). You could use "commit --no-verify" if you go the pre-commit hook route.

This is purely a Porcelain issue to support your workflow, and we should allow you to express a little bit of Policy like this, if it helps to make the workflow safer/saner. I think it is too premature to worry about "the line of separation"; it is not like we would be making this check part of the default behaviour anytime soon.

Another question not in your list:

     * What should happen if the submodule is not even checked out nor
       cloned?

This would happen when a top-level integrator who does not even have to check out nor modify a particular submodule has to merge two branches that have different commits for a submodule. Perhaps their common ancestor in the superproject history had a commit B bound at the submodule, and one branch of the superproject kept it as-is while the other branch updated it to a different commit A. The three-way merge will bind commit A in the resulting merge commit, but there is no way for the top-level integrator who does not even have a clone of the submodule to see how A and B relate to each other.

I think the right answer to the above question is "ignore the check". The branch that changed the submodule from commit B to commit A should have known what it was doing when it made that change, and the top-level integrator should be able to trust that change---that is why he is merging the branch in the first place.