Bugfix/spack spec: read and use the environment concretizer:unification option (#38248)

* Bugfix: spack.yaml concretizer:unify needs to be read and used
* Optional: add environment test to ensure configuration scheme is used
* Activate environment in unit tests
  A more proper solution would be to keep
  an environment instance configuration as
  an attribute, but that is a bigger refactor
* Delay evaluation of Environment.unify
* Slightly simplify unit tests

---------

Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

2 files changed, 62 insertions, 4 deletions

diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index a174b4a11e..9d475973e8 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -731,7 +731,7 @@ class Environment:
 
         self.txlock = lk.Lock(self._transaction_lock_path)
 
-        self.unify = None
+        self._unify = None
         self.new_specs: List[Spec] = []
         self.new_installs: List[Spec] = []
         self.views: Dict[str, ViewDescriptor] = {}
@@ -755,6 +755,16 @@ class Environment:
             self.manifest = EnvironmentManifestFile(manifest_dir)
             self._read()
 
+    @property
+    def unify(self):
+        if self._unify is None:
+            self._unify = spack.config.get("concretizer:unify", False)
+        return self._unify
+
+    @unify.setter
+    def unify(self, value):
+        self._unify = value
+
     def __reduce__(self):
         return _create_environment, (self.path,)
 
@@ -816,9 +826,6 @@ class Environment:
         else:
             self.views = {}
 
-        # Retrieve unification scheme for the concretizer
-        self.unify = spack.config.get("concretizer:unify", False)
-
         # Retrieve dev-build packages:
         self.dev_specs = copy.deepcopy(env_configuration.get("develop", {}))
         for name, entry in self.dev_specs.items():
diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py
index 965e841d0c..bcb459f100 100644
--- a/lib/spack/spack/test/env.py
+++ b/lib/spack/spack/test/env.py
@@ -500,3 +500,54 @@ def test_error_message_when_using_too_new_lockfile(tmp_path):
     ev.initialize_environment_dir(env_dir, init_file)
     with pytest.raises(ev.SpackEnvironmentError, match="You need to use a newer Spack version."):
         ev.Environment(env_dir)
+
+
+@pytest.mark.regression("38240")
+@pytest.mark.parametrize(
+    "unify_in_lower_scope,unify_in_spack_yaml",
+    [
+        (True, False),
+        (True, "when_possible"),
+        (False, True),
+        (False, "when_possible"),
+        ("when_possible", False),
+        ("when_possible", True),
+    ],
+)
+def test_environment_concretizer_scheme_used(tmp_path, unify_in_lower_scope, unify_in_spack_yaml):
+    """Tests that "unify" settings in spack.yaml always take precedence over settings in lower
+    configuration scopes.
+    """
+    manifest = tmp_path / "spack.yaml"
+    manifest.write_text(
+        f"""\
+spack:
+  specs:
+  - mpileaks
+  concretizer:
+    unify: {str(unify_in_spack_yaml).lower()}
+"""
+    )
+
+    with spack.config.override("concretizer:unify", unify_in_lower_scope):
+        with ev.Environment(manifest.parent) as e:
+            assert e.unify == unify_in_spack_yaml
+
+
+@pytest.mark.parametrize("unify_in_config", [True, False, "when_possible"])
+def test_environment_config_scheme_used(tmp_path, unify_in_config):
+    """Tests that "unify" settings in lower configuration scopes is taken into account,
+    if absent in spack.yaml.
+    """
+    manifest = tmp_path / "spack.yaml"
+    manifest.write_text(
+        """\
+spack:
+  specs:
+  - mpileaks
+"""
+    )
+
+    with spack.config.override("concretizer:unify", unify_in_config):
+        with ev.Environment(manifest.parent) as e:
+            assert e.unify == unify_in_config