From 7490d63c38f5cd8dd927ee0adda4e28578bbd60d Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 28 May 2021 14:12:05 -0700 Subject: [PATCH] Separable module configuration -- without the bugs this time (#23703) 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. --- etc/spack/defaults/config.yaml | 7 - etc/spack/defaults/linux/modules.yaml | 11 +- etc/spack/defaults/modules.yaml | 23 ++- lib/spack/docs/environments.rst | 2 + lib/spack/docs/module_file_support.rst | 150 ++++++++++++++++-- lib/spack/spack/cmd/env.py | 3 + lib/spack/spack/cmd/install.py | 10 +- lib/spack/spack/cmd/modules/__init__.py | 40 ++++- lib/spack/spack/cmd/modules/lmod.py | 3 +- lib/spack/spack/config.py | 19 ++- lib/spack/spack/environment.py | 49 ++++-- lib/spack/spack/hooks/__init__.py | 4 + .../spack/hooks/module_file_generation.py | 45 ++++-- lib/spack/spack/main.py | 4 +- lib/spack/spack/modules/common.py | 105 ++++++++---- lib/spack/spack/modules/lmod.py | 43 ++--- lib/spack/spack/modules/tcl.py | 24 +-- lib/spack/spack/schema/modules.py | 139 ++++++++++------ lib/spack/spack/subprocess_context.py | 6 + lib/spack/spack/test/cmd/env.py | 79 ++++++++- lib/spack/spack/test/cmd/module.py | 8 +- lib/spack/spack/test/config.py | 23 ++- lib/spack/spack/test/conftest.py | 10 +- lib/spack/spack/test/data/config/config.yaml | 3 - lib/spack/spack/test/data/config/modules.yaml | 5 +- .../data/modules/lmod/alter_environment.yaml | 2 +- .../test/data/modules/lmod/with_view.yaml | 6 + .../data/modules/tcl/alter_environment.yaml | 2 +- .../tcl/invalid_token_in_env_var_name.yaml | 2 +- lib/spack/spack/test/modules/common.py | 2 +- lib/spack/spack/test/modules/conftest.py | 8 +- lib/spack/spack/test/modules/lmod.py | 37 +++++ lib/spack/spack/test/modules/tcl.py | 23 ++- lib/spack/spack/user_environment.py | 4 +- share/spack/qa/setup-env-test.sh | 4 +- share/spack/spack-completion.bash | 6 +- 36 files changed, 690 insertions(+), 221 deletions(-) create mode 100644 lib/spack/spack/test/data/modules/lmod/with_view.yaml diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml index 61c23f42df..2400686e60 100644 --- a/etc/spack/defaults/config.yaml +++ b/etc/spack/defaults/config.yaml @@ -33,13 +33,6 @@ config: template_dirs: - $spack/share/spack/templates - - # Locations where different types of modules should be installed. - module_roots: - tcl: $spack/share/spack/modules - lmod: $spack/share/spack/lmod - - # Temporary locations Spack can try to use for builds. # # Recommended options are given below. diff --git a/etc/spack/defaults/linux/modules.yaml b/etc/spack/defaults/linux/modules.yaml index a86a4794f1..353a6ea9ab 100644 --- a/etc/spack/defaults/linux/modules.yaml +++ b/etc/spack/defaults/linux/modules.yaml @@ -14,8 +14,9 @@ # ~/.spack/modules.yaml # ------------------------------------------------------------------------- modules: - prefix_inspections: - lib: - - LD_LIBRARY_PATH - lib64: - - LD_LIBRARY_PATH + default: + prefix_inspections: + lib: + - LD_LIBRARY_PATH + lib64: + - LD_LIBRARY_PATH diff --git a/etc/spack/defaults/modules.yaml b/etc/spack/defaults/modules.yaml index d6d245930c..27b7c45f66 100644 --- a/etc/spack/defaults/modules.yaml +++ b/etc/spack/defaults/modules.yaml @@ -14,8 +14,7 @@ # ~/.spack/modules.yaml # ------------------------------------------------------------------------- modules: - enable: - - tcl + # Paths to check when creating modules for all module sets prefix_inspections: bin: - PATH @@ -34,6 +33,20 @@ modules: '': - CMAKE_PREFIX_PATH - lmod: - hierarchy: - - mpi + # These are configurations for the module set named "default" + default: + # These values are defaulted in the code. They are not defaulted here so + # that we can enable backwards compatibility with the old syntax more + # easily (old value is in the config yaml, config:module_roots) + # Where to install modules + # roots: + # tcl: $spack/share/spack/modules + # lmod: $spack/share/spack/lmod + # What type of modules to use + enable: + - tcl + + # Default configurations if lmod is enabled + lmod: + hierarchy: + - mpi diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 12f6f4121c..79382ab9ac 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -723,6 +723,8 @@ Spack Environment managed views are updated every time the environment is written out to the lock file ``spack.lock``, so the concrete environment and the view are always compatible. +.. _configuring_environment_views: + """"""""""""""""""""""""""""" Configuring environment views """"""""""""""""""""""""""""" diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst index d46ec3143d..029c6dbbbd 100644 --- a/lib/spack/docs/module_file_support.rst +++ b/lib/spack/docs/module_file_support.rst @@ -71,9 +71,24 @@ Module file customization ------------------------- Module files are generated by post-install hooks after the successful -installation of a package. The table below summarizes the essential -information associated with the different file formats -that can be generated by Spack: +installation of a package. + +.. note:: + + Spack only generates modulefiles when a package is installed. If + you attempt to install a package and it is already installed, Spack + will not regenerate modulefiles for the package. This may to + inconsistent modulefiles if the Spack module configuration has + changed since the package was installed, either by editing a file + or changing scopes or environments. + + Later in this section there is a subsection on :ref:`regenerating + modules ` that will allow you to bring + your modules to a consistent state. + +The table below summarizes the essential information associated with +the different file formats that can be generated by Spack: + +-----------------------------+--------------------+-------------------------------+----------------------------------------------+----------------------+ | | **Hook name** | **Default root directory** | **Default template file** | **Compatible tools** | @@ -163,6 +178,46 @@ the installation folder of each package for the presence of a set of subdirector (``bin``, ``man``, ``share/man``, etc.). If any is found its full path is prepended to the environment variables listed below the folder name. +Spack modules can be configured for multiple module sets. The default +module set is named ``default``. All Spack commands which operate on +modules default to apply the ``default`` module set, but can be +applied to any module set in the configuration. Settings applied at +the root of the configuration (e.g. ``modules:enable`` rather than +``modules:default:enable``) are applied to the default module set for +backwards compatibility. + +""""""""""""""""""""""""" +Changing the modules root +""""""""""""""""""""""""" + +As shown in the table above, the default module root for ``lmod`` is +``$spack/share/spack/lmod`` and the default root for ``tcl`` is +``$spack/share/spack/modules``. This can be overridden for any module +set by changing the ``roots`` key of the configuration. + +.. code-block:: yaml + + modules: + default: + roots: + tcl: /path/to/install/tcl/modules + my_custom_lmod_modules: + roots: + lmod: /path/to/install/custom/lmod/modules + ... + +This configuration will create two module sets. The default module set +will install its ``tcl`` modules to ``/path/to/install/tcl/modules`` +(and still install its lmod modules, if any, to the default +location). The set ``my_custom_lmod_modules`` will install its lmod +modules to ``/path/to/install/custom/lmod/modules`` (and still install +its tcl modules, if any, to the default location). + +Obviously, having multiple module sets install modules to the default +location could be confusing to users of your modules. In the next +section, we will discuss enabling and disabling module types (module +file generators) for each module set. + """""""""""""""""""" Activate other hooks """""""""""""""""""" @@ -178,13 +233,14 @@ to the generator being customized: .. code-block:: yaml modules: - enable: - - tcl - - lmod - tcl: - # contains environment modules specific customizations - lmod: - # contains lmod specific customizations + default: + enable: + - tcl + - lmod + tcl: + # contains environment modules specific customizations + lmod: + # contains lmod specific customizations In general, the configuration options that you can use in ``modules.yaml`` will either change the layout of the module files on the filesystem, or they will affect @@ -399,10 +455,16 @@ that are already in the LMod hierarchy. Customize environment modifications """"""""""""""""""""""""""""""""""" -You can control which prefixes in a Spack package are added to environment -variables with the ``prefix_inspections`` section; this section maps relative -prefixes to the list of environment variables which should be updated with -those prefixes. +You can control which prefixes in a Spack package are added to +environment variables with the ``prefix_inspections`` section; this +section maps relative prefixes to the list of environment variables +which should be updated with those prefixes. + +The ``prefix_inspections`` configuration is different from other +settings in that a ``prefix_inspections`` configuration at the +``modules`` level of the configuration file applies to all module +sets. This allows users to make general overrides to the default +inspections and customize them per-module-set. .. code-block:: yaml @@ -415,10 +477,66 @@ those prefixes. '': - CMAKE_PREFIX_PATH -In this case, for a Spack package ``foo`` installed to ``/spack/prefix/foo``, -the generated module file for ``foo`` would update ``PATH`` to contain +Prefix inspections are only applied if the relative path inside the +installation prefix exists. In this case, for a Spack package ``foo`` +installed to ``/spack/prefix/foo``, if ``foo`` installs executables to +``bin`` but no libraries in ``lib``, the generated module file for +``foo`` would update ``PATH`` to contain ``/spack/prefix/foo/bin`` and +``CMAKE_PREFIX_PATH`` to contain ``/spack/prefix/foo``, but would not +update ``LIBRARY_PATH``. + +There is a special case for prefix inspections relative to environment +views. If all of the following conditions hold for a module set +configuration: + +#. The configuration is for an :ref:`environment ` and + will never be applied outside the environment, +#. The environment in question is configured to use a :ref:`view + `, +#. The :ref:`environment view is configured + ` with a projection that ensures + every package is linked to a unique directory, + +then the module set may be configured to create modules relative to +the environment view. This is specified by the ``use_view`` +configuration option in the module set. If ``True``, the module set is +constructed relative to the default view of the +environment. Otherwise, the value must be the name of the environment +view relative to which to construct modules, or ``False-ish`` to +disable the feature explicitly (the default is ``False``). + +If the ``use_view`` value is set in the config, then the prefix +inspections for the package are done relative to the package's path in +the view. + +.. code-block:: yaml + + spack: + modules: + view_relative_modules: + use_view: my_view + prefix_inspections: + bin: + - PATH + view: + my_view: + projections: + root: /path/to/my/view + all: '{name}-{hash}' + +The ``spack`` key is relevant to :ref:`environment ` +configuration, and the view key is discussed in detail in the section +on :ref:`Configuring environment views +`. With this configuration the +generated module for package ``foo`` would set ``PATH`` to include +``/path/to/my/view/foo-/bin`` instead of ``/spack/prefix/foo/bin``. +The ``use_view`` option is useful when deploying a large software +stack to users who are likely to inspect the modules to find full +paths to software, when it is desirable to present the users with a +simpler set of paths than those generated by the Spack install tree. + """""""""""""""""""""""""""""""""""" Filter out environment modifications """""""""""""""""""""""""""""""""""" 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 @@ -363,6 +363,9 @@ def env_loads_setup_parser(subparser): """list modules for an installed environment '(see spack module loads)'""" 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') diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 5a0b47215a..16026bd5f2 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: @@ -340,7 +340,7 @@ def get_tests(specs): # 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: @@ -354,9 +354,9 @@ def get_tests(specs): 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 @@ 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 @@ 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 @@ def one_spec_or_raise(specs): 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..420a432fe9 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -571,16 +571,17 @@ def get_config(self, section, scope=None): 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'] } """ @@ -648,7 +649,11 @@ def get(self, path, default=None, scope=None): while parts: key = parts.pop(0) - value = value.get(key, default) + # cannot use value.get(key, default) in case there is another part + # and default is not a dict + if key not in value: + return default + value = value[key] return value diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index b649d112dc..bbb4298667 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -20,6 +20,7 @@ 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 @@ -459,12 +460,15 @@ def __init__(self, base_path, root, projections={}, select=[], exclude=[], 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, @@ -745,7 +749,7 @@ def _re_read(self): if not os.path.exists(self.manifest_path): return - self.clear() + self.clear(re_read=True) self._read() def _read(self): @@ -843,15 +847,26 @@ def _set_user_specs_from_lockfile(self): ) } - 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): @@ -1588,6 +1603,7 @@ def install_specs(self, specs=None, args=None, **install_args): # 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: @@ -1816,17 +1832,16 @@ def _read_lockfile_dict(self, d): 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 @@ -1862,7 +1877,6 @@ def write(self, regenerate_views=True): 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: @@ -1878,9 +1892,16 @@ def write(self, regenerate_views=True): # 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 @@ * 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 @@ def __call__(self, *args, **kwargs): # 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..2007a77af7 100644 --- a/lib/spack/spack/hooks/module_file_generation.py +++ b/lib/spack/spack/hooks/module_file_generation.py @@ -11,24 +11,45 @@ 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 + set_names = set(spack.config.get('modules', {}).keys()) + # If we have old-style modules enabled, we put those in the default set + old_default_enabled = spack.config.get('modules:enable') + if old_default_enabled: + set_names.add('default') + for name in set_names: + enabled = spack.config.get('modules:%s:enable' % name) + if name == 'default': + # combine enabled modules from default and old format + enabled = spack.config.merge_yaml(old_default_enabled, enabled) + if not enabled: + tty.debug('NO MODULE WRITTEN: list of enabled module files is empty') + continue - 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 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 shell_set(var, value): '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..1e5f261578 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -40,6 +40,7 @@ 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 @@ #: 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,31 @@ 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 """ + defaults = { + 'lmod': '$spack/share/spack/modules', + 'tcl': '$spack/share/spack/lmod', + } # 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) + + # Merge config values into the defaults so we prefer configured values + roots = spack.config.merge_yaml(defaults, roots) + path = roots.get(name, os.path.join(spack.paths.share_path, name)) return spack.util.path.canonicalize_path(path) @@ -326,7 +346,10 @@ def upstream_module(self, spec, module_type): 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 +365,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 +387,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 +414,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 +487,7 @@ def blacklisted(self): """ # 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 +549,7 @@ def environment_blacklist(self): 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 +578,10 @@ def spec(self): """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 +681,30 @@ def configure_options(self): @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 +712,12 @@ def environment_modifications(self): # 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 +732,17 @@ def environment_modifications(self): # 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 +760,8 @@ def autoload(self): 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 +771,7 @@ def verbose(self): 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 +780,9 @@ def __init__(self, spec): 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 @@ #: 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 @@ def core_compilers(self): 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 @@ def core_compilers(self): @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 @@ def missing(self): @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 @@ def conditionally_unlocked_paths(self): """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 @@ #: 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 @@ 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 @@ } +#: 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', + 'additionalProperties': False, + 'patternProperties': { + # prefix-relative path to be inspected for existence + r'^[\w-]*': array_of_strings + } + }, + 'roots': { + 'type': 'object', + 'properties': { + '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', + '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', - 'default': {}, - 'additionalProperties': False, - 'properties': { - 'prefix_inspections': { + 'patternProperties': { + set_regex: { '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'] - }, + '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..40bcab0e31 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')) @@ -508,6 +508,20 @@ def test_read_config_override_all(mock_low_high_config, write_config_file): } +@pytest.mark.regression('23663') +def test_read_with_default(mock_low_high_config): + # this very synthetic example ensures that config.get(path, default) + # returns default if any element of path doesn't exist, regardless + # of the type of default. + spack.config.set('modules', {'enable': []}) + + default_conf = spack.config.get('modules:default', 'default') + assert default_conf == 'default' + + default_enable = spack.config.get('modules:default:enable', []) + assert default_enable == [] + + def test_read_config_override_key(mock_low_high_config, write_config_file): write_config_file('config', config_low, 'low') write_config_file('config', config_override_key, 'high') @@ -987,8 +1001,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 165d45ce53..8fe566fee7 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -779,11 +779,11 @@ def __init__(self, configuration, writer_key): 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): @@ -796,7 +796,9 @@ def __init__(self, root_for_conf, writer_mod, writer_key, monkeypatch): 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 @@ def test_projections_all(self, factory, module_configuration): 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 @@ def test_blacklist_implicits( # 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 @@ def test_autoload_with_constraints( # 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..c4ea2b8ac2 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:prefix_inspections', {}) + if inspections: return inspections inspections = { diff --git a/share/spack/qa/setup-env-test.sh b/share/spack/qa/setup-env-test.sh index bef94dfe1f..bf3bfe63f3 100755 --- a/share/spack/qa/setup-env-test.sh +++ b/share/spack/qa/setup-env-test.sh @@ -104,11 +104,11 @@ contains "usage: spack module " spack -m module --help contains "usage: spack module " spack -m module title 'Testing `spack load`' -contains "export LD_LIBRARY_PATH=$(spack -m location -i b)/lib" spack -m load --only package --sh b +contains "export PATH=$(spack -m location -i b)/bin" spack -m load --only package --sh b succeeds spack -m load b fails spack -m load -l # test a variable MacOS clears and one it doesn't for recursive loads -contains "export LD_LIBRARY_PATH=$(spack -m location -i a)/lib:$(spack -m location -i b)/lib" spack -m load --sh a +contains "export PATH=$(spack -m location -i a)/bin:$(spack -m location -i b)/bin" spack -m load --sh a succeeds spack -m load --only dependencies a succeeds spack -m load --only package a fails spack -m load d diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index bc0f57b18c..163a2ecf5f 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -867,7 +867,7 @@ _spack_env_st() { _spack_env_loads() { if $list_options then - SPACK_COMPREPLY="-h --help -m --module-type --input-only -p --prefix -x --exclude -r --dependencies" + SPACK_COMPREPLY="-h --help -n --module-set-name -m --module-type --input-only -p --prefix -x --exclude -r --dependencies" else _environments fi @@ -1227,7 +1227,7 @@ _spack_module() { _spack_module_lmod() { if $list_options then - SPACK_COMPREPLY="-h --help" + SPACK_COMPREPLY="-h --help -n --name" else SPACK_COMPREPLY="refresh find rm loads setdefault" fi @@ -1281,7 +1281,7 @@ _spack_module_lmod_setdefault() { _spack_module_tcl() { if $list_options then - SPACK_COMPREPLY="-h --help" + SPACK_COMPREPLY="-h --help -n --name" else SPACK_COMPREPLY="refresh find rm loads" fi