Thread (36 messages) 36 messages, 3 authors, 2018-10-16

Re: [PATCH 18/19] submodule: don't add submodule as odb for push

From: Jonathan Tan <hidden>
Date: 2018-10-11 23:00:40

The submodule was added as an alternative in eb21c732d6 (push: teach
--recurse-submodules the on-demand option, 2012-03-29), but was
not explained, why.

In similar code, submodule_has_commits, the submodule is added as an
alternative to perform a quick check if we need to dive into the submodule.

However in push_submodule
(a) for_each_remote_ref_submodule will also provide the quick check and
(b) after that we don't need to have submodule objects around, as all
    further code is to spawn a separate process.
After some investigation, I think I understand. I would explain it this
way:

  In push_submodule(), because we do not actually need access to objects
  in the submodule, do not invoke add_submodule_odb().
  (for_each_remote_ref_submodule() does not require access to those
  objects, and the actual push is done by spawning another process,
  which handles object access by itself.)

I'm not sure if it's worth mentioning the commit in which the call was
introduced, since nothing seems to have changed between then and now
(the same bug is present when it was introduced, and now).

I also checked the users of push_submodule() (transport_push()) and
indeed it doesn't seem to make use of the additional objects brought in
by add_submodule_odb().

Do you know if pushing of submodules is exercised by any test?

Other than that, the code itself looks good.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help