dirstate: make functions for backup aware of transaction activity
authorFUJIWARA Katsunori <foozy@lares.dti.ne.jp>
Wed, 14 Oct 2015 02:49:17 +0900
changeset 26633 020b12d591f3
parent 26632 59b5e8844eb0
child 26634 09bb1ee7e73e
dirstate: make functions for backup aware of transaction activity Some comments in this patch assume that subsequent patch changes 'dirstate.write()' like as below: def write(self, repo): if not self._dirty: return tr = repo.currenttransaction() if tr: tr.addfilegenerator('dirstate', (self._filename,), self._writedirstate, location='plain') return # omit actual writing out st = self._opener('dirstate', "w", atomictemp=True) self._writedirstate(st) This patch makes '_savebackup()' write in-memory changes out, and it causes clearing 'self._dirty'. If dirstate isn't changed after '_savebackup()', subsequent 'dirstate.write()' never invokes 'tr.addfilegenerator()' because 'not self._dirty' is true. Then, 'tr.writepending()' unintentionally returns False, if there is no other (e.g. changelog) changes pending, even though dirstate changes are already written out at '_savebackup()'. To avoid such situation, this patch makes '_savebackup()' explicitly invoke 'tr.addfilegenerator()', if transaction is running. '_savebackup()' should get awareness of transaction before 'write()', because the former depends on the behavior of the latter before this patch.
mercurial/dirstate.py
--- a/mercurial/dirstate.py	Wed Oct 14 02:49:17 2015 +0900
+++ b/mercurial/dirstate.py	Wed Oct 14 02:49:17 2015 +0900
@@ -53,6 +53,7 @@
         self._filecache = {}
         self._parentwriters = 0
         self._filename = 'dirstate'
+        self._pendingfilename = '%s.pending' % self._filename
 
     def beginparentchange(self):
         '''Marks the beginning of a set of changes that involve changing
@@ -1052,10 +1053,34 @@
             return list(files)
         return [f for f in dmap if match(f)]
 
+    def _actualfilename(self, repo):
+        if repo.currenttransaction():
+            return self._pendingfilename
+        else:
+            return self._filename
+
     def _savebackup(self, repo, suffix):
         '''Save current dirstate into backup file with suffix'''
-        self.write()
-        filename = self._filename
+        filename = self._actualfilename(repo)
+
+        # use '_writedirstate' instead of 'write' to write changes certainly,
+        # because the latter omits writing out if transaction is running.
+        # output file will be used to create backup of dirstate at this point.
+        self._writedirstate(self._opener(filename, "w", atomictemp=True))
+
+        tr = repo.currenttransaction()
+        if tr:
+            # ensure that subsequent tr.writepending returns True for
+            # changes written out above, even if dirstate is never
+            # changed after this
+            tr.addfilegenerator('dirstate', (self._filename,),
+                                self._writedirstate, location='plain')
+
+            # ensure that pending file written above is unlinked at
+            # failure, even if tr.writepending isn't invoked until the
+            # end of this transaction
+            tr.registertmp(filename, location='plain')
+
         self._opener.write(filename + suffix, self._opener.tryread(filename))
 
     def _restorebackup(self, repo, suffix):
@@ -1063,10 +1088,10 @@
         # this "invalidate()" prevents "wlock.release()" from writing
         # changes of dirstate out after restoring from backup file
         self.invalidate()
-        filename = self._filename
+        filename = self._actualfilename(repo)
         self._opener.rename(filename + suffix, filename)
 
     def _clearbackup(self, repo, suffix):
         '''Clear backup file with suffix'''
-        filename = self._filename
+        filename = self._actualfilename(repo)
         self._opener.unlink(filename + suffix)