Patchwork D6722: fncache: make debugrebuildfncache not fail on broken fncache

login
register
mail settings
Submitter phabricator
Date Aug. 12, 2019, 6:13 p.m.
Message ID <differential-rev-PHID-DREV-r7gr34ag7zr5w3htqxek-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41250/
State Superseded
Headers show

Comments

phabricator - Aug. 12, 2019, 6:13 p.m.
valentin.gatienbaron created this revision.
Herald added subscribers: mercurial-devel, mjpieters.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The code reading the fncache changed in 5.0, to complain if the file
  is not \n terminated. This makes apparent the fact that the fncache
  gets corrupted.
  
  Make it possible to recover, instead of having `hg
  debugrebuildfncache` failing by saying `(run hg debugrebuildfncache)`.
  
  The corruption itself is most likely due to hg not using fsync in
  general, and so various bad things can happen. Here, the reported
  problems happened when running out of disk space. So I suspect that
  because the fncache is much bigger than the average commit/pull, when
  running out of disk space, the bulk of the pull may succeed, but the
  new fncache may get half-written and still renamed into place.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/repair.py
  mercurial/store.py
  tests/test-fncache.t

CHANGE DETAILS




To: valentin.gatienbaron, #hg-reviewers
Cc: mjpieters, mercurial-devel
phabricator - Aug. 12, 2019, 8:45 p.m.
This revision is now accepted and ready to land.
pulkit added a comment.
pulkit accepted this revision.


  Queued these for stable branch. Many thanks!

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6722/new/

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

To: valentin.gatienbaron, #hg-reviewers, pulkit
Cc: pulkit, mjpieters, mercurial-devel

Patch

diff --git a/tests/test-fncache.t b/tests/test-fncache.t
--- a/tests/test-fncache.t
+++ b/tests/test-fncache.t
@@ -435,16 +435,18 @@ 
   data/.bar.i
   data/foo.i
 
-debugrebuildfncache fails to recover from truncated line in fncache
+debugrebuildfncache recovers from truncated line in fncache
 
   $ printf a > .hg/store/fncache
   $ hg debugrebuildfncache
-  abort: fncache does not ends with a newline
-  (use 'hg debugrebuildfncache' to rebuild the fncache)
-  [255]
+  fncache does not ends with a newline
+  adding data/.bar.i
+  adding data/foo.i
+  2 items added, 0 removed from fncache
 
   $ cat .hg/store/fncache | sort
-  a
+  data/.bar.i
+  data/foo.i
 
   $ cd ..
 
diff --git a/mercurial/store.py b/mercurial/store.py
--- a/mercurial/store.py
+++ b/mercurial/store.py
@@ -458,7 +458,15 @@ 
         # set of new additions to fncache
         self.addls = set()
 
-    def _load(self):
+    def ensureloaded(self, warn=None):
+        '''read the fncache file if not already read.
+
+        If the file on disk is corrupted, raise. If warn is provided,
+        warn and keep going instead.'''
+        if self.entries is None:
+            self._load(warn)
+
+    def _load(self, warn=None):
         '''fill the entries from the fncache file'''
         self._dirty = False
         try:
@@ -482,20 +490,27 @@ 
                 pass
 
         if chunk:
-            raise error.Abort(_("fncache does not ends with a newline"),
-                              hint=_("use 'hg debugrebuildfncache' to rebuild"
-                                     " the fncache"))
-        self._checkentries(fp)
+            msg = _("fncache does not ends with a newline")
+            if warn:
+                warn(msg + '\n')
+            else:
+                raise error.Abort(msg,
+                                  hint=_("use 'hg debugrebuildfncache' to "
+                                         "rebuild the fncache"))
+        self._checkentries(fp, warn)
         fp.close()
 
-    def _checkentries(self, fp):
+    def _checkentries(self, fp, warn):
         """ make sure there is no empty string in entries """
         if '' in self.entries:
             fp.seek(0)
             for n, line in enumerate(util.iterfile(fp)):
                 if not line.rstrip('\n'):
                     t = _('invalid entry in fncache, line %d') % (n + 1)
-                    raise error.Abort(t)
+                    if warn:
+                        warn(t + '\n')
+                    else:
+                        raise error.Abort(t)
 
     def write(self, tr):
         if self._dirty:
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -392,9 +392,7 @@ 
 
     with repo.lock():
         fnc = repo.store.fncache
-        # Trigger load of fncache.
-        if 'irrelevant' in fnc:
-            pass
+        fnc.ensureloaded(warn=ui.warn)
 
         oldentries = set(fnc.entries)
         newentries = set()