use renameat2 for atomic view updates.

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)
-
+            spack.util.atomic_update.atomic_update(new_root, self.root)
-            # 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)
         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
 

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.

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"))

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