diff options
| -rw-r--r-- | lib/spack/spack/environment/environment.py | 51 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/env.py | 25 |
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): |