Fix spack package inheritance for module variables (#10097)
* we weren't properly setting module variables for the root package in a DAG -- just for transitive dependencies.
This commit is contained in:
		 Greg Becker
					Greg Becker
				
			
				
					committed by
					
						 Todd Gamblin
						Todd Gamblin
					
				
			
			
				
	
			
			
			 Todd Gamblin
						Todd Gamblin
					
				
			
						parent
						
							05f9c68c30
						
					
				
				
					commit
					d2d0ab06b7
				
			| @@ -370,10 +370,8 @@ def set_build_environment_variables(pkg, env, dirty): | ||||
|     return env | ||||
|  | ||||
|  | ||||
| def set_module_variables_for_package(pkg, module): | ||||
|     """Populate the module scope of install() with some useful functions. | ||||
|        This makes things easier for package writers. | ||||
|     """ | ||||
| def _set_variables_for_single_module(pkg, module): | ||||
|     """Helper function to set module variables for single module.""" | ||||
|     # number of jobs spack will build with. | ||||
|     jobs = spack.config.get('config:build_jobs') or multiprocessing.cpu_count() | ||||
|     if not pkg.parallel: | ||||
| @@ -444,6 +442,20 @@ def static_to_shared_library(static_lib, shared_lib=None, **kwargs): | ||||
|     m.static_to_shared_library = static_to_shared_library | ||||
|  | ||||
|  | ||||
| def set_module_variables_for_package(pkg): | ||||
|     """Populate the module scope of install() with some useful functions. | ||||
|        This makes things easier for package writers. | ||||
|     """ | ||||
|     # If a user makes their own package repo, e.g. | ||||
|     # spack.pkg.mystuff.libelf.Libelf, and they inherit from an existing class | ||||
|     # like spack.pkg.original.libelf.Libelf, then set the module variables | ||||
|     # for both classes so the parent class can still use them if it gets | ||||
|     # called. parent_class_modules includes pkg.module. | ||||
|     modules = parent_class_modules(pkg.__class__) | ||||
|     for mod in modules: | ||||
|         _set_variables_for_single_module(pkg, mod) | ||||
|  | ||||
|  | ||||
| def _static_to_shared_library(arch, compiler, static_lib, shared_lib=None, | ||||
|                               **kwargs): | ||||
|     """ | ||||
| @@ -591,6 +603,8 @@ def get_std_meson_args(pkg): | ||||
| def parent_class_modules(cls): | ||||
|     """ | ||||
|     Get list of superclass modules that descend from spack.package.PackageBase | ||||
|  | ||||
|     Includes cls.__module__ | ||||
|     """ | ||||
|     if (not issubclass(cls, spack.package.PackageBase) or | ||||
|         issubclass(spack.package.PackageBase, cls)): | ||||
| @@ -634,23 +648,15 @@ def setup_package(pkg, dirty): | ||||
|     spec = pkg.spec | ||||
|     for dspec in pkg.spec.traverse(order='post', root=False, | ||||
|                                    deptype=('build', 'test')): | ||||
|         # If a user makes their own package repo, e.g. | ||||
|         # spack.pkg.mystuff.libelf.Libelf, and they inherit from | ||||
|         # an existing class like spack.pkg.original.libelf.Libelf, | ||||
|         # then set the module variables for both classes so the | ||||
|         # parent class can still use them if it gets called. | ||||
|         spkg = dspec.package | ||||
|         modules = parent_class_modules(spkg.__class__) | ||||
|         for mod in modules: | ||||
|             set_module_variables_for_package(spkg, mod) | ||||
|         set_module_variables_for_package(spkg, spkg.module) | ||||
|         set_module_variables_for_package(spkg) | ||||
|  | ||||
|         # Allow dependencies to modify the module | ||||
|         dpkg = dspec.package | ||||
|         dpkg.setup_dependent_package(pkg.module, spec) | ||||
|         dpkg.setup_dependent_environment(spack_env, run_env, spec) | ||||
|  | ||||
|     set_module_variables_for_package(pkg, pkg.module) | ||||
|     set_module_variables_for_package(pkg) | ||||
|     pkg.setup_environment(spack_env, run_env) | ||||
|  | ||||
|     # Loading modules, in particular if they are meant to be used outside | ||||
|   | ||||
| @@ -515,22 +515,13 @@ def environment_modifications(self): | ||||
|         # before asking for package-specific modifications | ||||
|         for item in dependencies(self.spec, 'all'): | ||||
|             package = self.spec[item.name].package | ||||
|             modules = build_environment.parent_class_modules(package.__class__) | ||||
|             for mod in modules: | ||||
|                 build_environment.set_module_variables_for_package( | ||||
|                     package, mod | ||||
|                 ) | ||||
|             build_environment.set_module_variables_for_package( | ||||
|                 package, package.module | ||||
|             ) | ||||
|             build_environment.set_module_variables_for_package(package) | ||||
|             package.setup_dependent_package( | ||||
|                 self.spec.package.module, self.spec | ||||
|             ) | ||||
|             package.setup_dependent_environment(_, env, self.spec) | ||||
|         # Package specific modifications | ||||
|         build_environment.set_module_variables_for_package( | ||||
|             self.spec.package, self.spec.package.module | ||||
|         ) | ||||
|         build_environment.set_module_variables_for_package(self.spec.package) | ||||
|         self.spec.package.setup_environment(_, env) | ||||
|  | ||||
|         # Modifications required from modules.yaml | ||||
|   | ||||
| @@ -215,3 +215,18 @@ def _set_wrong_cc(x): | ||||
|     paths = os.environ['PATH'].split(':') | ||||
|  | ||||
|     assert paths.index(spack_path) < paths.index(module_path) | ||||
|  | ||||
|  | ||||
| def test_package_inheritance_module_setup(config, mock_packages): | ||||
|     s = spack.spec.Spec('multimodule-inheritance') | ||||
|     s.concretize() | ||||
|     pkg = s.package | ||||
|  | ||||
|     spack.build_environment.setup_package(pkg, False) | ||||
|  | ||||
|     os.environ['TEST_MODULE_VAR'] = 'failed' | ||||
|  | ||||
|     assert pkg.use_module_variable() == 'test_module_variable' | ||||
|     assert os.environ['TEST_MODULE_VAR'] == 'test_module_variable' | ||||
|  | ||||
|     os.environ.pop('TEST_MODULE_VAR') | ||||
|   | ||||
| @@ -0,0 +1,20 @@ | ||||
| # Copyright 2013-2018 Lawrence Livermore National Security, LLC and other | ||||
| # Spack Project Developers. See the top-level COPYRIGHT file for details. | ||||
| # | ||||
| # SPDX-License-Identifier: (Apache-2.0 OR MIT) | ||||
|  | ||||
| import spack.pkg.builtin.mock.simple_inheritance as si | ||||
|  | ||||
|  | ||||
| class MultimoduleInheritance(si.BaseWithDirectives): | ||||
|     """Simple package which inherits a method and several directives""" | ||||
|  | ||||
|     homepage = "http://www.example.com" | ||||
|     url = "http://www.example.com/multimodule-1.0.tar.gz" | ||||
|  | ||||
|     version('1.0', '0123456789abcdef0123456789abcdef') | ||||
|  | ||||
|     depends_on('openblas', when='+openblas') | ||||
|  | ||||
|     def install(self, spec, prefix): | ||||
|         pass | ||||
| @@ -13,6 +13,12 @@ class BaseWithDirectives(Package): | ||||
|     variant('openblas', description='Activates openblas', default=True) | ||||
|     provides('service1') | ||||
|  | ||||
|     def use_module_variable(self): | ||||
|         """Must be called in build environment. Allows us to test parent class | ||||
|          using module variables set up by build_environment.""" | ||||
|         env['TEST_MODULE_VAR'] = 'test_module_variable' | ||||
|         return env['TEST_MODULE_VAR'] | ||||
|  | ||||
|  | ||||
| class SimpleInheritance(BaseWithDirectives): | ||||
|     """Simple package which acts as a build dependency""" | ||||
|   | ||||
		Reference in New Issue
	
	Block a user