| Message ID | cover-00.13-00000000000-20221104T132117Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bisect: v2.30.0 "run" regressions + make it built-in | expand |
On Fri, Nov 04, 2022 at 02:22:37PM +0100, Æ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". Can you and Đoàn work together to find a way that your two series can work together? Reading both, this is definitely a "one or the other" situation currently, and I would much rather pick up a more targeted fix in the short-term. Thanks, Taylor
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@redhat.com/ > 2. https://lore.kernel.org/git/pull.1132.v6.git.1661885419.gitgitgadget@gmail.com/ > 3. https://lore.kernel.org/git/92b3b116ef8f879192d9deb94d68b73e29d5dcd6.1661885419.git.gitgitgadget@gmail.com/ > 4. https://lore.kernel.org/git/081f3f7f9501012404fb9e59ab6d94f632180b53.1667561761.git.congdanhqx@gmail.com/ > > 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 > >
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@redhat.com/ 2. https://lore.kernel.org/git/pull.1132.v6.git.1661885419.gitgitgadget@gmail.com/ 3. https://lore.kernel.org/git/92b3b116ef8f879192d9deb94d68b73e29d5dcd6.1661885419.git.gitgitgadget@gmail.com/ 4. https://lore.kernel.org/git/081f3f7f9501012404fb9e59ab6d94f632180b53.1667561761.git.congdanhqx@gmail.com/ 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