Re: js/bisect-in-c (was: What's cooking in git.git (Oct 2022, #09; Mon, 31))

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 11, 2022, 14:11 UTC
Message-ID: <221111.86a64xo86x.gmgdl@evledraar.gmail.com>
In-Reply-To: <8r1s24sp-8p26-sr61-3pp0-8o7r6pr72641@tzk.qr>

On Thu, Nov 10 2022, Johannes Schindelin wrote:

Show 42 quoted lines

> Hi Taylor,
>
> On Wed, 2 Nov 2022, Taylor Blau wrote:
>
>> On Wed, Nov 02, 2022 at 06:22:17PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >
>> > On Mon, Oct 31 2022, Taylor Blau wrote:
>> >
>> > > What's cooking in git.git (Oct 2022, #09; Mon, 31)
>> > > --------------------------------------------------
>> > >
>> > > * js/bisect-in-c (2022-08-30) 17 commits
>> > >  . bisect: no longer try to clean up left-over `.git/head-name` files
>> > >  . bisect: remove Cogito-related code
>> > >  . Turn `git bisect` into a full built-in
>> > >  . bisect: move even the command-line parsing to `bisect--helper`
>> > >  . bisect--helper: make `state` optional
>> > >  . bisect--helper: calling `bisect_state()` without an argument is a bug
>> > >  . bisect: avoid double-quoting when printing the failed command
>> > >  . bisect run: fix the error message
>> > >  . bisect: verify that a bogus option won't try to start a bisection
>> > >  . bisect--helper: migrate to OPT_SUBCOMMAND()
>> > >  . bisect--helper: make the order consistently `argc, argv`
>> > >  . bisect--helper: make `terms` an explicit singleton
>> > >  . bisect--helper: simplify exit code computation
>> > >  . bisect--helper: really retire `--bisect-autostart`
>> > >  . bisect--helper: really retire --bisect-next-check
>> > >  . bisect--helper: retire the --no-log option
>> > >  . Merge branch 'sg/parse-options-subcommand' into js/bisect-in-c
>> > >
>> > >  Final bits of "git bisect.sh" have been rewritten in C.
>> > >
>> > >  Needs review.
>> > >  cf. <xmqqv8pr8903.fsf@gitster.g>
>> > >  source: <pull.1132.v6.git.1661885419.gitgitgadget@gmail.com>
>> >
>> > I see this has been ejected out of "seen", presumably due to the
>> > outstanding conflicts.
>>
>> If I recall correctly, I ejected this one due to its age and lack of
>> attention. If you want to reroll it or if Johannes wants to take a look,
>> I'd be appreciative.

[Replying here instead of <s024qpqn-0roo-3rr2-nr4p-74p9296r6p02@tzk.qr>, but take this as a reply to that as well]

Show 7 quoted lines

> My last information was that I sent an iteration that was designed to
> address all outstanding concerns, including a rather major haul to put
> this on top of the new `OPT_SUBCOMMAND` feature that wasn't even dreamed
> of when I sent v1 of `bisect-in-c`, and then I only saw "Needs review."
> for several weeks and nobody objecting but also in particular Ævar (who
> raised concerns against this patch series several times over the last 10
> months) not chiming in with a "go ahead".

I think that "go ahead" from me was pretty unambiguous in [1], i.e. I said:

	"I think as it stands that it's a net improvement on "master". I
	would not mind it advanced in its current state. We can fix any
	outstanding issues with it later, particularly due to what seem
	like time constraints on your end[...]"

> So basically, I thought this was finally done and the next thing I hear is
> that it is ejected. [...]

What you're eliding here is that on the 4th Lukáš Doktor reported a regression in "git bisect" that pre-dated your cooking topic[2].

You didn't participate in that discussion, except now to say that you weren't motivated to review parts of the proposed fixes for that regression it[3].

Which is relevant because in your topic you spend a lot of time converting various code for the supposed needs of a migration to OPT_SUBCOMMAND()[4]. Just the diffstat for that was:

 1 file changed, 128 insertions(+), 134 deletions(-)

Which didn't get all the way to the full OPT_SUBCOMMAND(). That still needed [5] and [6], which are, respectively:

 1 file changed, 14 insertions(+), 15 deletions(-)
 2 files changed, 288 insertions(+), 194 deletions(-)

Whereas Đoàn's "dd/bisect-helper-subcommand", which is now in "next", and which was aiming for the shortest route to fixing that regression, and which also gave us OPT_SUBCOMMAND() is:

 3 files changed, 142 insertions(+), 120 deletions(-)

This is all around 2 months after Junio's feedback on your latest iteration was also questioning the need for that rewrite to get to the goal of OPT_SUBCOMMAND()[7].

> That's quite a frustrating experience, I must admit.
> At least I am not a new contributor who would be very much deterred from
> contributing any further by such an experience.

It's been frustrating for me too, particularly the as this isn't the first time that I feel like you're outright misrepresenting something I've said or done.

But I really don't think any of this translates to some general unfriendliness on the list that new contributors would run into.

To take just the above example: New contributors aren't sending hundreds of lines of patches to get to a stated goal of "X" by doing "A", and then ignoring or dismissing reviews when it's pointed out to them that they can get to "X" in a much easier manner by doing "B" instead.

Your original topic is a daunting:

 7 files changed, 406 insertions(+), 388 deletions(-)

Which, as noted above the replacement of Đoàn's "dd/bisect-helper-subcommand" is:

 3 files changed, 142 insertions(+), 120 deletions(-)

And his "dd/git-bisect-builtin" in seen is:

 5 files changed, 77 insertions(+), 58 deletions(-)

Does that get us all the same thing sas js/bisect-in-c? No. But it does get us to the primary goal whith fewer changes, which I think is the main reason why your series has had such a hard time advancing.

1. https://lore.kernel.org/git/220819.86sfls6zhh.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/1cb1c033-0525-7e62-8c09-81019bf26060@redhat.com/
3. https://lore.kernel.org/git/2477861r-9363-75sn-q415-o19206q70p90@tzk.qr/
4. https://lore.kernel.org/git/92b3b116ef8f879192d9deb94d68b73e29d5dcd6.1661885419.git.gitgitgadget@gmail.com/
5. https://lore.kernel.org/git/c9dc0281e38bf9bc0bce72de172b5dbadbcbb1f5.1661885419.git.gitgitgadget@gmail.com/
6. https://lore.kernel.org/git/e97e187bbec93b47f35e3dd42b4831f1c1d8658d.1661885419.git.gitgitgadget@gmail.com/
7. https://lore.kernel.org/git/xmqqmtblighr.fsf@gitster.g/