Cray manifest: compiler duplicates (#31173)

* remove unhelpful comment
* Filter compiler duplicates while reading manifest
* more-specific version matching edited to use module-specific version (to avoid an issue where a user might add a compiler with the same version to the initial test configuration

lib/spack/spack/compilers/__init__.py

@@ -252,6 +252,13 @@ def find_new_compilers(path_hints=None, scope=None):
             merged configuration.
     """
     compilers = find_compilers(path_hints)
+    return select_new_compilers(compilers, scope)
+
+
+def select_new_compilers(compilers, scope=None):
+    """Given a list of compilers, remove those that are already defined in
+    the configuration.
+    """
     compilers_not_in_config = []
     for c in compilers:
         arch_spec = spack.spec.ArchSpec((None, c.operating_system, c.target))

lib/spack/spack/cray_manifest.py

@@ -39,10 +39,6 @@ def translated_compiler_name(manifest_compiler_name):
     elif manifest_compiler_name in spack.compilers.supported_compilers():
         return manifest_compiler_name
     else:
-        # Try to fail quickly. This can occur in two cases: (1) the compiler
-        # definition (2) a spec can specify a compiler that doesn't exist; the
-        # first will be caught when creating compiler definition. The second
-        # will result in Specs with associated undefined compilers.
         raise spack.compilers.UnknownCompilerError(
             "Manifest parsing - unknown compiler: {0}"
             .format(manifest_compiler_name))
@@ -186,6 +182,8 @@ def read(path, apply_updates):
     tty.debug("{0}: {1} compilers read from manifest".format(
         path,
         str(len(compilers))))
+    # Filter out the compilers that already appear in the configuration
+    compilers = spack.compilers.select_new_compilers(compilers)
     if apply_updates and compilers:
         spack.compilers.add_compilers_to_config(
             compilers, init_config=False)

lib/spack/spack/test/cray_manifest.py

@@ -33,7 +33,7 @@
   },
   "compiler": {
     "name": "gcc",
-    "version": "10.2.0"
+    "version": "10.2.0.cray"
   },
   "dependencies": {
     "packagey": {
@@ -157,7 +157,7 @@ def spec_json(self):
 # Intended to match example_compiler_entry above
 _common_compiler = JsonCompilerEntry(
     name='gcc',
-    version='10.2.0',
+    version='10.2.0.cray',
     arch={
         "os": "centos8",
         "target": "x86_64"
@@ -319,7 +319,7 @@ def create_manifest_content():
             "cpe-version": "22.06"
         },
         'specs': list(x.to_dict() for x in generate_openmpi_entries()),
-        'compilers': []
+        'compilers': [_common_compiler.compiler_json()]
     }
 
 
@@ -347,6 +347,27 @@ def test_read_cray_manifest(
         assert concretized_specs[0]['hwloc'].dag_hash() == 'hwlocfakehashaaa'
 
 
+def test_read_cray_manifest_twice_no_compiler_duplicates(
+        tmpdir, mutable_config, mock_packages, mutable_database):
+    if spack.config.get('config:concretizer') == 'clingo':
+        pytest.skip("The ASP-based concretizer is currently picky about "
+                    " OS matching and will fail.")
+
+    with tmpdir.as_cwd():
+        test_db_fname = 'external-db.json'
+        with open(test_db_fname, 'w') as db_file:
+            json.dump(create_manifest_content(), db_file)
+
+        # Read the manifest twice
+        cray_manifest.read(test_db_fname, True)
+        cray_manifest.read(test_db_fname, True)
+
+        compilers = spack.compilers.all_compilers()
+        filtered = list(c for c in compilers if
+                        c.spec == spack.spec.CompilerSpec('gcc@10.2.0.cray'))
+        assert(len(filtered) == 1)
+
+
 def test_read_old_manifest_v1_2(
         tmpdir, mutable_config, mock_packages, mutable_database):
     """Test reading a file using the older format