From 85ef1be780c82392227961529d67fd2b498e1beb Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Wed, 29 Jan 2020 17:22:44 -0800 Subject: [PATCH] environments: synchronize read and uninstall (#14676) * `Environment.__init__` is now synchronized with all writing operations * `spack uninstall` now synchronizes its updates to any associated environment * A side effect of this is that the environment is no longer updated piecemeal as specs are uninstalled - all specs are removed from the environment before they are uninstalled --- lib/spack/spack/cmd/uninstall.py | 18 ++++++++---------- lib/spack/spack/environment.py | 20 ++++++++++++-------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py index 0ad42f4dfb..2757d5d232 100644 --- a/lib/spack/spack/cmd/uninstall.py +++ b/lib/spack/spack/cmd/uninstall.py @@ -6,6 +6,7 @@ from __future__ import print_function import sys +import itertools import spack.cmd import spack.environment as ev @@ -205,9 +206,6 @@ def do_uninstall(env, specs, force): # want to uninstall. spack.package.Package.uninstall_by_spec(item, force=True) - if env: - _remove_from_env(item, env) - # A package is ready to be uninstalled when nothing else references it, # unless we are requested to force uninstall it. is_ready = lambda x: not spack.store.db.query_by_spec_hash(x)[1].ref_count @@ -226,10 +224,6 @@ def do_uninstall(env, specs, force): for item in ready: item.do_uninstall(force=force) - # write any changes made to the active environment - if env: - env.write() - def get_uninstall_list(args, specs, env): # Gets the list of installed specs that match the ones give via cli @@ -317,9 +311,13 @@ def uninstall_specs(args, specs): if not args.yes_to_all: confirm_removal(anything_to_do) - # just force-remove things in the remove list - for spec in remove_list: - _remove_from_env(spec, env) + if env: + # Remove all the specs that are supposed to be uninstalled or just + # removed. + with env.write_transaction(): + for spec in itertools.chain(remove_list, uninstall_list): + _remove_from_env(spec, env) + env.write() # Uninstall everything on the list do_uninstall(env, uninstall_list, args.force) diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 87276eacbc..dd2d932849 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -567,6 +567,9 @@ def __init__(self, path, init_file=None, with_view=None): self.clear() if init_file: + # If we are creating the environment from an init file, we don't + # need to lock, because there are no Spack operations that alter + # the init file. with fs.open_if_filename(init_file) as f: if hasattr(f, 'name') and f.name.endswith('.lock'): self._read_manifest(default_manifest_yaml) @@ -575,7 +578,8 @@ def __init__(self, path, init_file=None, with_view=None): else: self._read_manifest(f, raw_yaml=default_manifest_yaml) else: - self._read() + with lk.ReadTransaction(self.txlock): + self._read() if with_view is False: self.views = {} @@ -1472,13 +1476,13 @@ def write(self, regenerate_views=True): # Remove yaml sections that are shadowing defaults # construct garbage path to ensure we don't find a manifest by accident - bare_env = Environment(os.path.join(self.manifest_path, 'garbage'), - with_view=self.view_path_default) - keys_present = list(yaml_dict.keys()) - for key in keys_present: - if yaml_dict[key] == config_dict(bare_env.yaml).get(key, None): - if key not in raw_yaml_dict: - del yaml_dict[key] + with fs.temp_cwd() as env_dir: + bare_env = Environment(env_dir, with_view=self.view_path_default) + keys_present = list(yaml_dict.keys()) + for key in keys_present: + if yaml_dict[key] == config_dict(bare_env.yaml).get(key, None): + if key not in raw_yaml_dict: + del yaml_dict[key] # if all that worked, write out the manifest file at the top level # Only actually write if it has changed or was never written