Re: js/bisect-in-c, was Re: What's cooking in git.git (Aug 2022, #05; Mon, 15)

Hi Junio,

On Wed, 17 Aug 2022, Junio C Hamano wrote:

The exit codes before and after this patch series are a red herring. The _current_ code prints this when calling `git bisect start -h`:

	$ git bisect start -h
	usage: git bisect--helper --bisect-reset [<commit>]
	   or: git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]
	   or: git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>] [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
	   or: git bisect--helper --bisect-next
	   or: git bisect--helper --bisect-state (bad|new) [<rev>]
	   or: git bisect--helper --bisect-state (good|old) [<rev>...]
	   or: git bisect--helper --bisect-replay <filename>
	   or: git bisect--helper --bisect-skip [(<rev>|<range>)...]
	   or: git bisect--helper --bisect-visualize
	   or: git bisect--helper --bisect-run <cmd>...

	    --bisect-reset        reset the bisection state
	    --bisect-next-check   check whether bad or good terms exist
	    --bisect-terms        print out the bisect terms
	    --bisect-start        start the bisect session
	    --bisect-next         find the next bisection commit
	    --bisect-state        mark the state of ref (or refs)
	    --bisect-log          list the bisection steps so far
	    --bisect-replay       replay the bisection process from the given file
	    --bisect-skip         skip some commits for checkout
	    --bisect-visualize    visualize the bisection
	    --bisect-run          use <cmd>... to automatically bisect
	    --no-log              no log for BISECT_WRITE

Notice how this talks about `bisect--helper` and about `--bisect-reset`.

Also, the _current_ code exits with code 0 when calling `git bisect -h`.

This has been the case even as far back as v2.25.1, and possibly even longer.

Given these issues, I was mistakenly assuming that it would be okay to postpone these problems that are exclusively related to incorrect invocation of `git bisect`, and that it would make sense to focus on the conversion from shell code to C in _this_ patch series, and take care of these problems afterwards, instead of hodgepodging fixes for them into the same patch series as the conversion to C, the latter being hard enough to review as it stands, so much so that it received only a single high quality review.

But I see that you somehow got the idea that the review that lacked attention to the common code path somehow was a valid review and you somehow got it in your mind that this was valid feedback and that the patch series needs to be reworked so that it _also_ addresses issues that have been broken _before_ it.

Fine.

I'll try to get to it next week. It does leave a foul taste that we're not separating concerns properly in the Git project, but block a patch series that has a specific, already large scope, just because one reviewer wants it to have another scope and for some reason that now must be the scope of the patch series.

Ciao, Dscho