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

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

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