Patchwork [2,of,3] dirstate: wrap setparent calls with begin/endparentchange (issue4353)

# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1409942180 25200
#      Fri Sep 05 11:36:20 2014 -0700
# Node ID ca5a5cd5a9840dd3d391a6204f5e3edb8b097b44
# Parent  8c6d502c92c8ed8dfa79b8fac4c8a2cc9e6c4153
dirstate: wrap setparent calls with begin/endparentchange (issue4353)

This wraps all the locations of dirstate.setparent with the appropriate
begin/endparentchange calls. This will prevent exceptions during those calls
from causing incoherent dirstates (issue4353).

I'm not sure how to test this, since it should only ever occur if there's an
exception, like ctrl+c.

On 09/05/2014 12:36 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1409942180 25200
> #      Fri Sep 05 11:36:20 2014 -0700
> # Node ID ca5a5cd5a9840dd3d391a6204f5e3edb8b097b44
> # Parent  8c6d502c92c8ed8dfa79b8fac4c8a2cc9e6c4153
> dirstate: wrap setparent calls with begin/endparentchange (issue4353)
>
> This wraps all the locations of dirstate.setparent with the appropriate
> begin/endparentchange calls. This will prevent exceptions during those calls
> from causing incoherent dirstates (issue4353).
>
> I'm not sure how to test this, since it should only ever occur if there's an
> exception, like ctrl+c.

The easiest way to do that is to write an extension that monkeypatches a 
function to raise KeyboardInterrupt. See the patch in 
http://bz.selenic.com/show_bug.cgi?id=4344#c0 for an example (it raises 
util.Abort instead, which is just as good).

