diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index cabb255e23b..0cc28cfe81b 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -38,6 +38,7 @@ import spack.subprocess_context import spack.traverse import spack.user_environment as uenv +import spack.util.atomic_update import spack.util.cpus import spack.util.environment import spack.util.hash @@ -467,6 +468,9 @@ def from_dict(base_path, d): @property def _current_root(self): + if spack.util.atomic_update.use_renameat2(): + return self.root + if not os.path.islink(self.root): return None @@ -484,6 +488,9 @@ def _next_root(self, specs): return os.path.join(root_dir, "._%s" % root_name, content_hash) def content_hash(self, specs): + print("CONTENT_HASH") + print(" ", specs) + print(" ", self.to_dict()) d = syaml.syaml_dict( [ ("descriptor", self.to_dict()), @@ -491,6 +498,7 @@ def content_hash(self, specs): ] ) contents = sjson.dump(d) + print(" ", spack.util.hash.b32_hash(contents)) return spack.util.hash.b32_hash(contents) def get_projection_for_spec(self, spec): @@ -597,6 +605,10 @@ def regenerate(self, concretized_root_specs): tty.debug("View at %s does not need regeneration." % self.root) return + if spack.util.atomic_update.use_renameat2(): + if os.path.isdir(new_root): + shutil.rmtree(new_root) + _error_on_nonempty_view_dir(new_root) # construct view at new_root @@ -605,26 +617,15 @@ def regenerate(self, concretized_root_specs): view = self.view(new=new_root) - root_dirname = os.path.dirname(self.root) - tmp_symlink_name = os.path.join(root_dirname, "._view_link") - # Create a new view try: fs.mkdirp(new_root) view.add_specs(*specs, with_dependencies=False) - - # create symlink from tmp_symlink_name to new_root - if os.path.exists(tmp_symlink_name): - os.unlink(tmp_symlink_name) - symlink(new_root, tmp_symlink_name) - - # mv symlink atomically over root symlink to old_root - fs.rename(tmp_symlink_name, self.root) + spack.util.atomic_update.atomic_update(new_root, self.root) except Exception as e: # Clean up new view and temporary symlink on any failure. try: shutil.rmtree(new_root, ignore_errors=True) - os.unlink(tmp_symlink_name) except (IOError, OSError): pass diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index 500adb4e43b..f768d9e28b2 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/lib/spack/spack/filesystem_view.py @@ -772,6 +772,22 @@ def get_relative_projection_for_spec(self, spec): p = spack.projections.get_projection(self.projections, spec) return spec.format(p) if p else "" + def get_all_specs(self): + md_dirs = [] + for root, dirs, files in os.walk(self._root): + if spack.store.layout.metadata_dir in dirs: + md_dirs.append(os.path.join(root, spack.store.layout.metadata_dir)) + + specs = [] + for md_dir in md_dirs: + if os.path.exists(md_dir): + for name_dir in os.listdir(md_dir): + filename = os.path.join(md_dir, name_dir, spack.store.layout.spec_file_name) + spec = get_spec_from_file(filename) + if spec: + specs.append(spec) + return specs + def get_projection_for_spec(self, spec): """ Return the projection for a spec in this view. diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index ce7862cd12a..9ed8d458182 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -52,6 +52,16 @@ sep = os.sep +if spack.util.atomic_update.use_renameat2(): + use_renameat2 = [True, False] +else: + use_renameat2 = [False] + +@pytest.fixture(params=use_renameat2) +def atomic_update_implementations(request, monkeypatch): + monkeypatch.setattr(spack.util.atomic_update, "use_renameat2", lambda: request.param) + yield + def check_mpileaks_and_deps_in_view(viewdir): """Check that the expected install directories exist.""" @@ -597,7 +607,9 @@ def test_init_from_yaml(tmpdir): @pytest.mark.usefixtures("config") -def test_env_view_external_prefix(tmpdir_factory, mutable_database, mock_packages): +def test_env_view_external_prefix( + tmpdir_factory, mutable_database, mock_packages, atomic_update_implementations +): fake_prefix = tmpdir_factory.mktemp("a-prefix") fake_bin = fake_prefix.join("bin") fake_bin.ensure(dir=True) @@ -1178,7 +1190,9 @@ def test_store_different_build_deps(tmpdir): assert x_read["y"].dag_hash() != y_read.dag_hash() -def test_env_updates_view_install(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_updates_view_install( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): view_dir = tmpdir.join("view") env("create", "--with-view=%s" % view_dir, "test") with ev.read("test"): @@ -1188,7 +1202,9 @@ def test_env_updates_view_install(tmpdir, mock_stage, mock_fetch, install_mocker check_mpileaks_and_deps_in_view(view_dir) -def test_env_view_fails(tmpdir, mock_packages, mock_stage, mock_fetch, install_mockery): +def test_env_view_fails( + tmpdir, mock_packages, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): # We currently ignore file-file conflicts for the prefix merge, # so in principle there will be no errors in this test. But # the .spack metadata dir is handled separately and is more strict. @@ -1205,7 +1221,9 @@ def test_env_view_fails(tmpdir, mock_packages, mock_stage, mock_fetch, install_m install("--fake") -def test_env_view_fails_dir_file(tmpdir, mock_packages, mock_stage, mock_fetch, install_mockery): +def test_env_view_fails_dir_file( + tmpdir, mock_packages, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): # This environment view fails to be created because a file # and a dir are in the same path. Test that it mentions the problematic path. view_dir = tmpdir.join("view") @@ -1220,7 +1238,7 @@ def test_env_view_fails_dir_file(tmpdir, mock_packages, mock_stage, mock_fetch, def test_env_view_succeeds_symlinked_dir_file( - tmpdir, mock_packages, mock_stage, mock_fetch, install_mockery + tmpdir, mock_packages, mock_stage, mock_fetch, install_mockery, atomic_update_implementations ): # A symlinked dir and an ordinary dir merge happily view_dir = tmpdir.join("view") @@ -1234,7 +1252,9 @@ def test_env_view_succeeds_symlinked_dir_file( assert os.path.exists(os.path.join(x_dir, "file_in_symlinked_dir")) -def test_env_without_view_install(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_without_view_install( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): # Test enabling a view after installing specs env("create", "--without-view", "test") @@ -1255,7 +1275,9 @@ def test_env_without_view_install(tmpdir, mock_stage, mock_fetch, install_mocker check_mpileaks_and_deps_in_view(view_dir) -def test_env_config_view_default(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_config_view_default( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): # This config doesn't mention whether a view is enabled test_config = """\ env: @@ -1273,7 +1295,9 @@ def test_env_config_view_default(tmpdir, mock_stage, mock_fetch, install_mockery assert os.path.isdir(os.path.join(e.default_view.view()._root, ".spack", "mpileaks")) -def test_env_updates_view_install_package(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_updates_view_install_package( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): view_dir = tmpdir.join("view") env("create", "--with-view=%s" % view_dir, "test") with ev.read("test"): @@ -1282,7 +1306,9 @@ def test_env_updates_view_install_package(tmpdir, mock_stage, mock_fetch, instal assert os.path.exists(str(view_dir.join(".spack/mpileaks"))) -def test_env_updates_view_add_concretize(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_updates_view_add_concretize( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): view_dir = tmpdir.join("view") env("create", "--with-view=%s" % view_dir, "test") install("--fake", "mpileaks") @@ -1293,7 +1319,9 @@ def test_env_updates_view_add_concretize(tmpdir, mock_stage, mock_fetch, install check_mpileaks_and_deps_in_view(view_dir) -def test_env_updates_view_uninstall(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_updates_view_uninstall( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): view_dir = tmpdir.join("view") env("create", "--with-view=%s" % view_dir, "test") with ev.read("test"): @@ -1308,7 +1336,7 @@ def test_env_updates_view_uninstall(tmpdir, mock_stage, mock_fetch, install_mock def test_env_updates_view_uninstall_referenced_elsewhere( - tmpdir, mock_stage, mock_fetch, install_mockery + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations ): view_dir = tmpdir.join("view") env("create", "--with-view=%s" % view_dir, "test") @@ -1325,24 +1353,30 @@ def test_env_updates_view_uninstall_referenced_elsewhere( check_viewdir_removal(view_dir) -def test_env_updates_view_remove_concretize(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_updates_view_remove_concretize( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): view_dir = tmpdir.join("view") + env("create", "--with-view=%s" % view_dir, "test") install("--fake", "mpileaks") + with ev.read("test"): add("mpileaks") concretize() check_mpileaks_and_deps_in_view(view_dir) - with ev.read("test"): + with ev.read("test") as e: remove("mpileaks") concretize() check_viewdir_removal(view_dir) -def test_env_updates_view_force_remove(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_env_updates_view_force_remove( + tmpdir, mock_stage, mock_fetch, install_mockery, atomic_update_implementations +): view_dir = tmpdir.join("view") env("create", "--with-view=%s" % view_dir, "test") with ev.read("test"): @@ -1889,7 +1923,7 @@ def test_stack_definition_conditional_add_write(tmpdir): def test_stack_combinatorial_view( - tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations ): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) @@ -1924,7 +1958,9 @@ def test_stack_combinatorial_view( ) -def test_stack_view_select(tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery): +def test_stack_view_select( + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations +): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) with open(filename, "w") as f: @@ -1964,7 +2000,9 @@ def test_stack_view_select(tmpdir, mock_fetch, mock_packages, mock_archive, inst ) -def test_stack_view_exclude(tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery): +def test_stack_view_exclude( + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations +): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) with open(filename, "w") as f: @@ -2005,7 +2043,7 @@ def test_stack_view_exclude(tmpdir, mock_fetch, mock_packages, mock_archive, ins def test_stack_view_select_and_exclude( - tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations ): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) @@ -2047,7 +2085,9 @@ def test_stack_view_select_and_exclude( ) -def test_view_link_roots(tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery): +def test_view_link_roots( + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations +): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) with open(filename, "w") as f: @@ -2091,7 +2131,9 @@ def test_view_link_roots(tmpdir, mock_fetch, mock_packages, mock_archive, instal ) -def test_view_link_run(tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery): +def test_view_link_run( + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations +): yaml = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) envdir = str(tmpdir) @@ -2133,7 +2175,13 @@ def test_view_link_run(tmpdir, mock_fetch, mock_packages, mock_archive, install_ @pytest.mark.parametrize("link_type", ["hardlink", "copy", "symlink"]) def test_view_link_type( - link_type, tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery + link_type, + tmpdir, + mock_fetch, + mock_packages, + mock_archive, + install_mockery, + atomic_update_implementations ): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) @@ -2163,7 +2211,9 @@ def test_view_link_type( assert os.path.islink(file_to_test) == (link_type == "symlink") -def test_view_link_all(tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery): +def test_view_link_all( + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations +): filename = str(tmpdir.join("spack.yaml")) viewdir = str(tmpdir.join("view")) with open(filename, "w") as f: @@ -2274,7 +2324,7 @@ def test_stack_view_no_activate_without_default( def test_stack_view_multiple_views( - tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery + tmpdir, mock_fetch, mock_packages, mock_archive, install_mockery, atomic_update_implementations ): filename = str(tmpdir.join("spack.yaml")) default_viewdir = str(tmpdir.join("default-view")) @@ -2843,10 +2893,14 @@ def test_failed_view_cleanup(tmpdir, mock_stage, mock_fetch, install_mockery): assert os.path.samefile(resolved_view, view) -def test_environment_view_target_already_exists(tmpdir, mock_stage, mock_fetch, install_mockery): +def test_environment_view_target_already_exists( + tmpdir, mock_stage, mock_fetch, install_mockery, monkeypatch +): """When creating a new view, Spack should check whether the new view dir already exists. If so, it should not be removed or modified.""" + # Only works for symlinked atomic views + monkeypatch.setattr(spack.util.atomic_update, "use_renameat2", lambda: False) # Create a new environment view = str(tmpdir.join("view")) diff --git a/lib/spack/spack/util/atomic_update.py b/lib/spack/spack/util/atomic_update.py new file mode 100644 index 00000000000..ea5093476dc --- /dev/null +++ b/lib/spack/spack/util/atomic_update.py @@ -0,0 +1,75 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from contextlib import contextmanager +import os +import llnl.util.filesystem as fs +from llnl.util.symlink import symlink + +try: + from ctypes import CDLL + libc = CDLL("/lib64/libc.so.6", 0x04) # 0x04 is RTLD_NOLOAD +except BaseException: + libc = None + + +def use_renameat2(): + return hasattr(libc, "renameat2") + + +def atomic_update(oldpath, newpath): + """ + atomically update newpath to contain the information at oldpath + + on linux systems supporting renameat2, the paths are swapped. + on other systems, oldpath is not affected but all paths are abstracted + by a symlink to allow for atomic updates. + """ + if use_renameat2(): + return atomic_update_renameat2(oldpath, newpath) + else: + return atomic_update_symlink(oldpath, newpath) + + +@contextmanager +def open_safely(path): + fd = os.open(path, os.O_CLOEXEC | os.O_PATH) + try: + yield fd + finally: + os.close(fd) + + +def atomic_update_renameat2(src, dest): + dest_exists = os.path.exists(dest) + if not dest_exists: + fs.touch(dest) + with open_safely(src) as srcfd: + with open_safely(dest) as destfd: + try: + libc.renameat2(srcfd, src.encode(), destfd, dest.encode(), 2) # 2 is RENAME_EXCHANGE + if not dest_exists: + os.unlink(src) + except Exception: + if not dest_exists: + os.unlink(dest) + # Some filesystems don't support this + # fail over to symlink method + atomic_update_symlink(src, dest) + + +def atomic_update_symlink(src, dest): + # Create temporary symlink to point to src + tmp_symlink_name = os.path.join(os.path.dirname(dest), "._tmp_symlink") + if os.path.exists(tmp_symlink_name): + os.unlink(tmp_symlink_name) + symlink(src, tmp_symlink_name) + + # atomically mv the symlink to destpath (still points to srcpath) + try: + fs.rename(tmp_symlink_name, dest) + except Exception: + os.unlink(tmp_symlink_name) + raise