Patchwork [04,of,19] commit: refactor mergestate management into workingctx

login
register
mail settings
Submitter David Schleimer
Date Feb. 10, 2013, 11:29 p.m.
Message ID <f5a6e68310f238e490c6.1360538994@dev010.prn1.facebook.com>
Download mbox | patch
Permalink /patch/944/
State Superseded
Commit 8048c519dc6a5f12d10cfdff155c6d8aac295b45
Headers show

Comments

David Schleimer - Feb. 10, 2013, 11:29 p.m.
# HG changeset patch
# User David Schleimer <dschleimer@fb.com>
# Date 1360330569 28800
# Node ID f5a6e68310f238e490c6157cf2e4c9725baa1691
# Parent  ae3daa803078b08571d1ad602d5d32c92e15475d
commit: refactor mergestate management into workingctx

This pulls the logic around validating that there are no unresolved
conflicts at commit time into the workingctx.  It also pulls the state
necessary for that detection, and the post-cleanup logic for that
state into the workingcontext object.
David Schleimer - Feb. 11, 2013, 11:40 a.m.
> Non-trivial duplicate code, can a brother get a helper function?

Done, but I'm going to hold off on resending this giant series until I can get some more feedback, in an effort to avoid blowing up Matt's inbox too much.

--David
Bryan O'Sullivan - Feb. 11, 2013, 8:50 p.m.
On Mon, Feb 11, 2013 at 6:56 AM, Kevin Bullock <
kbullock+mercurial@ringworld.org> wrote:

> If I get a chance this evening, and no-one else gets to it first, I'll
> queue up 1-3. That should make the re-send a bit less heavy.
>

I just pushed 1-3 to crew. Will take a look through the rest of the series
now.
Pierre-Yves David - Feb. 14, 2013, 10:27 a.m.
On Sun, Feb 10, 2013 at 03:29:54PM -0800, David Schleimer wrote:

> +    def unresolved(self):
> +        assert self._unresolved is not None # must call status first
> +        return self._unresolved

I would be happy If we could get ride of those assert statement by either
- having the status automatically called if cache is empty
- having a real exception raise

It looks like there were alredy here before you started your refactoring, so it
is not really related to your series

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -10,6 +10,7 @@ 
 import ancestor, mdiff, error, util, scmutil, subrepo, patch, encoding, phases
 import copies
 import match as matchmod
+import merge as mergemod
 import os, errno, stat
 import obsolete as obsmod
 import repoview
@@ -816,10 +817,16 @@ 
             self._unknown = changes[4]
             self._ignored = changes[5]
             self._clean = changes[6]
+
+            self._ms = mergemod.mergestate(self._repo)
+            self._unresolved = [f for f in changes[0]
+                                if f in self._ms and self._ms[f] == 'u']
         else:
             self._unknown = None
             self._ignored = None
             self._clean = None
+            self._ms = None
+            self._unresolved = None
 
         self._extra = {}
         if extra:
@@ -952,6 +959,11 @@ 
         if clean:
             self._clean = stat[6]
         self._status = stat[:4]
+
+        self._ms = mergemod.mergestate(self._repo)
+        self._unresolved = [f for f in stat[0]
+                            if f in self._ms and self._ms[f] == 'u']
+
         return stat
 
     def manifest(self):
@@ -982,6 +994,9 @@ 
     def clean(self):
         assert self._clean is not None  # must call status first
         return self._clean
+    def unresolved(self):
+        assert self._unresolved is not None # must call status first
+        return self._unresolved
     def branch(self):
         return encoding.tolocal(self._extra['branch'])
     def closesbranch(self):
@@ -1153,6 +1168,7 @@ 
         for f in self.removed():
             self._repo.dirstate.drop(f)
         self._repo.dirstate.setparents(node)
+        self._ms.reset()
 
     def dirs(self):
         return set(self._repo.dirstate.dirs())
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -11,7 +11,6 @@ 
 import lock, transaction, store, encoding, base85
 import scmutil, util, extensions, hook, error, revset
 import match as matchmod
-import merge as mergemod
 import tags as tagsmod
 from lock import release
 import weakref, errno, os, time, inspect
@@ -1240,11 +1239,9 @@ 
             if merge and cctx.deleted():
                 raise util.Abort(_("cannot commit merge with missing files"))
 
-            ms = mergemod.mergestate(self)
-            for f in changes[0]:
-                if f in ms and ms[f] == 'u':
-                    raise util.Abort(_("unresolved merge conflicts "
-                                       "(see hg help resolve)"))
+            if cctx.unresolved():
+                raise util.Abort(_("unresolved merge conflicts "
+                                   "(see hg help resolve)"))
 
             if editor:
                 cctx._text = editor(self, cctx, subs)
@@ -1280,7 +1277,6 @@ 
             # update bookmarks, dirstate and mergestate
             bookmarks.update(self, [p1, p2], ret)
             cctx.markcommitted(ret)
-            ms.reset()
         finally:
             wlock.release()