Re: [RFC 2/2] Don't push a repository with unpushed submodules
- From
Heiko Voigt <hvoigt@hvoigt.net>
- Date
- Jun 28, 2011, 19:30 UTC
- Message-ID
- <20110628193034.GB3700@book.hvoigt.net>
- In-Reply-To
- <7v1uydvmh0.fsf@alter.siamese.dyndns.org>
Hi,
On Tue, Jun 28, 2011 at 11:29:15AM -0700, Junio C Hamano wrote:
Show 30 quoted lines
> Fredrik Gustafsson <iveqy@iveqy.com> writes:
>
> > +static int is_submodule_commit_pushed(const char *path, const unsigned char sha1[20])
> > +{
> > + int is_pushed = 0;
> > + if (!add_submodule_odb(path) && lookup_commit_reference(sha1)) {
> > + struct child_process cp;
> > + const char *argv[] = {"rev-list", NULL, "--not", "--remotes", "-n", "1" , NULL};
> > + struct strbuf buf = STRBUF_INIT;
> > +
> > + argv[1] = sha1_to_hex(sha1);
> > + memset(&cp, 0, sizeof(cp));
> > + cp.argv = argv;
> > + cp.env = local_repo_env;
> > + cp.git_cmd = 1;
> > + cp.no_stdin = 1;
> > + cp.out = -1;
> > + cp.dir = path;
> > + if (!run_command(&cp) && !strbuf_read(&buf, cp.out, 41))
> > + is_pushed = 1;
> > + close(cp.out);
> > + strbuf_release(&buf);
> > + }
>
> What if
>
> (1) you are binding somebody else's project as your own submodule, you do
> not make any local changes (you won't be pushing them out anyway),
> and you do not have remote tracking branches in that submodule
> project?In this scenario the superproject can not be cloned that way that it would contain the submodule right? I would consider this a rather exotic way to work since pushing means to share your work somehow. This way you would share your work in the superproject but not from the submodule? How about not doing the is-pushed check when the submodule has no remotes? If we assume that only people having remote tracking branches want to share them via push this would allow your usecase.
> (2) you have a project you can push to that is bound as a submodule, you > have pushed the commit that is bound in the superproject's tree to > that submodule project, but you do not have remote tracking branches > in that submodule project?
This could also be solved with the above proposal.
Show 15 quoted lines
> (3) you have a project you can push to that is bound as a submodule, you > have multiple remotes defined (e.g. one for the public server you > intend for this check to be in effect, the other is just for your > private backup), and you have pushed the necessary commit to your > backup but not to the public one? > > The above check would fail in cases (1) and (2) even though it does not > have to. It would succeed in case (3), but people would not be able to use > the superproject as the necessary commit is not there but only on your > work and backup repositories. > > What am I missing? > > I am not sure if the check imposed on the client end is such a great > idea.
This check is solely meant as a convenience security measure. It should and can not enforce a tight check whether a superproject (including its submodules) can be cloned/checked out at all times. But it ensures that a developer has pushed his submodule commits "somewhere" which is enough in practice.
One typical scenario is that people are working together using shared remotes. In this scenario this patch provides a consistency check which catches typical mistakes.
If you fork a project you might change or add a new url for a submodule locally since you cannot directly push to upstream. This is situation 3 in your above description. All people working with you know which url you are using for the submodule. In this situation the check helps you and can not be enforced on the remote side since only the client knows which remotes the submodule has.
Maybe we should provide a configuration option for this check so that people who know what they are doing could switch it off?
Show 9 quoted lines
> I suspect that it would depend on the superproject which submodule > commit must exist "out there" for others to fetch. If you assume a closed > environment where all the superprojects and necessary submodule projects > are housed at a central location everybody pushes into and have tight > control over how project participants clone and check out the > superprojects and submodules, you do not have to worry about any of the > above, but then the server-side can perform the check when it accepts a > push (and you would already be doing other checks there in your hooks > anyway in the industrial setup, I would guess).
As mentioned above a check on the remote end is only applicable if you have a certain defined remote for the submodule in a superproject. This also has to be in an environment which has control over all projects/submodules. The presented solution does not just cover that but also the case where you fork and use different remotes than upstream.
Cheers Heiko