Patchwork D7833: transaction: allow finalizer to add finalizer

login
register
mail settings
Submitter phabricator
Date Jan. 11, 2020, 5:02 p.m.
Message ID <differential-rev-PHID-DREV-ctk4chzhc6hjtipmx6dh-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44258/
State Superseded
Headers show

Comments

phabricator - Jan. 11, 2020, 5:02 p.m.
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It will make some code (persistent nodemap related) simpler to write, because
  higher level code can blindly queue finalization without thinking too hard about
  the context.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/transaction.py

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 16, 2020, 4:49 p.m.
martinvonz added inline comments.

INLINE COMMENTS

> transaction.py:511
> +                callbacks = self._finalizecallback
> +                self._finalizecallback = {}
> +                categories = sorted(callbacks)

Any reason to not set this to `None` as we do on line 516? Also, can we remove that line now?

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - Jan. 16, 2020, 4:51 p.m.
This revision is now accepted and ready to land.
martinvonz added inline comments.
martinvonz accepted this revision.

INLINE COMMENTS

> martinvonz wrote in transaction.py:511
> Any reason to not set this to `None` as we do on line 516? Also, can we remove that line now?

Ah, never mind, I just found the previous patch in this stack and now it's clear.

REPOSITORY
  rHG Mercurial

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

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

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

Patch

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -506,9 +506,12 @@ 
             self._validator(self)  # will raise exception if needed
             self._validator = None  # Help prevent cycles.
             self._generatefiles(group=gengroupprefinalize)
-            categories = sorted(self._finalizecallback)
-            for cat in categories:
-                self._finalizecallback[cat](self)
+            while self._finalizecallback:
+                callbacks = self._finalizecallback
+                self._finalizecallback = {}
+                categories = sorted(callbacks)
+                for cat in categories:
+                    callbacks[cat](self)
             # Prevent double usage and help clear cycles.
             self._finalizecallback = None
             self._generatefiles(group=gengrouppostfinalize)