Manifest parsing: skip invalid manifest files (#32467)

* catch json schema errors and reraise as property of SpackError
* no need to catch subclass of given error
* Builtin json library for Python 2 uses more generic type
* Correct instantiation of SpackError (requires a string rather than an exception)
* Use exception chaining (where possible)

3 files changed, 57 insertions, 4 deletions

diff --git a/lib/spack/spack/cmd/external.py b/lib/spack/spack/cmd/external.py
index 993533d395..af186bb218 100644
--- a/lib/spack/spack/cmd/external.py
+++ b/lib/spack/spack/cmd/external.py
@@ -219,7 +219,7 @@ def _collect_and_consume_cray_manifest_files(
         tty.debug("Reading manifest file: " + path)
         try:
             cray_manifest.read(path, not dry_run)
-        except (spack.compilers.UnknownCompilerError, spack.error.SpackError) as e:
+        except spack.error.SpackError as e:
             if fail_on_error:
                 raise
             else:
diff --git a/lib/spack/spack/cray_manifest.py b/lib/spack/spack/cray_manifest.py
index 15b0d9293b..d15c8450a3 100644
--- a/lib/spack/spack/cray_manifest.py
+++ b/lib/spack/spack/cray_manifest.py
@@ -4,8 +4,10 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import json
+import sys
 
 import jsonschema
+import jsonschema.exceptions
 import six
 
 import llnl.util.tty as tty
@@ -161,10 +163,21 @@ def entries_to_specs(entries):
 
 
 def read(path, apply_updates):
-    with open(path, "r") as json_file:
-        json_data = json.load(json_file)
+    if sys.version_info >= (3, 0):
+        decode_exception_type = json.decoder.JSONDecodeError
+    else:
+        decode_exception_type = ValueError
+
+    try:
+        with open(path, "r") as json_file:
+            json_data = json.load(json_file)
 
-    jsonschema.validate(json_data, manifest_schema)
+        jsonschema.validate(json_data, manifest_schema)
+    except (jsonschema.exceptions.ValidationError, decode_exception_type) as e:
+        raise six.raise_from(
+            ManifestValidationError("error parsing manifest JSON:", str(e)),
+            e,
+        )
 
     specs = entries_to_specs(json_data["specs"])
     tty.debug("{0}: {1} specs read from manifest".format(path, str(len(specs))))
@@ -179,3 +192,8 @@ def read(path, apply_updates):
     if apply_updates:
         for spec in specs.values():
             spack.store.db.add(spec, directory_layout=None)
+
+
+class ManifestValidationError(spack.error.SpackError):
+    def __init__(self, msg, long_msg=None):
+        super(ManifestValidationError, self).__init__(msg, long_msg)
diff --git a/lib/spack/spack/test/cray_manifest.py b/lib/spack/spack/test/cray_manifest.py
index addf4e5287..4d030e8e11 100644
--- a/lib/spack/spack/test/cray_manifest.py
+++ b/lib/spack/spack/test/cray_manifest.py
@@ -365,3 +365,38 @@ def test_read_old_manifest_v1_2(tmpdir, mutable_config, mock_packages, mutable_d
 """
         )
     cray_manifest.read(manifest_file_path, True)
+
+
+def test_convert_validation_error(tmpdir, mutable_config, mock_packages, mutable_database):
+    manifest_dir = str(tmpdir.mkdir("manifest_dir"))
+    # Does not parse as valid JSON
+    invalid_json_path = os.path.join(manifest_dir, "invalid-json.json")
+    with open(invalid_json_path, "w") as f:
+        f.write(
+            """\
+{
+"""
+        )
+    with pytest.raises(cray_manifest.ManifestValidationError) as e:
+        cray_manifest.read(invalid_json_path, True)
+    str(e)
+
+    # Valid JSON, but does not conform to schema (schema-version is not a string
+    # of length > 0)
+    invalid_schema_path = os.path.join(manifest_dir, "invalid-schema.json")
+    with open(invalid_schema_path, "w") as f:
+        f.write(
+            """\
+{
+  "_meta": {
+    "file-type": "cray-pe-json",
+    "system-type": "EX",
+    "schema-version": ""
+  },
+  "specs": []
+}
+"""
+        )
+    with pytest.raises(cray_manifest.ManifestValidationError) as e:
+        cray_manifest.read(invalid_schema_path, True)
+    str(e)