Re: [RFC 2/2] Don't push a repository with unpushed submodules

From: Fredrik Gustafsson <iveqy@iveqy.com>
Date: Jun 28, 2011, 23:02 UTC
Message-ID: <20110628230248.GA4436@paksenarrion.iveqy.com>
In-Reply-To: <4E0A506B.6040407@xiplink.com>

On Tue, Jun 28, 2011 at 06:06:35PM -0400, Marc Branchaud wrote:

Show 15 quoted lines

> On 11-06-26 02:29 PM, Fredrik Gustafsson wrote:
> > When working with submodules it is easy to forget to push a submodule
> > to the server but pushing a super-project that contains a commit for
> > that submodule. The result is that the superproject points at a
> > submodule commit that is not avaliable on the server.
> > 
> > Check that all submodule commits that are about to be pushed are present
> > on a remote of the submodule and require forcing if that is not the
> > case.
> 
> I have a few concerns about what this is trying to do.
> 
> First, I expect performance will be terrible in repositories with large
> and/or many submodules.  I'd go so far as to say that it's pretty much a
> show-stopper for our repository.

This is not acceptable (in my opinion). The point of making this (only) a client side check was to not have a huge performance impact.

Would you care for testing this in your enviroment or give me enough data to be able to set up a test enviroment size-wize like yours?

> Second, there are many times where I'm working in a submodule on branch
> "TopicA" but not in branch "TopicB".  If I've made submodule changes in
> TopicA then try to push up TopicB, won't I have have to tell push to "-f"?
> But that turns off other checks that I'd rather keep.

I don't quite follow you here. Anyway, only the commits of the superproject that you are about to push is checked, and only the submodules that are changed in any of those commits are examined.

So if you're working in TopicA in a submodule and tries to push TopicB in a superproject that uses TopicC in the submodule, TopicA will not be required to be pushed. (if so, is it a bug).

Show 5 quoted lines

> I'd feel a lot better about this patch if the check was *off* by default and
> there was a config setting / command-line option to turn it on.
> 
> I also agree with Junio that this kind of verification makes more sense in a
> hook on the server side.

Serverside, we cannot guarantee that all submodules are reachable, they might be on different servers, maybe not even connected to eachothers. Even if they are connected this would requiring network traffic. Making this check an even bigger performance killer. This check is not supposed to guarantee a sane server-repo (that would be much harder) and therefore this check is "overkill" to have on the server-side. Client side we always have all information needed for this.

Note the problem: "Prevent the developer of pushing a superrepo that has submodule (commits) only locally avaliable"

That's the problem we're trying to solve, NOT:

"Prevent the developer of pushing a superrepo that has submodule (commits) not avaliable for an other developer"

The second problem is just too complex and too slow to solve in a generic way.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com