From patchwork Fri Oct  9 18:59:27 2015
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [3, of,
 5] dirstate: make functions for backup aware of transaction activity
From: Katsunori FUJIWARA <foozy@lares.dti.ne.jp>
X-Patchwork-Id: 10926
Message-Id: <0c77ac7b070e4109765e.1444417167@feefifofum>
To: mercurial-devel@selenic.com
Date: Sat, 10 Oct 2015 03:59:27 +0900

# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1444416862 -32400
#      Sat Oct 10 03:54:22 2015 +0900
# Node ID 0c77ac7b070e4109765ee2e7759c40de7a52b2fc
# Parent  b551f3883a6bb1285e83f257b81d10f11ab26341
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', backup=False)
            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.

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -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
@@ -1046,10 +1047,35 @@
             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', backup=False)
+
+            # 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):
@@ -1057,10 +1083,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)