changeset 26633:020b12d591f3

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.

--- 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)