[Differential] [Request, 53 lines] D68: dirstate: update backup functions to take full backup filename

simpkins (Adam Simpkins) phabricator at mercurial-scm.org
Wed Jul 12 23:01:50 UTC 2017


simpkins created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Update the dirstate functions so that the caller supplies the full backup
  filename rather than just a prefix and suffix.
  
  The localrepo code was already hard-coding the fact that the backup name must
  be (exactly prefix + "dirstate" + suffix): it relied on this in _journalfiles()
  and undofiles().  Making the caller responsible for specifying the full backup
  name removes the need for the localrepo code to assume that dirstate._filename
  is always "dirstate".

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D68

AFFECTED FILES
  hgext/shelve.py
  mercurial/dirstate.py
  mercurial/dirstateguard.py
  mercurial/localrepo.py

CHANGE DETAILS

Index: mercurial/localrepo.py
===================================================================
--- mercurial/localrepo.py
+++ mercurial/localrepo.py
@@ -1122,7 +1122,7 @@
             else:
                 # discard all changes (including ones already written
                 # out) in this transaction
-                repo.dirstate.restorebackup(None, prefix='journal.')
+                repo.dirstate.restorebackup(None, 'journal.dirstate')
 
                 repo.invalidate(clearfilecache=True)
 
@@ -1182,7 +1182,7 @@
 
     @unfilteredmethod
     def _writejournal(self, desc):
-        self.dirstate.savebackup(None, prefix='journal.')
+        self.dirstate.savebackup(None, 'journal.dirstate')
         self.vfs.write("journal.branch",
                           encoding.fromlocal(self.dirstate.branch()))
         self.vfs.write("journal.desc",
@@ -1270,7 +1270,7 @@
             # prevent dirstateguard from overwriting already restored one
             dsguard.close()
 
-            self.dirstate.restorebackup(None, prefix='undo.')
+            self.dirstate.restorebackup(None, 'undo.dirstate')
             try:
                 branch = self.vfs.read('undo.branch')
                 self.dirstate.setbranch(encoding.tolocal(branch))
Index: mercurial/dirstateguard.py
===================================================================
--- mercurial/dirstateguard.py
+++ mercurial/dirstateguard.py
@@ -31,8 +31,8 @@
         self._repo = repo
         self._active = False
         self._closed = False
-        self._suffix = '.backup.%s.%d' % (name, id(self))
-        repo.dirstate.savebackup(repo.currenttransaction(), self._suffix)
+        self._backupname = 'dirstate.backup.%s.%d' % (name, id(self))
+        repo.dirstate.savebackup(repo.currenttransaction(), self._backupname)
         self._active = True
 
     def __del__(self):
@@ -45,25 +45,24 @@
 
     def close(self):
         if not self._active: # already inactivated
-            msg = (_("can't close already inactivated backup: dirstate%s")
-                   % self._suffix)
+            msg = (_("can't close already inactivated backup: %s")
+                   % self._backupname)
             raise error.Abort(msg)
 
         self._repo.dirstate.clearbackup(self._repo.currenttransaction(),
-                                         self._suffix)
+                                         self._backupname)
         self._active = False
         self._closed = True
 
     def _abort(self):
         self._repo.dirstate.restorebackup(self._repo.currenttransaction(),
-                                           self._suffix)
+                                           self._backupname)
         self._active = False
 
     def release(self):
         if not self._closed:
             if not self._active: # already inactivated
-                msg = (_("can't release already inactivated backup:"
-                         " dirstate%s")
-                       % self._suffix)
+                msg = (_("can't release already inactivated backup: %s")
+                       % self._backupname)
                 raise error.Abort(msg)
             self._abort()
Index: mercurial/dirstate.py
===================================================================
--- mercurial/dirstate.py
+++ mercurial/dirstate.py
@@ -1300,10 +1300,10 @@
         else:
             return self._filename
 
-    def savebackup(self, tr, suffix='', prefix=''):
-        '''Save current dirstate into backup file with suffix'''
-        assert len(suffix) > 0 or len(prefix) > 0
+    def savebackup(self, tr, backupname):
+        '''Save current dirstate into backup file'''
         filename = self._actualfilename(tr)
+        assert backupname != filename
 
         # use '_writedirstate' instead of 'write' to write changes certainly,
         # because the latter omits writing out if transaction is running.
@@ -1324,27 +1324,20 @@
             # end of this transaction
             tr.registertmp(filename, location='plain')
 
-        backupname = prefix + self._filename + suffix
-        assert backupname != filename
         self._opener.tryunlink(backupname)
         # hardlink backup is okay because _writedirstate is always called
         # with an "atomictemp=True" file.
         util.copyfile(self._opener.join(filename),
                       self._opener.join(backupname), hardlink=True)
 
-    def restorebackup(self, tr, suffix='', prefix=''):
-        '''Restore dirstate by backup file with suffix'''
-        assert len(suffix) > 0 or len(prefix) > 0
+    def restorebackup(self, tr, backupname):
+        '''Restore dirstate by backup file'''
         # this "invalidate()" prevents "wlock.release()" from writing
         # changes of dirstate out after restoring from backup file
         self.invalidate()
         filename = self._actualfilename(tr)
-        # using self._filename to avoid having "pending" in the backup filename
-        self._opener.rename(prefix + self._filename + suffix, filename,
-                            checkambig=True)
+        self._opener.rename(backupname, filename, checkambig=True)
 
-    def clearbackup(self, tr, suffix='', prefix=''):
-        '''Clear backup file with suffix'''
-        assert len(suffix) > 0 or len(prefix) > 0
-        # using self._filename to avoid having "pending" in the backup filename
-        self._opener.unlink(prefix + self._filename + suffix)
+    def clearbackup(self, tr, backupname):
+        '''Clear backup file'''
+        self._opener.unlink(backupname)
Index: hgext/shelve.py
===================================================================
--- hgext/shelve.py
+++ hgext/shelve.py
@@ -297,9 +297,10 @@
     '''Abort current transaction for shelve/unshelve, but keep dirstate
     '''
     tr = repo.currenttransaction()
-    repo.dirstate.savebackup(tr, suffix='.shelve')
+    backupname = 'dirstate.shelve'
+    repo.dirstate.savebackup(tr, backupname)
     tr.abort()
-    repo.dirstate.restorebackup(None, suffix='.shelve')
+    repo.dirstate.restorebackup(None, backupname)
 
 def createcmd(ui, repo, pats, opts):
     """subcommand that creates a new shelve"""


EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: simpkins, #hg-reviewers
Cc: mercurial-devel, indygreg


More information about the Mercurial-devel mailing list