refactor for views to store metadata

lib/spack/spack/environment/environment.py

@@ -479,21 +479,28 @@ def _current_root(self):
         """
         Return the directory in which the view has been constructed.
 
-        If the view is using renameat2 for atomic updates, self.root is a directory and the root
+        Query the view if it stores metadata on where it was constructed.
-        directory of the view is the same as self.root.
 
         If the view us using symlinks for atomic updates, self.root is a link and we read the link
         to find the real root directory.
 
-        If self.root does not exist or is a regular file, the view has not been
+        If self.root is not a view with metadata and is not a link, the view has not been
-        constructed on the filesystem.
+        constructed.
         """
-        if not os.path.islink(self.root):
+        # Get the view as self.root even if it is actually a symlink
-            if os.path.isdir(self.root):
+        # We will not operate on this view object, only query metadata
-                return self.root
+        # We don't want to pass a created_path to this view, so that we can read where it says it
-            else:
+        # was created.
-                return None
+        view = self.view(self.root, created_path=False)
+        orig_path = view.metadata.get("created_path", None)
+        if orig_path:
+            return orig_path
 
+        # Backwards compat only applies for symlinked views
+        if not os.path.islink(self.root):
+            return None
+
+        # For backards compat, check link for symlink views if no "created_path"
         root = os.readlink(self.root)
         if os.path.isabs(root):
             return root
@@ -529,7 +536,7 @@ def get_projection_for_spec(self, spec):
         rel_path = os.path.relpath(view_path, self._current_root)
         return os.path.join(self.root, rel_path)
 
-    def view(self, new=None):
+    def view(self, new=None, created_path=True):
         """
         Generate the FilesystemView object for this ViewDescriptor
 
@@ -551,14 +558,17 @@ def view(self, new=None):
                 "View root is at %s" % self.root
             )
             raise SpackEnvironmentViewError(msg)
-        return SimpleFilesystemView(
+
-            root,
+        kwargs = {
-            spack.store.layout,
+            "ignore_conflicts": True,
-            ignore_conflicts=True,
+            "projections": self.projections,
-            projections=self.projections,
+            "link": self.link_type,
-            link=self.link_type,
+            "final_destination": self.root,
-            final_destination=self.root,
+        }
-        )
+        if created_path:
+            kwargs["metadata"] = {"created_path": root}
+
+        return SimpleFilesystemView(root, spack.store.layout, **kwargs)
 
     def __contains__(self, spec):
         """Is the spec described by the view descriptor
@@ -675,9 +685,6 @@ def regenerate(self, concretized_root_specs, force=False):
         # will be /dirname/._basename_<hash>.
         # This allows for atomic swaps when we update the view
 
-        # Check which atomic update method we need
-        update_method = self.update_method_to_use(force)
-
         # cache the roots because the way we determine which is which does
         # not work while we are updating
         new_root = self._next_root(specs)
@@ -687,21 +694,10 @@ def regenerate(self, concretized_root_specs, force=False):
             tty.debug("View at %s does not need regeneration." % self.root)
             return
 
-        print(new_root)
+        # Check which atomic update method we need
-        #        print(
+        update_method = self.update_method_to_use(force)
-        #            [
+
-        #                (s, os.stat(os.path.join(os.path.dirname(new_root), s)).st_mtime)
-        #                for s in os.listdir(os.path.dirname(new_root))
-        #            ]
-        #        )
-        print(specs)
         if update_method == "exchange" and os.path.isdir(new_root):
-            # If new_root is the newest thing in its directory, no need to update
-            parent = os.path.dirname(new_root)
-            siblings = [os.path.join(parent, s) for s in os.listdir(parent)]
-            if max(siblings, key=lambda p: os.stat(p).st_mtime) == new_root:
-                tty.debug("View at %s does not need regeneration." % self.root)
-                return
             shutil.rmtree(new_root)
 
         _error_on_nonempty_view_dir(new_root)
@@ -717,7 +713,14 @@ def regenerate(self, concretized_root_specs, force=False):
             fs.mkdirp(new_root)
             view.add_specs(*specs, with_dependencies=False)
             if update_method == "exchange":
