On Thu, Jul 13, 2017 at 11:37 AM, Junio C Hamano <gitster@xxxxxxxxx> 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