Re: [WIP PATCH 1/3] git bisect old/new

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 4, 2015, 08:29 UTC
Message-ID: <CAP8UFD3K8NFWgiuusi47Puy4q8JZPD7X9wOj7H7yq+NYui0tsQ@mail.gmail.com>
In-Reply-To: <1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr>

On Thu, Jun 4, 2015 at 9:59 AM, Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr> wrote:

Show 60 quoted lines

> From: Christian Couder <chriscool@tuxfamily.org>
>
> When not looking for a regression during a bisect but for a fix or a
> change in another given property, it can be confusing to use 'good'
> and 'bad'.
>
> This patch introduce `git bisect new` and `git bisect old` as an
> alternative to 'bad' and good': the commits which have the most
> recent version of the property must be marked as `new` and the ones
> with the older version as `old`.
>
> The output will be the first commit after the change in the property.
> During a new/old bisect session you cannot use bad/good commands and
> vice-versa.
>
> `git bisect replay` works fine for old/new bisect sessions.
>
> Some commands are still not available for old/new:
>
>      * git bisect start [<new> [<old>...]] is not possible: the
>        commits will be treated as bad and good.
>      * git rev-list --bisect does not treat the revs/bisect/new and
>        revs/bisect/old-SHA1 files.
>      * git bisect visualize seem to work partially: the tags are
>        displayed correctly but the tree is not limited to the bisect
>        section.
>
> Related discussions:
>
>         - http://thread.gmane.org/gmane.comp.version-control.git/86063
>                 introduced bisect fix unfixed to find fix.
>         - http://thread.gmane.org/gmane.comp.version-control.git/182398
>                 discussion around bisect yes/no or old/new.
>         - http://thread.gmane.org/gmane.comp.version-control.git/199758
>                 last discussion and reviews
>
> Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
> Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
> Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
> Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
> Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
> Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
> Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> ---
> We took account of most of the "easy" reviews that were discuted two years ago.
> We hope we didn't missed any.
> http://thread.gmane.org/gmane.comp.version-control.git/199758
>
> We corrected various issues that were not reported:
> -one that caused a "fatal ... not a valid ref" at the end of the bisection.
> -the autostart was causing issues, the following lines were working :
> git bisect new HEAD
> git bisect bad HEAD
> git bisect good aGoodCommit
>
> The hard review which we were thinking on was the issue of the maintaining
> of old/new and allow easy support of new "tags" like yes/no in the future.
> I tried to remove the maximum of bad/good and old/new which were hard wrote in
> the code but I'm not completly satisfied. This patch is clearly a v1.

Thanks for working on this. Here are some suggestions that I should probably have told you before, but didn't:

- Take ownership of all the patches.
- Patch 3/3 renames some variables in bisect.c, do the same thing in
git-bisect.sh for consistency.
- Squash all the patches together.
- Try to find a way to break down the resulting patch into a logical
patch series which adds tests at each logical step. This might be
difficult. You might want to add features to git bisect--helper first
for example, then test those features, then add features to git bisect
and then test those features.

Best, Christian.

Show 15 quoted lines

> We're currently working on:
>
> * rebasing the history of the patch
> * git rev-list --bisect does not treat the revs/bisect/new and
> revs/bisect/old-SHA1 files.
> * git bisect visualize seem to work partially: the tags are displayed
> correctly but the tree is not limited to the bisect section.
> * adding tests about old/new
>
> Some other problems that might also be considerred later :
> * change/add valid terms (e.g "unfixed/fixed" instead of "old/new")
> *
> * git bisect start [<new> [<old>...]] is not possible: the commits
> will be treated as bad and good.
>