-                spack.util.atomic_update.atomic_update_renameat2(new_root, self.root)
+                # Swap the view to the directory of the previous view if one exists so that
+                # the view that is swapped out will be named appropriately
+                if old_root:
+                    os.rename(new_root, old_root)
+                    exchange_location = old_root
+                else:
+                    exchange_location = new_root
+                spack.util.atomic_update.atomic_update_renameat2(exchange_location, self.root)
             else:
                 spack.util.atomic_update.atomic_update_symlink(new_root, self.root)
         except Exception as e:

lib/spack/spack/filesystem_view.py

@@ -43,6 +43,7 @@
 
 
 _projections_path = ".spack/projections.yaml"
+_metadata_path = ".spack/metadata.yaml"
 
 
 def view_symlink(src, dst, **kwargs):
@@ -155,6 +156,7 @@ def __init__(self, root, layout, **kwargs):
         self.layout = layout
 
         self.projections = kwargs.get("projections", {})
+        self.metadata = kwargs.get("metadata", {})
 
         self.ignore_conflicts = kwargs.get("ignore_conflicts", False)
         self.verbose = kwargs.get("verbose", False)
@@ -284,8 +286,34 @@ def __init__(self, root, layout, **kwargs):
                 msg += " which does not match projections passed manually."
                 raise ConflictingProjectionsError(msg)
 
+        self.metadata_path = os.path.join(self._root, _metadata_path)
+        if not self.metadata:
+            self.projections = self.read_metadata()
+        elif not os.path.exists(self.metadata_path):
+            self.write_metadata()
+        else:
+            if self.metadata != self.read_metadata():
+                msg = f"View at {self._root} has metadata file"
+                msg += " which does not match metadata passed manually."
+                raise ConflictingMetadataError(msg)
+
         self._croot = colorize_root(self._root) + " "
 
+    def write_metadata(self):
+        if self.metadata:
+            mkdirp(os.path.dirname(self.metadata_path))
+            with open(self.metadata_path, "w") as f:
+                f.write(s_yaml.dump_config({"metadata": self.metadata}))
+
+    def read_metadata(self):
+        if os.path.exists(self.metadata_path):
+            with open(self.metadata_path, "r") as f:
+                # no schema as this is not user modified
+                metadata_data = s_yaml.load(f)
+                return metadata_data["metadata"]
+        else:
+            return {}
+
     def write_projections(self):
         if self.projections:
             mkdirp(os.path.dirname(self.projections_path))
@@ -848,3 +876,7 @@ def get_dependencies(specs):
 
 class ConflictingProjectionsError(SpackError):
     """Raised when a view has a projections file and is given one manually."""
+
+
+class ConflictingMetadataError(SpackError):
+    """Raised when a view has a metadata file and is given one manually."""

lib/spack/spack/test/cmd/env.py

@@ -8,7 +8,6 @@
 import os
 import shutil
 import sys
-import time
 from argparse import Namespace
 
 import pytest
@@ -3371,33 +3370,29 @@ def test_view_update_unnecessary(update_method, tmpdir, install_mockery, mock_fe
     # Create a "previous" view
     # Wait after each view regeneration to ensure timestamps are different
     view.regenerate([libelf])
-    time.sleep(1)
 
     # monkeypatch so that any attempt to actually regenerate the view fails
     def raises(*args, **kwargs):
         raise AssertionError
 
-    old_view = view.view
+    old_view = view.update_method_to_use
-    monkeypatch.setattr(view, "view", raises)
+    monkeypatch.setattr(view, "update_method_to_use", raises)
 
     # regenerating the view is a no-op, so doesn't raise
     # will raise if the view isn't identical
     view.regenerate([libelf])
-    time.sleep(1)
     with pytest.raises(AssertionError):
         view.regenerate([libelf, libdwarf])
 
     # Create another view so there are multiple old views around
-    monkeypatch.setattr(view, "view", old_view)
+    monkeypatch.setattr(view, "update_method_to_use", old_view)
     view.regenerate([libelf, libdwarf])
-    time.sleep(1)
 
     # Redo the monkeypatch
-    monkeypatch.setattr(view, "view", raises)
+    monkeypatch.setattr(view, "update_method_to_use", raises)
 
     # no raise for no-op regeneration
     # raise when it's not a no-op
     view.regenerate([libelf, libdwarf])
-    time.sleep(1)
     with pytest.raises(AssertionError):
         view.regenerate([libelf])