Re: [PATCH] submodule: use cheaper check for submodule pushes

[Stefan Beller <sbeller@google.com>]

On Thu, Jul 13, 2017 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:

>
> I think Jonathan's question (which I concurred) is if we also ended
> up relying on the side effect of calling that function (i.e. being
> able to now find objects that are not in our repository but in the
> submodule's object store).  By looking at the eb21c732d6, we can
> tell that the original didn't mean to and didn't add any code that
> relies on the ability to be able to read from the submodule object
> store.  I am not sure if that is still true after 5 years (i.e. is
> there any new code added in the meantime that made us depend on the
> ability to read from submodule object store?).

Yes we are safe, because the function itself only spawns a child process
(not using any of the objects).

It's only caller push_unpushed_submodules also doesn't rely on objects
loaded after calling push_submodule.

The caller of push_unpushed_submodules (transport.c, transport_push)
also doesn't need submodule objects loaded.

> My hunch (and hope) is that we are probably safe, but that is a lot
> weaker than "yes this is a good change we want to apply".

Given the above (I went through the code), all I can do is repeating
"yes this is a good change we want to apply".

Thanks,
Stefan