[Patch 1 of 2] add support for multiple possible bisect results

Patrick Mézard pmezard at gmail.com
Sat Aug 2 12:46:30 CDT 2008


Bernhard Leiner a écrit :
> # HG changeset patch
> # User Bernhard Leiner <bleiner at gmail.com>
> # Date 1217682662 -7200
> # Node ID 81aa4924ed9d7ceb4a97c2a7a2d27ac40c18b856
> # Parent  e726c476ee455fda5433a629fe5e96d6d100b450
> 
> The _real_ reason for both issue issue1228 and issue1182 is that bisect can not
> handle cases where there are multiple possibilites for the result.

[skip]

I think the patches are good, and will be pushed when the following remarks are addressed:

> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -328,10 +328,19 @@
>      # actually bisect
>      node, changesets, good = hbisect.bisect(repo.changelog, state)
>      if changesets == 0:
> -        ui.write(_("The first %s revision is:\n") % (good and "good" or "bad"))
>          displayer = cmdutil.show_changeset(ui, repo, {})
> -        displayer.show(changenode=node)
> -    elif node is not None:
> +        if len(node) == 1:
> +            # narrowed it down to a single revision
> +            ui.write(_("The first %s revision is:\n") % (good and
> "good" or "bad"))

How did you submit the patches ? I had to unfold a couple of lines manually before patching. Try using the patchbomb extension if you can.

> +            displayer.show(changenode=node[0])
> +        else:
> +            # multiple possible revisions
> +            ui.write(_("Due to skipped revisions, the first "
> +                       "%s revision could be any of:\n")
> +                     % (good and "good" or "bad"))

Perhaps we could compute the good/bad string at a single place.

> +            for n in node:
> +                displayer.show(changenode=n)
> +    else:
>          # compute the approximate number of remaining tests
>          tests, size = 0, 2
>          while size <= changesets:
> diff --git a/mercurial/hbisect.py b/mercurial/hbisect.py
> --- a/mercurial/hbisect.py
> +++ b/mercurial/hbisect.py
> @@ -61,9 +61,11 @@
>                          visit.append(prev)
> 
>      # have we narrowed it down to one entry?
> +    # or have all other possible candidates besides 'bad' have been skipped?
>      tot = len(candidates)
> -    if tot == 1:
> -        return (bad, 0, good)
> +    if tot == 1 or [c for c in candidates if (c not in skip) and (c
> != badrev)] == []:

Let's compute the list on another line, and test for the empty list using boolean evaluation directly like "not unskipped".

> +        return ([changelog.node(rev) for rev in candidates], 0, good)
> +

Can we return a list of nodes in all cases ? And if you still have a couple of spare minutes, add a docstring telling what bisect() actually returns.

>      perfect = tot / 2
> 
>      # find the best node to test

Finally, could you submit those patches against -stable instead ? I know we are changing the bisect API but this is a minor point IMHO, and we really want this in the next bug fix release. The rebasing is trivial, only the .out has to be updated.

--
Patrick Mézard



More information about the Mercurial-devel mailing list