From beeca6bb541787dab06a91272c0e18136076463e Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Thu, 20 Apr 2017 03:53:41 -0700 Subject: Revert "Override partial installs by default" (#3918) * Revert "Override partial installs by default (#3530)" This reverts commit a65c37f15dff4b4d60784fd4fcc55874ce9d6d11. --- lib/spack/spack/cmd/install.py | 9 -- lib/spack/spack/directory_layout.py | 27 +---- lib/spack/spack/package.py | 51 +-------- lib/spack/spack/test/install.py | 123 --------------------- share/spack/spack-completion.bash | 2 +- .../repos/builtin.mock/packages/canfail/package.py | 41 ------- 6 files changed, 8 insertions(+), 245 deletions(-) delete mode 100644 var/spack/repos/builtin.mock/packages/canfail/package.py diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 08dc080e8e..fb01fc2d5e 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -72,9 +72,6 @@ the dependencies""" subparser.add_argument( '--fake', action='store_true', dest='fake', help="fake install. just remove prefix and create a fake file") - subparser.add_argument( - '--force', action='store_true', dest='force', - help='Install again even if package is already installed.') cd_group = subparser.add_mutually_exclusive_group() arguments.add_common_arguments(cd_group, ['clean', 'dirty']) @@ -322,12 +319,6 @@ def install(parser, args, **kwargs): tty.error('The `spack install` command requires a spec to install.') for spec in specs: - if args.force: - for s in spec.traverse(): - if s.package.installed: - tty.msg("Clearing %s for new installation" % s.name) - s.package.do_uninstall(force=True) - # Check if we were asked to produce some log for dashboards if args.log_format is not None: # Compute the filename for logging diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py index e220a2d430..9d09875484 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -239,17 +239,6 @@ class YamlDirectoryLayout(DirectoryLayout): return join_path(self.path_for_spec(spec), self.metadata_dir, self.packages_dir) - def _completion_marker_file(self, spec): - return join_path(self.path_for_spec(spec), self.metadata_dir, - 'complete') - - def mark_complete(self, spec): - with open(self._completion_marker_file(spec), 'w'): - pass - - def completed_install(self, spec): - return os.path.exists(self._completion_marker_file(spec)) - def create_install_directory(self, spec): _check_concrete(spec) @@ -263,34 +252,26 @@ class YamlDirectoryLayout(DirectoryLayout): def check_installed(self, spec): _check_concrete(spec) path = self.path_for_spec(spec) + spec_file_path = self.spec_file_path(spec) if not os.path.isdir(path): return None - elif not self.completed_install(spec): - raise InconsistentInstallDirectoryError( - 'The prefix %s contains a partial install' % path) - - self.check_metadata_consistency(spec) - return path - - def check_metadata_consistency(self, spec): - spec_file_path = self.spec_file_path(spec) if not os.path.isfile(spec_file_path): raise InconsistentInstallDirectoryError( 'Install prefix exists but contains no spec.yaml:', - " " + spec.prefix) + " " + path) installed_spec = self.read_spec(spec_file_path) if installed_spec == spec: - return + return path # DAG hashes currently do not include build dependencies. # # TODO: remove this when we do better concretization and don't # ignore build-only deps in hashes. elif installed_spec == spec.copy(deps=('link', 'run')): - return + return path if spec.dag_hash() == installed_spec.dag_hash(): raise SpecHashCollisionError(spec, installed_spec) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index e6eea35a80..108ddeff07 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -856,7 +856,7 @@ class PackageBase(with_metaclass(PackageMeta, object)): @property def installed(self): - return spack.store.layout.completed_install(self.spec) + return os.path.isdir(self.prefix) @property def prefix_lock(self): @@ -1174,16 +1174,10 @@ class PackageBase(with_metaclass(PackageMeta, object)): (self.name, self.spec.external)) return - self.repair_partial(keep_prefix) - # Ensure package is not already installed layout = spack.store.layout with self._prefix_read_lock(): - if (keep_prefix and os.path.isdir(self.prefix) and - (not self.installed)): - tty.msg( - "Continuing from partial install of %s" % self.name) - elif layout.check_installed(self.spec): + if layout.check_installed(self.spec): tty.msg( "%s is already installed in %s" % (self.name, self.prefix)) rec = spack.store.db.get_record(self.spec) @@ -1311,11 +1305,9 @@ class PackageBase(with_metaclass(PackageMeta, object)): try: # Create the install prefix and fork the build process. - if (not keep_prefix) or (not os.path.isdir(self.prefix)): - spack.store.layout.create_install_directory(self.spec) + spack.store.layout.create_install_directory(self.spec) # Fork a child to do the actual installation spack.build_environment.fork(self, build_process, dirty=dirty) - spack.store.layout.mark_complete(self.spec) # If we installed then we should keep the prefix keep_prefix = True if self.last_phase is None else keep_prefix # note: PARENT of the build process adds the new package to @@ -1340,43 +1332,6 @@ class PackageBase(with_metaclass(PackageMeta, object)): if not keep_prefix: self.remove_prefix() - def repair_partial(self, continue_with_partial=False): - """If continue_with_partial is not set, this ensures that the package - is either fully-installed or that the prefix is removed. If the - package is installed but there is no DB entry then this adds a - record. If continue_with_partial is not set this also clears the - stage directory to start an installation from scratch. - """ - layout = spack.store.layout - with self._prefix_read_lock(): - if (os.path.isdir(self.prefix) and not self.installed and - not continue_with_partial): - spack.hooks.pre_uninstall(self) - self.remove_prefix() - try: - spack.store.db.remove(self.spec) - except KeyError: - pass - spack.hooks.post_uninstall(self) - tty.msg("Removed partial install for %s" % - self.spec.short_spec) - elif self.installed and layout.check_installed(self.spec): - try: - spack.store.db.get_record(self.spec) - except KeyError: - tty.msg("Repairing db for %s" % self.name) - spack.store.db.add(self.spec) - - if continue_with_partial and not self.installed: - try: - layout.check_metadata_consistency(self.spec) - except directory_layout.DirectoryLayoutError: - self.remove_prefix() - - if not continue_with_partial: - self.stage.destroy() - self.stage.create() - def _do_install_pop_kwargs(self, kwargs): """Pops kwargs from do_install before starting the installation diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index 08694f7ee8..f10c3a37e9 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -30,8 +30,6 @@ from spack.directory_layout import YamlDirectoryLayout from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite from spack.spec import Spec -import os - @pytest.fixture() def install_mockery(tmpdir, config, builtin_mock): @@ -79,123 +77,6 @@ def test_install_and_uninstall(mock_archive): raise -def mock_remove_prefix(*args): - raise MockInstallError( - "Intentional error", - "Mock remove_prefix method intentionally fails") - - -@pytest.mark.usefixtures('install_mockery') -def test_partial_install(mock_archive): - spec = Spec('canfail') - spec.concretize() - pkg = spack.repo.get(spec) - fake_fetchify(mock_archive.url, pkg) - remove_prefix = spack.package.Package.remove_prefix - try: - spack.package.Package.remove_prefix = mock_remove_prefix - try: - pkg.do_install() - except MockInstallError: - pass - spack.package.Package.remove_prefix = remove_prefix - setattr(pkg, 'succeed', True) - pkg.do_install() - assert pkg.installed - finally: - spack.package.Package.remove_prefix = remove_prefix - try: - delattr(pkg, 'succeed') - except AttributeError: - pass - - -@pytest.mark.usefixtures('install_mockery') -def test_partial_install_keep_prefix(mock_archive): - spec = Spec('canfail') - spec.concretize() - pkg = spack.repo.get(spec) - fake_fetchify(mock_archive.url, pkg) - remove_prefix = spack.package.Package.remove_prefix - try: - spack.package.Package.remove_prefix = mock_remove_prefix - try: - pkg.do_install() - except MockInstallError: - pass - # Don't repair remove_prefix at this point, set keep_prefix so that - # Spack continues with a partial install - setattr(pkg, 'succeed', True) - pkg.do_install(keep_prefix=True) - assert pkg.installed - finally: - spack.package.Package.remove_prefix = remove_prefix - try: - delattr(pkg, 'succeed') - except AttributeError: - pass - - -@pytest.mark.usefixtures('install_mockery') -def test_partial_install_keep_prefix_check_metadata(mock_archive): - spec = Spec('canfail') - spec.concretize() - pkg = spack.repo.get(spec) - fake_fetchify(mock_archive.url, pkg) - remove_prefix = spack.package.Package.remove_prefix - try: - spack.package.Package.remove_prefix = mock_remove_prefix - try: - pkg.do_install() - except MockInstallError: - pass - os.remove(spack.store.layout.spec_file_path(spec)) - spack.package.Package.remove_prefix = remove_prefix - setattr(pkg, 'succeed', True) - pkg.do_install(keep_prefix=True) - assert pkg.installed - spack.store.layout.check_metadata_consistency(spec) - finally: - spack.package.Package.remove_prefix = remove_prefix - try: - delattr(pkg, 'succeed') - except AttributeError: - pass - - -@pytest.mark.usefixtures('install_mockery') -def test_install_succeeds_but_db_add_fails(mock_archive): - """If an installation succeeds but the database is not updated, make sure - that the database is updated for a future install.""" - spec = Spec('cmake') - spec.concretize() - pkg = spack.repo.get(spec) - fake_fetchify(mock_archive.url, pkg) - remove_prefix = spack.package.Package.remove_prefix - db_add = spack.store.db.add - - def mock_db_add(*args, **kwargs): - raise MockInstallError( - "Intentional error", "Mock db add method intentionally fails") - - try: - spack.package.Package.remove_prefix = mock_remove_prefix - spack.store.db.add = mock_db_add - try: - pkg.do_install() - except MockInstallError: - pass - assert pkg.installed - - spack.package.Package.remove_prefix = remove_prefix - spack.store.db.add = db_add - pkg.do_install() - assert spack.store.db.get_record(spec) - except: - spack.package.Package.remove_prefix = remove_prefix - raise - - @pytest.mark.usefixtures('install_mockery') def test_store(mock_archive): spec = Spec('cmake-client').concretized() @@ -221,7 +102,3 @@ def test_failing_build(mock_archive): pkg = spec.package with pytest.raises(spack.build_environment.ChildError): pkg.do_install() - - -class MockInstallError(spack.error.SpackError): - pass diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 1167690fa2..819dcc06ab 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -407,7 +407,7 @@ function _spack_install { then compgen -W "-h --help --only -j --jobs --keep-prefix --keep-stage -n --no-checksum -v --verbose --fake --clean --dirty - --run-tests --log-format --log-file --force" -- "$cur" + --run-tests --log-format --log-file" -- "$cur" else compgen -W "$(_all_packages)" -- "$cur" fi diff --git a/var/spack/repos/builtin.mock/packages/canfail/package.py b/var/spack/repos/builtin.mock/packages/canfail/package.py deleted file mode 100644 index bef4f6e838..0000000000 --- a/var/spack/repos/builtin.mock/packages/canfail/package.py +++ /dev/null @@ -1,41 +0,0 @@ -############################################################################## -# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC. -# Produced at the Lawrence Livermore National Laboratory. -# -# This file is part of Spack. -# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. -# LLNL-CODE-647188 -# -# For details, see https://github.com/llnl/spack -# Please also see the LICENSE file for our notice and the LGPL. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU Lesser General Public License (as -# published by the Free Software Foundation) version 2.1, February 1999. -# -# This program is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and -# conditions of the GNU Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this program; if not, write to the Free Software -# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -############################################################################## -from spack import * - - -class Canfail(Package): - """Package which fails install unless a special attribute is set""" - - homepage = "http://www.example.com" - url = "http://www.example.com/a-1.0.tar.gz" - - version('1.0', '0123456789abcdef0123456789abcdef') - - def install(self, spec, prefix): - try: - getattr(self, 'succeed') - touch(join_path(prefix, 'an_installation_file')) - except AttributeError: - raise InstallError() -- cgit v1.2.3-60-g2f50