diff options
author | Greg Becker <becker33@llnl.gov> | 2021-05-14 15:03:28 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-14 15:03:28 -0700 |
commit | cefbe48c89209dc3df654795644973b1885cdea4 (patch) | |
tree | b65b3625a56faaaaeb4005b24ea49dd711a1c431 /lib | |
parent | fc392d2f56beae448dd5d8888dbb2b8cde6d9e64 (diff) | |
download | spack-cefbe48c89209dc3df654795644973b1885cdea4.tar.gz spack-cefbe48c89209dc3df654795644973b1885cdea4.tar.bz2 spack-cefbe48c89209dc3df654795644973b1885cdea4.tar.xz spack-cefbe48c89209dc3df654795644973b1885cdea4.zip |
Separable module configurations (#22588)
Currently, module configurations are inconsistent because modulefiles are generated with the configs for the active environment, but are shared among all environments (and spack outside any environment).
This PR fixes that by allowing Spack environments (or other spack config scopes) to define additional sets of modules to generate. Each set of modules can enable either lmod or tcl modules, and contains all of the previously available module configuration. The user defines the name of each module set -- the set configured in Spack by default is named "default", and is the one returned by module manipulation commands in the absence of user intervention.
As part of this change, the module roots configuration moved from the `config` section to inside each module configuration.
Additionally, it adds a feature that the modulefiles for an environment can be configured to be relative to an environment view rather than the underlying prefix. This will not be enabled by default, as it should only be enabled within an environment and for non-default views constructed with separate projections per-spec.
TODO:
- [x] code changes to support multiple module sets
- [x] code changes to support modules relative to a view
- [x] Tests for multiple module configurations
- [x] Tests for modules relative to a view
- [x] Backwards compatibility for module roots from config section
- [x] Backwards compatibility for default module set without the name specified
- [x] Tests for backwards compatibility
Diffstat (limited to 'lib')
29 files changed, 491 insertions, 183 deletions
diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index 11b422a9bd..c980366352 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -364,6 +364,9 @@ def env_loads_setup_parser(subparser): subparser.add_argument( 'env', nargs='?', help='name of env to generate loads file for') subparser.add_argument( + '-n', '--module-set-name', default='default', + help='module set for which to generate load operations') + subparser.add_argument( '-m', '--module-type', choices=('tcl', 'lmod'), help='type of module system to generate loads for') spack.cmd.modules.add_loads_arguments(subparser) diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 53e924cb9e..eb126460d2 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -261,7 +261,7 @@ def install_specs(cli_args, kwargs, specs): with env.write_transaction(): specs_to_install.append( env.concretize_and_add(abstract, concrete)) - env.write(regenerate_views=False) + env.write(regenerate=False) # Install the validated list of cli specs if specs_to_install: @@ -338,7 +338,7 @@ environment variables: # save view regeneration for later, so that we only do it # once, as it can be slow. - env.write(regenerate_views=False) + env.write(regenerate=False) specs = env.all_specs() if not args.log_file and not reporter.filename: @@ -352,9 +352,9 @@ environment variables: tty.debug("Regenerating environment views for {0}" .format(env.name)) with env.write_transaction(): - # It is not strictly required to synchronize view regeneration - # but doing so can prevent redundant work in the filesystem. - env.regenerate_views() + # write env to trigger view generation and modulefile + # generation + env.write() return else: msg = "install requires a package argument or active environment" diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py index 7fbce3bb9d..3d6975801f 100644 --- a/lib/spack/spack/cmd/modules/__init__.py +++ b/lib/spack/spack/cmd/modules/__init__.py @@ -13,6 +13,7 @@ import sys from llnl.util import filesystem, tty import spack.cmd +import spack.config import spack.modules import spack.repo import spack.modules.common @@ -25,6 +26,11 @@ level = "short" def setup_parser(subparser): + subparser.add_argument( + '-n', '--name', + action='store', dest='module_set_name', default='default', + help="Named module set to use from modules configuration." + ) sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='subparser_name') refresh_parser = sp.add_parser('refresh', help='regenerate module files') @@ -111,6 +117,19 @@ def one_spec_or_raise(specs): return specs[0] +def check_module_set_name(name): + modules_config = spack.config.get('modules') + valid_names = set([key for key, value in modules_config.items() + if isinstance(value, dict) and value.get('enable', [])]) + if 'enable' in modules_config and modules_config['enable']: + valid_names.add('default') + + if name not in valid_names: + msg = "Cannot use invalid module set %s." % name + msg += " Valid module set names are %s" % list(valid_names) + raise spack.config.ConfigError(msg) + + _missing_modules_warning = ( "Modules have been omitted for one or more specs, either" " because they were blacklisted or because the spec is" @@ -121,6 +140,7 @@ _missing_modules_warning = ( def loads(module_type, specs, args, out=None): """Prompt the list of modules associated with a list of specs""" + check_module_set_name(args.module_set_name) out = sys.stdout if out is None else out # Get a comprehensive list of specs @@ -142,7 +162,8 @@ def loads(module_type, specs, args, out=None): modules = list( (spec, spack.modules.common.get_module( - module_type, spec, get_full_path=False, required=False)) + module_type, spec, get_full_path=False, + module_set_name=args.module_set_name, required=False)) for spec in specs) module_commands = { @@ -177,6 +198,7 @@ def loads(module_type, specs, args, out=None): def find(module_type, specs, args): """Retrieve paths or use names of module files""" + check_module_set_name(args.module_set_name) single_spec = one_spec_or_raise(specs) @@ -190,12 +212,14 @@ def find(module_type, specs, args): try: modules = [ spack.modules.common.get_module( - module_type, spec, args.full_path, required=False) + module_type, spec, args.full_path, + module_set_name=args.module_set_name, required=False) for spec in dependency_specs_to_retrieve] modules.append( spack.modules.common.get_module( - module_type, single_spec, args.full_path, required=True)) + module_type, single_spec, args.full_path, + module_set_name=args.module_set_name, required=True)) except spack.modules.common.ModuleNotFoundError as e: tty.die(e.message) @@ -209,13 +233,16 @@ def rm(module_type, specs, args): """Deletes the module files associated with every spec in specs, for every module type in module types. """ + check_module_set_name(args.module_set_name) module_cls = spack.modules.module_types[module_type] - module_exist = lambda x: os.path.exists(module_cls(x).layout.filename) + module_exist = lambda x: os.path.exists( + module_cls(x, args.module_set_name).layout.filename) specs_with_modules = [spec for spec in specs if module_exist(spec)] - modules = [module_cls(spec) for spec in specs_with_modules] + modules = [module_cls(spec, args.module_set_name) + for spec in specs_with_modules] if not modules: tty.die('No module file matches your query') @@ -239,6 +266,7 @@ def refresh(module_type, specs, args): """Regenerates the module files for every spec in specs and every module type in module types. """ + check_module_set_name(args.module_set_name) # Prompt a message to the user about what is going to change if not specs: @@ -263,7 +291,7 @@ def refresh(module_type, specs, args): # Skip unknown packages. writers = [ - cls(spec) for spec in specs + cls(spec, args.module_set_name) for spec in specs if spack.repo.path.exists(spec.name)] # Filter blacklisted packages early diff --git a/lib/spack/spack/cmd/modules/lmod.py b/lib/spack/spack/cmd/modules/lmod.py index 61f2fc28d8..3546e2b87a 100644 --- a/lib/spack/spack/cmd/modules/lmod.py +++ b/lib/spack/spack/cmd/modules/lmod.py @@ -40,7 +40,8 @@ def setdefault(module_type, specs, args): # https://lmod.readthedocs.io/en/latest/060_locating.html#marking-a-version-as-default # spack.cmd.modules.one_spec_or_raise(specs) - writer = spack.modules.module_types['lmod'](specs[0]) + writer = spack.modules.module_types['lmod']( + specs[0], args.module_set_name) module_folder = os.path.dirname(writer.layout.filename) module_basename = os.path.basename(writer.layout.filename) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 574902f508..aa94f89f94 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -571,16 +571,17 @@ class Configuration(object): YAML config file that looks like this:: config: - install_tree: $spack/opt/spack - module_roots: - lmod: $spack/share/spack/lmod + install_tree: + root: $spack/opt/spack + build_stage: + - $tmpdir/$user/spack-stage ``get_config('config')`` will return:: - { 'install_tree': '$spack/opt/spack', - 'module_roots: { - 'lmod': '$spack/share/spack/lmod' + { 'install_tree': { + 'root': '$spack/opt/spack', } + 'build_stage': ['$tmpdir/$user/spack-stage'] } """ diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index fbdcb67761..a37cad8633 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -19,6 +19,7 @@ from llnl.util.tty.color import colorize import spack.concretize import spack.error import spack.hash_types as ht +import spack.hooks import spack.repo import spack.schema.env import spack.spec @@ -458,12 +459,15 @@ class ViewDescriptor(object): self.root = spack.util.path.canonicalize_path(root) self.projections = projections self.select = select - self.select_fn = lambda x: any(x.satisfies(s) for s in self.select) self.exclude = exclude - self.exclude_fn = lambda x: not any(x.satisfies(e) - for e in self.exclude) self.link = link + def select_fn(self, spec): + return any(spec.satisfies(s) for s in self.select) + + def exclude_fn(self, spec): + return not any(spec.satisfies(e) for e in self.exclude) + def __eq__(self, other): return all([self.root == other.root, self.projections == other.projections, @@ -741,7 +745,7 @@ class Environment(object): if not os.path.exists(self.manifest_path): return - self.clear() + self.clear(re_read=True) self._read() def _read(self): @@ -839,15 +843,26 @@ class Environment(object): ) } - def clear(self): + def clear(self, re_read=False): + """Clear the contents of the environment + + Arguments: + re_read (boolean): If True, do not clear ``new_specs`` nor + ``new_installs`` values. These values cannot be read from + yaml, and need to be maintained when re-reading an existing + environment. + """ self.spec_lists = {user_speclist_name: SpecList()} # specs from yaml self.dev_specs = {} # dev-build specs from yaml self.concretized_user_specs = [] # user specs from last concretize self.concretized_order = [] # roots of last concretize, in order self.specs_by_hash = {} # concretized specs by hash - self.new_specs = [] # write packages for these on write() self._repo = None # RepoPath for this env (memoized) self._previous_active = None # previously active environment + if not re_read: + # things that cannot be recreated from file + self.new_specs = [] # write packages for these on write() + self.new_installs = [] # write modules for these on write() @property def internal(self): @@ -1584,6 +1599,7 @@ class Environment(object): # Ensure links are set appropriately for spec in specs_to_install: if spec.package.installed: + self.new_installs.append(spec) try: self._install_log_links(spec) except OSError as e: @@ -1812,17 +1828,16 @@ class Environment(object): self.concretized_order = [ old_hash_to_new.get(h, h) for h in self.concretized_order] - def write(self, regenerate_views=True): + def write(self, regenerate=True): """Writes an in-memory environment to its location on disk. Write out package files for each newly concretized spec. Also - regenerate any views associated with the environment, if - regenerate_views is True. + regenerate any views associated with the environment and run post-write + hooks, if regenerate is True. Arguments: - regenerate_views (bool): regenerate views as well as - writing if True. - + regenerate (bool): regenerate views and run post-write hooks as + well as writing if True. """ # Intercept environment not using the latest schema format and prevent # them from being modified @@ -1858,7 +1873,6 @@ class Environment(object): fs.mkdirp(pkg_dir) spack.repo.path.dump_provenance(dep, pkg_dir) - self.new_specs = [] # write the lock file last with fs.write_tmp_and_move(self.lock_path) as f: @@ -1874,9 +1888,16 @@ class Environment(object): # call. But, having it here makes the views consistent witht the # concretized environment for most operations. Which is the # special case? - if regenerate_views: + if regenerate: self.regenerate_views() + # Run post_env_hooks + spack.hooks.post_env_write(self) + + # new specs and new installs reset at write time + self.new_specs = [] + self.new_installs = [] + def _update_and_write_manifest(self, raw_yaml_dict, yaml_dict): """Update YAML manifest for this environment based on changes to spec lists and views and write it. diff --git a/lib/spack/spack/hooks/__init__.py b/lib/spack/spack/hooks/__init__.py index 3c15b978d3..d4b8cd8eca 100644 --- a/lib/spack/spack/hooks/__init__.py +++ b/lib/spack/spack/hooks/__init__.py @@ -22,6 +22,7 @@ Currently the following hooks are supported: * on_phase_error(pkg, phase_name, log_file) * on_phase_error(pkg, phase_name, log_file) * on_analyzer_save(pkg, result) + * post_env_write(env) This can be used to implement support for things like module systems (e.g. modules, lmod, etc.) or to add other custom @@ -91,3 +92,6 @@ on_install_failure = _HookRunner('on_install_failure') # Analyzer hooks on_analyzer_save = _HookRunner('on_analyzer_save') + +# Environment hooks +post_env_write = _HookRunner('post_env_write') diff --git a/lib/spack/spack/hooks/module_file_generation.py b/lib/spack/spack/hooks/module_file_generation.py index 363654efc4..99d149f29c 100644 --- a/lib/spack/spack/hooks/module_file_generation.py +++ b/lib/spack/spack/hooks/module_file_generation.py @@ -11,24 +11,37 @@ import llnl.util.tty as tty def _for_each_enabled(spec, method_name): """Calls a method for each enabled module""" - enabled = spack.config.get('modules:enable') - if not enabled: - tty.debug('NO MODULE WRITTEN: list of enabled module files is empty') - return - - for name in enabled: - generator = spack.modules.module_types[name](spec) - try: - getattr(generator, method_name)() - except RuntimeError as e: - msg = 'cannot perform the requested {0} operation on module files' - msg += ' [{1}]' - tty.warn(msg.format(method_name, str(e))) + for name in spack.config.get('modules', {}): + enabled = spack.config.get('modules:%s:enable' % name) + if not enabled: + tty.debug('NO MODULE WRITTEN: list of enabled module files is empty') + return + + for type in enabled: + generator = spack.modules.module_types[type](spec, name) + try: + getattr(generator, method_name)() + except RuntimeError as e: + msg = 'cannot perform the requested {0} operation on module files' + msg += ' [{1}]' + tty.warn(msg.format(method_name, str(e))) def post_install(spec): + import spack.environment # break import cycle + if spack.environment.get_env({}, ''): + # If the installed through an environment, we skip post_install + # module generation and generate the modules on env_write so Spack + # can manage interactions between env views and modules + return + _for_each_enabled(spec, 'write') def post_uninstall(spec): _for_each_enabled(spec, 'remove') + + +def post_env_write(env): + for spec in env.new_installs: + _for_each_enabled(spec, 'write') diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index 6359d6dacf..fa1406be14 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -647,7 +647,9 @@ def print_setup_info(*info): 'tcl': list(), 'lmod': list() } - module_roots = spack.config.get('config:module_roots') + module_roots = spack.config.get('modules:default:roots', {}) + module_roots = spack.config.merge_yaml( + module_roots, spack.config.get('config:module_roots', {})) module_roots = dict( (k, v) for k, v in module_roots.items() if k in module_to_roots ) diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index ed843e03af..ae67ea5143 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -40,6 +40,7 @@ import llnl.util.filesystem from llnl.util.lang import dedupe import llnl.util.tty as tty import spack.build_environment as build_environment +import spack.environment as ev import spack.error import spack.paths import spack.schema.environment @@ -52,8 +53,13 @@ import spack.util.spack_yaml as syaml #: config section for this file -def configuration(): - return spack.config.get('modules', {}) +def configuration(module_set_name): + config_path = 'modules:%s' % module_set_name + config = spack.config.get(config_path, {}) + if not config and module_set_name == 'default': + # return old format for backward compatibility + return spack.config.get('modules', {}) + return config #: Valid tokens for naming scheme and env variable names @@ -204,17 +210,23 @@ def merge_config_rules(configuration, spec): return spec_configuration -def root_path(name): +def root_path(name, module_set_name): """Returns the root folder for module file installation. Args: name: name of the module system to be used (e.g. 'tcl') + module_set_name: name of the set of module configs to use Returns: root folder for module file installation """ # Root folders where the various module files should be written - roots = spack.config.get('config:module_roots', {}) + roots = spack.config.get('modules:%s:roots' % module_set_name, {}) + + # For backwards compatibility, read the old module roots for default set + if module_set_name == 'default': + roots = spack.config.merge_yaml( + spack.config.get('config:module_roots', {}), roots) path = roots.get(name, os.path.join(spack.paths.share_path, name)) return spack.util.path.canonicalize_path(path) @@ -326,7 +338,10 @@ class UpstreamModuleIndex(object): return None -def get_module(module_type, spec, get_full_path, required=True): +def get_module( + module_type, spec, get_full_path, + module_set_name='default', required=True +): """Retrieve the module file for a given spec and module type. Retrieve the module file for the given spec if it is available. If the @@ -342,6 +357,8 @@ def get_module(module_type, spec, get_full_path, required=True): then an exception is raised (regardless of whether it is required) get_full_path: if ``True``, this returns the full path to the module. Otherwise, this returns the module name. + module_set_name: the named module configuration set from modules.yaml + for which to retrieve the module. Returns: The module name or path. May return ``None`` if the module is not @@ -362,7 +379,7 @@ def get_module(module_type, spec, get_full_path, required=True): else: return module.use_name else: - writer = spack.modules.module_types[module_type](spec) + writer = spack.modules.module_types[module_type](spec, module_set_name) if not os.path.isfile(writer.layout.filename): if not writer.conf.blacklisted: err_msg = "No module available for package {0} at {1}".format( @@ -389,20 +406,22 @@ class BaseConfiguration(object): default_projections = { 'all': '{name}-{version}-{compiler.name}-{compiler.version}'} - def __init__(self, spec): + def __init__(self, spec, module_set_name): # Module where type(self) is defined self.module = inspect.getmodule(self) # Spec for which we want to generate a module file self.spec = spec + self.name = module_set_name # Dictionary of configuration options that should be applied # to the spec - self.conf = merge_config_rules(self.module.configuration(), self.spec) + self.conf = merge_config_rules( + self.module.configuration(self.name), self.spec) @property def projections(self): """Projection from specs to module names""" # backwards compatiblity for naming_scheme key - conf = self.module.configuration() + conf = self.module.configuration(self.name) if 'naming_scheme' in conf: default = {'all': conf['naming_scheme']} else: @@ -460,7 +479,7 @@ class BaseConfiguration(object): """ # A few variables for convenience of writing the method spec = self.spec - conf = self.module.configuration() + conf = self.module.configuration(self.name) # Compute the list of whitelist rules that match wlrules = conf.get('whitelist', []) @@ -522,7 +541,7 @@ class BaseConfiguration(object): def _create_list_for(self, what): whitelist = [] for item in self.conf[what]: - conf = type(self)(item) + conf = type(self)(item, self.name) if not conf.blacklisted: whitelist.append(item) return whitelist @@ -551,11 +570,10 @@ class BaseFileLayout(object): """Spec under consideration""" return self.conf.spec - @classmethod - def dirname(cls): + def dirname(self): """Root folder for module files of this type.""" - module_system = str(inspect.getmodule(cls).__name__).split('.')[-1] - return root_path(module_system) + module_system = str(self.conf.module.__name__).split('.')[-1] + return root_path(module_system, self.conf.name) @property def use_name(self): @@ -655,10 +673,30 @@ class BaseContext(tengine.Context): @tengine.context_property def environment_modifications(self): """List of environment modifications to be processed.""" - # Modifications guessed inspecting the spec prefix + # Modifications guessed by inspecting the spec prefix + std_prefix_inspections = spack.config.get( + 'modules:prefix_inspections', {}) + set_prefix_inspections = spack.config.get( + 'modules:%s:prefix_inspections' % self.conf.name, {}) + prefix_inspections = spack.config.merge_yaml( + std_prefix_inspections, set_prefix_inspections) + + use_view = spack.config.get( + 'modules:%s:use_view' % self.conf.name, False) + + spec = self.spec.copy() # defensive copy before setting prefix + if use_view: + if use_view is True: + use_view = ev.default_view_name + + env = ev.get_env({}, 'post_env_write_hook', required=True) + view = env.views[use_view].view() + + spec.prefix = view.get_projection_for_spec(spec) + env = spack.util.environment.inspect_path( - self.spec.prefix, - spack.config.get('modules:prefix_inspections', {}), + spec.prefix, + prefix_inspections, exclude=spack.util.environment.is_system_path ) @@ -666,12 +704,12 @@ class BaseContext(tengine.Context): # before asking for package-specific modifications env.extend( build_environment.modifications_from_dependencies( - self.spec, context='run' + spec, context='run' ) ) # Package specific modifications - build_environment.set_module_variables_for_package(self.spec.package) - self.spec.package.setup_run_environment(env) + build_environment.set_module_variables_for_package(spec.package) + spec.package.setup_run_environment(env) # Modifications required from modules.yaml env.extend(self.conf.env) @@ -686,17 +724,17 @@ class BaseContext(tengine.Context): # tokens uppercase. transform = {} for token in _valid_tokens: - transform[token] = lambda spec, string: str.upper(string) + transform[token] = lambda s, string: str.upper(string) for x in env: # Ensure all the tokens are valid in this context msg = 'some tokens cannot be expanded in an environment variable name' # noqa: E501 _check_tokens_are_valid(x.name, message=msg) # Transform them - x.name = self.spec.format(x.name, transform=transform) + x.name = spec.format(x.name, transform=transform) try: # Not every command has a value - x.value = self.spec.format(x.value) + x.value = spec.format(x.value) except AttributeError: pass x.name = str(x.name).replace('-', '_') @@ -714,7 +752,8 @@ class BaseContext(tengine.Context): def _create_module_list_of(self, what): m = self.conf.module - return [m.make_layout(x).use_name + name = self.conf.name + return [m.make_layout(x, name).use_name for x in getattr(self.conf, what)] @tengine.context_property @@ -724,7 +763,7 @@ class BaseContext(tengine.Context): class BaseModuleFileWriter(object): - def __init__(self, spec): + def __init__(self, spec, module_set_name): self.spec = spec # This class is meant to be derived. Get the module of the @@ -733,9 +772,9 @@ class BaseModuleFileWriter(object): m = self.module # Create the triplet of configuration/layout/context - self.conf = m.make_configuration(spec) - self.layout = m.make_layout(spec) - self.context = m.make_context(spec) + self.conf = m.make_configuration(spec, module_set_name) + self.layout = m.make_layout(spec, module_set_name) + self.context = m.make_context(spec, module_set_name) # Check if a default template has been defined, # throw if not found diff --git a/lib/spack/spack/modules/lmod.py b/lib/spack/spack/modules/lmod.py index bb4476a7b0..bc47761421 100644 --- a/lib/spack/spack/modules/lmod.py +++ b/lib/spack/spack/modules/lmod.py @@ -22,36 +22,42 @@ from .common import BaseContext, BaseModuleFileWriter #: lmod specific part of the configuration -def configuration(): - return spack.config.get('modules:lmod', {}) +def configuration(module_set_name): + config_path = 'modules:%s:lmod' % module_set_name + config = spack.config.get(config_path, {}) + if not config and module_set_name == 'default': + # return old format for backward compatibility + return spack.config.get('modules:lmod', {}) + return config #: Caches the configuration {spec_hash: configuration} configuration_registry = {} # type: Dict[str, Any] -def make_configuration(spec): +def make_configuration(spec, module_set_name): """Returns the lmod configuration for spec""" - key = spec.dag_hash() + key = (spec.dag_hash(), module_set_name) try: return configuration_registry[key] except KeyError: - return configuration_registry.setdefault(key, LmodConfiguration(spec)) + return configuration_registry.setdefault( + key, LmodConfiguration(spec, module_set_name)) -def make_layout(spec): +def make_layout(spec, module_set_name): """Returns the layout information for spec """ - conf = make_configuration(spec) + conf = make_configuration(spec, module_set_name) return LmodFileLayout(conf) -def make_context(spec): +def make_context(spec, module_set_name): """Returns the context information for spec""" - conf = make_configuration(spec) + conf = make_configuration(spec, module_set_name) return LmodContext(conf) -def guess_core_compilers(store=False): +def guess_core_compilers(name, store=False): """Guesses the list of core compilers installed in the system. Args: @@ -81,11 +87,12 @@ def guess_core_compilers(store=False): # in the default modify scope (i.e. within the directory hierarchy # of Spack itself) modules_cfg = spack.config.get( - 'modules', scope=spack.config.default_modify_scope() + 'modules:' + name, {}, scope=spack.config.default_modify_scope() ) modules_cfg.setdefault('lmod', {})['core_compilers'] = core_compilers spack.config.set( - 'modules', modules_cfg, scope=spack.config.default_modify_scope() + 'modules:' + name, modules_cfg, + scope=spack.config.default_modify_scope() ) return core_compilers or None @@ -104,9 +111,9 @@ class LmodConfiguration(BaseConfiguration): specified in the configuration file or the sequence is empty """ - value = configuration().get( + value = configuration(self.name).get( 'core_compilers' - ) or guess_core_compilers(store=True) + ) or guess_core_compilers(self.name, store=True) if not value: msg = 'the key "core_compilers" must be set in modules.yaml' @@ -116,14 +123,14 @@ class LmodConfiguration(BaseConfiguration): @property def core_specs(self): """Returns the list of "Core" specs""" - return configuration().get('core_specs', []) + return configuration(self.name).get('core_specs', []) @property def hierarchy_tokens(self): """Returns the list of tokens that are part of the modulefile hierarchy. 'compiler' is always present. """ - tokens = configuration().get('hierarchy', []) + tokens = configuration(self.name).get('hierarchy', []) # Check if all the tokens in the hierarchy are virtual specs. # If not warn the user and raise an error. @@ -407,7 +414,7 @@ class LmodContext(BaseContext): @tengine.context_property def unlocked_paths(self): """Returns the list of paths that are unlocked unconditionally.""" - layout = make_layout(self.spec) + layout = make_layout(self.spec, self.conf.name) return [os.path.join(*parts) for parts in layout.unlocked_paths[None]] @tengine.context_property @@ -415,7 +422,7 @@ class LmodContext(BaseContext): """Returns the list of paths that are unlocked conditionally. Each item in the list is a tuple with the structure (condition, path). """ - layout = make_layout(self.spec) + layout = make_layout(self.spec, self.conf.name) value = [] conditional_paths = layout.unlocked_paths conditional_paths.pop(None) diff --git a/lib/spack/spack/modules/tcl.py b/lib/spack/spack/modules/tcl.py index e1d2ac7fe3..d2f980bbc7 100644 --- a/lib/spack/spack/modules/tcl.py +++ b/lib/spack/spack/modules/tcl.py @@ -20,32 +20,38 @@ from .common import BaseContext, BaseModuleFileWriter #: TCL specific part of the configuration -def configuration(): - return spack.config.get('modules:tcl', {}) +def configuration(module_set_name): + config_path = 'modules:%s:tcl' % module_set_name + config = spack.config.get(config_path, {}) + if not config and module_set_name == 'default': + # return old format for backward compatibility + return spack.config.get('modules:tcl', {}) + return config #: Caches the configuration {spec_hash: configuration} configuration_registry = {} # type: Dict[str, Any] -def make_configuration(spec): +def make_configuration(spec, module_set_name): """Returns the tcl configuration for spec""" - key = spec.dag_hash() + key = (spec.dag_hash(), module_set_name) try: return configuration_registry[key] except KeyError: - return configuration_registry.setdefault(key, TclConfiguration(spec)) + return configuration_registry.setdefault( + key, TclConfiguration(spec, module_set_name)) -def make_layout(spec): +def make_layout(spec, module_set_name): """Returns the layout information for spec """ - conf = make_configuration(spec) + conf = make_configuration(spec, module_set_name) return TclFileLayout(conf) -def make_context(spec): +def make_context(spec, module_set_name): """Returns the context information for spec""" - conf = make_configuration(spec) + conf = make_configuration(spec, module_set_name) return TclContext(conf) diff --git a/lib/spack/spack/schema/modules.py b/lib/spack/spack/schema/modules.py index 39db0bf9a7..07a495af13 100644 --- a/lib/spack/spack/schema/modules.py +++ b/lib/spack/spack/schema/modules.py @@ -20,6 +20,10 @@ spec_regex = r'(?!hierarchy|core_specs|verbose|hash_length|whitelist|' \ r'blacklist|projections|naming_scheme|core_compilers|all)' \ r'(^\w[\w-]*)' +#: Matches a valid name for a module set +# Banned names are valid entries at that level in the previous schema +set_regex = r'(?!enable|lmod|tcl|dotkit|prefix_inspections)^\w[\w-]*' + #: Matches an anonymous spec, i.e. a spec without a root name anonymous_spec_regex = r'^[\^@%+~]' @@ -112,74 +116,105 @@ module_type_configuration = { } -# Properties for inclusion into other schemas (requires definitions) -properties = { - 'modules': { +#: The "real" module properties -- the actual configuration parameters. +#: They are separate from ``properties`` because they can appear both +#: at the top level of a Spack ``modules:`` config (old, deprecated format), +#: and within a named module set (new format with multiple module sets). +module_config_properties = { + 'use_view': {'anyOf': [ + {'type': 'string'}, + {'type': 'boolean'} + ]}, + 'prefix_inspections': { 'type': 'object', - 'default': {}, 'additionalProperties': False, + 'patternProperties': { + # prefix-relative path to be inspected for existence + r'^[\w-]*': array_of_strings + } + }, + 'roots': { + 'type': 'object', 'properties': { - 'prefix_inspections': { + 'tcl': {'type': 'string'}, + 'lmod': {'type': 'string'}, + }, + }, + 'enable': { + 'type': 'array', + 'default': [], + 'items': { + 'type': 'string', + 'enum': ['tcl', 'dotkit', 'lmod'] + }, + 'deprecatedProperties': { + 'properties': ['dotkit'], + 'message': 'cannot enable "dotkit" in modules.yaml ' + '[support for "dotkit" has been dropped ' + 'in v0.13.0]', + 'error': False + }, + }, + 'lmod': { + 'allOf': [ + # Base configuration + module_type_configuration, + { 'type': 'object', - 'patternProperties': { - # prefix-relative path to be inspected for existence - r'\w[\w-]*': array_of_strings - } - }, - 'enable': { - 'type': 'array', - 'default': [], - 'items': { - 'type': 'string', - 'enum': ['tcl', 'dotkit', 'lmod'] + 'properties': { + 'core_compilers': array_of_strings, + 'hierarchy': array_of_strings, + 'core_specs': array_of_strings, }, + } # Specific lmod extensions + ] + }, + 'tcl': { + 'allOf': [ + # Base configuration + module_type_configuration, + {} # Specific tcl extensions + ] + }, + 'dotkit': { + 'allOf': [ + # Base configuration + module_type_configuration, + {} # Specific dotkit extensions + ] + }, +} + + +# Properties for inclusion into other schemas (requires definitions) +properties = { + 'modules': { + 'type': 'object', + 'patternProperties': { + set_regex: { + 'type': 'object', + 'default': {}, + 'additionalProperties': False, + 'properties': module_config_properties, 'deprecatedProperties': { 'properties': ['dotkit'], - 'message': 'cannot enable "dotkit" in modules.yaml ' - '[support for "dotkit" has been dropped ' - 'in v0.13.0]', + 'message': 'the "dotkit" section in modules.yaml has no effect' + ' [support for "dotkit" has been dropped in v0.13.0]', 'error': False - }, - }, - 'lmod': { - 'allOf': [ - # Base configuration - module_type_configuration, - { - 'type': 'object', - 'properties': { - 'core_compilers': array_of_strings, - 'hierarchy': array_of_strings, - 'core_specs': array_of_strings, - }, - } # Specific lmod extensions - ] - }, - 'tcl': { - 'allOf': [ - # Base configuration - module_type_configuration, - {} # Specific tcl extensions - ] - }, - 'dotkit': { - 'allOf': [ - # Base configuration - module_type_configuration, - {} # Specific dotkit extensions - ] + } }, }, + # Available here for backwards compatibility + 'properties': module_config_properties, 'deprecatedProperties': { 'properties': ['dotkit'], 'message': 'the "dotkit" section in modules.yaml has no effect' - ' [support for "dotkit" has been dropped in v0.13.0]', + ' [support for "dotkit" has been dropped in v0.13.0]', 'error': False - }, - }, + } + } } - #: Full schema with metadata schema = { '$schema': 'http://json-schema.org/schema#', diff --git a/lib/spack/spack/subprocess_context.py b/lib/spack/spack/subprocess_context.py index 3eee2125d2..0b41d796fa 100644 --- a/lib/spack/spack/subprocess_context.py +++ b/lib/spack/spack/subprocess_context.py @@ -65,19 +65,25 @@ class PackageInstallContext(object): needs to be transmitted to a child process. """ def __init__(self, pkg): + import spack.environment as ev # break import cycle if _serialize: self.serialized_pkg = serialize(pkg) + self.serialized_env = serialize(ev._active_environment) else: self.pkg = pkg + self.env = ev._active_environment self.spack_working_dir = spack.main.spack_working_dir self.test_state = TestState() def restore(self): + import spack.environment as ev # break import cycle self.test_state.restore() spack.main.spack_working_dir = self.spack_working_dir if _serialize: + ev._active_environment = pickle.load(self.serialized_env) return pickle.load(self.serialized_pkg) else: + ev._active_environment = self.env return self.pkg diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index f6933dc349..002bd14c0f 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2,7 +2,7 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - +import glob import os from six import StringIO @@ -2484,3 +2484,80 @@ def test_custom_version_concretize_together(tmpdir): e.concretize() assert any('hdf5@myversion' in spec for _, spec in e.concretized_specs()) + + +def test_modules_relative_to_views(tmpdir, install_mockery, mock_fetch): + spack_yaml = """ +spack: + specs: + - trivial-install-test-package + modules: + default: + enable:: [tcl] + use_view: true + roots: + tcl: modules +""" + _env_create('test', StringIO(spack_yaml)) + + with ev.read('test') as e: + install() + + spec = e.specs_by_hash[e.concretized_order[0]] + view_prefix = e.default_view.view().get_projection_for_spec(spec) + modules_glob = '%s/modules/**/*' % e.path + modules = glob.glob(modules_glob) + assert len(modules) == 1 + module = modules[0] + + with open(module, 'r') as f: + contents = f.read() + + assert view_prefix in contents + assert spec.prefix not in contents + + +def test_multiple_modules_post_env_hook(tmpdir, install_mockery, mock_fetch): + spack_yaml = """ +spack: + specs: + - trivial-install-test-package + modules: + default: + enable:: [tcl] + use_view: true + roots: + tcl: modules + full: + enable:: [tcl] + roots: + tcl: full_modules +""" + _env_create('test', StringIO(spack_yaml)) + + with ev.read('test') as e: + install() + + spec = e.specs_by_hash[e.concretized_order[0]] + view_prefix = e.default_view.view().get_projection_for_spec(spec) + modules_glob = '%s/modules/**/*' % e.path + modules = glob.glob(modules_glob) + assert len(modules) == 1 + module = modules[0] + + full_modules_glob = '%s/full_modules/**/*' % e.path + full_modules = glob.glob(full_modules_glob) + assert len(full_modules) == 1 + full_module = full_modules[0] + + with open(module, 'r') as f: + contents = f.read() + + with open(full_module, 'r') as f: + full_contents = f.read() + + assert view_prefix in contents + assert spec.prefix not in contents + + assert view_prefix not in full_contents + assert spec.prefix in full_contents diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py index 9acb21fdef..7b281eeba3 100644 --- a/lib/spack/spack/test/cmd/module.py +++ b/lib/spack/spack/test/cmd/module.py @@ -32,7 +32,7 @@ def ensure_module_files_are_there( def _module_files(module_type, *specs): specs = [spack.spec.Spec(x).concretized() for x in specs] writer_cls = spack.modules.module_types[module_type] - return [writer_cls(spec).layout.filename for spec in specs] + return [writer_cls(spec, 'default').layout.filename for spec in specs] @pytest.fixture( @@ -200,8 +200,10 @@ def test_setdefault_command( spack.spec.Spec(preferred).concretized().package.do_install(fake=True) writers = { - preferred: writer_cls(spack.spec.Spec(preferred).concretized()), - other_spec: writer_cls(spack.spec.Spec(other_spec).concretized()) + preferred: writer_cls( + spack.spec.Spec(preferred).concretized(), 'default'), + other_spec: writer_cls( + spack.spec.Spec(other_spec).concretized(), 'default') } # Create two module files for the same software diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index d47851462a..3e21bddbd6 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -374,9 +374,9 @@ def test_substitute_config_variables(mock_low_high_config, monkeypatch): # relative paths with source information are relative to the file spack.config.set( - 'config:module_roots', {'lmod': 'foo/bar/baz'}, scope='low') + 'modules:default', {'roots': {'lmod': 'foo/bar/baz'}}, scope='low') spack.config.config.clear_caches() - path = spack.config.get('config:module_roots:lmod') + path = spack.config.get('modules:default:roots:lmod') assert spack_path.canonicalize_path(path) == os.path.normpath( os.path.join(mock_low_high_config.scopes['low'].path, 'foo/bar/baz')) @@ -987,8 +987,9 @@ def test_bad_config_yaml(tmpdir): check_schema(spack.schema.config.schema, """\ config: verify_ssl: False - module_roots: - fmod: /some/fake/location + install_tree: + root: + extra_level: foo """) diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 6cc3ad16db..b76cd1be1b 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -763,11 +763,11 @@ class MockConfig(object): self._configuration = configuration self.writer_key = writer_key - def configuration(self): + def configuration(self, module_set_name): return self._configuration - def writer_configuration(self): - return self.configuration()[self.writer_key] + def writer_configuration(self, module_set_name): + return self.configuration(module_set_name)[self.writer_key] class ConfigUpdate(object): @@ -780,7 +780,9 @@ class ConfigUpdate(object): def __call__(self, filename): file = os.path.join(self.root_for_conf, filename + '.yaml') with open(file) as f: - mock_config = MockConfig(syaml.load_config(f), self.writer_key) + config_settings = syaml.load_config(f) + spack.config.set('modules:default', config_settings) + mock_config = MockConfig(config_settings, self.writer_key) self.monkeypatch.setattr( spack.modules.common, diff --git a/lib/spack/spack/test/data/config/config.yaml b/lib/spack/spack/test/data/config/config.yaml index 09ab7709a3..d5c5f914fb 100644 --- a/lib/spack/spack/test/data/config/config.yaml +++ b/lib/spack/spack/test/data/config/config.yaml @@ -14,6 +14,3 @@ config: checksum: true dirty: false concretizer: {0} - module_roots: - tcl: {1} - lmod: {2} diff --git a/lib/spack/spack/test/data/config/modules.yaml b/lib/spack/spack/test/data/config/modules.yaml index f610087fb1..e2ddd841c5 100644 --- a/lib/spack/spack/test/data/config/modules.yaml +++ b/lib/spack/spack/test/data/config/modules.yaml @@ -14,8 +14,9 @@ # ~/.spack/modules.yaml # ------------------------------------------------------------------------- modules: - enable: - - tcl + default: + enable: + - tcl prefix_inspections: bin: - PATH diff --git a/lib/spack/spack/test/data/modules/lmod/alter_environment.yaml b/lib/spack/spack/test/data/modules/lmod/alter_environment.yaml index f61c94362e..314dd1ddf5 100644 --- a/lib/spack/spack/test/data/modules/lmod/alter_environment.yaml +++ b/lib/spack/spack/test/data/modules/lmod/alter_environment.yaml @@ -9,7 +9,7 @@ lmod: all: filter: - environment_blacklist': + environment_blacklist: - CMAKE_PREFIX_PATH environment: set: diff --git a/lib/spack/spack/test/data/modules/lmod/with_view.yaml b/lib/spack/spack/test/data/modules/lmod/with_view.yaml new file mode 100644 index 0000000000..28220fe445 --- /dev/null +++ b/lib/spack/spack/test/data/modules/lmod/with_view.yaml @@ -0,0 +1,6 @@ +enable: + - lmod +use_view: default +lmod: + core_compilers: + - 'clang@3.3' diff --git a/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml b/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml index ecb0f56254..74d9724695 100644 --- a/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml +++ b/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml @@ -3,7 +3,7 @@ enable: tcl: all: filter: - environment_blacklist': + environment_blacklist: - CMAKE_PREFIX_PATH environment: set: diff --git a/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml b/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml index bed866fe90..6012a2d3b0 100644 --- a/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml +++ b/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml @@ -3,7 +3,7 @@ enable: tcl: all: filter: - environment_blacklist': + environment_blacklist: - CMAKE_PREFIX_PATH environment: set: diff --git a/lib/spack/spack/test/modules/common.py b/lib/spack/spack/test/modules/common.py index 0918cf2dfd..8270b01c71 100644 --- a/lib/spack/spack/test/modules/common.py +++ b/lib/spack/spack/test/modules/common.py @@ -70,7 +70,7 @@ def test_modules_written_with_proper_permissions(mock_module_filename, # The code tested is common to all module types, but has to be tested from # one. TCL picked at random - generator = spack.modules.tcl.TclModulefileWriter(spec) + generator = spack.modules.tcl.TclModulefileWriter(spec, 'default') generator.write() assert mock_package_perms & os.stat( diff --git a/lib/spack/spack/test/modules/conftest.py b/lib/spack/spack/test/modules/conftest.py index dbfac6b0bc..ea61a3b955 100644 --- a/lib/spack/spack/test/modules/conftest.py +++ b/lib/spack/spack/test/modules/conftest.py @@ -19,11 +19,11 @@ def modulefile_content(request): writer_cls = getattr(request.module, 'writer_cls') - def _impl(spec_str): + def _impl(spec_str, module_set_name='default'): # Write the module file spec = spack.spec.Spec(spec_str) spec.concretize() - generator = writer_cls(spec) + generator = writer_cls(spec, module_set_name) generator.write(overwrite=True) # Get its filename @@ -56,9 +56,9 @@ def factory(request): # Class of the module file writer writer_cls = getattr(request.module, 'writer_cls') - def _mock(spec_string): + def _mock(spec_string, module_set_name='default'): spec = spack.spec.Spec(spec_string) spec.concretize() - return writer_cls(spec), spec + return writer_cls(spec, module_set_name), spec return _mock diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py index 7239c487aa..097aaf526f 100644 --- a/lib/spack/spack/test/modules/lmod.py +++ b/lib/spack/spack/test/modules/lmod.py @@ -5,12 +5,17 @@ import re import pytest +import spack.environment as ev +import spack.main import spack.modules.lmod +import spack.spec mpich_spec_string = 'mpich@3.0.4' mpileaks_spec_string = 'mpileaks' libdwarf_spec_string = 'libdwarf arch=x64-linux' +install = spack.main.SpackCommand('install') + #: Class of the writer tested in this module writer_cls = spack.modules.lmod.LmodModulefileWriter @@ -314,3 +319,35 @@ class TestLmod(object): assert writer.conf.projections == expected projection = writer.spec.format(writer.conf.projections['all']) assert projection in writer.layout.use_name + + def test_config_backwards_compat(self, mutable_config): + settings = { + 'enable': ['lmod'], + 'lmod': { + 'core_compilers': ['%gcc@0.0.0'] + } + } + + spack.config.set('modules:default', settings) + new_format = spack.modules.lmod.configuration('default') + + spack.config.set('modules', settings) + old_format = spack.modules.lmod.configuration('default') + + assert old_format == new_format + assert old_format == settings['lmod'] + + def test_modules_relative_to_view( + self, tmpdir, modulefile_content, module_configuration, install_mockery): + with ev.Environment(str(tmpdir), with_view=True) as e: + module_configuration('with_view') + install('cmake') + + spec = spack.spec.Spec('cmake').concretized() + + content = modulefile_content('cmake') + expected = e.default_view.view().get_projection_for_spec(spec) + # Rather than parse all lines, ensure all prefixes in the content + # point to the right one + assert any(expected in line for line in content) + assert not any(spec.prefix in line for line in content) diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index e5f2797e39..464d91c278 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -359,14 +359,14 @@ class TestTcl(object): # the tests database mpileaks_specs = database.query('mpileaks') for item in mpileaks_specs: - writer = writer_cls(item) + writer = writer_cls(item, 'default') assert not writer.conf.blacklisted # callpath is a dependency of mpileaks, and has been pulled # in implicitly callpath_specs = database.query('callpath') for item in callpath_specs: - writer = writer_cls(item) + writer = writer_cls(item, 'default') assert writer.conf.blacklisted @pytest.mark.regression('9624') @@ -385,3 +385,22 @@ class TestTcl(object): # Test the mpileaks that should NOT have the autoloaded dependencies content = modulefile_content('mpileaks ^mpich') assert len([x for x in content if 'is-loaded' in x]) == 0 + + def test_config_backwards_compat(self, mutable_config): + settings = { + 'enable': ['tcl'], + 'tcl': { + 'all': { + 'conflict': ['{name}'] + } + } + } + + spack.config.set('modules:default', settings) + new_format = spack.modules.tcl.configuration('default') + + spack.config.set('modules', settings) + old_format = spack.modules.tcl.configuration('default') + + assert old_format == new_format + assert old_format == settings['tcl'] diff --git a/lib/spack/spack/user_environment.py b/lib/spack/spack/user_environment.py index 7dd63dcdea..15c8d2822a 100644 --- a/lib/spack/spack/user_environment.py +++ b/lib/spack/spack/user_environment.py @@ -26,8 +26,8 @@ def prefix_inspections(platform): A dictionary mapping subdirectory names to lists of environment variables to modify with that directory if it exists. """ - inspections = spack.config.get('modules:prefix_inspections', None) - if inspections is not None: + inspections = spack.config.get('modules:default:prefix_inspections', {}) + if inspections: return inspections inspections = { |