Patchwork D9811: tagcache: distinguish between invalid and missing entries

login
register
mail settings
Submitter phabricator
Date Jan. 17, 2021, 9:55 p.m.
Message ID <differential-rev-PHID-DREV-wv22ejpztt27o6kng2mt-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48123/
State Superseded
Headers show

Comments

phabricator - Jan. 17, 2021, 9:55 p.m.
mharbison72 created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The TortoiseHg repo has typically not had a newly applied tag accessible by name
  for recent releases, for unknown reasons.  Deleting and rebuilding the tag cache
  doesn't fix it, though deleting the cache and running `hg log -r $new_tag` does.
  Eventually the situation does sort itself out for new clones from the server.
  In an effort to figure out what the issue is, Pierre-Yves David suggested
  listing these entries in the debug output more specifically.
  
  This isn't complete yet- the second test change that says "missing" is more like
  "invalid", since it was truncated.  The problem there is the code that reads the
  raw array truncates any partial records and then fills it with 0xFF, which
  signifies that it is missing.  As a side note, that means the check for the
  length when validating an existing entry never fails.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/bundle2.py
  mercurial/debugcommands.py
  mercurial/tags.py
  tests/test-tags.t

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-tags.t b/tests/test-tags.t
--- a/tests/test-tags.t
+++ b/tests/test-tags.t
@@ -104,7 +104,7 @@ 
   0010: ff ff ff ff ff ff ff ff b9 15 46 36 26 b7 b4 a7 |..........F6&...|
   0020: 73 e0 9e e3 c5 2f 51 0e 19 e0 5e 1f f9 66 d8 59 |s..../Q...^..f.Y|
   $ hg debugtagscache
-  0 acb14030fe0a21b60322c440ad2d20cf7685a376 missing/invalid
+  0 acb14030fe0a21b60322c440ad2d20cf7685a376 missing
   1 b9154636be938d3d431e75a7c906504a079bfe07 26b7b4a773e09ee3c52f510e19e05e1ff966d859
 
 Repeat with cold tag cache:
@@ -381,7 +381,7 @@ 
 
   $ hg debugtagscache | tail -2
   4 0c192d7d5e6b78a714de54a2e9627952a877e25a 0c04f2a8af31de17fab7422878ee5a2dadbc943d
-  5 8dbfe60eff306a54259cfe007db9e330e7ecf866 missing/invalid
+  5 8dbfe60eff306a54259cfe007db9e330e7ecf866 missing
   $ hg tags
   tip                                5:8dbfe60eff30
   bar                                1:78391a272241
@@ -389,6 +389,34 @@ 
   4 0c192d7d5e6b78a714de54a2e9627952a877e25a 0c04f2a8af31de17fab7422878ee5a2dadbc943d
   5 8dbfe60eff306a54259cfe007db9e330e7ecf866 0c04f2a8af31de17fab7422878ee5a2dadbc943d
 
