Hi Ævar, I see you Cc:ed me on this, but I have to admit that I am not motivated to review this patch series because it seems to be designed to interfere with `js/bisect-in-c` instead of being an honest effort to help me get that patch series over the finish line. I'd much rather see you assist me in the most efficient/minimal way possible to get `js/bisect-in-c` into a shape that can be integrated. Ciao, Johannes On Fri, 4 Nov 2022, Ævar Arnfjörð Bjarmason wrote: > This fixes the regression Lukáš Doktor reported in [1], and also gets > us the full way to a builtin/bisect.c and "git rm git-bisect.sh". > > Only 1-4/13 here are needed to fix the "git bisect run <cmd> [...] > --log" regression Lukáš reported, but as Jeff points out we'd still > conflate "--bisect-*" with the user arguments. That's fixed in 11/13 > here. > > The 1-4/13 here also fixes other but probably more minor "git bisect > run" regressions in v2.30.0, we changed the output in a few ways > without intending it. 4/13 gets us mostly back to v2.29.0 behavior, > 5/13 keeps the best of it and the current output. > > I think for the v2.30.0 regressions we're better off with just > something like 1-4/13 here for now, and possibly 5/13 too. > > But getting to the point of fixing the root cause of "--bisect-*" > being conflated is going to take quite a bit of churn. In the > side-thread Đoàn's diffstat is on the order of 1/2 of the size of this > series, and this gives us built-in "bisect". > > The 6-13 here is something I had already for a couple of days, I was > seeing if I could distill Johannes's [2] down to something much > smaller, to just make a beeline towards a built-in bisect. > > Johannes's refactors the "term" passing in [3], and Đoàn ends up > needing to do much the same in [4]. > > Here in 9/13 I instead just extend the OPT_SUBCOMMAND() API so it's > able to accept function callbacks with custom signatures, which > eliminates the need for most of that refactoring. 11/13 then makes use > of it. > > 1. https://lore.kernel.org/git/1cb1c033-0525-7e62-8c09-81019bf26060@xxxxxxxxxx/ > 2. https://lore.kernel.org/git/pull.1132.v6.git.1661885419.gitgitgadget@xxxxxxxxx/ > 3. https://lore.kernel.org/git/92b3b116ef8f879192d9deb94d68b73e29d5dcd6.1661885419.git.gitgitgadget@xxxxxxxxx/ > 4. https://lore.kernel.org/git/081f3f7f9501012404fb9e59ab6d94f632180b53.1667561761.git.congdanhqx@xxxxxxxxx/ > > Johannes Schindelin (3): > bisect--helper: remove dead --bisect-{next-check,autostart} code > bisect--helper: make `state` optional > Turn `git bisect` into a full built-in > > Ævar Arnfjörð Bjarmason (10): > bisect tests: test for v2.30.0 "bisect run" regressions > bisect: refactor bisect_run() to match CodingGuidelines > bisect: fix output regressions in v2.30.0 > bisect run: fix "--log" eating regression in v2.30.0 > bisect run: keep some of the post-v2.30.0 output > bisect test: test exit codes on bad usage > bisect--helper: emit usage for "git bisect" > bisect--helper: have all functions take state, argc, argv, prefix > parse-options API: don't restrict OPT_SUBCOMMAND() to one *_fn type > bisect--helper: convert to OPT_SUBCOMMAND_CB() > > Makefile | 3 +- > builtin.h | 2 +- > builtin/{bisect--helper.c => bisect.c} | 250 +++++++++++++------------ > git-bisect.sh | 84 --------- > git.c | 2 +- > parse-options.c | 9 +- > parse-options.h | 31 ++- > t/t6030-bisect-porcelain.sh | 109 +++++++++++ > 8 files changed, 277 insertions(+), 213 deletions(-) > rename builtin/{bisect--helper.c => bisect.c} (86%) > delete mode 100755 git-bisect.sh > > -- > 2.38.0.1452.g710f45c7951 > >
