Re: "git bisect run" strips "--log" from the list of arguments

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Nov 4, 2022, 12:37 UTC
Message-ID: <20221104123722.GA6942@szeder.dev>
In-Reply-To: <Y2TfUFkLUa2tHdS7@coredump.intra.peff.net>

On Fri, Nov 04, 2022 at 05:45:52AM -0400, Jeff King wrote:

Show 29 quoted lines

> On Fri, Nov 04, 2022 at 07:31:26AM +0100, Lukáš Doktor wrote:
> 
> > Steps to Reproduce:
> > 
> > 1. git bisect start BAD GOOD
> > 2. git bisect run ./myscript arg1 --log arg2 --log -- arg3 --log arg4
> > 
> > Results with 2.34.1:
> > 
> >     running  './myscript' 'arg1' 'arg2' '--' 'arg3' '--log' 'arg4'
> > 
> > Results with 2.33.0:
> > 
> >     running ./myscript arg1 --log arg2 --log -- arg3 --log arg4
> 
> Thanks for an easy reproduction recipe. I used this as an easy-to-see
> test case, which works in any repo:
> 
>   git bisect start HEAD HEAD~2 >/dev/null 2>&1
>   git bisect bisect run echo --log 2>&1 | grep running
> 
> > Is this expected? In https://bugzilla.redhat.com/show_bug.cgi?id=2139883 Todd suggested it might be related to
> > 
> >     d1bbbe45df (bisect--helper: reimplement `bisect_run` shell function in C, 2021-09-13) 
> > 
> > but I haven't tried it myself.
> 
> Yes, it bisects to that commit. +cc Christian, who mentored this gsoc
> project.

It might bisect to that commit, but I don't think that commit is the real culprit here.

'git-bisect.sh' used to have a 'bisect_write' function (not subcommand!), whose third positional parameter was a "nolog" flag. This flag was only used when 'bisect_start' invoked it to write the starting good and bad revisions. Then 0f30233a11 (bisect--helper: `bisect_write` shell function in C, 2019-01-02) ported it to C as a command mode of 'bisect--helper', and:

  - Added the '--no-log' option, but since 'bisect--helper' has
    command modes not subcommands, all other command modes see and
    handle that option as well.
  - Converted all callsites of 'bisect_write' to invocations of 'git
    bisect--helper --bisect-write', but while doing so that one
    callsite in 'bisect_start' was misconverted, and instead of
    passing the '--no-log' option, it still passed 'nolog' as
    parameter.  Consequently, 'git bisect start' wrote a couple of
    extra lines to '.git/BISECT_LOG'.

This bogus state didn't last long, however, because in the same patch series 06f5608c14 (bisect--helper: `bisect_start` shell function partially in C, 2019-01-02) the C reimplementation of bisect_start() started calling the bisect_write() C function, this time with the right 'nolog' function parameter. From then on there was no need for the '--no-log' option in 'bisect--helper'. Eventually all bisect subcommands were ported to C as 'bisect--helper' command modes, each calling the bisect_write() C function instead, but when the '--bisect-write' command mode was removed in 68efed8c8a (bisect--helper: retire `--bisect-write` subcommand, 2021-02-03) it forgot to remove that '--no-log' option.