Patchwork D12125: transaction: do not rely on a global variable to post_finalize file

login
register
mail settings
Submitter phabricator
Date Jan. 31, 2022, 6:49 p.m.
Message ID <differential-rev-PHID-DREV-pcmyqzz727yyr7khyuae-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50458/
State New
Headers show

Comments

phabricator - Jan. 31, 2022, 6:49 p.m.
marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  We can just add a new argument to the `addfilegenerator` function. This is more
  explicit and therefor clearer and less error prone.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/bookmarks.py
  mercurial/dirstate.py
  mercurial/transaction.py
  tests/testlib/crash_transaction_late.py

CHANGE DETAILS




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

Patch

diff --git a/tests/testlib/crash_transaction_late.py b/tests/testlib/crash_transaction_late.py
--- a/tests/testlib/crash_transaction_late.py
+++ b/tests/testlib/crash_transaction_late.py
@@ -9,7 +9,6 @@ 
 
 from mercurial import (
     error,
-    transaction,
 )
 
 
@@ -18,14 +17,15 @@ 
 
 
 def reposetup(ui, repo):
-
-    transaction.postfinalizegenerators.add(b'late-abort')
-
     class LateAbortRepo(repo.__class__):
         def transaction(self, *args, **kwargs):
             tr = super(LateAbortRepo, self).transaction(*args, **kwargs)
             tr.addfilegenerator(
-                b'late-abort', [b'late-abort'], abort, order=9999999
+                b'late-abort',
+                [b'late-abort'],
+                abort,
+                order=9999999,
+                post_finalize=True,
             )
             return tr
 
diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -25,16 +25,6 @@ 
 
 version = 2
 
-# These are the file generators that should only be executed after the
-# finalizers are done, since they rely on the output of the finalizers (like
-# the changelog having been written).
-postfinalizegenerators = {
-    b'bookmarks',
-    b'dirstate-0-key-pre',
-    b'dirstate-1-main',
-    b'dirstate-2-key-post',
-}
-
 GEN_GROUP_ALL = b'all'
 GEN_GROUP_PRE_FINALIZE = b'prefinalize'
 GEN_GROUP_POST_FINALIZE = b'postfinalize'
@@ -335,7 +325,13 @@ 
 
     @active
     def addfilegenerator(
-        self, genid, filenames, genfunc, order=0, location=b''
+        self,
+        genid,
+        filenames,
+        genfunc,
+        order=0,
+        location=b'',
+        post_finalize=False,
     ):
         """add a function to generates some files at transaction commit
 
@@ -358,10 +354,14 @@ 
         The `location` arguments may be used to indicate the files are located
         outside of the the standard directory for transaction. It should match
         one of the key of the `transaction.vfsmap` dictionary.
+
+        The `post_finalize` argument can be set to `True` for file generation
+        that must be run after the transaction has been finalized.
         """
         # For now, we are unable to do proper backup and restore of custom vfs
         # but for bookmarks that are handled outside this mechanism.
-        self._filegenerators[genid] = (order, filenames, genfunc, location)
+        entry = (order, filenames, genfunc, location, post_finalize)
+        self._filegenerators[genid] = entry
 
     @active
     def removefilegenerator(self, genid):
@@ -381,13 +381,12 @@ 
 
         for id, entry in sorted(pycompat.iteritems(self._filegenerators)):
             any = True
-            order, filenames, genfunc, location = entry
+            order, filenames, genfunc, location, post_finalize = entry
 
             # for generation at closing, check if it's before or after finalize
-            is_post = id in postfinalizegenerators
-            if skip_post and is_post:
+            if skip_post and post_finalize:
                 continue
-            elif skip_pre and not is_post:
+            elif skip_pre and not post_finalize:
                 continue
 
             vfs = self._vfsmap[location]
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -730,12 +730,14 @@ 
                     (self._filename_tk,),
                     lambda f: self._write_tracked_key(tr, f),
                     location=b'plain',
+                    post_finalize=True,
                 )
             tr.addfilegenerator(
                 b'dirstate-1-main',
                 (self._filename,),
                 lambda f: self._writedirstate(tr, f),
                 location=b'plain',
+                post_finalize=True,
             )
             if write_key:
                 tr.addfilegenerator(
@@ -743,6 +745,7 @@ 
                     (self._filename_tk,),
                     lambda f: self._write_tracked_key(tr, f),
                     location=b'plain',
+                    post_finalize=True,
                 )
             return
 
@@ -1425,6 +1428,7 @@ 
                 (self._filename,),
                 lambda f: self._writedirstate(tr, f),
                 location=b'plain',
+                post_finalize=True,
             )
 
             # ensure that pending file written above is unlinked at
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -212,7 +212,11 @@ 
         The transaction is then responsible for updating the file content."""
         location = b'' if bookmarksinstore(self._repo) else b'plain'
         tr.addfilegenerator(
-            b'bookmarks', (b'bookmarks',), self._write, location=location
+            b'bookmarks',
+            (b'bookmarks',),
+            self._write,
+            location=location,
+            post_finalize=True,
         )
         tr.hookargs[b'bookmark_moved'] = b'1'