Patchwork D8187: nodemap: make sure hooks have access to an up-to-date version

login
register
mail settings
Submitter phabricator
Date Feb. 28, 2020, 6:56 p.m.
Message ID <differential-rev-PHID-DREV-p7ee5ilx35t6whzgzx4k-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45399/
State Superseded
Headers show

Comments

phabricator - Feb. 28, 2020, 6:56 p.m.
marmoute created this revision.
Herald added a reviewer: indygreg.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We make sure hooks can read persistent nodemap data and that they access
  something up-to-date with the pending transaction.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/configitems.py
  mercurial/revlog.py
  mercurial/revlogutils/nodemap.py
  tests/test-persistent-nodemap.t

CHANGE DETAILS




To: marmoute, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 10, 2020, 8:42 p.m.
This revision now requires changes to proceed.
durin42 added a comment.
durin42 requested changes to this revision.


  No longer applies on top of @.

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, indygreg, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
phabricator - March 11, 2020, 1:36 a.m.
marmoute added a comment.


  In D8187#123258 <https://phab.mercurial-scm.org/D8187#123258>, @durin42 wrote:
  
  > No longer applies on top of @.
  
  I don't understand this feedback. There are about 15 other changesets before it in this stack, so this is not meant to be directly applied on the head of default.

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, indygreg, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
phabricator - March 11, 2020, 3:20 p.m.
durin42 added a comment.


  In D8187#123306 <https://phab.mercurial-scm.org/D8187#123306>, @marmoute wrote:
  
  > In D8187#123258 <https://phab.mercurial-scm.org/D8187#123258>, @durin42 wrote:
  >
  >> No longer applies on top of @.
  >
  > I don't understand this feedback. There are about 15 other changesets before it in this stack, so this is not meant to be directly applied on the head of default.
  
  I mentioned this in IRC, but I'll put it here for completeness: the parent pointers in this series are borked, and yadda was showing them /extremely/ out of order, to the point that this was about the 5th change in the series. I think I've managed to piece together a correct order, but if it doesn't apply I'll drop the whole stack until the parent pointers are contiguous again.

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, indygreg, #hg-reviewers, durin42
Cc: durin42, mercurial-devel

Patch

diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -281,3 +281,39 @@ 
   data-unused: 0
   $ hg log -r "$OTHERNODE" -T '{rev}\n'
   5002
+
+Check transaction related property
+==================================
+
+An up to date nodemap should be available to shell hooks,
+
+  $ echo dsljfl > a
+  $ hg add a
+  $ hg ci -m a
+  $ hg debugnodemap --metadata
+  uid: ???????????????? (glob)
+  tip-rev: 5003
+  tip-node: c91af76d172f1053cca41b83f7c2e4e514fe2bcf
+  data-length: 123008
+  data-unused: 0
+  $ echo babar2 > babar
+  $ hg ci -m 'babar2' --config "hooks.pretxnclose.nodemap-test=hg debugnodemap --metadata"
+  uid: ???????????????? (glob)
+  tip-rev: 5004
+  tip-node: ba87cd9559559e4b91b28cb140d003985315e031
+  data-length: 123328 (pure !)
+  data-length: 123328 (rust !)
+  data-length: 123136 (no-pure no-rust !)
+  data-unused: 192 (pure !)
+  data-unused: 192 (rust !)
+  data-unused: 0 (no-pure no-rust !)
+  $ hg debugnodemap --metadata
+  uid: ???????????????? (glob)
+  tip-rev: 5004
+  tip-node: ba87cd9559559e4b91b28cb140d003985315e031
+  data-length: 123328 (pure !)
+  data-length: 123328 (rust !)
+  data-length: 123136 (no-pure no-rust !)
+  data-unused: 192 (pure !)
+  data-unused: 192 (rust !)
+  data-unused: 0 (no-pure no-rust !)
diff --git a/mercurial/revlogutils/nodemap.py b/mercurial/revlogutils/nodemap.py
--- a/mercurial/revlogutils/nodemap.py
+++ b/mercurial/revlogutils/nodemap.py
@@ -75,6 +75,9 @@ 
     callback_id = b"revlog-persistent-nodemap-%s" % revlog.nodemap_file
     if tr.hasfinalize(callback_id):
         return  # no need to register again
+    tr.addpending(
+        callback_id, lambda tr: _persist_nodemap(tr, revlog, pending=True)
+    )
     tr.addfinalize(callback_id, lambda tr: _persist_nodemap(tr, revlog))
 
 
@@ -101,7 +104,7 @@ 
         notr._postclose[k](None)
 
 
-def _persist_nodemap(tr, revlog):
+def _persist_nodemap(tr, revlog, pending=False):
     """Write nodemap data on disk for a given revlog
     """
     if getattr(revlog, 'filteredrevs', ()):
@@ -169,7 +172,10 @@ 
     target_docket.tip_node = revlog.node(target_docket.tip_rev)
     # EXP-TODO: if this is a cache, this should use a cache vfs, not a
     # store vfs
-    with revlog.opener(revlog.nodemap_file, b'w', atomictemp=True) as fp:
+    file_path = revlog.nodemap_file
+    if pending:
+        file_path += b'.a'
+    with revlog.opener(file_path, b'w', atomictemp=True) as fp:
         fp.write(target_docket.serialize())
     revlog._nodemap_docket = target_docket
     if feed_data:
@@ -304,7 +310,10 @@ 
 
 def _rawdata_filepath(revlog, docket):
     """The (vfs relative) nodemap's rawdata file for a given uid"""
-    prefix = revlog.nodemap_file[:-2]
+    if revlog.nodemap_file.endswith(b'.n.a'):
+        prefix = revlog.nodemap_file[:-4]
+    else:
+        prefix = revlog.nodemap_file[:-2]
     return b"%s-%s.nd" % (prefix, docket.uid)
 
 
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -436,7 +436,14 @@ 
         self.datafile = datafile or (indexfile[:-2] + b".d")
         self.nodemap_file = None
         if persistentnodemap:
-            self.nodemap_file = indexfile[:-2] + b".n"
+            if indexfile.endswith(b'.a'):
+                pending_path = indexfile[:-4] + b".n.a"
+                if opener.exists(pending_path):
+                    self.nodemap_file = pending_path
+                else:
+                    self.nodemap_file = indexfile[:-4] + b".n"
+            else:
+                self.nodemap_file = indexfile[:-2] + b".n"
 
         self.opener = opener
         #  When True, indexfile is opened with checkambig=True at writing, to
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -408,7 +408,6 @@ 
 # TODO before getting `persistent-nodemap` out of experimental
 #
 # * code/tests around aborted transaction
-# * code/tests around pending data for hooks
 # * regenerate a new nodemap when the unused/total ration is to high
 # * decide for a "status" of the persistent nodemap and associated location
 #   - part of the store next the revlog itself (new requirements)