From bd305d48a9844c60729dc932083803f4b04eb24b Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 29 Mar 2023 12:49:42 -0700 Subject: [PATCH] enable spack env view regenerate -f --- lib/spack/spack/cmd/env.py | 9 ++++- lib/spack/spack/environment/environment.py | 46 ++++++++++++++-------- lib/spack/spack/test/cmd/env.py | 11 +++++- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index 085757ca0fe..4df75195725 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -420,6 +420,13 @@ def actions(): # def env_view_setup_parser(subparser): """manage a view associated with the environment""" + subparser.add_argument( + "-f", + "--force", + action="store_true", + dest="force", + help="regenerate even if regeneration cannot be done atomically" + ) subparser.add_argument( "action", choices=ViewAction.actions(), help="action to take for the environment's view" ) @@ -433,7 +440,7 @@ def env_view(args): if env: if args.action == ViewAction.regenerate: - env.regenerate_views() + env.regenerate_views(force=args.force) elif args.action == ViewAction.enable: if args.view_path: view_path = args.view_path diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index ea66ee7e059..10c61984bc2 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -604,46 +604,60 @@ def raise_if_invalid_exchange(self): msg = "This operating system does not support the 'exchange' atomic update method." msg += f"\n If the view at {self.root} does not already exist on the filesystem," msg += "change its update_method to 'symlink' or 'auto'." - msg += f"\n If the view at {self.root} exists already, either remove it for a" - msg += " non-atomic update or run on a newer OS." + msg += f"\n If the view at {self.root} exists already, either change the" + msg += " update_method and run `spack env view regenerate --force`" + msg += " or run on a newer OS." raise RuntimeError(msg) - def raise_if_symlink_before_exchange(self): + def raise_if_symlink_before_exchange(self, force): if os.path.islink(self.root): + if force: + try: + os.unlink(self.root) + return + except OSError: + pass msg = f"The view at {self.root} cannot be updated with the 'exchange' update method" msg += " because it was originally constructed with the 'symlink' method." - msg += " Either change the update method to 'symlink' or remove the view for a" - msg += " non-atomic update" + msg += " Either change the update method to 'symlink' or" + msg += " run `spack env view regenerate --force` for a non-atomic update" raise RuntimeError(msg) - def raise_if_exchange_before_symlink(self): + def raise_if_exchange_before_symlink(self, force): if os.path.isdir(self.root) and not os.path.islink(self.root): + if force: + try: + shutil.rmtree(self.root) + return + except: + pass msg = f"The view at {self.root} cannot be updated with the 'symlink' update method" msg += " because it was originally constructed with the 'exchange' method." - msg += " Either change the update method to 'exchange' or remove the view for a" - msg += " non-atomic update" + msg += " Either change the update method to 'exchange' or" + msg += " run `spack env view regenerate --force` for a non-atomic update" raise RuntimeError(msg) - def use_renameat2(self): + def use_renameat2(self, force=False): # If it's set explicitly, respect that if self.update_method == "exchange": - self.raise_if_symlink_before_exchange() + self.raise_if_symlink_before_exchange(force) self.raise_if_invalid_exchange() return True if self.update_method == "symlink": - self.raise_if_exchange_before_symlink() + self.raise_if_exchange_before_symlink(force) return False # If it's set to "auto", detect which it should be if os.path.islink(self.root): return False elif os.path.isdir(self.root): - self.raise_if_invalid_exchange() + if not spack.util.atomic_update.renameat2(): + self.raise_if_exchange_before_symlink(force) return True return bool(spack.util.atomic_update.renameat2()) - def regenerate(self, concretized_root_specs): + def regenerate(self, concretized_root_specs, force=False): specs = self.specs_for_view(concretized_root_specs) # To ensure there are no conflicts with packages being installed @@ -656,7 +670,7 @@ def regenerate(self, concretized_root_specs): # This allows for atomic swaps when we update the view # Check which atomic update method we need - use_renameat2 = self.use_renameat2() + use_renameat2 = self.use_renameat2(force) # cache the roots because the way we determine which is which does # not work while we are updating @@ -1615,14 +1629,14 @@ def update_default_view(self, viewpath): else: self.views.pop(name, None) - def regenerate_views(self): + def regenerate_views(self, force=False): if not self.views: tty.debug("Skip view update, this environment does not" " maintain a view") return concretized_root_specs = [s for _, s in self.concretized_specs()] for view in self.views.values(): - view.regenerate(concretized_root_specs) + view.regenerate(concretized_root_specs, force) def check_views(self): """Checks if the environments default view can be activated.""" diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 84e0f5609ed..59715ff084b 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -52,7 +52,8 @@ sep = os.sep -if spack.util.atomic_update.renameat2(): +supports_renameat2 = bool(spack.util.atomic_update.renameat2()) +if supports_renameat2: use_renameat2 = [True, False] else: use_renameat2 = [False] @@ -3303,13 +3304,16 @@ def test_view_update_mismatch(update_method, tmpdir, install_mockery, mock_fetch if update_method == "symlink": os.makedirs(root) checker = "cannot be updated with the 'symlink' update method" - elif True in use_renameat2: + forceable = True + elif supports_renameat2: link = str(tmpdir.join("symlink")) os.makedirs(link) os.symlink(link, root) checker = "cannot be updated with the 'exchange' update method" + forceable = True else: checker = "does not support the 'exchange' atomic update method" + forceable = False view = ev.environment.ViewDescriptor( base_path=str(tmpdir), root=root, update_method=update_method @@ -3321,6 +3325,9 @@ def test_view_update_mismatch(update_method, tmpdir, install_mockery, mock_fetch with pytest.raises(RuntimeError, match=checker): view.regenerate([spec]) + if forceable: + view.regenerate([spec], force=True) + assert os.path.exists(view.root) @pytest.mark.parametrize("update_method", ["symlink", "exchange"]) def test_view_update_fails(update_method, tmpdir, install_mockery, mock_fetch, monkeypatch):