Re: [PATCHv3 00/11] Expose the submodule parallelism to the user

On Wed, Nov 4, 2015 at 9:54 AM, Junio C Hamano [off-list ref] wrote:

Stefan Beller [off-list ref] writes:

quoted

Where does it apply?
---
This series applies on top of d075d2604c0f92045caa8d5bb6ab86cf4921a4ae (Merge
branch 'rs/daemon-plug-child-leak' into sb/submodule-parallel-update) and replaces
the previous patches in sb/submodule-parallel-update

What does it do?
---
This series should finish the on going efforts of parallelizing
submodule network traffic. The patches contain tests for clone,
fetch and submodule update to use the actual parallelism both via
command line as well as a configured option. I decided to go with
"submodule.jobs" for all three for now.

The order of patches and where the series builds makes me suspect
that I have been expecting too much from the "parallel-fetch" topic.

I've been hoping that it would be useful for the project as a whole
to polish the other topic and make it available to wider audience
sooner by itself (both from "end users get improved Git early"
aspect and from "the core machinery to be reused in follow-up
improvements are made closer to perfection sooner" perspective).  So
I've been expecting that "Let's fix it on Windows" change directly
on top of sb/submodule-parallel-fetch to make that topic usable
before everything else.

I can resend the patches on top of sb/submodule-parallel-fetch
(though looking at sb/submodule-parallel-fetch..d075d2604c0f920
[Merge branch 'rs/daemon-plug-child-leak' into sb/submodule-parallel-update]
I don't expect conflicts, so it would be a verbatim resend)

Other patches in this series may require
the child_process_cleanup() change, so they may be applied on top of
the merge between sb/submodule-parallel-fetch (updated for Windows)
and rs/daemon-plug-child-leak topic.

I assumed the rs/daemon-plug-child-leak topic is no feature, but cleanup.
Which is why I would have expected a sb/submodule-parallel-fetch-for-windows
pointing at maybe the third patch of the series on top of
rs/daemon-plug-child-leak

That does not seem to be what's happening here (note: I am not
complaining; I am just trying to make sure expectation matches
reality).  Am I reading you correctly?

I really wanted to send out just one series, my bad.
The ordering made sense to me (first the run-command related fixes
and then the new features in later patches)

I think sb/submodule-parallel-fetch + sb/submodule-parallel-update
as a single topic would need more time to mature to be in a tagged
release than we have in the remainder of this cycle.

I agree on that.

 It is likely
that the former topic has a chance to get rebased after 2.7 happens.
And that would allow us to (1) use the child_process_cleanup() from
get-go instead of _deinit and to (2) get the machinery right both
for UNIX and Windows from get-go.  Which would make the result
easier to understand.  As this is one of the more important areas,
it matters to keep the resulting code and the rationale behind it
understandable by reading "log --reverse -p".

So you are saying that reading the Windows cleanup patch
before the s/deinit/clear/ Patch by Rene makes it way easier to understand?
Which is why you would prefer another history. (Merging an updated
sb/submodule-parallel-fetch again to  rs/daemon-plug-child-leak or even
sb/submodule-parallel-update)

Thanks,
Stefan