summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/spack/spack/environment/environment.py51
-rw-r--r--lib/spack/spack/test/cmd/env.py25
2 files changed, 49 insertions, 27 deletions
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index ad900f53ff..ddfae0c783 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -1573,11 +1573,12 @@ class Environment(object):
os.remove(build_log_link)
symlink(spec.package.build_log_path, build_log_link)
- def uninstalled_specs(self):
- """Return a list of all uninstalled (and non-dev) specs."""
- # Do the installed check across all specs within a single
- # DB read transaction to reduce time spent in lock acquisition.
- uninstalled_specs = []
+ def _partition_roots_by_install_status(self):
+ """Partition root specs into those that do not have to be passed to the
+ installer, and those that should be, taking into account development
+ specs. This is done in a single read transaction per environment instead
+ of per spec."""
+ installed, uninstalled = [], []
with spack.store.db.read_transaction():
for concretized_hash in self.concretized_order:
spec = self.specs_by_hash[concretized_hash]
@@ -1585,8 +1586,15 @@ class Environment(object):
spec.satisfies('dev_path=*') or
spec.satisfies('^dev_path=*')
):
- uninstalled_specs.append(spec)
- return uninstalled_specs
+ uninstalled.append(spec)
+ else:
+ installed.append(spec)
+ return installed, uninstalled
+
+ def uninstalled_specs(self):
+ """Return root specs that are not installed, or are installed, but
+ are development specs themselves or have those among their dependencies."""
+ return self._partition_roots_by_install_status()[1]
def install_all(self, **install_args):
"""Install all concretized specs in an environment.
@@ -1604,22 +1612,21 @@ class Environment(object):
# If "spack install" is invoked repeatedly for a large environment
# where all specs are already installed, the operation can take
# a large amount of time due to repeatedly acquiring and releasing
- # locks, this does an initial check across all specs within a single
- # DB read transaction to reduce time spent in this case. In the next
- # three lines we remove any already-installed root specs from the list
- # to install. However, uninstalled_specs() only considers root specs,
- # so this will allow dep specs to be unnecessarily re-installed.
- uninstalled_roots = self.uninstalled_specs()
- specs_to_install = specs or uninstalled_roots
- specs_to_install = [s for s in specs_to_install
- if s not in self.roots() or s in uninstalled_roots]
-
- # ensure specs already installed are marked explicit
- all_specs = specs or [cs for _, cs in self.concretized_specs()]
- specs_installed = [s for s in all_specs if s.installed]
- if specs_installed:
+ # locks. As a small optimization, drop already installed root specs.
+ installed_roots, uninstalled_roots = self._partition_roots_by_install_status()
+ if specs:
+ specs_to_install = [s for s in specs if s not in installed_roots]
+ specs_dropped = [s for s in specs if s in installed_roots]
+ else:
+ specs_to_install = uninstalled_roots
+ specs_dropped = installed_roots
+
+ # We need to repeat the work of the installer thanks to the above optimization:
+ # Already installed root specs should be marked explicitly installed in the
+ # database.
+ if specs_dropped:
with spack.store.db.write_transaction(): # do all in one transaction
- for spec in specs_installed:
+ for spec in specs_dropped:
spack.store.db.update_explicit(spec, True)
if not specs_to_install:
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 7fcdecdbdb..0aa3ae24f7 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -145,17 +145,32 @@ def test_concretize():
assert any(x.name == 'mpileaks' for x in env_specs)
-def test_env_uninstalled_specs(install_mockery, mock_fetch):
+def test_env_specs_partition(install_mockery, mock_fetch):
e = ev.create('test')
e.add('cmake-client')
e.concretize()
- assert any(s.name == 'cmake-client' for s in e.uninstalled_specs())
+
+ # Single not installed root spec.
+ roots_already_installed, roots_to_install = e._partition_roots_by_install_status()
+ assert len(roots_already_installed) == 0
+ assert len(roots_to_install) == 1
+ assert roots_to_install[0].name == 'cmake-client'
+
+ # Single installed root.
e.install_all()
- assert not any(s.name == 'cmake-client' for s in e.uninstalled_specs())
+ roots_already_installed, roots_to_install = e._partition_roots_by_install_status()
+ assert len(roots_already_installed) == 1
+ assert roots_already_installed[0].name == 'cmake-client'
+ assert len(roots_to_install) == 0
+
+ # One installed root, one not installed root.
e.add('mpileaks')
e.concretize()
- assert not any(s.name == 'cmake-client' for s in e.uninstalled_specs())
- assert any(s.name == 'mpileaks' for s in e.uninstalled_specs())
+ roots_already_installed, roots_to_install = e._partition_roots_by_install_status()
+ assert len(roots_already_installed) == 1
+ assert len(roots_to_install) == 1
+ assert roots_already_installed[0].name == 'cmake-client'
+ assert roots_to_install[0].name == 'mpileaks'
def test_env_install_all(install_mockery, mock_fetch):