Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > A re-roll of [1] which aims to address the concerns about the previous > 8-part series being too large to fix a release regression. "If it > isn't bolted down, throw it overboard!". > > The main change here is: > > * The new "ungroup" parameter is now passed via an "extern" parameter. > * Tests for existing run-command.c behavior (not narrowly needed for > the regression fix) are gone. > * Adding an INIT macro is gone, instead we explicitly initialize to NULL. > * Stray bugfix for existing hook test is gone. > > etc. I think all of those still make sense, but they're something I > can rebase on this topic once it (hopefully) lands. In the meantime > the updated commit messages for the remaining two (see start of the > range-diff below) argue for this being a a safe API change, even if > the interface is a bit nasty. So the approach taken here is that we assume the reported one is the only regression and keep going with run_process_parallel() API. I still share the sentiment with Dscho that it is generally a bad idea, when dealing with a regression, to double-down and dig in your heels to keep the change that caused a regression with paper over patches, but too much time has passed since the release, and a patch or two on top does look like a quicker way forward. I left a few comments on the implementation, but modulo these small details, the code looks OK (provided that the assumption holds true, that is, of course). Thanks.
