Patchwork [4,of,5] transaction: support cache file in backupentries

login
register
mail settings
Submitter Pierre-Yves David
Date Nov. 13, 2014, 11:39 p.m.
Message ID <9f78cb8b801097375968.1415921960@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6725/
State Accepted
Commit 006e9ef05c31025f105c34c8d4ed842c08f53cdc
Headers show

Comments

Pierre-Yves David - Nov. 13, 2014, 11:39 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1415877429 0
#      Thu Nov 13 11:17:09 2014 +0000
# Node ID 9f78cb8b801097375968fa94568f7cebcc7e37ea
# Parent  b86227ec8b547d605306421d8d293294b7659fc6
transaction: support cache file in backupentries

We do not want to abort if anything wrong happen while handling a cache file.
Cache file have way to be invalidated and if old/bad version stay no
misbehavior will happen. Proper value will eventually be computed and the wrong
will be righten.

This changeset use the transaction reporter (usually writing on stderr) to write
details about failed cache handling. This will only apply to write operation
using a transaction. The usual update during read only operation will stay a
debug message.

I was on the way to bring these message back to debug level when I realised it
could be a feature. People with write access to the repository are likely to
have the power to fix error related to cache (and it is valuable to fix them).
So let the things as is for now.

Patch

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -43,35 +43,45 @@  def _playback(journal, report, opener, v
                 if inst.errno != errno.ENOENT:
                     raise
 
     backupfiles = []
     for l, f, b, c in backupentries:
+        if l not in vfsmap and c:
+            report("couldn't handle %s: unknown cache location %s\n"
+                        % (b, l))
         vfs = vfsmap[l]
-        if f and b:
-            filepath = vfs.join(f)
-            backuppath = vfs.join(b)
-            try:
-                util.copyfile(backuppath, filepath)
-                backupfiles.append(b)
-            except IOError:
-                report(_("failed to recover %s\n") % f)
+        try:
+            if f and b:
+                filepath = vfs.join(f)
+                backuppath = vfs.join(b)
+                try:
+                    util.copyfile(backuppath, filepath)
+                    backupfiles.append(b)
+                except IOError:
+                    report(_("failed to recover %s\n") % f)
+            else:
+                target = f or b
+                try:
+                    vfs.unlink(target)
+                except (IOError, OSError), inst:
+                    if inst.errno != errno.ENOENT:
+                        raise
+        except (IOError, OSError, util.Abort), inst:
+            if not c:
                 raise
-        else:
-            target = f or b
-            try:
-                vfs.unlink(target)
-            except (IOError, OSError), inst:
-                if inst.errno != errno.ENOENT:
-                    raise
 
     opener.unlink(journal)
     backuppath = "%s.backupfiles" % journal
     if opener.exists(backuppath):
         opener.unlink(backuppath)
-    for f in backupfiles:
-        if opener.exists(f):
-            opener.unlink(f)
+    try:
+        for f in backupfiles:
+            if opener.exists(f):
+                opener.unlink(f)
+    except (IOError, OSError, util.Abort), inst:
+        # only pure backup file remains, it is sage to ignore any error
+        pass
 
 class transaction(object):
     def __init__(self, report, opener, vfsmap, journal, after=None,
                  createmode=None, onclose=None, onabort=None):
         """Begin a new transaction
@@ -363,25 +373,47 @@  class transaction(object):
         if self.count != 0:
             return
         self.file.close()
         self._backupsfile.close()
         # cleanup temporary files
-        for l, f, b, _c in self._backupentries:
+        for l, f, b, c in self._backupentries:
+            if l not in self._vfsmap and c:
+                self.report("couldn't remote %s: unknown cache location %s\n"
+                            % (b, l))
+                continue
             vfs = self._vfsmap[l]
             if not f and b and vfs.exists(b):
-                vfs.unlink(b)
+                try:
+                    vfs.unlink(b)
+                except (IOError, OSError, util.Abort), inst:
+                    if not c:
+                        raise
+                    # Abort may be raise by read only opener
+                    self.report("couldn't remote %s: %s\n"
+                                % (vfs.join(b), inst))
         self.entries = []
         if self.after:
             self.after()
         if self.opener.isfile(self.journal):
             self.opener.unlink(self.journal)
         if self.opener.isfile(self._backupjournal):
             self.opener.unlink(self._backupjournal)
-            for _l, _f, b, _c in self._backupentries:
+            for _l, _f, b, c in self._backupentries:
+                if l not in self._vfsmap and c:
+                    self.report("couldn't remote %s: unknown cache location"
+                                "%s\n" % (b, l))
+                    continue
                 vfs = self._vfsmap[l]
                 if b and vfs.exists(b):
-                    vfs.unlink(b)
+                    try:
+                        vfs.unlink(b)
+                    except (IOError, OSError, util.Abort), inst:
+                        if not c:
+                            raise
+                        # Abort may be raise by read only opener
+                        self.report("couldn't remote %s: %s\n"
+                                    % (vfs.join(b), inst))
         self._backupentries = []
         self.journal = None
         # run post close action
         categories = sorted(self._postclosecallback)
         for cat in categories: