| Submitter | Durham Goode |
|---|---|
| Date | Sept. 9, 2014, 8:21 p.m. |
| Message ID | <b647743bc6fb059c05c2.1410294087@dev2000.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/5756/ |
| State | Accepted |
| Headers | show |
Comments
On 09/09/2014 10:21 PM, 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 b647743bc6fb059c05c2e075b2d71d4bdb2654cf > # Parent 897041f6b025778193c6da5b9795da09a91c866e > 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. This series was bugging me because it linked the dirstate change to the lock. Linking such related concept to the lock seems like a mistake. However reading it again, I realize that the dirstate writing is -already- attached to the lock and this series just makes is sightly less awful. So +1 We should aims to detach dirstate from the lock at some point.
Patch
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py --- a/mercurial/dirstate.py +++ b/mercurial/dirstate.py @@ -44,6 +44,30 @@ self._lastnormaltime = 0 self._ui = ui self._filecache = {} + self._parentwriters = 0 + + def beginparentchange(self): + '''Marks the beginning of a set of changes that involve changing + the dirstate parents. If there is an exception during this time, + the dirstate will not be written when the wlock is released. This + prevents writing an incoherent dirstate where the parent doesn't + match the contents. + ''' + self._parentwriters += 1 + + def endparentchange(self): + '''Marks the end of a set of changes that involve changing the + dirstate parents. Once all parent changes have been marked done, + the wlock will be free to write the dirstate on release. + ''' + if self._parentwriters > 0: + self._parentwriters -= 1 + + def pendingparentchange(self): + '''Returns true if the dirstate is in the middle of a set of changes + that modify the dirstate parent. + ''' + return self._parentwriters > 0 @propertycache def _map(self): @@ -300,6 +324,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 @@ -1102,7 +1102,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,