Ensure a spec cannot be instantiate twice for the same context

Having duplicates nodes with the same context leads to creation of
corrupted trees

Part of it/e3-core#54

Author: Nikokrock

src/e3/anod/context.py

@@ -181,22 +181,54 @@ def load(
 
         if key not in self.cache:
             # Spec is not in cache so create a new instance
-            self.cache[key] = self.repo.load(name)(
-                qualifier=qualifier, env=env, kind=kind
+            result = self.repo.load(name)(qualifier=qualifier, env=env, kind=kind)
+
+            # Once a spec is loaded the qualifier has been expanded/normalized
+            # to its final value (a dict with default values set).
+            # After normalization, check if a spec instance with the same
+            # context already exists to avoid creating duplicate instances for
+            # logically equivalent spec instances and ensure cache correctness.
+            # For that purpose recompute the cache key using the normalized
+            # qualifier value.
+            if result.enable_name_generator and kind != "source":
+                final_qualifier_key = qualifier_dict_to_str(result.args)
+            else:
+                final_qualifier_key = result.qualifier
+            final_key = (
+                name,
+                env.build,
+                env.host,
+                env.target,
+                final_qualifier_key,
+                kind,
+                source_name,
             )
-            if sandbox is not None:
-                self.cache[key].bind_to_sandbox(sandbox)
-
-            # Update tracking of dependencies
-            self.dependencies[self.cache[key].uid] = {}
-
-            # Update the list of available sources. ??? Should be done
-            # once per spec (and not once per spec instance). Need some
-            # spec cleanup to achieve that ???
-            source_builders = self.cache[key].source_pkg_build
-            if source_builders is not None:
-                for s in source_builders:
-                    self.sources[s.name] = (name, s)
+
+            if final_key in self.cache:
+                # The spec is in fact already in the cache so abandon the
+                # instantiation, after updating the cache
+                logger.debug(f"loading cached spec instance {result.uid}({kind})")
+                self.cache[key] = self.cache[final_key]
+            else:
+                logger.debug(f"loading new spec instance {result.uid}({kind})")
+                # By updating both entries in the cache we avoid useless
+                # instantiation of the specs
+                self.cache[final_key] = result
+                self.cache[key] = result
+
+                if sandbox is not None:
+                    self.cache[key].bind_to_sandbox(sandbox)
+
+                # Update tracking of dependencies
+                self.dependencies[self.cache[key].uid] = {}
+
+                # Update the list of available sources. ??? Should be done
+                # once per spec (and not once per spec instance). Need some
+                # spec cleanup to achieve that ???
+                source_builders = self.cache[key].source_pkg_build
+                if source_builders is not None:
+                    for s in source_builders:
+                        self.sources[s.name] = (name, s)
 
         return self.cache[key]
 

src/e3/anod/loader.py

@@ -232,7 +232,7 @@ def load(self, repository: AnodSpecRepository) -> Callable[..., Anod]:
                 assert self.anod_class is not None
             return self.anod_class
 
-        logger.debug("loading anod spec: %s", self.name)
+        logger.debug("loading anod spec class: %s", self.name)
 
         # Create a new module
         mod_name = "anod_" + self.name

tests/tests_e3/anod/context_data/spec15.anod

@@ -0,0 +1,15 @@
+from e3.anod.spec import Anod
+
+
+class Spec15(Anod):
+    enable_name_generator = True
+
+    def declare_qualifiers_and_components(self, qualifiers_manager):
+        qualifiers_manager.declare_tag_qualifier(name="q1", description="help q1")
+        qualifiers_manager.declare_key_value_qualifier(
+            name="q2", description="help of q2", default="default_q2"
+        )
+
+    @Anod.primitive()
+    def build(self):
+        pass

tests/tests_e3/anod/context_test.py

@@ -374,6 +374,65 @@ def test_add_anod_action14(self):
         result = ac.schedule(ac.always_download_source_resolver)
         assert "x86-linux.spec4.test" in list(result.vertex_data.keys())
 
+    def test_add_anod_action15(self):
+        """Check for duplicate nodes."""
+        # The goal of the test is to ensure that only one instance of a given Anod spec
+        # for a given context exists at a given time, independently of how the context
+        # was passed.
+
+        ac = self.create_context()
+        result = [
+            ac.add_anod_action(
+                "spec15", env=ac.default_env, qualifier=None, primitive="build"
+            ),
+            ac.add_anod_action(
+                "spec15", env=ac.default_env, qualifier={}, primitive="build"
+            ),
+            ac.add_anod_action(
+                "spec15", env=ac.default_env, qualifier={"q1": False}, primitive="build"
+            ),
+            ac.add_anod_action(
+                "spec15",
+                env=ac.default_env,
+                qualifier={"q1": False, "q2": "default_q2"},
+                primitive="build",
+            ),
+            ac.add_anod_action(
+                "spec15",
+                env=ac.default_env,
+                qualifier="q2=default_q2",
+                primitive="build",
+            ),
+            ac.add_anod_action(
+                "spec15",
+                env=ac.default_env,
+                qualifier="",
+                primitive="build",
+            ),
+        ]
+        for i in range(len(result) - 1):
+            assert result[i].anod_instance is result[i + 1].anod_instance
+
+        result = [
+            ac.add_anod_action(
+                "spec15", env=ac.default_env, qualifier="q1", primitive="build"
+            ),
+            ac.add_anod_action(
+                "spec15",
+                env=ac.default_env,
+                qualifier="q1,q2=default_q2",
+                primitive="build",
+            ),
+            ac.add_anod_action(
+                "spec15",
+                env=ac.default_env,
+                qualifier="q2=default_q2,q1",
+                primitive="build",
+            ),
+        ]
+        for i in range(len(result) - 1):
+            assert result[i].anod_instance is result[i + 1].anod_instance
+
     def test_source_fails_when_missing_source_primitive(self):
         """Source action should fail when the source primitive is undefined.