Improve error messages when Spack finds a too new DB / lockfile (#37614)

alalazo · web-flow · commit ebfc706c8c6e · 2023-05-12T08:13:10.000Z
This PR ensures that we'll get a comprehensible error message whenever an old
version of Spack tries to use a DB or a lockfile that is "too new".

* Fix error message when using a too new DB
* Add a unit-test to ensure we have a comprehensible error message
diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
@@ -798,9 +798,8 @@ def check(cond, msg):
 
         # TODO: better version checking semantics.
         version = vn.Version(db["version"])
-        spec_reader = reader(version)
         if version > _db_version:
-            raise InvalidDatabaseVersionError(_db_version, version)
+            raise InvalidDatabaseVersionError(self, _db_version, version)
         elif version < _db_version:
             if not any(old == version and new == _db_version for old, new in _skip_reindex):
                 tty.warn(
@@ -814,6 +813,8 @@ def check(cond, msg):
                     for k, v in self._data.items()
                 )
 
+        spec_reader = reader(version)
+
         def invalid_record(hash_key, error):
             return CorruptDatabaseError(
                 f"Invalid record in Spack database: hash: {hash_key}, cause: "
@@ -1642,16 +1643,19 @@ class CorruptDatabaseError(SpackError):
 
 
 class NonConcreteSpecAddError(SpackError):
-    """Raised when attemptint to add non-concrete spec to DB."""
+    """Raised when attempting to add non-concrete spec to DB."""
 
 
 class MissingDependenciesError(SpackError):
     """Raised when DB cannot find records for dependencies"""
 
 
 class InvalidDatabaseVersionError(SpackError):
-    def __init__(self, expected, found):
-        super(InvalidDatabaseVersionError, self).__init__(
-            "Expected database version %s but found version %s." % (expected, found),
-            "`spack reindex` may fix this, or you may need a newer " "Spack version.",
+    """Exception raised when the database metadata is newer than current Spack."""
+
+    def __init__(self, database, expected, found):
+        msg = (
+            f"you need a newer Spack version to read the database in '{database.root}'. "
+            f"The expected database version is '{expected}', but '{found}' was found."
         )
+        super(InvalidDatabaseVersionError, self).__init__(msg)
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
@@ -2127,10 +2127,12 @@ def _read_lockfile_dict(self, d):
             reader = READER_CLS[current_lockfile_format]
         except KeyError:
             msg = (
-                f"Spack {spack.__version__} cannot read environment lockfiles using the "
-                f"v{current_lockfile_format} format"
+                f"Spack {spack.__version__} cannot read the lockfile '{self.lock_path}', using "
+                f"the v{current_lockfile_format} format."
             )
-            raise RuntimeError(msg)
+            if lockfile_format_version < current_lockfile_format:
+                msg += " You need to use a newer Spack version."
+            raise SpackEnvironmentError(msg)
 
         # First pass: Put each spec in the map ignoring dependencies
         for lockfile_key, node_dict in json_specs_by_hash.items():
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
@@ -30,6 +30,7 @@
 import spack.repo
 import spack.spec
 import spack.store
+import spack.version as vn
 from spack.schema.database_index import schema
 from spack.util.executable import Executable
 
@@ -1051,3 +1052,16 @@ def test_query_installed_when_package_unknown(database, tmpdir):
             assert not s.installed_upstream
             with pytest.raises(spack.repo.UnknownNamespaceError):
                 s.package
+
+
+def test_error_message_when_using_too_new_db(database, monkeypatch):
+    """Sometimes the database format needs to be bumped. When that happens, we have forward
+    incompatibilities that need to be reported in a clear way to the user, in case we moved
+    back to an older version of Spack. This test ensures that the error message for a too
+    new database version stays comprehensible across refactoring of the database code.
+    """
+    monkeypatch.setattr(spack.database, "_db_version", vn.Version("0"))
+    with pytest.raises(
+        spack.database.InvalidDatabaseVersionError, match="you need a newer Spack version"
+    ):
+        spack.database.Database(database.root)._read()
diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py
@@ -474,3 +474,29 @@ def test_initialize_from_random_file_as_manifest(tmp_path, filename):
     assert not os.path.exists(env_dir / ev.lockfile_name)
     assert os.path.exists(env_dir / ev.manifest_name)
     assert filecmp.cmp(env_dir / ev.manifest_name, init_file, shallow=False)
+
+
+def test_error_message_when_using_too_new_lockfile(tmp_path):
+    """Sometimes the lockfile format needs to be bumped. When that happens, we have forward
+    incompatibilities that need to be reported in a clear way to the user, in case we moved
+    back to an older version of Spack. This test ensures that the error message for a too
+    new lockfile version stays comprehensible across refactoring of the environment code.
+    """
+    init_file = tmp_path / ev.lockfile_name
+    env_dir = tmp_path / "env_dir"
+    init_file.write_text(
+        """
+{
+    "_meta": {
+        "file-type": "spack-lockfile",
+        "lockfile-version": 100,
+        "specfile-version": 3
+    },
+    "roots": [],
+    "concrete_specs": {}
+}\n
+"""
+    )
+    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)
