Patchwork [1,of,4,V2] dirstate: add begin/endparentchange to dirstate

login
register
mail settings
Submitter Durham Goode
Date Sept. 5, 2014, 10:47 p.m.
Message ID <8c6d502c92c8ed8dfa79.1409957233@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/5708/
State Superseded
Headers show

Comments

Durham Goode - Sept. 5, 2014, 10:47 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1409942069 25200
#      Fri Sep 05 11:34:29 2014 -0700
# Node ID 8c6d502c92c8ed8dfa79b8fac4c8a2cc9e6c4153
# Parent  188b8aa2120b03eead618ba150319074f4e3b42b
dirstate: add begin/endparentchange to dirstate

It's possible for the dirstate to become incoherent (issue4353) if there is an
exception in the middle of the dirstate parent and entries being written (like
if the user ctrl+c's). This change adds begin/endparentchange which a future
patch will require to be set before changing the dirstate parent.  This will
allow us to prevent writing the dirstate in the event of an exception while
changing the parent.
Pierre-Yves David - Sept. 5, 2014, 11:35 p.m.
On 09/06/2014 12:47 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1409942069 25200
> #      Fri Sep 05 11:34:29 2014 -0700
> # Node ID 8c6d502c92c8ed8dfa79b8fac4c8a2cc9e6c4153
> # Parent  188b8aa2120b03eead618ba150319074f4e3b42b
> dirstate: add begin/endparentchange to dirstate
>
> It's possible for the dirstate to become incoherent (issue4353) if there is an
> exception in the middle of the dirstate parent and entries being written (like
> if the user ctrl+c's). This change adds begin/endparentchange which a future
> patch will require to be set before changing the dirstate parent.  This will
> allow us to prevent writing the dirstate in the event of an exception while
> changing the parent.

Can you elaborate a bit more about how this new mechanism will work? 
That should go as inline documentation, so that future reader of the 
code can benefit from it too.


>
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -44,6 +44,17 @@
>           self._lastnormaltime = 0
>           self._ui = ui
>           self._filecache = {}
> +        self._parentwriters = 0
> +
> +    def beginparentchange(self):
> +        self._parentwriters += 1
> +
> +    def endparentchange(self):
> +        if self._parentwriters > 0:
> +            self._parentwriters -= 1
> +
> +    def pendingparentchange(self):
> +        return self._parentwriters > 0
>
>       @propertycache
>       def _map(self):
> @@ -300,6 +311,7 @@
>                   delattr(self, a)
>           self._lastnormaltime = 0
>           self._dirty = False
> +        self._parentwriters = 0
>
>       def copy(self, source, dest):
>           """Mark dest as a copy of source. Unmark dest if source is None."""
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1108,7 +1108,11 @@
>               return l
>
>           def unlock():
> -            self.dirstate.write()
> +            if self.dirstate.pendingparentchange():
> +                self.dirstate.invalidate()
> +            else:
> +                self.dirstate.write()
> +
>               self._filecache['dirstate'].refresh()
>
>           l = self._lock(self.vfs, "wlock", wait, unlock,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -44,6 +44,17 @@ 
         self._lastnormaltime = 0
         self._ui = ui
         self._filecache = {}
+        self._parentwriters = 0
+
+    def beginparentchange(self):
+        self._parentwriters += 1
+
+    def endparentchange(self):
+        if self._parentwriters > 0:
+            self._parentwriters -= 1
+
+    def pendingparentchange(self):
+        return self._parentwriters > 0
 
     @propertycache
     def _map(self):
@@ -300,6 +311,7 @@ 
                 delattr(self, a)
         self._lastnormaltime = 0
         self._dirty = False
+        self._parentwriters = 0
 
     def copy(self, source, dest):
         """Mark dest as a copy of source. Unmark dest if source is None."""
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1108,7 +1108,11 @@ 
             return l
 
         def unlock():
-            self.dirstate.write()
+            if self.dirstate.pendingparentchange():
+                self.dirstate.invalidate()
+            else:
+                self.dirstate.write()
+
             self._filecache['dirstate'].refresh()
 
         l = self._lock(self.vfs, "wlock", wait, unlock,