From 3f46f03c837da9959b4257c710e521fc9bf777bb Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Mon, 14 Oct 2019 17:50:38 -0700 Subject: bugfix: install --only dependencies works in env (#13090) * bugfix: install --only dependents works in env includes regression testing --- lib/spack/spack/cmd/install.py | 28 +++++-------- lib/spack/spack/package.py | 12 ++++++ lib/spack/spack/test/cmd/install.py | 48 ++++++++++++++++++++++ .../packages/dependent-install/package.py | 4 +- 4 files changed, 73 insertions(+), 19 deletions(-) diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index b625078a0b..876fb58f51 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -39,8 +39,15 @@ def update_kwargs_from_args(args, kwargs): 'fake': args.fake, 'dirty': args.dirty, 'use_cache': args.use_cache, - 'cache_only': args.cache_only + 'cache_only': args.cache_only, + 'explicit': True # Always true for install command }) + + kwargs.update({ + 'install_dependencies': ('dependencies' in args.things_to_install), + 'install_package': ('package' in args.things_to_install) + }) + if hasattr(args, 'setup'): setups = set() for arglist_s in args.setup: @@ -188,8 +195,8 @@ def default_log_file(spec): def install_spec(cli_args, kwargs, abstract_spec, spec): """Do the actual installation.""" - # handle active environment, if any - def install(spec, kwargs): + try: + # handle active environment, if any env = ev.get_env(cli_args, 'install') if env: env.install(abstract_spec, spec, **kwargs) @@ -197,17 +204,6 @@ def install_spec(cli_args, kwargs, abstract_spec, spec): else: spec.package.do_install(**kwargs) - try: - if cli_args.things_to_install == 'dependencies': - # Install dependencies as-if they were installed - # for root (explicit=False in the DB) - kwargs['explicit'] = False - for s in spec.dependencies(): - install(s, kwargs) - else: - kwargs['explicit'] = True - install(spec, kwargs) - except spack.build_environment.InstallError as e: if cli_args.show_log_on_error: e.print_context() @@ -242,10 +238,6 @@ def install(parser, args, **kwargs): # Parse cli arguments and construct a dictionary # that will be passed to Package.do_install API update_kwargs_from_args(args, kwargs) - kwargs.update({ - 'install_dependencies': ('dependencies' in args.things_to_install), - 'install_package': ('package' in args.things_to_install) - }) if args.run_tests: tty.warn("Deprecated option: --run-tests: use --test=all instead") diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 686bbc2e1e..b6a2d2de8d 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1519,6 +1519,15 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): dirty = kwargs.get('dirty', False) restage = kwargs.get('restage', False) + # install_self defaults True and is popped so that dependencies are + # always installed regardless of whether the root was installed + install_self = kwargs.pop('install_package', True) + # explicit defaults False so that dependents are implicit regardless + # of whether their dependents are implicitly or explicitly installed. + # Spack ensures root packages of install commands are always marked to + # install explicit + explicit = kwargs.pop('explicit', False) + # For external packages the workflow is simplified, and basically # consists in module file generation and registration in the DB if self.spec.external: @@ -1568,6 +1577,9 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): if install_deps: Package._install_bootstrap_compiler(self, **kwargs) + if not install_self: + return + # Then, install the package proper tty.msg(colorize('@*{Installing} @*g{%s}' % self.name)) diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index 3892a1da7b..3be548907a 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -21,10 +21,13 @@ import spack.cmd.install from spack.error import SpackError from spack.spec import Spec from spack.main import SpackCommand +import spack.environment as ev from six.moves.urllib.error import HTTPError, URLError install = SpackCommand('install') +env = SpackCommand('env') +add = SpackCommand('add') @pytest.fixture(scope='module') @@ -600,3 +603,48 @@ def test_cache_only_fails(tmpdir, mock_fetch, install_mockery, capfd): assert False except spack.main.SpackCommandError: pass + + +def test_install_only_dependencies(tmpdir, mock_fetch, install_mockery): + dep = Spec('dependency-install').concretized() + root = Spec('dependent-install').concretized() + + install('--only', 'dependencies', 'dependent-install') + + assert os.path.exists(dep.prefix) + assert not os.path.exists(root.prefix) + + +@pytest.mark.regression('12002') +def test_install_only_dependencies_in_env(tmpdir, mock_fetch, install_mockery, + mutable_mock_env_path): + env('create', 'test') + + with ev.read('test'): + dep = Spec('dependency-install').concretized() + root = Spec('dependent-install').concretized() + + install('-v', '--only', 'dependencies', 'dependent-install') + + assert os.path.exists(dep.prefix) + assert not os.path.exists(root.prefix) + + +@pytest.mark.regression('12002') +def test_install_only_dependencies_of_all_in_env( + tmpdir, mock_fetch, install_mockery, mutable_mock_env_path +): + env('create', '--without-view', 'test') + + with ev.read('test'): + roots = [Spec('dependent-install@1.0').concretized(), + Spec('dependent-install@2.0').concretized()] + + add('dependent-install@1.0') + add('dependent-install@2.0') + install('--only', 'dependencies') + + for root in roots: + assert not os.path.exists(root.prefix) + for dep in root.traverse(root=False): + assert os.path.exists(dep.prefix) diff --git a/var/spack/repos/builtin.mock/packages/dependent-install/package.py b/var/spack/repos/builtin.mock/packages/dependent-install/package.py index 1a0f13ffea..54c60ce99e 100644 --- a/var/spack/repos/builtin.mock/packages/dependent-install/package.py +++ b/var/spack/repos/builtin.mock/packages/dependent-install/package.py @@ -13,8 +13,10 @@ class DependentInstall(Package): url = "http://www.example.com/a-1.0.tar.gz" version('1.0', '0123456789abcdef0123456789abcdef') + version('2.0', '0123456789abcdef0123456789abcdef') - depends_on('dependency-install') + depends_on('dependency-install@2.0', when='@2.0') + depends_on('dependency-install@1.0', when='@1.0') def install(self, spec, prefix): touch(join_path(prefix, 'an_installation_file')) -- cgit v1.2.3-70-g09d2