bugfix: install --only dependencies works in env (#13090)
* bugfix: install --only dependents works in env includes regression testing
This commit is contained in:
		| @@ -39,8 +39,15 @@ def update_kwargs_from_args(args, kwargs): | |||||||
|         'fake': args.fake, |         'fake': args.fake, | ||||||
|         'dirty': args.dirty, |         'dirty': args.dirty, | ||||||
|         'use_cache': args.use_cache, |         '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'): |     if hasattr(args, 'setup'): | ||||||
|         setups = set() |         setups = set() | ||||||
|         for arglist_s in args.setup: |         for arglist_s in args.setup: | ||||||
| @@ -188,8 +195,8 @@ def default_log_file(spec): | |||||||
| def install_spec(cli_args, kwargs, abstract_spec, spec): | def install_spec(cli_args, kwargs, abstract_spec, spec): | ||||||
|     """Do the actual installation.""" |     """Do the actual installation.""" | ||||||
|  |  | ||||||
|     # handle active environment, if any |     try: | ||||||
|     def install(spec, kwargs): |         # handle active environment, if any | ||||||
|         env = ev.get_env(cli_args, 'install') |         env = ev.get_env(cli_args, 'install') | ||||||
|         if env: |         if env: | ||||||
|             env.install(abstract_spec, spec, **kwargs) |             env.install(abstract_spec, spec, **kwargs) | ||||||
| @@ -197,17 +204,6 @@ def install(spec, kwargs): | |||||||
|         else: |         else: | ||||||
|             spec.package.do_install(**kwargs) |             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: |     except spack.build_environment.InstallError as e: | ||||||
|         if cli_args.show_log_on_error: |         if cli_args.show_log_on_error: | ||||||
|             e.print_context() |             e.print_context() | ||||||
| @@ -242,10 +238,6 @@ def install(parser, args, **kwargs): | |||||||
|     # Parse cli arguments and construct a dictionary |     # Parse cli arguments and construct a dictionary | ||||||
|     # that will be passed to Package.do_install API |     # that will be passed to Package.do_install API | ||||||
|     update_kwargs_from_args(args, kwargs) |     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: |     if args.run_tests: | ||||||
|         tty.warn("Deprecated option: --run-tests: use --test=all instead") |         tty.warn("Deprecated option: --run-tests: use --test=all instead") | ||||||
|   | |||||||
| @@ -1519,6 +1519,15 @@ def do_install(self, **kwargs): | |||||||
|         dirty = kwargs.get('dirty', False) |         dirty = kwargs.get('dirty', False) | ||||||
|         restage = kwargs.get('restage', 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 |         # For external packages the workflow is simplified, and basically | ||||||
|         # consists in module file generation and registration in the DB |         # consists in module file generation and registration in the DB | ||||||
|         if self.spec.external: |         if self.spec.external: | ||||||
| @@ -1568,6 +1577,9 @@ def do_install(self, **kwargs): | |||||||
|         if install_deps: |         if install_deps: | ||||||
|             Package._install_bootstrap_compiler(self, **kwargs) |             Package._install_bootstrap_compiler(self, **kwargs) | ||||||
|  |  | ||||||
|  |         if not install_self: | ||||||
|  |             return | ||||||
|  |  | ||||||
|         # Then, install the package proper |         # Then, install the package proper | ||||||
|         tty.msg(colorize('@*{Installing} @*g{%s}' % self.name)) |         tty.msg(colorize('@*{Installing} @*g{%s}' % self.name)) | ||||||
|  |  | ||||||
|   | |||||||
| @@ -21,10 +21,13 @@ | |||||||
| from spack.error import SpackError | from spack.error import SpackError | ||||||
| from spack.spec import Spec | from spack.spec import Spec | ||||||
| from spack.main import SpackCommand | from spack.main import SpackCommand | ||||||
|  | import spack.environment as ev | ||||||
|  |  | ||||||
| from six.moves.urllib.error import HTTPError, URLError | from six.moves.urllib.error import HTTPError, URLError | ||||||
|  |  | ||||||
| install = SpackCommand('install') | install = SpackCommand('install') | ||||||
|  | env = SpackCommand('env') | ||||||
|  | add = SpackCommand('add') | ||||||
|  |  | ||||||
|  |  | ||||||
| @pytest.fixture(scope='module') | @pytest.fixture(scope='module') | ||||||
| @@ -600,3 +603,48 @@ def test_cache_only_fails(tmpdir, mock_fetch, install_mockery, capfd): | |||||||
|             assert False |             assert False | ||||||
|         except spack.main.SpackCommandError: |         except spack.main.SpackCommandError: | ||||||
|             pass |             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) | ||||||
|   | |||||||
| @@ -13,8 +13,10 @@ class DependentInstall(Package): | |||||||
|     url      = "http://www.example.com/a-1.0.tar.gz" |     url      = "http://www.example.com/a-1.0.tar.gz" | ||||||
|  |  | ||||||
|     version('1.0', '0123456789abcdef0123456789abcdef') |     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): |     def install(self, spec, prefix): | ||||||
|         touch(join_path(prefix, 'an_installation_file')) |         touch(join_path(prefix, 'an_installation_file')) | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Greg Becker
					Greg Becker