From 7aef3e608fddc57648daffaec23432e6893f26ff Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Sat, 20 Jan 2024 18:47:13 +0200
Subject: [PATCH] hooks: fix plugins registering other plugins in a hook

when the other plugins implement the same hook itself.

Fix #438.
Regressed in 63b7e908b4b22c30d86cd2cff240b3b7aa6da596 - pluggy 1.1.0.

Went with the simple solution described in the issue for now, basically
undoing most of the rationale for 63b7e908b4b22c30d86cd2cf (though I
think it's still better to have a single `_hookimpls` list).
---
 src/pluggy/_hooks.py          |  6 ++-
 testing/test_pluginmanager.py | 76 +++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py
index 6623c147..7c8420f4 100644
--- a/src/pluggy/_hooks.py
+++ b/src/pluggy/_hooks.py
@@ -497,7 +497,8 @@ def __call__(self, **kwargs: object) -> Any:
         ), "Cannot directly call a historic hook - use call_historic instead."
         self._verify_all_args_are_provided(kwargs)
         firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
-        return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
+        # Copy because plugins may register other plugins during iteration (#438).
+        return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
 
     def call_historic(
         self,
@@ -518,7 +519,8 @@ def call_historic(
         self._call_history.append((kwargs, result_callback))
         # Historizing hooks don't return results.
         # Remember firstresult isn't compatible with historic.
-        res = self._hookexec(self.name, self._hookimpls, kwargs, False)
+        # Copy because plugins may register other plugins during iteration (#438).
+        res = self._hookexec(self.name, self._hookimpls.copy(), kwargs, False)
         if result_callback is None:
             return
         if isinstance(res, list):
diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py
index 81b86b65..08e2f37e 100644
--- a/testing/test_pluginmanager.py
+++ b/testing/test_pluginmanager.py
@@ -675,3 +675,79 @@ def he_method1(self):
         assert saveindent[0] > indent
     finally:
         undo()
+
+
+@pytest.mark.parametrize("historic", [False, True])
+def test_register_while_calling(
+    pm: PluginManager,
+    historic: bool,
+) -> None:
+    """Test that registering an impl of a hook while it is being called does
+    not affect the call itself, only later calls.
+
+    For historic hooks however, the hook is called immediately on registration.
+
+    Regression test for #438.
+    """
+    hookspec = HookspecMarker("example")
+
+    class Hooks:
+        @hookspec(historic=historic)
+        def configure(self) -> int:
+            raise NotImplementedError()
+
+    class Plugin1:
+        @hookimpl
+        def configure(self) -> int:
+            return 1
+
+    class Plugin2:
+        def __init__(self) -> None:
+            self.already_registered = False
+
+        @hookimpl
+        def configure(self) -> int:
+            if not self.already_registered:
+                pm.register(Plugin4())
+                pm.register(Plugin5())
+                pm.register(Plugin6())
+                self.already_registered = True
+            return 2
+
+    class Plugin3:
+        @hookimpl
+        def configure(self) -> int:
+            return 3
+
+    class Plugin4:
+        @hookimpl(tryfirst=True)
+        def configure(self) -> int:
+            return 4
+
+    class Plugin5:
+        @hookimpl()
+        def configure(self) -> int:
+            return 5
+
+    class Plugin6:
+        @hookimpl(trylast=True)
+        def configure(self) -> int:
+            return 6
+
+    pm.add_hookspecs(Hooks)
+    pm.register(Plugin1())
+    pm.register(Plugin2())
+    pm.register(Plugin3())
+
+    if not historic:
+        result = pm.hook.configure()
+        assert result == [3, 2, 1]
+        result = pm.hook.configure()
+        assert result == [4, 5, 3, 2, 1, 6]
+    else:
+        result = []
+        pm.hook.configure.call_historic(result.append)
+        assert result == [4, 5, 6, 3, 2, 1]
+        result = []
+        pm.hook.configure.call_historic(result.append)
+        assert result == [4, 5, 3, 2, 1, 6]