Patchwork [6,of,6] commands: use dirstateguard instead of begin/end-parentchange for backout

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Oct. 8, 2015, 6:59 p.m.
Message ID <e3faacb44a1208939575.1444330745@feefifofum>
Download mbox | patch
Permalink /patch/10886/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Oct. 8, 2015, 6:59 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1444330427 -32400
#      Fri Oct 09 03:53:47 2015 +0900
# Node ID e3faacb44a12089395756401d91b45fe67436c44
# Parent  f4ff0385ec32d5192ce67fc6228ec87333335b14
commands: use dirstateguard instead of begin/end-parentchange for backout

Before this patch, "hg backout" uses 'begin'/'end'-'parentchange()'
of 'dirstate' class to avoid writing incomplete dirstate changes out
at failure.

But this framework doesn't work as expected, if 'dirstate.write()' is
invoked between them. In fact, in-memory dirstate changes may be
written out at 'repo.status()' implied by 'merge.update()', even
before this patch.

To restore dirstate as expected at failure of "hg backout", this patch
uses 'dirstateguard' instead of 'begin'/'end'-'parentchange()'.
Pierre-Yves David - Oct. 9, 2015, 12:29 p.m.
On 10/08/2015 11:59 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1444330427 -32400
> #      Fri Oct 09 03:53:47 2015 +0900
> # Node ID e3faacb44a12089395756401d91b45fe67436c44
> # Parent  f4ff0385ec32d5192ce67fc6228ec87333335b14
> commands: use dirstateguard instead of begin/end-parentchange for backout

Pushed to the clowncopter thanks.

Do you think it would be possible to setup a devel-warning for any 
dirstate changes that happen outside a transaction or a dirstateguard range?
Katsunori FUJIWARA - Oct. 9, 2015, 6:39 p.m.
At Fri, 09 Oct 2015 05:29:17 -0700,
Pierre-Yves David wrote:
> 
> 
> 
> On 10/08/2015 11:59 AM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1444330427 -32400
> > #      Fri Oct 09 03:53:47 2015 +0900
> > # Node ID e3faacb44a12089395756401d91b45fe67436c44
> > # Parent  f4ff0385ec32d5192ce67fc6228ec87333335b14
> > commands: use dirstateguard instead of begin/end-parentchange for backout
> 
> Pushed to the clowncopter thanks.
> 
> Do you think it would be possible to setup a devel-warning for any 
> dirstate changes that happen outside a transaction or a dirstateguard range?

What about the way below ?

  1. introduce a kind of the stack into dirstate

     for example, in constructor of dirstate:

         self._guards = []

  2. push "scope id" into the above stack at starting scope of
     dirstateguard or transaction

     for example, at starting a transaction in localrepo:

         dirstate._guards.append('transaction.%s' % (id(tr)))

  3. examine whether this stack is empty or not at changing dirstate
     and show develwarning

     Adding examination straightforward to functions making dirstate
     dirty seems to have some performance impact, doesn't it ? (or is
     it cheeper enough than maybe preceding file I/O ?)

     If it is reasonable to assume that 'devel' configurations in
     'dirstate.ui' are kept during its lifetime, how about wrapping
     functions above by checker decorator at construction time ?

BTW, would you suppose enclosing code block like
'merge.recordupdates()' by dirstateguard or so ?

> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -547,14 +547,14 @@ 
         bheads = repo.branchheads(branch)
         rctx = scmutil.revsingle(repo, hex(parent))
         if not opts.get('merge') and op1 != node:
+            dsguard = cmdutil.dirstateguard(repo, 'backout')
             try:
                 ui.setconfig('ui', 'forcemerge', opts.get('tool', ''),
                              'backout')
-                repo.dirstate.beginparentchange()
                 stats = mergemod.update(repo, parent, True, True, False,
                                         node, False)
                 repo.setparents(op1, op2)
-                repo.dirstate.endparentchange()
+                dsguard.close()
                 hg._showstats(repo, stats)
                 if stats[3]:
                     repo.ui.status(_("use 'hg resolve' to retry unresolved "
@@ -567,6 +567,7 @@ 
                     return 0
             finally:
                 ui.setconfig('ui', 'forcemerge', '', '')
+                lockmod.release(dsguard)
         else:
             hg.clean(repo, node, show_stats=False)
             repo.dirstate.setbranch(branch)