+If the 4 bytes of node hash for a record don't match an existing node, the entry
+is flagged as invalid.
+
+  >>> import os
+  >>> with open(".hg/cache/hgtagsfnodes1", "rb+") as fp:
+  ...     fp.seek(-24, os.SEEK_END) and None
+  ...     fp.write(b'\xde\xad') and None
+
+  $ f --size --hexdump .hg/cache/hgtagsfnodes1
+  .hg/cache/hgtagsfnodes1: size=144
+  0000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+  0010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+  0020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+  0030: 7a 94 12 77 0c 04 f2 a8 af 31 de 17 fa b7 42 28 |z..w.....1....B(|
+  0040: 78 ee 5a 2d ad bc 94 3d 6f a4 50 21 7d 3b 71 8c |x.Z-...=o.P!};q.|
+  0050: 96 4e f3 7b 89 e5 50 eb da fd 57 89 e7 6c e1 b0 |.N.{..P...W..l..|
+  0060: 0c 19 2d 7d 0c 04 f2 a8 af 31 de 17 fa b7 42 28 |..-}.....1....B(|
+  0070: 78 ee 5a 2d ad bc 94 3d de ad e6 0e 0c 04 f2 a8 |x.Z-...=........|
+  0080: af 31 de 17 fa b7 42 28 78 ee 5a 2d ad bc 94 3d |.1....B(x.Z-...=|
+
+  $ hg debugtagscache | tail -2
+  4 0c192d7d5e6b78a714de54a2e9627952a877e25a 0c04f2a8af31de17fab7422878ee5a2dadbc943d
+  5 8dbfe60eff306a54259cfe007db9e330e7ecf866 invalid
+
+  $ hg tags
+  tip                                5:8dbfe60eff30
+  bar                                1:78391a272241
+
 #if unix-permissions no-root
 Errors writing to .hgtags fnodes cache are silently ignored
 
@@ -405,7 +433,7 @@ 
   $ hg blackbox -l 6
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> tags
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> couldn't write cache/hgtagsfnodes1: [Errno *] * (glob)
-  1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> 3/4 cache hits/lookups in * seconds (glob)
+  1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> 2/4 cache hits/lookups in * seconds (glob)
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> writing .hg/cache/tags2-visible with 1 tags
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> tags exited 0 after * seconds (glob)
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> blackbox -l 6
@@ -420,7 +448,7 @@ 
   $ hg blackbox -l 6
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> tags
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> writing 24 bytes to cache/hgtagsfnodes1
-  1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> 3/4 cache hits/lookups in * seconds (glob)
+  1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> 2/4 cache hits/lookups in * seconds (glob)
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> writing .hg/cache/tags2-visible with 1 tags
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> tags exited 0 after * seconds (glob)
   1970/01/01 00:00:00 bob @b968051b5cf3f624b771779c6d5f84f1d4c3fb5d (5000)> blackbox -l 6
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -733,6 +733,7 @@ 
         if rawlen < wantedlen:
             if self._dirtyoffset is None:
                 self._dirtyoffset = rawlen
+            # TODO: zero fill entire record, because it's invalid not missing?
             self._raw.extend(b'\xff' * (wantedlen - rawlen))
 
     def getfnode(self, node, computemissing=True):
@@ -740,7 +741,8 @@ 
 
         If the value is in the cache, the entry will be validated and returned.
         Otherwise, the filenode will be computed and returned unless
-        "computemissing" is False, in which case None will be returned without
+        "computemissing" is False.  In that case, None will be returned if
+        the entry is missing or False if the entry is invalid without
         any potentially expensive computation being performed.
 
         If an .hgtags does not exist at the specified revision, nullid is
@@ -771,6 +773,8 @@ 
         # If we get here, the entry is either missing or invalid.
 
         if not computemissing:
+            if record != _fnodesmissingrec:
+                return False
             return None
 
         fnode = None
@@ -788,7 +792,7 @@ 
                 # we cannot rely on readfast because we don't know against what
                 # parent the readfast delta is computed
                 p1fnode = None
-        if p1fnode is not None:
+        if p1fnode:
             mctx = ctx.manifestctx()
             fnode = mctx.readfast().get(b'.hgtags')
             if fnode is None:
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -3780,7 +3780,13 @@ 
     for r in repo:
         node = repo[r].node()
         tagsnode = cache.getfnode(node, computemissing=False)
-        tagsnodedisplay = hex(tagsnode) if tagsnode else b'missing/invalid'
+        if tagsnode:
+            tagsnodedisplay = hex(tagsnode)
+        elif tagsnode is False:
+            tagsnodedisplay = b'invalid'
+        else:
+            tagsnodedisplay = b'missing'
+
         ui.write(b'%d %s %s\n' % (r, hex(node), tagsnodedisplay))
 
 
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1769,7 +1769,7 @@ 
     for node in outgoing.ancestorsof:
         # Don't compute missing, as this may slow down serving.
         fnode = cache.getfnode(node, computemissing=False)
-        if fnode is not None:
+        if fnode:
             chunks.extend([node, fnode])
 
     if chunks: