From ebfc706c8c6e16455a9e6f476a48d0f0489a8aec Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Fri, 12 May 2023 10:13:10 +0200 Subject: Improve error messages when Spack finds a too new DB / lockfile (#37614) 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 --- lib/spack/spack/database.py | 18 +++++++++++------- lib/spack/spack/environment/environment.py | 8 +++++--- lib/spack/spack/test/database.py | 14 ++++++++++++++ lib/spack/spack/test/env.py | 26 ++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index f75e540a70..9fa041ea35 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -798,9 +798,8 @@ class Database(object): # 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 @@ class Database(object): 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,7 +1643,7 @@ 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): @@ -1650,8 +1651,11 @@ class MissingDependenciesError(SpackError): 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 index 4d0b1da569..7e1faed6df 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -2127,10 +2127,12 @@ class Environment: 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 index 219c68c47d..3416ec9f65 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -30,6 +30,7 @@ import spack.package_base 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 index e40f42fa04..965e841d0c 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -474,3 +474,29 @@ spack: 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) -- cgit v1.2.3-70-g09d2