Pass Database layout in constructor (#46219)

Ensures that Database instances do not reference a global
`spack.store.STORE.layout`. Simplify Database.{add,reindex} signature.

lib/spack/spack/binary_distribution.py

@@ -105,7 +105,7 @@ class BuildCacheDatabase(spack_db.Database):
     record_fields = ("spec", "ref_count", "in_buildcache")
 
     def __init__(self, root):
-        super().__init__(root, lock_cfg=spack_db.NO_LOCK)
+        super().__init__(root, lock_cfg=spack_db.NO_LOCK, layout=None)
         self._write_transaction_impl = llnl.util.lang.nullcontext
         self._read_transaction_impl = llnl.util.lang.nullcontext
 
@@ -788,7 +788,9 @@ def sign_specfile(key: str, specfile_path: str) -> str:
     return signed_specfile_path
 
 
-def _read_specs_and_push_index(file_list, read_method, cache_prefix, db, temp_dir, concurrency):
+def _read_specs_and_push_index(
+    file_list, read_method, cache_prefix, db: BuildCacheDatabase, temp_dir, concurrency
+):
     """Read all the specs listed in the provided list, using thread given thread parallelism,
         generate the index, and push it to the mirror.
 
@@ -812,7 +814,7 @@ def _read_specs_and_push_index(file_list, read_method, cache_prefix, db, temp_di
         else:
             continue
 
-        db.add(fetched_spec, None)
+        db.add(fetched_spec)
         db.mark(fetched_spec, "in_buildcache", True)
 
     # Now generate the index, compute its hash, and push the two files to
@@ -1765,7 +1767,7 @@ def _oci_update_index(
 
     for spec_dict in spec_dicts:
         spec = Spec.from_dict(spec_dict)
-        db.add(spec, directory_layout=None)
+        db.add(spec)
         db.mark(spec, "in_buildcache", True)
 
     # Create the index.json file
@@ -2562,7 +2564,7 @@ def install_root_node(spec, unsigned=False, force=False, sha256=None):
         tty.msg('Installing "{0}" from a buildcache'.format(spec.format()))
         extract_tarball(spec, download_result, force)
         spack.hooks.post_install(spec, False)
-        spack.store.STORE.db.add(spec, spack.store.STORE.layout)
+        spack.store.STORE.db.add(spec)
 
 
 def install_single_spec(spec, unsigned=False, force=False):

lib/spack/spack/cray_manifest.py

@@ -14,12 +14,14 @@
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.compilers
 import spack.deptypes as dt
 import spack.error
 import spack.hash_types as hash_types
 import spack.platforms
 import spack.repo
 import spack.spec
+import spack.store
 from spack.schema.cray_manifest import schema as manifest_schema
 
 #: Cray systems can store a Spack-compatible description of system
@@ -237,7 +239,7 @@ def read(path, apply_updates):
                 tty.debug(f"Include this\n{traceback.format_exc()}")
     if apply_updates:
         for spec in specs.values():
-            spack.store.STORE.db.add(spec, directory_layout=None)
+            spack.store.STORE.db.add(spec)
 
 
 class ManifestValidationError(spack.error.SpackError):

lib/spack/spack/database.py

@@ -599,9 +599,11 @@ class Database:
     def __init__(
         self,
         root: str,
+        *,
         upstream_dbs: Optional[List["Database"]] = None,
         is_upstream: bool = False,
         lock_cfg: LockConfiguration = DEFAULT_LOCK_CFG,
+        layout: Optional[DirectoryLayout] = None,
     ) -> None:
         """Database for Spack installations.
 
@@ -624,6 +626,7 @@ def __init__(
         """
         self.root = root
         self.database_directory = os.path.join(self.root, _DB_DIRNAME)
+        self.layout = layout
 
         # Set up layout of database files within the db dir
         self._index_path = os.path.join(self.database_directory, "index.json")
@@ -907,7 +910,7 @@ def invalid_record(hash_key, error):
         self._data = data
         self._installed_prefixes = installed_prefixes
 
-    def reindex(self, directory_layout):
+    def reindex(self):
         """Build database index from scratch based on a directory layout.
 
         Locks the DB if it isn't locked already.
@@ -940,7 +943,7 @@ def _read_suppress_error():
             old_data = self._data
             old_installed_prefixes = self._installed_prefixes
             try:
-                self._construct_from_directory_layout(directory_layout, old_data)
+                self._construct_from_directory_layout(old_data)
             except BaseException:
                 # If anything explodes, restore old data, skip write.
                 self._data = old_data
@@ -949,7 +952,6 @@ def _read_suppress_error():
 
     def _construct_entry_from_directory_layout(
         self,
-        directory_layout: DirectoryLayout,
         old_data: Dict[str, InstallRecord],
         spec: "spack.spec.Spec",
         deprecator: Optional["spack.spec.Spec"] = None,
@@ -967,18 +969,17 @@ def _construct_entry_from_directory_layout(
                 explicit = old_info.explicit
                 inst_time = old_info.installation_time
 
-        self._add(spec, directory_layout, explicit=explicit, installation_time=inst_time)
+        self._add(spec, explicit=explicit, installation_time=inst_time)
         if deprecator:
             self._deprecate(spec, deprecator)
 
-    def _construct_from_directory_layout(
+    def _construct_from_directory_layout(self, old_data: Dict[str, InstallRecord]):
-        self, directory_layout: DirectoryLayout, old_data: Dict[str, InstallRecord]
-    ):
         # Read first the spec files in the prefixes. They should be considered authoritative with
         # respect to DB reindexing, as entries in the DB may be corrupted in a way that still makes
         # them readable. If we considered DB entries authoritative instead, we would perpetuate
         # errors over a reindex.
-        with directory_layout.disable_upstream_check():
+        assert self.layout is not None, "Cannot reindex a database without a known layout"
+        with self.layout.disable_upstream_check():
             # Initialize data in the reconstructed DB
             self._data = {}
             self._installed_prefixes = set()
@@ -986,14 +987,12 @@ def _construct_from_directory_layout(
             # Start inspecting the installed prefixes
             processed_specs = set()
 
-            for spec in directory_layout.all_specs():
+            for spec in self.layout.all_specs():
-                self._construct_entry_from_directory_layout(directory_layout, old_data, spec)
+                self._construct_entry_from_directory_layout(old_data, spec)
                 processed_specs.add(spec)
 
-            for spec, deprecator in directory_layout.all_deprecated_specs():
+            for spec, deprecator in self.layout.all_deprecated_specs():
-                self._construct_entry_from_directory_layout(
+                self._construct_entry_from_directory_layout(old_data, spec, deprecator)
-                    directory_layout, old_data, spec, deprecator
-                )
                 processed_specs.add(spec)
 
             for entry in old_data.values():
@@ -1012,7 +1011,6 @@ def _construct_from_directory_layout(
                 try:
                     self._add(
                         spec=entry.spec,
-                        directory_layout=None if entry.spec.external else directory_layout,
                         explicit=entry.explicit,
                         installation_time=entry.installation_time,
                     )
@@ -1115,20 +1113,16 @@ def _read(self):
     def _add(
         self,
         spec: "spack.spec.Spec",
-        directory_layout: Optional[DirectoryLayout] = None,
         explicit: bool = False,
         installation_time: Optional[float] = None,
         allow_missing: bool = False,
     ):
         """Add an install record for this spec to the database.
 
-        Assumes spec is installed in ``directory_layout.path_for_spec(spec)``.
-
         Also ensures dependencies are present and updated in the DB as either installed or missing.
 
         Args:
             spec: spec to be added
-            directory_layout: layout of the spec installation
             explicit:
                 Possible values: True, False, any
 
@@ -1157,7 +1151,6 @@ def _add(
                 continue
             self._add(
                 edge.spec,
-                directory_layout,
                 explicit=False,
                 installation_time=installation_time,
                 # allow missing build-only deps. This prevents excessive warnings when a spec is
@@ -1167,11 +1160,11 @@ def _add(
             )
 
         # Make sure the directory layout agrees whether the spec is installed
-        if not spec.external and directory_layout:
+        if not spec.external and self.layout:
-            path = directory_layout.path_for_spec(spec)
+            path = self.layout.path_for_spec(spec)
             installed = False
             try:
-                directory_layout.ensure_installed(spec)
+                self.layout.ensure_installed(spec)
                 installed = True
                 self._installed_prefixes.add(path)
             except DirectoryLayoutError as e:
@@ -1225,7 +1218,7 @@ def _add(
         self._data[key].explicit = explicit
 
     @_autospec
-    def add(self, spec, directory_layout, explicit=False):
+    def add(self, spec: "spack.spec.Spec", *, explicit=False) -> None:
         """Add spec at path to database, locking and reading DB to sync.
 
         ``add()`` will lock and read from the DB on disk.
@@ -1234,7 +1227,7 @@ def add(self, spec, directory_layout, explicit=False):
         # TODO: ensure that spec is concrete?
         # Entire add is transactional.
         with self.write_transaction():
-            self._add(spec, directory_layout, explicit=explicit)
+            self._add(spec, explicit=explicit)
 
     def _get_matching_spec_key(self, spec, **kwargs):
         """Get the exact spec OR get a single spec that matches."""

lib/spack/spack/installer.py

@@ -451,7 +451,7 @@ def _process_external_package(pkg: "spack.package_base.PackageBase", explicit: b
 
         # Add to the DB
         tty.debug(f"{pre} registering into DB")
-        spack.store.STORE.db.add(spec, None, explicit=explicit)
+        spack.store.STORE.db.add(spec, explicit=explicit)
 
 
 def _process_binary_cache_tarball(
@@ -493,7 +493,7 @@ def _process_binary_cache_tarball(
             pkg._post_buildcache_install_hook()
 
         pkg.installed_from_binary_cache = True
-        spack.store.STORE.db.add(pkg.spec, spack.store.STORE.layout, explicit=explicit)
+        spack.store.STORE.db.add(pkg.spec, explicit=explicit)
         return True
 
 
@@ -1668,7 +1668,7 @@ def _install_task(self, task: BuildTask, install_status: InstallStatus) -> None:
             )
             # Note: PARENT of the build process adds the new package to
             # the database, so that we don't need to re-read from file.
-            spack.store.STORE.db.add(pkg.spec, spack.store.STORE.layout, explicit=explicit)
+            spack.store.STORE.db.add(pkg.spec, explicit=explicit)
 
             # If a compiler, ensure it is added to the configuration
             if task.compiler:

lib/spack/spack/rewiring.py

@@ -116,7 +116,7 @@ def rewire_node(spec, explicit):
     # spec being added to look for mismatches)
     spack.store.STORE.layout.write_spec(spec, spack.store.STORE.layout.spec_file_path(spec))
     # add to database, not sure about explicit
-    spack.store.STORE.db.add(spec, spack.store.STORE.layout, explicit=explicit)
+    spack.store.STORE.db.add(spec, explicit=explicit)
 
     # run post install hooks
     spack.hooks.post_install(spec, explicit)

lib/spack/spack/store.py

@@ -173,7 +173,12 @@ def __init__(
         self.hash_length = hash_length
         self.upstreams = upstreams
         self.lock_cfg = lock_cfg
-        self.db = spack.database.Database(root, upstream_dbs=upstreams, lock_cfg=lock_cfg)
+        self.layout = spack.directory_layout.DirectoryLayout(
+            root, projections=projections, hash_length=hash_length
+        )
+        self.db = spack.database.Database(
+            root, upstream_dbs=upstreams, lock_cfg=lock_cfg, layout=self.layout
+        )
 
         timeout_format_str = (
             f"{str(lock_cfg.package_timeout)}s" if lock_cfg.package_timeout else "No timeout"
@@ -187,13 +192,9 @@ def __init__(
             self.root, default_timeout=lock_cfg.package_timeout
         )
 
-        self.layout = spack.directory_layout.DirectoryLayout(
-            root, projections=projections, hash_length=hash_length
-        )
-
     def reindex(self) -> None:
         """Convenience function to reindex the store DB with its own layout."""
-        return self.db.reindex(self.layout)
+        return self.db.reindex()
 
     def __reduce__(self):
         return Store, (

lib/spack/spack/test/cmd/buildcache.py

@@ -379,9 +379,8 @@ def test_buildcache_create_install(
 def test_correct_specs_are_pushed(
     things_to_install, expected, tmpdir, monkeypatch, default_mock_concretization, temporary_store
 ):
-    # Concretize dttop and add it to the temporary database (without prefixes)
     spec = default_mock_concretization("dttop")
-    temporary_store.db.add(spec, directory_layout=None)
+    spec.package.do_install(fake=True)
     slash_hash = f"/{spec.dag_hash()}"
 
     class DontUpload(spack.binary_distribution.Uploader):

lib/spack/spack/test/cmd/config.py

@@ -591,14 +591,12 @@ def test_config_prefer_upstream(
     """
 
     mock_db_root = str(tmpdir_factory.mktemp("mock_db_root"))
-    prepared_db = spack.database.Database(mock_db_root)
+    prepared_db = spack.database.Database(mock_db_root, layout=gen_mock_layout("/a/"))
-
-    upstream_layout = gen_mock_layout("/a/")
 
     for spec in ["hdf5 +mpi", "hdf5 ~mpi", "boost+debug~icu+graph", "dependency-install", "patch"]:
         dep = spack.spec.Spec(spec)
         dep.concretize()
-        prepared_db.add(dep, upstream_layout)
+        prepared_db.add(dep)
 
     downstream_db_root = str(tmpdir_factory.mktemp("mock_downstream_db_root"))
     db_for_test = spack.database.Database(downstream_db_root, upstream_dbs=[prepared_db])

lib/spack/spack/test/concretize.py

@@ -13,6 +13,7 @@
 
 import llnl.util.lang
 
+import spack.binary_distribution
 import spack.compiler
 import spack.compilers
 import spack.concretize
@@ -1287,7 +1288,7 @@ def mock_fn(*args, **kwargs):
             return [first_spec]
 
         if mock_db:
-            temporary_store.db.add(first_spec, None)
+            temporary_store.db.add(first_spec)
         else:
             monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", mock_fn)
 
@@ -1352,7 +1353,7 @@ def test_no_reuse_when_variant_condition_does_not_hold(self, mutable_database, m
     def test_reuse_with_flags(self, mutable_database, mutable_config):
         spack.config.set("concretizer:reuse", True)
         spec = Spec("pkg-a cflags=-g cxxflags=-g").concretized()
-        spack.store.STORE.db.add(spec, None)
+        spec.package.do_install(fake=True)
 
         testspec = Spec("pkg-a cflags=-g")
         testspec.concretize()

lib/spack/spack/test/database.py

@@ -40,20 +40,21 @@
 @pytest.fixture()
 def upstream_and_downstream_db(tmpdir, gen_mock_layout):
     mock_db_root = str(tmpdir.mkdir("mock_db_root"))
-    upstream_write_db = spack.database.Database(mock_db_root)
+    upstream_layout = gen_mock_layout("/a/")
-    upstream_db = spack.database.Database(mock_db_root, is_upstream=True)
+    upstream_write_db = spack.database.Database(mock_db_root, layout=upstream_layout)
+    upstream_db = spack.database.Database(mock_db_root, is_upstream=True, layout=upstream_layout)
     # Generate initial DB file to avoid reindex
     with open(upstream_write_db._index_path, "w") as db_file:
         upstream_write_db._write_to_file(db_file)
-    upstream_layout = gen_mock_layout("/a/")
 
     downstream_db_root = str(tmpdir.mkdir("mock_downstream_db_root"))
-    downstream_db = spack.database.Database(downstream_db_root, upstream_dbs=[upstream_db])
+    downstream_db = spack.database.Database(
+        downstream_db_root, upstream_dbs=[upstream_db], layout=gen_mock_layout("/b/")
+    )
     with open(downstream_db._index_path, "w") as db_file:
         downstream_db._write_to_file(db_file)
-    downstream_layout = gen_mock_layout("/b/")
 
-    yield upstream_write_db, upstream_db, upstream_layout, downstream_db, downstream_layout
+    yield upstream_write_db, upstream_db, downstream_db
 
 
 @pytest.mark.parametrize(
@@ -69,14 +70,14 @@ def upstream_and_downstream_db(tmpdir, gen_mock_layout):
 def test_query_by_install_tree(
     install_tree, result, upstream_and_downstream_db, mock_packages, monkeypatch, config
 ):
-    up_write_db, up_db, up_layout, down_db, down_layout = upstream_and_downstream_db
+    up_write_db, up_db, down_db = upstream_and_downstream_db
 
     # Set the upstream DB to contain "pkg-c" and downstream to contain "pkg-b")
     b = spack.spec.Spec("pkg-b").concretized()
     c = spack.spec.Spec("pkg-c").concretized()
-    up_write_db.add(c, up_layout)
+    up_write_db.add(c)
     up_db._read()
-    down_db.add(b, down_layout)
+    down_db.add(b)
 
     specs = down_db.query(install_tree=install_tree.format(u=up_db.root, d=down_db.root))
     assert [s.name for s in specs] == result
@@ -86,9 +87,7 @@ def test_spec_installed_upstream(
     upstream_and_downstream_db, mock_custom_repository, config, monkeypatch
 ):
     """Test whether Spec.installed_upstream() works."""
-    upstream_write_db, upstream_db, upstream_layout, downstream_db, downstream_layout = (
+    upstream_write_db, upstream_db, downstream_db = upstream_and_downstream_db
-        upstream_and_downstream_db
-    )
 
     # a known installed spec should say that it's installed
     with spack.repo.use_repositories(mock_custom_repository):
@@ -96,7 +95,7 @@ def test_spec_installed_upstream(
         assert not spec.installed
         assert not spec.installed_upstream
 
-        upstream_write_db.add(spec, upstream_layout)
+        upstream_write_db.add(spec)
         upstream_db._read()
 
         monkeypatch.setattr(spack.store.STORE, "db", downstream_db)
@@ -112,9 +111,7 @@ def test_spec_installed_upstream(
 
 @pytest.mark.usefixtures("config")
 def test_installed_upstream(upstream_and_downstream_db, tmpdir):
-    upstream_write_db, upstream_db, upstream_layout, downstream_db, downstream_layout = (
+    upstream_write_db, upstream_db, downstream_db = upstream_and_downstream_db
-        upstream_and_downstream_db
-    )
 
     builder = spack.repo.MockRepositoryBuilder(tmpdir.mkdir("mock.repo"))
     builder.add_package("x")
@@ -125,7 +122,7 @@ def test_installed_upstream(upstream_and_downstream_db, tmpdir):
     with spack.repo.use_repositories(builder.root):
         spec = spack.spec.Spec("w").concretized()
         for dep in spec.traverse(root=False):
-            upstream_write_db.add(dep, upstream_layout)
+            upstream_write_db.add(dep)
         upstream_db._read()
 
         for dep in spec.traverse(root=False):
@@ -135,11 +132,11 @@ def test_installed_upstream(upstream_and_downstream_db, tmpdir):
                 upstream_db.get_by_hash(dep.dag_hash())
 
         new_spec = spack.spec.Spec("w").concretized()
-        downstream_db.add(new_spec, downstream_layout)
+        downstream_db.add(new_spec)
         for dep in new_spec.traverse(root=False):
             upstream, record = downstream_db.query_by_spec_hash(dep.dag_hash())
             assert upstream
-            assert record.path == upstream_layout.path_for_spec(dep)
+            assert record.path == upstream_db.layout.path_for_spec(dep)
         upstream, record = downstream_db.query_by_spec_hash(new_spec.dag_hash())
         assert not upstream
         assert record.installed
@@ -149,9 +146,7 @@ def test_installed_upstream(upstream_and_downstream_db, tmpdir):
 
 
 def test_removed_upstream_dep(upstream_and_downstream_db, tmpdir, capsys, config):
-    upstream_write_db, upstream_db, upstream_layout, downstream_db, downstream_layout = (
+    upstream_write_db, upstream_db, downstream_db = upstream_and_downstream_db
-        upstream_and_downstream_db
-    )
 
     builder = spack.repo.MockRepositoryBuilder(tmpdir.mkdir("mock.repo"))
     builder.add_package("z")
@@ -162,9 +157,9 @@ def test_removed_upstream_dep(upstream_and_downstream_db, tmpdir, capsys, config
         z = y["z"]
 
         # add dependency to upstream, dependents to downstream
-        upstream_write_db.add(z, upstream_layout)
+        upstream_write_db.add(z)
         upstream_db._read()
-        downstream_db.add(y, downstream_layout)
+        downstream_db.add(y)
 
         # remove the dependency from the upstream DB
         upstream_write_db.remove(z)
@@ -184,9 +179,7 @@ def test_add_to_upstream_after_downstream(upstream_and_downstream_db, tmpdir):
     DB. When a package is recorded as installed in both, the results should
     refer to the downstream DB.
     """
-    upstream_write_db, upstream_db, upstream_layout, downstream_db, downstream_layout = (
+    upstream_write_db, upstream_db, downstream_db = upstream_and_downstream_db
-        upstream_and_downstream_db
-    )
 
     builder = spack.repo.MockRepositoryBuilder(tmpdir.mkdir("mock.repo"))
     builder.add_package("x")
@@ -194,8 +187,8 @@ def test_add_to_upstream_after_downstream(upstream_and_downstream_db, tmpdir):
     with spack.repo.use_repositories(builder.root):
         spec = spack.spec.Spec("x").concretized()
 
-        downstream_db.add(spec, downstream_layout)
+        downstream_db.add(spec)
-        upstream_write_db.add(spec, upstream_layout)
+        upstream_write_db.add(spec)
         upstream_db._read()
 
         upstream, record = downstream_db.query_by_spec_hash(spec.dag_hash())
@@ -209,33 +202,22 @@ def test_add_to_upstream_after_downstream(upstream_and_downstream_db, tmpdir):
         try:
             orig_db = spack.store.STORE.db
             spack.store.STORE.db = downstream_db
-            assert queried_spec.prefix == downstream_layout.path_for_spec(spec)
+            assert queried_spec.prefix == downstream_db.layout.path_for_spec(spec)
         finally:
             spack.store.STORE.db = orig_db
 
 
-@pytest.mark.usefixtures("config", "temporary_store")
+def test_cannot_write_upstream(tmp_path, mock_packages, config):
-def test_cannot_write_upstream(tmpdir, gen_mock_layout):
-    roots = [str(tmpdir.mkdir(x)) for x in ["a", "b"]]
-    layouts = [gen_mock_layout(x) for x in ["/ra/", "/rb/"]]
-
-    builder = spack.repo.MockRepositoryBuilder(tmpdir.mkdir("mock.repo"))
-    builder.add_package("x")
-
     # Instantiate the database that will be used as the upstream DB and make
     # sure it has an index file
-    upstream_db_independent = spack.database.Database(roots[1])
+    with spack.database.Database(str(tmp_path)).write_transaction():
-    with upstream_db_independent.write_transaction():
         pass
 
-    upstream_dbs = spack.store._construct_upstream_dbs_from_install_roots([roots[1]])
+    # Create it as an upstream
+    db = spack.database.Database(str(tmp_path), is_upstream=True)
 
-    with spack.repo.use_repositories(builder.root):
+    with pytest.raises(spack.database.ForbiddenLockError):
-        spec = spack.spec.Spec("x")
+        db.add(spack.spec.Spec("pkg-a").concretized())
-        spec.concretize()
-
-        with pytest.raises(spack.database.ForbiddenLockError):
-            upstream_dbs[0].add(spec, layouts[1])
 
 
 @pytest.mark.usefixtures("config", "temporary_store")
@@ -250,14 +232,14 @@ def test_recursive_upstream_dbs(tmpdir, gen_mock_layout):
 
     with spack.repo.use_repositories(builder.root):
         spec = spack.spec.Spec("x").concretized()
-        db_c = spack.database.Database(roots[2])
+        db_c = spack.database.Database(roots[2], layout=layouts[2])
-        db_c.add(spec["z"], layouts[2])
+        db_c.add(spec["z"])
 
-        db_b = spack.database.Database(roots[1], upstream_dbs=[db_c])
+        db_b = spack.database.Database(roots[1], upstream_dbs=[db_c], layout=layouts[1])
-        db_b.add(spec["y"], layouts[1])
+        db_b.add(spec["y"])
 
-        db_a = spack.database.Database(roots[0], upstream_dbs=[db_b, db_c])
+        db_a = spack.database.Database(roots[0], upstream_dbs=[db_b, db_c], layout=layouts[0])
-        db_a.add(spec["x"], layouts[0])
+        db_a.add(spec["x"])
 
         upstream_dbs_from_scratch = spack.store._construct_upstream_dbs_from_install_roots(
             [roots[1], roots[2]]
@@ -368,7 +350,7 @@ def _check_db_sanity(database):
     _check_merkleiness()
 
 
-def _check_remove_and_add_package(database, spec):
+def _check_remove_and_add_package(database: spack.database.Database, spec):
     """Remove a spec from the DB, then add it and make sure everything's
     still ok once it is added.  This checks that it was
     removed, that it's back when added again, and that ref
@@ -388,7 +370,7 @@ def _check_remove_and_add_package(database, spec):
     assert concrete_spec not in remaining
 
     # add it back and make sure everything is ok.
-    database.add(concrete_spec, spack.store.STORE.layout)
+    database.add(concrete_spec)
     installed = database.query()
     assert concrete_spec in installed
     assert installed == original
@@ -398,7 +380,7 @@ def _check_remove_and_add_package(database, spec):
     database._check_ref_counts()
 
 
-def _mock_install(spec):
+def _mock_install(spec: str):
     s = spack.spec.Spec(spec).concretized()
     s.package.do_install(fake=True)
 
@@ -638,7 +620,7 @@ def test_080_root_ref_counts(mutable_database):
     assert mutable_database.get_record("mpich").ref_count == 1
 
     # Put the spec back
-    mutable_database.add(rec.spec, spack.store.STORE.layout)
+    mutable_database.add(rec.spec)
 
     # record is present again
     assert len(mutable_database.query("mpileaks ^mpich", installed=any)) == 1
@@ -1119,9 +1101,9 @@ def test_database_construction_doesnt_use_globals(tmpdir, config, nullify_global
 def test_database_read_works_with_trailing_data(tmp_path, default_mock_concretization):
     # Populate a database
     root = str(tmp_path)
-    db = spack.database.Database(root)
+    db = spack.database.Database(root, layout=None)
     spec = default_mock_concretization("pkg-a")
-    db.add(spec, directory_layout=None)
+    db.add(spec)
     specs_in_db = db.query_local()
     assert spec in specs_in_db
 

lib/spack/spack/test/install.py

@@ -11,6 +11,8 @@
 
 import llnl.util.filesystem as fs
 
+import spack.config
+import spack.database
 import spack.error
 import spack.mirror
 import spack.patch
@@ -255,8 +257,8 @@ def install_upstream(tmpdir_factory, gen_mock_layout, install_mockery):
     installs are using the upstream installs).
     """
     mock_db_root = str(tmpdir_factory.mktemp("mock_db_root"))
-    prepared_db = spack.database.Database(mock_db_root)
     upstream_layout = gen_mock_layout("/a/")
+    prepared_db = spack.database.Database(mock_db_root, layout=upstream_layout)
     spack.config.CONFIG.push_scope(
         spack.config.InternalConfigScope(
             name="install-upstream-fixture",
@@ -266,8 +268,7 @@ def install_upstream(tmpdir_factory, gen_mock_layout, install_mockery):
 
     def _install_upstream(*specs):
         for spec_str in specs:
-            s = spack.spec.Spec(spec_str).concretized()
+            prepared_db.add(Spec(spec_str).concretized())
-            prepared_db.add(s, upstream_layout)
         downstream_root = str(tmpdir_factory.mktemp("mock_downstream_db_root"))
         return downstream_root, upstream_layout
 
@@ -280,7 +281,7 @@ def test_installed_upstream_external(install_upstream, mock_fetch):
     """
     store_root, _ = install_upstream("externaltool")
     with spack.store.use_store(store_root):
-        dependent = spack.spec.Spec("externaltest")
+        dependent = Spec("externaltest")
         dependent.concretize()
 
         new_dependency = dependent["externaltool"]
@@ -299,8 +300,8 @@ def test_installed_upstream(install_upstream, mock_fetch):
     """
     store_root, upstream_layout = install_upstream("dependency-install")
     with spack.store.use_store(store_root):
-        dependency = spack.spec.Spec("dependency-install").concretized()
+        dependency = Spec("dependency-install").concretized()
-        dependent = spack.spec.Spec("dependent-install").concretized()
+        dependent = Spec("dependent-install").concretized()
 
         new_dependency = dependent["dependency-install"]
         assert new_dependency.installed_upstream
@@ -607,7 +608,7 @@ def test_install_from_binary_with_missing_patch_succeeds(
         s.to_json(f)
 
     # And register it in the database
-    temporary_store.db.add(s, directory_layout=temporary_store.layout, explicit=True)
+    temporary_store.db.add(s, explicit=True)
 
     # Push it to a binary cache
     mirror = spack.mirror.Mirror.from_local_path(str(tmp_path / "my_build_cache"))

lib/spack/spack/test/spec_syntax.py

@@ -864,8 +864,8 @@ def test_ambiguous_hash(mutable_database):
 
     assert x1 != x2  # doesn't hold when only the dag hash is modified.
 
-    mutable_database.add(x1, directory_layout=None)
+    mutable_database.add(x1)
-    mutable_database.add(x2, directory_layout=None)
+    mutable_database.add(x2)
 
     # ambiguity in first hash character
     s1 = SpecParser("/x").next_spec()