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

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

On 11-06-26 02:29 PM, Fredrik Gustafsson wrote:

quoted

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

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