From c9bb1a937b4d8102b4aec76c5739835e77aa94de Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 5 Apr 2023 13:20:54 -0700 Subject: [PATCH] refactor for views to store metadata --- lib/spack/spack/environment/environment.py | 75 +++++++++++----------- lib/spack/spack/filesystem_view.py | 32 +++++++++ lib/spack/spack/test/cmd/env.py | 13 ++-- 3 files changed, 75 insertions(+), 45 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index c4e4ed0713d..9d5eac64ef4 100644 --- a/lib/spack/spack/environment/environment.py +++ b/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 - directory of the view is the same as self.root. + Query the view if it stores metadata on where it was constructed. 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 - constructed on the filesystem. + If self.root is not a view with metadata and is not a link, the view has not been + constructed. """ - if not os.path.islink(self.root): - if os.path.isdir(self.root): - return self.root - else: - return None + # Get the view as self.root even if it is actually a symlink + # We will not operate on this view object, only query metadata + # We don't want to pass a created_path to this view, so that we can read where it says it + # was created. + 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, - spack.store.layout, - ignore_conflicts=True, - projections=self.projections, - link=self.link_type, - final_destination=self.root, - ) + + kwargs = { + "ignore_conflicts": True, + "projections": self.projections, + "link": self.link_type, + "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_. # 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) - # print( - # [ - # (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) + # Check which atomic update method we need + update_method = self.update_method_to_use(force) + 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: diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index 85dee0eda86..0617ab26d06 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/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.""" diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 9b921c75b90..9c20c3b0048 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/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 - monkeypatch.setattr(view, "view", raises) + old_view = view.update_method_to_use + 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])