[PATCH 3 of 8 phases] mq: turn changeset draft on qfinish (except if qparent is secret)

[Matt Mackall]

On Tue, 2012-01-17 at 18:35 +0100, pierre-yves.david at logilab.fr wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1326791375 -3600
> # Node ID 86530e2ad6e1126f063ad34ca222aae767f3865a
> # Parent  56ce840b148b8eaef0b8e6a58f26c9e70dff4e13
> mq: turn changeset draft on qfinish (except if qparent is secret)
> 
> As mq aumatotically set changeset as secret, it should make them draft when he
> is done with it. We do not move them automatically to draft when we detect that
> something else have also set them as secret through their parents.
> 
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -767,10 +767,12 @@ class queue(object):
>  
>          if numrevs:
>              qfinished = self.applied[:numrevs]
>              del self.applied[:numrevs]
>              self.applieddirty = True
> +        else:
> +            qfinished=[]

Missing whitespace.

Consider the pattern:

x = <trivial value>
if y:
    x = <hard value>
 
>          unknown = []
>  
>          for (i, p) in sorted([(self.findseries(p), p) for p in patches],
>                               reverse=True):
> @@ -789,10 +791,11 @@ class queue(object):
>                  msg = _('unknown patches: %s\n')
>                  raise util.Abort(''.join(msg % p for p in unknown))
>  
>          self.parseseries()
>          self.seriesdirty = True
> +        return [entry.node for entry in qfinished]
>  
>      def _revpatches(self, repo, revs):
>          firstrev = repo[self.applied[0].node].rev()
>          patches = []
>          for i, rev in enumerate(revs):
> @@ -816,11 +819,25 @@ class queue(object):
>              patches.append(patch)
>          return patches
>  
>      def finish(self, repo, revs):
>          patches = self._revpatches(repo, sorted(revs))
> -        self._cleanup(patches, len(patches))
> +        qfinished = self._cleanup(patches, len(patches))
> +        if qfinished:
> +            oldqbase = repo[qfinished[0]]
> +            for p in oldqbase.parents():

Can we have two parents here? If not:

oldqbase.p1().phase()

> +                if p.phase() == phases.secret:
> +                    break # qfinished changeset will stay secret
> +                else:
> +                    # Make them draft
> +                    lock = repo.lock()

Is wlock held at this point? If so, this may result in an AB/BA
deadlock. Also note that this can race with someone manually setting
phases.

> +                    try:
> +                        phases.advanceboundary(repo, phases.draft,
> +                                               [oldqbase.node()])
> +                    finally:
> +                        lock.release()
> +

Extra whitespace.

>      def delete(self, repo, patches, opts):
>          if not patches and not opts.get('rev'):
>              raise util.Abort(_('qdelete requires at least one revision or '
>                                 'patch name'))

-- 
Mathematics is the supreme nostalgia of our time.