>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -220,6 +220,7 @@
>           cmdutil.revert(ui, repo, ctx, (wcpar, node.nullid), all=True)
>           stats = None
>       else:
> +        repo.dirstate.beginparentchange()
>           try:
>               # ui.forcemerge is an internal variable, do not document
>               repo.ui.setconfig('ui', 'forcemerge', opts.get('tool', ''),
> @@ -229,6 +230,7 @@
>           finally:
>               repo.ui.setconfig('ui', 'forcemerge', '', 'histedit')
>           repo.setparents(wcpar, node.nullid)
> +        repo.dirstate.endparentchange()
>           repo.dirstate.write()
>           # fix up dirstate for copies and renames
>       cmdutil.duplicatecopies(repo, ctx.rev(), ctx.p1().rev())
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -816,12 +816,14 @@
>                           merged.append(f)
>                       else:
>                           removed.append(f)
> +                repo.dirstate.beginparentchange()
>                   for f in removed:
>                       repo.dirstate.remove(f)
>                   for f in merged:
>                       repo.dirstate.merge(f)
>                   p1, p2 = repo.dirstate.parents()
>                   repo.setparents(p1, merge)
> +                repo.dirstate.endparentchange()
>   
>               if all_files and '.hgsubstate' in all_files:
>                   wctx = repo[None]
> @@ -1451,7 +1453,7 @@
>                   if keepchanges and tobackup:
>                       raise util.Abort(_("local changes found, refresh first"))
>                   self.backup(repo, tobackup)
> -
> +                repo.dirstate.beginparentchange()
>                   for f in a:
>                       util.unlinkpath(repo.wjoin(f), ignoremissing=True)
>                       repo.dirstate.drop(f)
> @@ -1460,6 +1462,7 @@
>                       repo.wwrite(f, fctx.data(), fctx.flags())
>                       repo.dirstate.normal(f)
>                   repo.setparents(qp, nullid)
> +                repo.dirstate.endparentchange()
>               for patch in reversed(self.applied[start:end]):
>                   self.ui.status(_("popping %s\n") % patch.name)
>               del self.applied[start:end]
> @@ -1599,6 +1602,7 @@
>               bmlist = repo[top].bookmarks()
>   
>               try:
> +                repo.dirstate.beginparentchange()
>                   if diffopts.git or diffopts.upgrade:
>                       copies = {}
>                       for dst in a:
> @@ -1651,6 +1655,7 @@
>   
>                   # assumes strip can roll itself back if interrupted
>                   repo.setparents(*cparents)
> +                repo.dirstate.endparentchange()
>                   self.applied.pop()
>                   self.applieddirty = True
>                   strip(self.ui, repo, [top], update=False, backup=False)
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -360,7 +360,9 @@
>                                             editor=editor)
>                   else:
>                       # Skip commit if we are collapsing
> +                    repo.dirstate.beginparentchange()
>                       repo.setparents(repo[p1].node())
> +                    repo.dirstate.endparentchange()
>                       newrev = None
>                   # Update the state
>                   if newrev is not None:
> @@ -466,7 +468,9 @@
>   def concludenode(repo, rev, p1, p2, commitmsg=None, editor=None, extrafn=None):
>       'Commit the changes and store useful information in extra'
>       try:
> +        repo.dirstate.beginparentchange()
>           repo.setparents(repo[p1].node(), repo[p2].node())
> +        repo.dirstate.endparentchange()
>           ctx = repo[rev]
>           if commitmsg is None:
>               commitmsg = ctx.description()
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -659,6 +659,7 @@
>   
>           n = None
>           if update:
> +            repo.dirstate.beginparentchange()
>               if p1 != parents[0]:
>                   updatefunc(repo, p1.node())
>               if p2 != parents[1]:
> @@ -698,6 +699,7 @@
>                   n = repo.commit(message, opts.get('user') or user,
>                                   opts.get('date') or date, match=m,
>                                   editor=editor, force=partial)
> +            repo.dirstate.endparentchange()
>           else:
>               if opts.get('exact') or opts.get('import_branch'):
>                   branch = branch or 'default'
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -483,9 +483,11 @@
>               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()
>                   hg._showstats(repo, stats)
>                   if stats[3]:
>                       repo.ui.status(_("use 'hg resolve' to retry unresolved "
> @@ -2746,7 +2748,9 @@
>   
>       wlock = repo.wlock()
>       try:
> +        repo.dirstate.beginparentchange()
>           repo.setparents(r1, r2)
> +        repo.dirstate.endparentchange()
>       finally:
>           wlock.release()
>   
> @@ -3306,10 +3310,12 @@
>                   cont = False
>   
>               # drop the second merge parent
> +            repo.dirstate.beginparentchange()
>               repo.setparents(current.node(), nullid)
>               repo.dirstate.write()
>               # fix up dirstate for copies and renames
>               cmdutil.duplicatecopies(repo, ctx.rev(), ctx.p1().rev())
> +            repo.dirstate.endparentchange()
>   
>               # commit
>               node = repo.commit(text=message, user=user,
> @@ -3918,6 +3924,7 @@
>       try:
>           try:
>               wlock = repo.wlock()
> +            repo.dirstate.beginparentchange()
>               if not opts.get('no_commit'):
>                   lock = repo.lock()
>                   tr = repo.transaction('import')
> @@ -3958,6 +3965,7 @@
>                   tr.close()
>               if msgs:
>                   repo.savecommitmessage('\n* * *\n'.join(msgs))
> +            repo.dirstate.endparentchange()
>               return ret
>           except: # re-raises
>               # wlock.release() indirectly calls dirstate.write(): since
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -1171,11 +1171,13 @@
>   
>           """
>   
> +        self._repo.dirstate.beginparentchange()
>           for f in self.modified() + self.added():
>               self._repo.dirstate.normal(f)
>           for f in self.removed():
>               self._repo.dirstate.drop(f)
>           self._repo.dirstate.setparents(node)
> +        self._repo.dirstate.endparentchange()
>   
>       def dirs(self):
>           return self._repo.dirstate.dirs()
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -764,6 +764,7 @@
>           return self[changeid].parents()
>   
>       def setparents(self, p1, p2=nullid):
> +        self.dirstate.beginparentchange()
>           copies = self.dirstate.setparents(p1, p2)
>           pctx = self[p1]
>           if copies:
> @@ -777,6 +778,7 @@
>               for f, s in sorted(self.dirstate.copies().items()):
>                   if f not in pctx and s not in pctx:
>                       self.dirstate.copy(None, f)
> +        self.dirstate.endparentchange()
>   
>       def filectx(self, path, changeid=None, fileid=None):
>           """changeid can be a changeset revision, node, or tag.
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1134,6 +1134,7 @@
>           stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
>   
>           if not partial:
> +            repo.dirstate.beginparentchange()
>               repo.setparents(fp1, fp2)
>               recordupdates(repo, actions, branchmerge)
>               # update completed, clear state
> @@ -1141,6 +1142,7 @@
>   
>               if not branchmerge:
>                   repo.dirstate.setbranch(p2.branch())
> +            repo.dirstate.endparentchange()
>       finally:
>           wlock.release()
>   
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel