Patchwork [8,of,8] bisect: extra a small initialisation outside of a loop

login
register
mail settings
Submitter Pierre-Yves David
Date Oct. 9, 2016, 8:57 a.m.
Message ID <4df38a2b962c5cccb1a3.1476003462@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/16977/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Pierre-Yves David - Oct. 9, 2016, 8:57 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1472008186 -7200
#      Wed Aug 24 05:09:46 2016 +0200
# Node ID 4df38a2b962c5cccb1a3f6716a7d6a872832b662
# Parent  e5012181a722d5cb544c935400c676fce2d7b0a8
# EXP-Topic bisect
bisect: extra a small initialisation outside of a loop

Having initialisation done during the first iteration is cute, but can be
avoided.
Yuya Nishihara - Oct. 9, 2016, 8:41 p.m.
On Sun, 09 Oct 2016 10:57:42 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1472008186 -7200
> #      Wed Aug 24 05:09:46 2016 +0200
> # Node ID 4df38a2b962c5cccb1a3f6716a7d6a872832b662
> # Parent  e5012181a722d5cb544c935400c676fce2d7b0a8
> # EXP-Topic bisect
> bisect: extra a small initialisation outside of a loop
> 
> Having initialisation done during the first iteration is cute, but can be
> avoided.
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -893,6 +893,8 @@ def bisect(ui, repo, rev=None, extra=Non
>              node, p2 = repo.dirstate.parents()
>              if p2 != nullid:
>                  raise error.Abort(_('current bisect revision is a merge'))
> +            if rev:
> +                node = repo[scmutil.revsingle(repo, rev, node)].node()

It seems we need to initialize node even if --noupdate is specified.

>          try:
>              while changesets:
>                  # update state
> @@ -910,9 +912,8 @@ def bisect(ui, repo, rev=None, extra=Non
>                      raise error.Abort(_("%s killed") % command)
>                  else:
>                      transition = "bad"
> -                ctx = scmutil.revsingle(repo, rev, node)
> -                rev = None # clear for future iterations
> -                state[transition].append(ctx.node())
> +                state[transition].append(node)
> +                ctx = repo[node]
Pierre-Yves David - Oct. 10, 2016, 2:41 p.m.
On 10/09/2016 10:41 PM, Yuya Nishihara wrote:
> On Sun, 09 Oct 2016 10:57:42 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1472008186 -7200
>> #      Wed Aug 24 05:09:46 2016 +0200
>> # Node ID 4df38a2b962c5cccb1a3f6716a7d6a872832b662
>> # Parent  e5012181a722d5cb544c935400c676fce2d7b0a8
>> # EXP-Topic bisect
>> bisect: extra a small initialisation outside of a loop
>>
>> Having initialisation done during the first iteration is cute, but can be
>> avoided.
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -893,6 +893,8 @@ def bisect(ui, repo, rev=None, extra=Non
>>              node, p2 = repo.dirstate.parents()
>>              if p2 != nullid:
>>                  raise error.Abort(_('current bisect revision is a merge'))
>> +            if rev:
>> +                node = repo[scmutil.revsingle(repo, rev, node)].node()
>
> It seems we need to initialize node even if --noupdate is specified.

Hum, yes, that patch result in 'rev' being ignored with in the noupdate 
case. getting this conditional one level up is fixing this. I assume you 
want a V2?

Cheers,
Yuya Nishihara - Oct. 10, 2016, 6:52 p.m.
On Mon, 10 Oct 2016 16:41:13 +0200, Pierre-Yves David wrote:
> On 10/09/2016 10:41 PM, Yuya Nishihara wrote:
> > On Sun, 09 Oct 2016 10:57:42 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> >> # Date 1472008186 -7200
> >> #      Wed Aug 24 05:09:46 2016 +0200
> >> # Node ID 4df38a2b962c5cccb1a3f6716a7d6a872832b662
> >> # Parent  e5012181a722d5cb544c935400c676fce2d7b0a8
> >> # EXP-Topic bisect
> >> bisect: extra a small initialisation outside of a loop
> >>
> >> Having initialisation done during the first iteration is cute, but can be
> >> avoided.
> >>
> >> diff --git a/mercurial/commands.py b/mercurial/commands.py
> >> --- a/mercurial/commands.py
> >> +++ b/mercurial/commands.py
> >> @@ -893,6 +893,8 @@ def bisect(ui, repo, rev=None, extra=Non
> >>              node, p2 = repo.dirstate.parents()
> >>              if p2 != nullid:
> >>                  raise error.Abort(_('current bisect revision is a merge'))
> >> +            if rev:
> >> +                node = repo[scmutil.revsingle(repo, rev, node)].node()
> >
> > It seems we need to initialize node even if --noupdate is specified.
> 
> Hum, yes, that patch result in 'rev' being ignored with in the noupdate 
> case. getting this conditional one level up is fixing this. I assume you 
> want a V2?

Yes, V2 please.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -893,6 +893,8 @@  def bisect(ui, repo, rev=None, extra=Non
             node, p2 = repo.dirstate.parents()
             if p2 != nullid:
                 raise error.Abort(_('current bisect revision is a merge'))
+            if rev:
+                node = repo[scmutil.revsingle(repo, rev, node)].node()
         try:
             while changesets:
                 # update state
@@ -910,9 +912,8 @@  def bisect(ui, repo, rev=None, extra=Non
                     raise error.Abort(_("%s killed") % command)
                 else:
                     transition = "bad"
-                ctx = scmutil.revsingle(repo, rev, node)
-                rev = None # clear for future iterations
-                state[transition].append(ctx.node())
+                state[transition].append(node)
+                ctx = repo[node]
                 ui.status(_('changeset %d:%s: %s\n') % (ctx, ctx, transition))
                 hbisect.checkstate(state)
                 # bisect