diff options
35 files changed, 1136 insertions, 242 deletions
diff --git a/etc/spack/defaults/darwin/packages.yaml b/etc/spack/defaults/darwin/packages.yaml index b2bcd560c6..948e90ea5a 100644 --- a/etc/spack/defaults/darwin/packages.yaml +++ b/etc/spack/defaults/darwin/packages.yaml @@ -21,11 +21,14 @@ packages: - gcc - intel providers: - elf: [libelf] - unwind: [apple-libunwind] + elf: + - libelf + unwind: + - apple-libunwind apple-libunwind: - paths: + buildable: false + externals: # Apple bundles libunwind version 35.3 with macOS 10.9 and later, # although the version number used here isn't critical - apple-libunwind@35.3: /usr - buildable: False + - spec: apple-libunwind@35.3 + prefix: /usr diff --git a/lib/spack/docs/build_settings.rst b/lib/spack/docs/build_settings.rst index 9f67d8c14f..3e7a21c3e8 100644 --- a/lib/spack/docs/build_settings.rst +++ b/lib/spack/docs/build_settings.rst @@ -57,10 +57,13 @@ directory. Here's an example of an external configuration: packages: openmpi: - paths: - openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64: /opt/openmpi-1.4.3 - openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug: /opt/openmpi-1.4.3-debug - openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64: /opt/openmpi-1.6.5-intel + externals: + - spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64" + prefix: /opt/openmpi-1.4.3 + - spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug" + prefix: /opt/openmpi-1.4.3-debug + - spec: "openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64" + prefix: /opt/openmpi-1.6.5-intel This example lists three installations of OpenMPI, one built with GCC, one built with GCC and debug information, and another built with Intel. @@ -76,13 +79,15 @@ of the installation prefixes. The following example says that module .. code-block:: yaml cmake: - modules: - cmake@3.7.2: CMake/3.7.2 - -Each ``packages.yaml`` begins with a ``packages:`` token, followed -by a list of package names. To specify externals, add a ``paths`` or ``modules`` -token under the package name, which lists externals in a -``spec: /path`` or ``spec: module-name`` format. Each spec should be as + externals: + - spec: cmake@3.7.2 + modules: + - CMake/3.7.2 + +Each ``packages.yaml`` begins with a ``packages:`` attribute, followed +by a list of package names. To specify externals, add an ``externals:`` +attribute under the package name, which lists externals. +Each external should specify a ``spec:`` string that should be as well-defined as reasonably possible. If a package lacks a spec component, such as missing a compiler or package version, then Spack will guess the missing component based @@ -106,10 +111,13 @@ be: packages: openmpi: - paths: - openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64: /opt/openmpi-1.4.3 - openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug: /opt/openmpi-1.4.3-debug - openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64: /opt/openmpi-1.6.5-intel + externals: + - spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64" + prefix: /opt/openmpi-1.4.3 + - spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug" + prefix: /opt/openmpi-1.4.3-debug + - spec: "openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64" + prefix: /opt/openmpi-1.6.5-intel buildable: False The addition of the ``buildable`` flag tells Spack that it should never build @@ -137,10 +145,13 @@ but more conveniently: mpi: buildable: False openmpi: - paths: - openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64: /opt/openmpi-1.4.3 - openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug: /opt/openmpi-1.4.3-debug - openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64: /opt/openmpi-1.6.5-intel + externals: + - spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64" + prefix: /opt/openmpi-1.4.3 + - spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug" + prefix: /opt/openmpi-1.4.3-debug + - spec: "openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64" + prefix: /opt/openmpi-1.6.5-intel Implementations can also be listed immediately under the virtual they provide: @@ -172,8 +183,9 @@ After running this command your ``packages.yaml`` may include new entries: packages: cmake: - paths: - cmake@3.17.2: /usr + externals: + - spec: cmake@3.17.2 + prefix: /usr Generally this is useful for detecting a small set of commonly-used packages; for now this is generally limited to finding build-only dependencies. diff --git a/lib/spack/docs/build_systems/intelpackage.rst b/lib/spack/docs/build_systems/intelpackage.rst index 66f473cbf8..8594c8d425 100644 --- a/lib/spack/docs/build_systems/intelpackage.rst +++ b/lib/spack/docs/build_systems/intelpackage.rst @@ -418,9 +418,13 @@ Adapt the following example. Be sure to maintain the indentation: # other content ... intel-mkl: - modules: - intel-mkl@2018.2.199 arch=linux-centos6-x86_64: intel-mkl/18/18.0.2 - intel-mkl@2018.3.222 arch=linux-centos6-x86_64: intel-mkl/18/18.0.3 + externals: + - spec: "intel-mkl@2018.2.199 arch=linux-centos6-x86_64" + modules: + - intel-mkl/18/18.0.2 + - spec: "intel-mkl@2018.3.222 arch=linux-centos6-x86_64" + modules: + - intel-mkl/18/18.0.3 The version numbers for the ``intel-mkl`` specs defined here correspond to file and directory names that Intel uses for its products because they were adopted @@ -451,12 +455,16 @@ mechanism. packages: intel-parallel-studio: - modules: - intel-parallel-studio@cluster.2018.2.199 +mkl+mpi+ipp+tbb+daal arch=linux-centos6-x86_64: intel/18/18.0.2 - intel-parallel-studio@cluster.2018.3.222 +mkl+mpi+ipp+tbb+daal arch=linux-centos6-x86_64: intel/18/18.0.3 + externals: + - spec: "intel-parallel-studio@cluster.2018.2.199 +mkl+mpi+ipp+tbb+daal arch=linux-centos6-x86_64" + modules: + - intel/18/18.0.2 + - spec: "intel-parallel-studio@cluster.2018.3.222 +mkl+mpi+ipp+tbb+daal arch=linux-centos6-x86_64" + modules: + - intel/18/18.0.3 buildable: False -One additional example illustrates the use of ``paths:`` instead of +One additional example illustrates the use of ``prefix:`` instead of ``modules:``, useful when external modulefiles are not available or not suitable: @@ -464,13 +472,15 @@ suitable: packages: intel-parallel-studio: - paths: - intel-parallel-studio@cluster.2018.2.199 +mkl+mpi+ipp+tbb+daal: /opt/intel - intel-parallel-studio@cluster.2018.3.222 +mkl+mpi+ipp+tbb+daal: /opt/intel + externals: + - spec: "intel-parallel-studio@cluster.2018.2.199 +mkl+mpi+ipp+tbb+daal" + prefix: /opt/intel + - spec: "intel-parallel-studio@cluster.2018.3.222 +mkl+mpi+ipp+tbb+daal" + prefix: /opt/intel buildable: False Note that for the Intel packages discussed here, the directory values in the -``paths:`` entries must be the high-level and typically version-less +``prefix:`` entries must be the high-level and typically version-less "installation directory" that has been used by Intel's product installer. Such a directory will typically accumulate various product versions. Amongst them, Spack will select the correct version-specific product directory based on diff --git a/lib/spack/docs/getting_started.rst b/lib/spack/docs/getting_started.rst index 550f2509e6..26deb1cef5 100644 --- a/lib/spack/docs/getting_started.rst +++ b/lib/spack/docs/getting_started.rst @@ -712,8 +712,9 @@ an OpenMPI installed in /opt/local, one would use: packages: openmpi: - paths: - openmpi@1.10.1: /opt/local + externals: + - spec: openmpi@1.10.1 + prefix: /opt/local buildable: False In general, Spack is easier to use and more reliable if it builds all of @@ -775,8 +776,9 @@ Then add the following to ``~/.spack/packages.yaml``: packages: openssl: - paths: - openssl@1.0.2g: /usr + externals: + - spec: openssl@1.0.2g + prefix: /usr buildable: False @@ -791,8 +793,9 @@ to add the following to ``packages.yaml``: packages: netlib-lapack: - paths: - netlib-lapack@3.6.1: /usr + externals: + - spec: netlib-lapack@3.6.1 + prefix: /usr buildable: False all: providers: @@ -1181,9 +1184,13 @@ Here's an example of an external configuration for cray modules: packages: mpich: - modules: - mpich@7.3.1%gcc@5.2.0 arch=cray_xc-haswell-CNL10: cray-mpich - mpich@7.3.1%intel@16.0.0.109 arch=cray_xc-haswell-CNL10: cray-mpich + externals: + - spec: "mpich@7.3.1%gcc@5.2.0 arch=cray_xc-haswell-CNL10" + modules: + - cray-mpich + - spec: "mpich@7.3.1%intel@16.0.0.109 arch=cray_xc-haswell-CNL10" + modules: + - cray-mpich all: providers: mpi: [mpich] @@ -1195,7 +1202,7 @@ via module load. .. note:: - For Cray-provided packages, it is best to use ``modules:`` instead of ``paths:`` + For Cray-provided packages, it is best to use ``modules:`` instead of ``prefix:`` in ``packages.yaml``, because the Cray Programming Environment heavily relies on modules (e.g., loading the ``cray-mpich`` module adds MPI libraries to the compiler wrapper link line). @@ -1211,19 +1218,31 @@ Here is an example of a full packages.yaml used at NERSC packages: mpich: - modules: - mpich@7.3.1%gcc@5.2.0 arch=cray_xc-CNL10-ivybridge: cray-mpich - mpich@7.3.1%intel@16.0.0.109 arch=cray_xc-SuSE11-ivybridge: cray-mpich + externals: + - spec: "mpich@7.3.1%gcc@5.2.0 arch=cray_xc-CNL10-ivybridge" + modules: + - cray-mpich + - spec: "mpich@7.3.1%intel@16.0.0.109 arch=cray_xc-SuSE11-ivybridge" + modules: + - cray-mpich buildable: False netcdf: - modules: - netcdf@4.3.3.1%gcc@5.2.0 arch=cray_xc-CNL10-ivybridge: cray-netcdf - netcdf@4.3.3.1%intel@16.0.0.109 arch=cray_xc-CNL10-ivybridge: cray-netcdf + externals: + - spec: "netcdf@4.3.3.1%gcc@5.2.0 arch=cray_xc-CNL10-ivybridge" + modules: + - cray-netcdf + - spec: "netcdf@4.3.3.1%intel@16.0.0.109 arch=cray_xc-CNL10-ivybridge" + modules: + - cray-netcdf buildable: False hdf5: - modules: - hdf5@1.8.14%gcc@5.2.0 arch=cray_xc-CNL10-ivybridge: cray-hdf5 - hdf5@1.8.14%intel@16.0.0.109 arch=cray_xc-CNL10-ivybridge: cray-hdf5 + externals: + - spec: "hdf5@1.8.14%gcc@5.2.0 arch=cray_xc-CNL10-ivybridge" + modules: + - cray-hdf5 + - spec: "hdf5@1.8.14%intel@16.0.0.109 arch=cray_xc-CNL10-ivybridge" + modules: + - cray-hdf5 buildable: False all: compiler: [gcc@5.2.0, intel@16.0.0.109] diff --git a/lib/spack/docs/workflows.rst b/lib/spack/docs/workflows.rst index 2f215f8209..7deb5eff54 100644 --- a/lib/spack/docs/workflows.rst +++ b/lib/spack/docs/workflows.rst @@ -1545,8 +1545,9 @@ Avoid double-installing CUDA by adding, e.g. packages: cuda: - paths: - cuda@9.0.176%gcc@5.4.0 arch=linux-ubuntu16-x86_64: /usr/local/cuda + externals: + - spec: "cuda@9.0.176%gcc@5.4.0 arch=linux-ubuntu16-x86_64" + prefix: /usr/local/cuda buildable: False to your ``packages.yaml``. diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index bee6e1e43c..5e08273677 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -2,7 +2,6 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - import collections import errno import hashlib @@ -377,17 +376,17 @@ def install(src, dest): copy(src, dest, _permissions=True) -def resolve_link_target_relative_to_the_link(l): +def resolve_link_target_relative_to_the_link(link): """ os.path.isdir uses os.path.exists, which for links will check the existence of the link target. If the link target is relative to the link, we need to construct a pathname that is valid from our cwd (which may not be the same as the link's directory) """ - target = os.readlink(l) + target = os.readlink(link) if os.path.isabs(target): return target - link_dir = os.path.dirname(os.path.abspath(l)) + link_dir = os.path.dirname(os.path.abspath(link)) return os.path.join(link_dir, target) @@ -1571,6 +1570,19 @@ def can_access_dir(path): @memoized +def can_write_to_dir(path): + """Return True if the argument is a directory in which we can write. + + Args: + path: path to be tested + + Returns: + True if ``path`` is an writeable directory, else False + """ + return os.path.isdir(path) and os.access(path, os.R_OK | os.X_OK | os.W_OK) + + +@memoized def files_in(*search_paths): """Returns all the files in paths passed as arguments. @@ -1683,3 +1695,18 @@ def prefixes(path): pass return paths + + +def md5sum(file): + """Compute the MD5 sum of a file. + + Args: + file (str): file to be checksummed + + Returns: + MD5 sum of the file's content + """ + md5 = hashlib.md5() + with open(file, "rb") as f: + md5.update(f.read()) + return md5.digest() diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 7ef2126766..7206ddb247 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -62,7 +62,7 @@ from spack.util.environment import ( from spack.util.environment import system_dirs from spack.error import NoLibrariesError, NoHeadersError from spack.util.executable import Executable -from spack.util.module_cmd import load_module, get_path_from_module, module +from spack.util.module_cmd import load_module, path_from_modules, module from spack.util.log_parse import parse_log_events, make_log_context @@ -642,7 +642,7 @@ def get_rpaths(pkg): # Second module is our compiler mod name. We use that to get rpaths from # module show output. if pkg.compiler.modules and len(pkg.compiler.modules) > 1: - rpaths.append(get_path_from_module(pkg.compiler.modules[1])) + rpaths.append(path_from_modules(pkg.compiler.modules[1])) return list(dedupe(filter_system_paths(rpaths))) @@ -706,8 +706,9 @@ def load_external_modules(pkg): pkg (PackageBase): package to load deps for """ for dep in list(pkg.spec.traverse()): - if dep.external_module: - load_module(dep.external_module) + external_modules = dep.external_modules or [] + for external_module in external_modules: + load_module(external_module) def setup_package(pkg, dirty): diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py index 954d4e4585..e684364d8a 100644 --- a/lib/spack/spack/cmd/config.py +++ b/lib/spack/spack/cmd/config.py @@ -2,16 +2,20 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - from __future__ import print_function + +import collections import os import re +import shutil +import llnl.util.filesystem as fs import llnl.util.tty as tty - import spack.config +import spack.cmd.common.arguments import spack.schema.env import spack.environment as ev +import spack.schema.packages import spack.util.spack_yaml as syaml from spack.util.editor import editor @@ -80,6 +84,19 @@ def setup_parser(subparser): # Make the add parser available later setup_parser.add_parser = add_parser + update = sp.add_parser( + 'update', help='update configuration files to the latest format' + ) + spack.cmd.common.arguments.add_common_arguments(update, ['yes_to_all']) + update.add_argument('section', help='section to update') + + revert = sp.add_parser( + 'revert', + help='revert configuration files to their state before update' + ) + spack.cmd.common.arguments.add_common_arguments(revert, ['yes_to_all']) + revert.add_argument('section', help='section to update') + def _get_scope_and_section(args): """Extract config scope and section from arguments.""" @@ -275,12 +292,164 @@ def config_remove(args): set_config(args, path, existing, scope) +def _can_update_config_file(scope_dir, cfg_file): + dir_ok = fs.can_write_to_dir(scope_dir) + cfg_ok = fs.can_access(cfg_file) + return dir_ok and cfg_ok + + +def config_update(args): + # Read the configuration files + spack.config.config.get_config(args.section, scope=args.scope) + updates = spack.config.config.format_updates[args.section] + + cannot_overwrite, skip_system_scope = [], False + for scope in updates: + cfg_file = spack.config.config.get_config_filename( + scope.name, args.section + ) + scope_dir = scope.path + can_be_updated = _can_update_config_file(scope_dir, cfg_file) + if not can_be_updated: + if scope.name == 'system': + skip_system_scope = True + msg = ('Not enough permissions to write to "system" scope. ' + 'Skipping update at that location [cfg={0}]') + tty.warn(msg.format(cfg_file)) + continue + cannot_overwrite.append((scope, cfg_file)) + + if cannot_overwrite: + msg = 'Detected permission issues with the following scopes:\n\n' + for scope, cfg_file in cannot_overwrite: + msg += '\t[scope={0}, cfg={1}]\n'.format(scope.name, cfg_file) + msg += ('\nEither ensure that you have sufficient permissions to ' + 'modify these files or do not include these scopes in the ' + 'update.') + tty.die(msg) + + if skip_system_scope: + updates = [x for x in updates if x.name != 'system'] + + # Report if there are no updates to be done + if not updates: + msg = 'No updates needed for "{0}" section.' + tty.msg(msg.format(args.section)) + return + + proceed = True + if not args.yes_to_all: + msg = ('The following configuration files are going to be updated to' + ' the latest schema format:\n\n') + for scope in updates: + cfg_file = spack.config.config.get_config_filename( + scope.name, args.section + ) + msg += '\t[scope={0}, file={1}]\n'.format(scope.name, cfg_file) + msg += ('\nIf the configuration files are updated, versions of Spack ' + 'that are older than this version may not be able to read ' + 'them. Spack stores backups of the updated files which can ' + 'be retrieved with "spack config revert"') + tty.msg(msg) + proceed = tty.get_yes_or_no('Do you want to proceed?', default=False) + + if not proceed: + tty.die('Operation aborted.') + + # Get a function to update the format + update_fn = spack.config.ensure_latest_format_fn(args.section) + for scope in updates: + cfg_file = spack.config.config.get_config_filename( + scope.name, args.section + ) + with open(cfg_file) as f: + data = syaml.load(f) or {} + data = data.pop(args.section, {}) + update_fn(data) + + # Make a backup copy and rewrite the file + bkp_file = cfg_file + '.bkp' + shutil.copy(cfg_file, bkp_file) + spack.config.config.update_config( + args.section, data, scope=scope.name, force=True + ) + msg = 'File "{0}" updated [backup={1}]' + tty.msg(msg.format(cfg_file, bkp_file)) + + +def _can_revert_update(scope_dir, cfg_file, bkp_file): + dir_ok = fs.can_write_to_dir(scope_dir) + cfg_ok = not os.path.exists(cfg_file) or fs.can_access(cfg_file) + bkp_ok = fs.can_access(bkp_file) + return dir_ok and cfg_ok and bkp_ok + + +def config_revert(args): + scopes = [args.scope] if args.scope else [ + x.name for x in spack.config.config.file_scopes + ] + + # Search for backup files in the configuration scopes + Entry = collections.namedtuple('Entry', ['scope', 'cfg', 'bkp']) + to_be_restored, cannot_overwrite = [], [] + for scope in scopes: + cfg_file = spack.config.config.get_config_filename(scope, args.section) + bkp_file = cfg_file + '.bkp' + + # If the backup files doesn't exist move to the next scope + if not os.path.exists(bkp_file): + continue + + # If it exists and we don't have write access in this scope + # keep track of it and report a comprehensive error later + entry = Entry(scope, cfg_file, bkp_file) + scope_dir = os.path.dirname(bkp_file) + can_be_reverted = _can_revert_update(scope_dir, cfg_file, bkp_file) + if not can_be_reverted: + cannot_overwrite.append(entry) + continue + + to_be_restored.append(entry) + + # Report errors if we can't revert a configuration + if cannot_overwrite: + msg = 'Detected permission issues with the following scopes:\n\n' + for e in cannot_overwrite: + msg += '\t[scope={0.scope}, cfg={0.cfg}, bkp={0.bkp}]\n'.format(e) + msg += ('\nEither ensure to have the right permissions before retrying' + ' or be more specific on the scope to revert.') + tty.die(msg) + + proceed = True + if not args.yes_to_all: + msg = ('The following scopes will be restored from the corresponding' + ' backup files:\n') + for entry in to_be_restored: + msg += '\t[scope={0.scope}, bkp={0.bkp}]\n'.format(entry) + msg += 'This operation cannot be undone.' + tty.msg(msg) + proceed = tty.get_yes_or_no('Do you want to proceed?', default=False) + + if not proceed: + tty.die('Operation aborted.') + + for _, cfg_file, bkp_file in to_be_restored: + shutil.copy(bkp_file, cfg_file) + os.unlink(bkp_file) + msg = 'File "{0}" reverted to old state' + tty.msg(msg.format(cfg_file)) + + def config(parser, args): - action = {'get': config_get, - 'blame': config_blame, - 'edit': config_edit, - 'list': config_list, - 'add': config_add, - 'rm': config_remove, - 'remove': config_remove} + action = { + 'get': config_get, + 'blame': config_blame, + 'edit': config_edit, + 'list': config_list, + 'add': config_add, + 'rm': config_remove, + 'remove': config_remove, + 'update': config_update, + 'revert': config_revert + } action[args.config_command](args) diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index e3c45cc27b..7bd8052528 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import os +import shutil import sys from collections import namedtuple @@ -14,6 +15,7 @@ from llnl.util.tty.color import colorize import spack.config import spack.schema.env +import spack.cmd.common.arguments import spack.cmd.install import spack.cmd.uninstall import spack.cmd.modules @@ -37,6 +39,8 @@ subcommands = [ ['status', 'st'], 'loads', 'view', + 'update', + 'revert' ] @@ -394,6 +398,80 @@ def env_loads(args): print(' source %s' % loads_file) +def env_update_setup_parser(subparser): + """update environments to the latest format""" + subparser.add_argument( + metavar='env', dest='env', + help='name or directory of the environment to activate' + ) + spack.cmd.common.arguments.add_common_arguments(subparser, ['yes_to_all']) + + +def env_update(args): + manifest_file = ev.manifest_file(args.env) + backup_file = manifest_file + ".bkp" + needs_update = not ev.is_latest_format(manifest_file) + + if not needs_update: + tty.msg('No update needed for the environment "{0}"'.format(args.env)) + return + + proceed = True + if not args.yes_to_all: + msg = ('The environment "{0}" is going to be updated to the latest ' + 'schema format.\nIf the environment is updated, versions of ' + 'Spack that are older than this version may not be able to ' + 'read it. Spack stores backups of the updated environment ' + 'which can be retrieved with "spack env revert"') + tty.msg(msg.format(args.env)) + proceed = tty.get_yes_or_no('Do you want to proceed?', default=False) + + if not proceed: + tty.die('Operation aborted.') + + ev.update_yaml(manifest_file, backup_file=backup_file) + msg = 'Environment "{0}" has been updated [backup={1}]' + tty.msg(msg.format(args.env, backup_file)) + + +def env_revert_setup_parser(subparser): + """restore environments to their state before update""" + subparser.add_argument( + metavar='env', dest='env', + help='name or directory of the environment to activate' + ) + spack.cmd.common.arguments.add_common_arguments(subparser, ['yes_to_all']) + + +def env_revert(args): + manifest_file = ev.manifest_file(args.env) + backup_file = manifest_file + ".bkp" + + # Check that both the spack.yaml and the backup exist, the inform user + # on what is going to happen and ask for confirmation + if not os.path.exists(manifest_file): + msg = 'cannot fine the manifest file of the environment [file={0}]' + tty.die(msg.format(manifest_file)) + if not os.path.exists(backup_file): + msg = 'cannot find the old manifest file to be restored [file={0}]' + tty.die(msg.format(backup_file)) + + proceed = True + if not args.yes_to_all: + msg = ('Spack is going to overwrite the current manifest file' + ' with a backup copy [manifest={0}, backup={1}]') + tty.msg(msg.format(manifest_file, backup_file)) + proceed = tty.get_yes_or_no('Do you want to proceed?', default=False) + + if not proceed: + tty.die('Operation aborted.') + + shutil.copy(backup_file, manifest_file) + os.remove(backup_file) + msg = 'Environment "{0}" reverted to old state' + tty.msg(msg.format(manifest_file)) + + #: Dictionary mapping subcommand names and aliases to functions subcommand_functions = {} diff --git a/lib/spack/spack/cmd/external.py b/lib/spack/spack/cmd/external.py index afdd40e2a0..6fe7825070 100644 --- a/lib/spack/spack/cmd/external.py +++ b/lib/spack/spack/cmd/external.py @@ -74,19 +74,28 @@ def _generate_pkg_config(external_pkg_entries): This does not generate the entire packages.yaml. For example, given some external entries for the CMake package, this could return:: - { 'paths': { - 'cmake@3.17.1': '/opt/cmake-3.17.1/', - 'cmake@3.16.5': '/opt/cmake-3.16.5/' - } + { + 'externals': [{ + 'spec': 'cmake@3.17.1', + 'prefix': '/opt/cmake-3.17.1/' + }, { + 'spec': 'cmake@3.16.5', + 'prefix': '/opt/cmake-3.16.5/' + }] } """ - paths_dict = syaml.syaml_dict() + + pkg_dict = syaml.syaml_dict() + pkg_dict['externals'] = [] for e in external_pkg_entries: if not _spec_is_valid(e.spec): continue - paths_dict[str(e.spec)] = e.base_dir - pkg_dict = syaml.syaml_dict() - pkg_dict['paths'] = paths_dict + + external_items = [('spec', str(e.spec)), ('prefix', e.base_dir)] + external_items.extend(e.spec.extra_attributes.items()) + pkg_dict['externals'].append( + syaml.syaml_dict(external_items) + ) return pkg_dict @@ -259,6 +268,14 @@ def _get_external_packages(packages_to_check, system_path_to_exe=None): else: resolved_specs[spec] = prefix + try: + spec.validate_detection() + except Exception as e: + msg = ('"{0}" has been detected on the system but will ' + 'not be added to packages.yaml [{1}]') + tty.warn(msg.format(spec, str(e))) + continue + pkg_to_entries[pkg.name].append( ExternalPackageEntry(spec=spec, base_dir=pkg_prefix)) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index a3d8101cad..425fcec8ee 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -30,6 +30,7 @@ schemas are in submodules of :py:mod:`spack.schema`. """ +import collections import copy import os import re @@ -352,6 +353,7 @@ class Configuration(object): self.scopes = OrderedDict() for scope in scopes: self.push_scope(scope) + self.format_updates = collections.defaultdict(list) def push_scope(self, scope): """Add a higher precedence scope to the Configuration.""" @@ -440,7 +442,7 @@ class Configuration(object): for scope in self.scopes.values(): scope.clear() - def update_config(self, section, update_data, scope=None): + def update_config(self, section, update_data, scope=None, force=False): """Update the configuration file for a particular scope. Overwrites contents of a section in a scope with update_data, @@ -449,7 +451,26 @@ class Configuration(object): update_data should have the top-level section name stripped off (it will be re-added). Data itself can be a list, dict, or any other yaml-ish structure. + + Configuration scopes that are still written in an old schema + format will fail to update unless ``force`` is True. + + Args: + section (str): section of the configuration to be updated + update_data (dict): data to be used for the update + scope (str): scope to be updated + force (str): force the update """ + if self.format_updates.get(section) and not force: + msg = ('The "{0}" section of the configuration needs to be written' + ' to disk, but is currently using a deprecated format. ' + 'Please update it using:\n\n' + '\tspack config [--scope=<scope] update {0}\n\n' + 'Note that previous versions of Spack will not be able to ' + 'use the updated configuration.') + msg = msg.format(section) + raise RuntimeError(msg) + _validate_section_name(section) # validate section name scope = self._validate_scope(scope) # get ConfigScope object @@ -514,6 +535,15 @@ class Configuration(object): if section not in data: continue + # We might be reading configuration files in an old format, + # thus read data and update it in memory if need be. + changed = _update_in_memory(data, section) + if changed: + self.format_updates[section].append(scope) + msg = ('OUTDATED CONFIGURATION FILE ' + '[section={0}, scope={1}, dir={2}]') + tty.debug(msg.format(section, scope.name, scope.path)) + merged_section = merge_yaml(merged_section, data) # no config files -- empty config. @@ -723,7 +753,7 @@ def get(path, default=None, scope=None): def set(path, value, scope=None): - """Convenience function for getting single values in config files. + """Convenience function for setting single values in config files. Accepts the path syntax described in ``get()``. """ @@ -999,6 +1029,41 @@ def default_list_scope(): return None +def _update_in_memory(data, section): + """Update the format of the configuration data in memory. + + This function assumes the section is valid (i.e. validation + is responsibility of the caller) + + Args: + data (dict): configuration data + section (str): section of the configuration to update + + Returns: + True if the data was changed, False otherwise + """ + update_fn = ensure_latest_format_fn(section) + changed = update_fn(data[section]) + return changed + + +def ensure_latest_format_fn(section): + """Return a function that takes as input a dictionary read from + a configuration file and update it to the latest format. + + The function returns True if there was any update, False otherwise. + + Args: + section (str): section of the configuration e.g. "packages", + "config", etc. + """ + # The line below is based on the fact that every module we need + # is already imported at the top level + section_module = getattr(spack.schema, section) + update_fn = getattr(section_module, 'update', lambda x: False) + return update_fn + + class ConfigError(SpackError): """Superclass for all Spack config related errors.""" diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index a580f1169d..1423acbddb 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1473,6 +1473,18 @@ class Environment(object): writing if True. """ + # Intercept environment not using the latest schema format and prevent + # them from being modified + manifest_exists = os.path.exists(self.manifest_path) + if manifest_exists and not is_latest_format(self.manifest_path): + msg = ('The environment "{0}" needs to be written to disk, but ' + 'is currently using a deprecated format. Please update it ' + 'using:\n\n' + '\tspack env update {0}\n\n' + 'Note that previous versions of Spack will not be able to ' + 'use the updated configuration.') + raise RuntimeError(msg.format(self.name)) + # ensure path in var/spack/environments fs.mkdirp(self.path) @@ -1723,5 +1735,92 @@ def deactivate_config_scope(env): spack.config.config.remove_scope(scope.name) +def manifest_file(env_name_or_dir): + """Return the absolute path to a manifest file given the environment + name or directory. + + Args: + env_name_or_dir (str): either the name of a valid environment + or a directory where a manifest file resides + + Raises: + AssertionError: if the environment is not found + """ + env_dir = None + if is_env_dir(env_name_or_dir): + env_dir = os.path.abspath(env_name_or_dir) + elif exists(env_name_or_dir): + env_dir = os.path.abspath(root(env_name_or_dir)) + + assert env_dir, "environment not found [env={0}]".format(env_name_or_dir) + return os.path.join(env_dir, manifest_name) + + +def update_yaml(manifest, backup_file): + """Update a manifest file from an old format to the current one. + + Args: + manifest (str): path to a manifest file + backup_file (str): file where to copy the original manifest + + Returns: + True if the manifest was updated, False otherwise. + + Raises: + AssertionError: in case anything goes wrong during the update + """ + # Check if the environment needs update + with open(manifest) as f: + data = syaml.load(f) + + top_level_key = _top_level_key(data) + needs_update = spack.schema.env.update(data[top_level_key]) + if not needs_update: + msg = "No update needed [manifest={0}]".format(manifest) + tty.debug(msg) + return False + + # Copy environment to a backup file and update it + msg = ('backup file "{0}" already exists on disk. Check its content ' + 'and remove it before trying to update again.') + assert not os.path.exists(backup_file), msg.format(backup_file) + + shutil.copy(manifest, backup_file) + with open(manifest, 'w') as f: + _write_yaml(data, f) + return True + + +def _top_level_key(data): + """Return the top level key used in this environment + + Args: + data (dict): raw yaml data of the environment + + Returns: + Either 'spack' or 'env' + """ + msg = ('cannot find top level attribute "spack" or "env"' + 'in the environment') + assert any(x in data for x in ('spack', 'env')), msg + if 'spack' in data: + return 'spack' + return 'env' + + +def is_latest_format(manifest): + """Return True if the manifest file is at the latest schema format, + False otherwise. + + Args: + manifest (str): manifest file to be analyzed + """ + with open(manifest) as f: + data = syaml.load(f) + top_level_key = _top_level_key(data) + changed = spack.schema.env.update(data[top_level_key]) + return not changed + + class SpackEnvironmentError(spack.error.SpackError): """Superclass for all errors to do with Spack environments.""" diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 2d4b488ac3..b8bbe7ce3f 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -272,9 +272,9 @@ def _process_external_package(pkg, explicit): pre = '{s.name}@{s.version} :'.format(s=pkg.spec) spec = pkg.spec - if spec.external_module: + if spec.external_modules: tty.msg('{0} has external module in {1}' - .format(pre, spec.external_module)) + .format(pre, spec.external_modules)) tty.debug('{0} is actually installed in {1}' .format(pre, spec.external_path)) else: diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py index 67325fc7ae..bdedfcfb3d 100644 --- a/lib/spack/spack/package_prefs.py +++ b/lib/spack/spack/package_prefs.py @@ -2,7 +2,6 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - import stat from six import string_types @@ -158,7 +157,7 @@ def spec_externals(spec): """Return a list of external specs (w/external directory path filled in), one for each known external installation.""" # break circular import. - from spack.util.module_cmd import get_path_from_module # NOQA: ignore=F401 + from spack.util.module_cmd import path_from_modules # NOQA: ignore=F401 allpkgs = spack.config.get('packages') names = set([spec.name]) @@ -167,24 +166,24 @@ def spec_externals(spec): external_specs = [] for name in names: pkg_config = allpkgs.get(name, {}) - pkg_paths = pkg_config.get('paths', {}) - pkg_modules = pkg_config.get('modules', {}) - if (not pkg_paths) and (not pkg_modules): - continue - - for external_spec, path in pkg_paths.items(): - external_spec = spack.spec.Spec( - external_spec, external_path=canonicalize_path(path)) - if external_spec.satisfies(spec): - external_specs.append(external_spec) - - for external_spec, module in pkg_modules.items(): - external_spec = spack.spec.Spec( - external_spec, external_module=module) + pkg_externals = pkg_config.get('externals', []) + for entry in pkg_externals: + spec_str = entry['spec'] + external_path = entry.get('prefix', None) + if external_path: + external_path = canonicalize_path(external_path) + external_modules = entry.get('modules', None) + external_spec = spack.spec.Spec.from_detection( + spack.spec.Spec( + spec_str, + external_path=external_path, + external_modules=external_modules + ), extra_attributes=entry.get('extra_attributes', {}) + ) if external_spec.satisfies(spec): external_specs.append(external_spec) - # defensively copy returned specs + # Defensively copy returned specs return [s.copy() for s in external_specs] diff --git a/lib/spack/spack/schema/__init__.py b/lib/spack/spack/schema/__init__.py index 755e3d9086..38cc36caef 100644 --- a/lib/spack/spack/schema/__init__.py +++ b/lib/spack/spack/schema/__init__.py @@ -90,11 +90,15 @@ def _make_validator(): is_error = deprecated['error'] if not is_error: for entry in deprecated_properties: - llnl.util.tty.warn(msg.format(property=entry)) + llnl.util.tty.warn( + msg.format(property=entry, entry=instance[entry]) + ) else: import jsonschema for entry in deprecated_properties: - yield jsonschema.ValidationError(msg.format(property=entry)) + yield jsonschema.ValidationError( + msg.format(property=entry, entry=instance[entry]) + ) return jsonschema.validators.extend( jsonschema.Draft4Validator, { diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 6ead76416b..18a2048557 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -8,9 +8,12 @@ .. literalinclude:: _spack_root/lib/spack/spack/schema/env.py :lines: 36- """ +import warnings + from llnl.util.lang import union_dicts import spack.schema.merged +import spack.schema.packages import spack.schema.projections #: legal first keys in the schema @@ -133,3 +136,22 @@ schema = { } } } + + +def update(data): + """Update the data in place to remove deprecated properties. + + Args: + data (dict): dictionary to be updated + + Returns: + True if data was changed, False otherwise + """ + if 'include' in data: + msg = ("included configuration files should be updated manually" + " [files={0}]") + warnings.warn(msg.format(', '.join(data['include']))) + + if 'packages' in data: + return spack.schema.packages.update(data['packages']) + return False diff --git a/lib/spack/spack/schema/packages.py b/lib/spack/spack/schema/packages.py index 4984471c73..e97e953b83 100644 --- a/lib/spack/spack/schema/packages.py +++ b/lib/spack/spack/schema/packages.py @@ -2,7 +2,6 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - """Schema for packages.yaml configuration files. .. literalinclude:: _spack_root/lib/spack/spack/schema/packages.py @@ -59,10 +58,6 @@ properties = { }, }, }, - 'modules': { - 'type': 'object', - 'default': {}, - }, 'providers': { 'type': 'object', 'default': {}, @@ -72,17 +67,39 @@ properties = { 'type': 'array', 'default': [], 'items': {'type': 'string'}, }, }, }, - 'paths': { - 'type': 'object', - 'default': {}, - }, 'variants': { 'oneOf': [ {'type': 'string'}, {'type': 'array', 'items': {'type': 'string'}}], }, + 'externals': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'spec': {'type': 'string'}, + 'prefix': {'type': 'string'}, + 'modules': {'type': 'array', + 'items': {'type': 'string'}}, + 'extra_attributes': {'type': 'object'} + }, + 'additionalProperties': True, + 'required': ['spec'] + } + }, + # Deprecated properties, will trigger an error with a + # message telling how to update. + 'paths': {'type': 'object'}, + 'modules': {'type': 'object'}, }, + 'deprecatedProperties': { + 'properties': ['modules', 'paths'], + 'message': 'the attribute "{property}" in the "packages" ' + 'section of the configuration has been ' + 'deprecated [entry={entry}]', + 'error': False + } }, }, }, @@ -97,3 +114,33 @@ schema = { 'additionalProperties': False, 'properties': properties, } + + +def update(data): + """Update in-place the data to remove deprecated properties. + + Args: + data (dict): dictionary to be updated + + Returns: + True if data was changed, False otherwise + """ + changed = False + for cfg_object in data.values(): + externals = [] + paths = cfg_object.pop('paths', {}) + for spec, prefix in paths.items(): + externals.append({ + 'spec': str(spec), + 'prefix': str(prefix) + }) + modules = cfg_object.pop('modules', {}) + for spec, module in modules.items(): + externals.append({ + 'spec': str(spec), + 'modules': [str(module)] + }) + if externals: + changed = True + cfg_object['externals'] = externals + return changed diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 227652168c..941656a964 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -959,7 +959,7 @@ class Spec(object): def __init__(self, spec_like=None, normal=False, concrete=False, external_path=None, - external_module=None, full_hash=None): + external_modules=None, full_hash=None): """Create a new Spec. Arguments: @@ -988,8 +988,6 @@ class Spec(object): self.variants = vt.VariantMap(self) self.architecture = None self.compiler = None - self.external_path = None - self.external_module = None self.compiler_flags = FlagMap(self) self._dependents = DependencyMap() self._dependencies = DependencyMap() @@ -1010,9 +1008,13 @@ class Spec(object): self._normal = normal self._concrete = concrete self.external_path = external_path - self.external_module = external_module + self.external_modules = external_modules self._full_hash = full_hash + # This attribute is used to store custom information for + # external specs. None signal that it was not set yet. + self.extra_attributes = None + if isinstance(spec_like, six.string_types): spec_list = SpecParser(self).parse(spec_like) if len(spec_list) > 1: @@ -1025,7 +1027,7 @@ class Spec(object): @property def external(self): - return bool(self.external_path) or bool(self.external_module) + return bool(self.external_path) or bool(self.external_modules) def get_dependency(self, name): dep = self._dependencies.get(name) @@ -1526,7 +1528,8 @@ class Spec(object): if self.external: d['external'] = syaml.syaml_dict([ ('path', self.external_path), - ('module', self.external_module), + ('module', self.external_modules), + ('extra_attributes', self.extra_attributes) ]) if not self._concrete: @@ -1695,21 +1698,21 @@ class Spec(object): for name in FlagMap.valid_compiler_flags(): spec.compiler_flags[name] = [] + spec.external_path = None + spec.external_modules = None if 'external' in node: - spec.external_path = None - spec.external_module = None # This conditional is needed because sometimes this function is # called with a node already constructed that contains a 'versions' # and 'external' field. Related to virtual packages provider # indexes. if node['external']: spec.external_path = node['external']['path'] - spec.external_module = node['external']['module'] - if spec.external_module is False: - spec.external_module = None - else: - spec.external_path = None - spec.external_module = None + spec.external_modules = node['external']['module'] + if spec.external_modules is False: + spec.external_modules = None + spec.extra_attributes = node['external'].get( + 'extra_attributes', syaml.syaml_dict() + ) # specs read in are concrete unless marked abstract spec._concrete = node.get('concrete', True) @@ -1970,6 +1973,44 @@ class Spec(object): tty.debug(e) raise sjson.SpackJSONError("error parsing JSON spec:", str(e)) + @staticmethod + def from_detection(spec_str, extra_attributes=None): + """Construct a spec from a spec string determined during external + detection and attach extra attributes to it. + + Args: + spec_str (str): spec string + extra_attributes (dict): dictionary containing extra attributes + + Returns: + spack.spec.Spec: external spec + """ + s = Spec(spec_str) + extra_attributes = syaml.sorted_dict(extra_attributes or {}) + # This is needed to be able to validate multi-valued variants, + # otherwise they'll still be abstract in the context of detection. + vt.substitute_abstract_variants(s) + s.extra_attributes = extra_attributes + return s + + def validate_detection(self): + """Validate the detection of an external spec. + + This method is used as part of Spack's detection protocol, and is + not meant for client code use. + """ + # Assert that _extra_attributes is a Mapping and not None, + # which likely means the spec was created with Spec.from_detection + msg = ('cannot validate "{0}" since it was not created ' + 'using Spec.from_detection'.format(self)) + assert isinstance(self.extra_attributes, collections.Mapping), msg + + # Validate the spec calling a package specific method + validate_fn = getattr( + self.package, 'validate_detected_spec', lambda x, y: None + ) + validate_fn(self, self.extra_attributes) + def _concretize_helper(self, concretizer, presets=None, visited=None): """Recursive helper function for concretize(). This concretizes everything bottom-up. As things are @@ -2115,8 +2156,8 @@ class Spec(object): feq(replacement.variants, spec.variants) and feq(replacement.external_path, spec.external_path) and - feq(replacement.external_module, - spec.external_module)): + feq(replacement.external_modules, + spec.external_modules)): continue # Refine this spec to the candidate. This uses # replace_with AND dup so that it can work in @@ -2250,7 +2291,7 @@ class Spec(object): t[-1] for t in ordered_hashes) for s in self.traverse(): - if s.external_module and not s.external_path: + if s.external_modules and not s.external_path: compiler = spack.compilers.compiler_for_spec( s.compiler, s.architecture) for mod in compiler.modules: @@ -2259,8 +2300,8 @@ class Spec(object): # get the path from the module # the package can override the default s.external_path = getattr(s.package, 'external_prefix', - md.get_path_from_module( - s.external_module)) + md.path_from_modules( + s.external_modules)) # Mark everything in the spec as concrete, as well. self._mark_concrete() @@ -3046,7 +3087,7 @@ class Spec(object): self._normal != other._normal and self.concrete != other.concrete and self.external_path != other.external_path and - self.external_module != other.external_module and + self.external_modules != other.external_modules and self.compiler_flags != other.compiler_flags) self._package = None @@ -3074,7 +3115,8 @@ class Spec(object): self.variants.spec = self self.external_path = other.external_path - self.external_module = other.external_module + self.external_modules = other.external_modules + self.extra_attributes = other.extra_attributes self.namespace = other.namespace # Cached fields are results of expensive operations. diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index afa7c7fc07..7e61a885ff 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -527,14 +527,10 @@ spack: ci_cmd('generate', '--output-file', outputfile) with open(outputfile) as f: - contents = f.read() - print('generated contents: ') - print(contents) - yaml_contents = syaml.load(contents) - for ci_key in yaml_contents.keys(): - if 'externaltool' in ci_key: - print('Erroneously staged "externaltool" pkg') - assert(False) + yaml_contents = syaml.load(f) + + # Check that the "externaltool" package was not erroneously staged + assert not any('externaltool' in key for key in yaml_contents) def test_ci_generate_debug_with_custom_spack(tmpdir, mutable_mock_env_path, diff --git a/lib/spack/spack/test/cmd/config.py b/lib/spack/spack/test/cmd/config.py index 6dbf50676d..524636fed6 100644 --- a/lib/spack/spack/test/cmd/config.py +++ b/lib/spack/spack/test/cmd/config.py @@ -2,17 +2,40 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) -import pytest import os -from llnl.util.filesystem import mkdirp +import pytest +import llnl.util.filesystem as fs import spack.config import spack.environment as ev -from spack.main import SpackCommand - -config = SpackCommand('config') -env = SpackCommand('env') +import spack.main +import spack.util.spack_yaml as syaml + +config = spack.main.SpackCommand('config') +env = spack.main.SpackCommand('env') + + +@pytest.fixture() +def packages_yaml_v015(mutable_config): + """Create a packages.yaml in the old format""" + def _create(scope=None): + old_data = { + 'packages': { + 'cmake': { + 'paths': {'cmake@3.14.0': '/usr'} + }, + 'gcc': { + 'modules': {'gcc@8.3.0': 'gcc-8'} + } + } + } + scope = scope or spack.config.default_modify_scope() + cfg_file = spack.config.config.get_config_filename(scope, 'packages') + with open(cfg_file, 'w') as f: + syaml.dump(old_data, stream=f) + return cfg_file + return _create def test_get_config_scope(mock_low_high_config): @@ -23,8 +46,8 @@ def test_get_config_scope_merged(mock_low_high_config): low_path = mock_low_high_config.scopes['low'].path high_path = mock_low_high_config.scopes['high'].path - mkdirp(low_path) - mkdirp(high_path) + fs.mkdirp(low_path) + fs.mkdirp(high_path) with open(os.path.join(low_path, 'repos.yaml'), 'w') as f: f.write('''\ @@ -403,3 +426,104 @@ def test_config_remove_from_env(mutable_empty_config, mutable_mock_env_path): """ assert output == expected + + +def test_config_update_packages(packages_yaml_v015): + """Test Spack updating old packages.yaml format for externals + to new format. Ensure that data is preserved and converted + properly. + """ + packages_yaml_v015() + config('update', '-y', 'packages') + + # Check the entries have been transformed + data = spack.config.get('packages') + check_update(data) + + +def test_config_update_not_needed(mutable_config): + data_before = spack.config.get('repos') + config('update', '-y', 'repos') + data_after = spack.config.get('repos') + assert data_before == data_after + + +def test_config_update_fail_on_permission_issue( + packages_yaml_v015, monkeypatch +): + # The first time it will update and create the backup file + packages_yaml_v015() + # Mock a global scope where we cannot write + monkeypatch.setattr( + spack.cmd.config, '_can_update_config_file', lambda x, y: False + ) + with pytest.raises(spack.main.SpackCommandError): + config('update', '-y', 'packages') + + +def test_config_revert(packages_yaml_v015): + cfg_file = packages_yaml_v015() + bkp_file = cfg_file + '.bkp' + + config('update', '-y', 'packages') + + # Check that the backup file exists, compute its md5 sum + assert os.path.exists(bkp_file) + md5bkp = fs.md5sum(bkp_file) + + config('revert', '-y', 'packages') + + # Check that the backup file does not exist anymore and + # that the md5 sum of the configuration file is the same + # as that of the old backup file + assert not os.path.exists(bkp_file) + assert md5bkp == fs.md5sum(cfg_file) + + +def test_config_revert_raise_if_cant_write(packages_yaml_v015, monkeypatch): + packages_yaml_v015() + config('update', '-y', 'packages') + + # Mock a global scope where we cannot write + monkeypatch.setattr( + spack.cmd.config, '_can_revert_update', lambda x, y, z: False + ) + # The command raises with an helpful error if a configuration + # file is to be deleted and we don't have sufficient permissions + with pytest.raises(spack.main.SpackCommandError): + config('revert', '-y', 'packages') + + +def test_updating_config_implicitly_raises(packages_yaml_v015): + # Trying to write implicitly to a scope with a configuration file + # in the old format raises an exception + packages_yaml_v015() + with pytest.raises(RuntimeError): + config('add', 'packages:cmake:buildable:false') + + +def test_updating_multiple_scopes_at_once(packages_yaml_v015): + # Create 2 config files in the old format + packages_yaml_v015(scope='user') + packages_yaml_v015(scope='site') + + # Update both of them at once + config('update', '-y', 'packages') + + for scope in ('user', 'site'): + data = spack.config.get('packages', scope=scope) + check_update(data) + + +def check_update(data): + """Check that the data from the packages_yaml_v015 + has been updated. + """ + assert 'externals' in data['cmake'] + externals = data['cmake']['externals'] + assert {'spec': 'cmake@3.14.0', 'prefix': '/usr'} in externals + assert 'paths' not in data['cmake'] + assert 'externals' in data['gcc'] + externals = data['gcc']['externals'] + assert {'spec': 'gcc@8.3.0', 'modules': ['gcc-8']} in externals + assert 'modules' not in data['gcc'] diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 87f7a58667..955693ca0f 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -448,8 +448,9 @@ env: external_config = StringIO("""\ packages: a: - paths: - a: {a_prefix} + externals: + - spec: a + prefix: {a_prefix} buildable: false """.format(a_prefix=str(fake_prefix))) external_config_dict = spack.util.spack_yaml.load_config(external_config) @@ -2041,3 +2042,73 @@ def test_env_write_only_non_default(): yaml = f.read() assert yaml == ev.default_manifest_yaml + + +@pytest.fixture +def packages_yaml_v015(tmpdir): + """Return the path to an existing manifest in the v0.15.x format + and the path to a non yet existing backup file. + """ + raw_yaml = """ +spack: + specs: + - mpich + packages: + cmake: + paths: + cmake@3.17.3: /usr +""" + manifest = tmpdir.ensure('spack.yaml') + backup_file = tmpdir.join('spack.yaml.bkp') + manifest.write(raw_yaml) + return manifest, backup_file + + +def test_update_anonymous_env(packages_yaml_v015): + manifest, backup_file = packages_yaml_v015 + env('update', '-y', str(manifest.dirname)) + + # The environment is now at the latest format + assert ev.is_latest_format(str(manifest)) + # A backup file has been created and it's not at the latest format + assert os.path.exists(str(backup_file)) + assert not ev.is_latest_format(str(backup_file)) + + +def test_double_update(packages_yaml_v015): + manifest, backup_file = packages_yaml_v015 + + # Update the environment + env('update', '-y', str(manifest.dirname)) + # Try to read the environment (it should not error) + ev.create('test', str(manifest)) + # Updating again does nothing since the manifest is up-to-date + env('update', '-y', str(manifest.dirname)) + + # The environment is at the latest format + assert ev.is_latest_format(str(manifest)) + # A backup file has been created and it's not at the latest format + assert os.path.exists(str(backup_file)) + assert not ev.is_latest_format(str(backup_file)) + + +def test_update_and_revert(packages_yaml_v015): + manifest, backup_file = packages_yaml_v015 + + # Update the environment + env('update', '-y', str(manifest.dirname)) + assert os.path.exists(str(backup_file)) + assert not ev.is_latest_format(str(backup_file)) + assert ev.is_latest_format(str(manifest)) + + # Revert to previous state + env('revert', '-y', str(manifest.dirname)) + assert not os.path.exists(str(backup_file)) + assert not ev.is_latest_format(str(manifest)) + + +def test_old_format_cant_be_updated_implicitly(packages_yaml_v015): + manifest, backup_file = packages_yaml_v015 + env('activate', str(manifest.dirname)) + with pytest.raises(spack.main.SpackCommandError): + add('hdf5') diff --git a/lib/spack/spack/test/cmd/external.py b/lib/spack/spack/test/cmd/external.py index 0bdf67fe3e..31175843ec 100644 --- a/lib/spack/spack/test/cmd/external.py +++ b/lib/spack/spack/test/cmd/external.py @@ -70,21 +70,20 @@ def test_find_external_two_instances_same_package(create_exe): def test_find_external_update_config(mutable_config): - pkg_to_entries = { - 'cmake': [ - ExternalPackageEntry(Spec('cmake@1.foo'), '/x/y1/'), - ExternalPackageEntry(Spec('cmake@3.17.2'), '/x/y2/'), - ] - } + entries = [ + ExternalPackageEntry(Spec.from_detection('cmake@1.foo'), '/x/y1/'), + ExternalPackageEntry(Spec.from_detection('cmake@3.17.2'), '/x/y2/'), + ] + pkg_to_entries = {'cmake': entries} spack.cmd.external._update_pkg_config(pkg_to_entries, False) pkgs_cfg = spack.config.get('packages') cmake_cfg = pkgs_cfg['cmake'] - cmake_paths_cfg = cmake_cfg['paths'] + cmake_externals = cmake_cfg['externals'] - assert cmake_paths_cfg['cmake@1.foo'] == '/x/y1/' - assert cmake_paths_cfg['cmake@3.17.2'] == '/x/y2/' + assert {'spec': 'cmake@1.foo', 'prefix': '/x/y1/'} in cmake_externals + assert {'spec': 'cmake@3.17.2', 'prefix': '/x/y2/'} in cmake_externals def test_get_executables(working_env, create_exe): @@ -103,15 +102,16 @@ def test_find_external_cmd(mutable_config, working_env, create_exe): which restricts the set of packages that Spack looks for. """ cmake_path1 = create_exe("cmake", "cmake version 1.foo") + prefix = os.path.dirname(os.path.dirname(cmake_path1)) os.environ['PATH'] = ':'.join([os.path.dirname(cmake_path1)]) external('find', 'cmake') pkgs_cfg = spack.config.get('packages') cmake_cfg = pkgs_cfg['cmake'] - cmake_paths_cfg = cmake_cfg['paths'] + cmake_externals = cmake_cfg['externals'] - assert 'cmake@1.foo' in cmake_paths_cfg + assert {'spec': 'cmake@1.foo', 'prefix': prefix} in cmake_externals def test_find_external_cmd_not_buildable( @@ -134,16 +134,18 @@ def test_find_external_cmd_full_repo( """ exe_path1 = create_exe( - "find-externals1-exe", "find-externals1 version 1.foo") + "find-externals1-exe", "find-externals1 version 1.foo" + ) + prefix = os.path.dirname(os.path.dirname(exe_path1)) os.environ['PATH'] = ':'.join([os.path.dirname(exe_path1)]) external('find') pkgs_cfg = spack.config.get('packages') pkg_cfg = pkgs_cfg['find-externals1'] - pkg_paths_cfg = pkg_cfg['paths'] + pkg_externals = pkg_cfg['externals'] - assert 'find-externals1@1.foo' in pkg_paths_cfg + assert {'spec': 'find-externals1@1.foo', 'prefix': prefix} in pkg_externals def test_find_external_merge(mutable_config, mutable_mock_repo): @@ -152,26 +154,31 @@ def test_find_external_merge(mutable_config, mutable_mock_repo): """ pkgs_cfg_init = { 'find-externals1': { - 'paths': { - 'find-externals1@1.1': '/preexisting-prefix/' - }, + 'externals': [{ + 'spec': 'find-externals1@1.1', + 'prefix': '/preexisting-prefix/' + }], 'buildable': False } } mutable_config.update_config('packages', pkgs_cfg_init) - - pkg_to_entries = { - 'find-externals1': [ - ExternalPackageEntry(Spec('find-externals1@1.1'), '/x/y1/'), - ExternalPackageEntry(Spec('find-externals1@1.2'), '/x/y2/'), - ] - } + entries = [ + ExternalPackageEntry( + Spec.from_detection('find-externals1@1.1'), '/x/y1/' + ), + ExternalPackageEntry( + Spec.from_detection('find-externals1@1.2'), '/x/y2/' + ) + ] + pkg_to_entries = {'find-externals1': entries} spack.cmd.external._update_pkg_config(pkg_to_entries, False) pkgs_cfg = spack.config.get('packages') pkg_cfg = pkgs_cfg['find-externals1'] - pkg_paths_cfg = pkg_cfg['paths'] + pkg_externals = pkg_cfg['externals'] - assert pkg_paths_cfg['find-externals1@1.1'] == '/preexisting-prefix/' - assert pkg_paths_cfg['find-externals1@1.2'] == '/x/y2/' + assert {'spec': 'find-externals1@1.1', + 'prefix': '/preexisting-prefix/'} in pkg_externals + assert {'spec': 'find-externals1@1.2', + 'prefix': '/x/y2/'} in pkg_externals diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index cfe0748c4b..a7a89cc78d 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -373,7 +373,7 @@ class TestConcretize(object): spec = Spec('externalmodule') spec.concretize() - assert spec['externalmodule'].external_module == 'external-module' + assert spec['externalmodule'].external_modules == ['external-module'] assert 'externalprereq' not in spec assert spec['externalmodule'].compiler.satisfies('gcc') diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index df46ed9fe8..c941ccad94 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -198,8 +198,9 @@ all: mpi: [mpich] mpich: buildable: false - paths: - mpich@3.0.4: /dummy/path + externals: + - spec: mpich@3.0.4 + prefix: /dummy/path """) spack.config.set('packages', conf, scope='concretize') @@ -229,8 +230,9 @@ all: mpi: [mpich] mpi: buildable: false - modules: - mpich@3.0.4: dummy + externals: + - spec: mpich@3.0.4 + modules: [dummy] """) spack.config.set('packages', conf, scope='concretize') diff --git a/lib/spack/spack/test/data/config/packages.yaml b/lib/spack/spack/test/data/config/packages.yaml index 63e63e525d..c2e8d558b3 100644 --- a/lib/spack/spack/test/data/config/packages.yaml +++ b/lib/spack/spack/test/data/config/packages.yaml @@ -4,15 +4,21 @@ packages: mpi: [openmpi, mpich] externaltool: buildable: False - paths: - externaltool@1.0%gcc@4.5.0: /path/to/external_tool - externaltool@0.9%gcc@4.5.0: /usr + externals: + - spec: externaltool@1.0%gcc@4.5.0 + prefix: /path/to/external_tool + - spec: externaltool@0.9%gcc@4.5.0 + prefix: /usr externalvirtual: buildable: False - paths: - externalvirtual@2.0%clang@3.3: /path/to/external_virtual_clang - externalvirtual@1.0%gcc@4.5.0: /path/to/external_virtual_gcc + externals: + - spec: externalvirtual@2.0%clang@3.3 + prefix: /path/to/external_virtual_clang + - spec: externalvirtual@1.0%gcc@4.5.0 + prefix: /path/to/external_virtual_gcc externalmodule: buildable: False - modules: - externalmodule@1.0%gcc@4.5.0: external-module + externals: + - spec: externalmodule@1.0%gcc@4.5.0 + modules: + - external-module diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py index 63276c0e7b..a161a22908 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -689,17 +689,17 @@ def test_115_reindex_with_packages_not_in_repo(mutable_database): def test_external_entries_in_db(mutable_database): rec = mutable_database.get_record('mpileaks ^zmpi') assert rec.spec.external_path is None - assert rec.spec.external_module is None + assert not rec.spec.external_modules rec = mutable_database.get_record('externaltool') assert rec.spec.external_path == '/path/to/external_tool' - assert rec.spec.external_module is None + assert not rec.spec.external_modules assert rec.explicit is False rec.spec.package.do_install(fake=True, explicit=True) rec = mutable_database.get_record('externaltool') assert rec.spec.external_path == '/path/to/external_tool' - assert rec.spec.external_module is None + assert not rec.spec.external_modules assert rec.explicit is True diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 68b70e0840..0b3f409641 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -157,11 +157,11 @@ def test_process_external_package_module(install_mockery, monkeypatch, capfd): monkeypatch.setattr(spack.database.Database, 'get_record', _none) spec.external_path = '/actual/external/path/not/checked' - spec.external_module = 'unchecked_module' + spec.external_modules = ['unchecked_module'] inst._process_external_package(spec.package, False) out = capfd.readouterr()[0] - assert 'has external module in {0}'.format(spec.external_module) in out + assert 'has external module in {0}'.format(spec.external_modules) in out def test_process_binary_cache_tarball_none(install_mockery, monkeypatch, @@ -257,15 +257,15 @@ def test_installer_ensure_ready_errors(install_mockery): fmt = r'cannot be installed locally.*{0}' # Force an external package error - path, module = spec.external_path, spec.external_module + path, modules = spec.external_path, spec.external_modules spec.external_path = '/actual/external/path/not/checked' - spec.external_module = 'unchecked_module' + spec.external_modules = ['unchecked_module'] msg = fmt.format('is external') with pytest.raises(inst.ExternalPackageError, match=msg): installer._ensure_install_ready(spec.package) # Force an upstream package error - spec.external_path, spec.external_module = path, module + spec.external_path, spec.external_modules = path, modules spec.package._installed_upstream = True msg = fmt.format('is upstream') with pytest.raises(inst.UpstreamPackageError, match=msg): diff --git a/lib/spack/spack/test/module_parsing.py b/lib/spack/spack/test/module_parsing.py index 0bf485913f..8dc06b058b 100644 --- a/lib/spack/spack/test/module_parsing.py +++ b/lib/spack/spack/test/module_parsing.py @@ -9,7 +9,7 @@ import spack from spack.util.module_cmd import ( module, - get_path_from_module, + path_from_modules, get_path_args_from_module_line, get_path_from_module_contents ) @@ -55,7 +55,7 @@ def test_get_path_from_module_faked(monkeypatch): return line monkeypatch.setattr(spack.util.module_cmd, 'module', fake_module) - path = get_path_from_module('mod') + path = path_from_modules(['mod']) assert path == '/path/to' @@ -116,10 +116,10 @@ def test_get_argument_from_module_line(): bad_lines = ['prepend_path(PATH,/lib/path)', 'prepend-path (LD_LIBRARY_PATH) /lib/path'] - assert all(get_path_args_from_module_line(l) == ['/lib/path'] - for l in simple_lines) - assert all(get_path_args_from_module_line(l) == ['/lib/path', '/pkg/path'] - for l in complex_lines) + assert all(get_path_args_from_module_line(x) == ['/lib/path'] + for x in simple_lines) + assert all(get_path_args_from_module_line(x) == ['/lib/path', '/pkg/path'] + for x in complex_lines) for bl in bad_lines: with pytest.raises(ValueError): get_path_args_from_module_line(bl) diff --git a/lib/spack/spack/util/module_cmd.py b/lib/spack/spack/util/module_cmd.py index 7017b2ecb6..3b11851b60 100644 --- a/lib/spack/spack/util/module_cmd.py +++ b/lib/spack/spack/util/module_cmd.py @@ -135,18 +135,32 @@ def get_path_args_from_module_line(line): return paths -def get_path_from_module(mod): - """Inspects a TCL module for entries that indicate the absolute path - at which the library supported by said module can be found. +def path_from_modules(modules): + """Inspect a list of TCL modules for entries that indicate the absolute + path at which the library supported by said module can be found. + + Args: + modules (list): module files to be loaded to get an external package + + Returns: + Guess of the prefix path where the package """ - # Read the module - text = module('show', mod).split('\n') - - p = get_path_from_module_contents(text, mod) - if p and not os.path.exists(p): - tty.warn("Extracted path from module does not exist:" - "\n\tExtracted path: " + p) - return p + best_choice = None + for module_name in modules: + # Read the current module and return a candidate path + text = module('show', module_name).split('\n') + candidate_path = get_path_from_module_contents(text, module_name) + + if candidate_path and not os.path.exists(candidate_path): + msg = ("Extracted path from module does not exist " + "[module={0}, path={0}]") + tty.warn(msg.format(module_name, candidate_path)) + + # If anything is found, then it's the best choice. This means + # that we give preference to the last module to be loaded + # for packages requiring to load multiple modules in sequence + best_choice = candidate_path or best_choice + return best_choice def get_path_from_module_contents(text, module_name): diff --git a/lib/spack/spack/util/spack_yaml.py b/lib/spack/spack/util/spack_yaml.py index 46e8e35543..565a9be4ea 100644 --- a/lib/spack/spack/util/spack_yaml.py +++ b/lib/spack/spack/util/spack_yaml.py @@ -13,7 +13,7 @@ """ import ctypes - +import collections from ordereddict_backport import OrderedDict from six import string_types, StringIO @@ -332,6 +332,22 @@ def dump_annotated(data, stream=None, *args, **kwargs): return getvalue() +def sorted_dict(dict_like): + """Return an ordered dict with all the fields sorted recursively. + + Args: + dict_like (dict): dictionary to be sorted + + Returns: + dictionary sorted recursively + """ + result = syaml_dict(sorted(dict_like.items())) + for key, value in result.items(): + if isinstance(value, collections.Mapping): + result[key] = sorted_dict(value) + return result + + class SpackYAMLError(spack.error.SpackError): """Raised when there are issues with YAML parsing.""" def __init__(self, msg, yaml_error): diff --git a/share/spack/qa/configuration/packages.yaml b/share/spack/qa/configuration/packages.yaml index 076095a7ca..bd853c2386 100644 --- a/share/spack/qa/configuration/packages.yaml +++ b/share/spack/qa/configuration/packages.yaml @@ -1,26 +1,32 @@ packages: cmake: buildable: False - paths: - cmake@3.12.4: /usr + externals: + - spec: cmake@3.12.4 + prefix: /usr r: buildable: False - paths: - r@3.4.4: /usr + externals: + - spec: r@3.4.4 + prefix: /usr perl: buildable: False - paths: - perl@5.26.1: /usr + externals: + - spec: perl@5.26.1 + prefix: /usr findutils: buildable: False - paths: - findutils@4.6.0: /usr + externals: + - spec: findutils@4.6.0 + prefix: /usr openssl: buildable: False - paths: - openssl@1.1.1: /usr + externals: + - spec: openssl@1.1.1 + prefix: /usr libpciaccess: buildable: False - paths: - libpciaccess@0.13.5: /usr + externals: + - spec: libpciaccess@0.13.5 + prefix: /usr diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index fc1b4e2a27..329b9f28ff 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -570,7 +570,7 @@ _spack_config() { then SPACK_COMPREPLY="-h --help --scope" else - SPACK_COMPREPLY="get blame edit list add remove rm" + SPACK_COMPREPLY="get blame edit list add remove rm update revert" fi } @@ -632,6 +632,24 @@ _spack_config_rm() { fi } +_spack_config_update() { + if $list_options + then + SPACK_COMPREPLY="-h --help -y --yes-to-all" + else + _config_sections + fi +} + +_spack_config_revert() { + if $list_options + then + SPACK_COMPREPLY="-h --help -y --yes-to-all" + else + _config_sections + fi +} + _spack_containerize() { SPACK_COMPREPLY="-h --help" } @@ -725,7 +743,7 @@ _spack_env() { then SPACK_COMPREPLY="-h --help" else - SPACK_COMPREPLY="activate deactivate create remove rm list ls status st loads view" + SPACK_COMPREPLY="activate deactivate create remove rm list ls status st loads view update revert" fi } @@ -803,6 +821,24 @@ _spack_env_view() { fi } +_spack_env_update() { + if $list_options + then + SPACK_COMPREPLY="-h --help -y --yes-to-all" + else + _environments + fi +} + +_spack_env_revert() { + if $list_options + then + SPACK_COMPREPLY="-h --help -y --yes-to-all" + else + _environments + fi +} + _spack_extensions() { if $list_options then diff --git a/var/spack/repos/builtin.mock/packages/find-externals1/package.py b/var/spack/repos/builtin.mock/packages/find-externals1/package.py index 25e26dcced..9f5f94aaab 100644 --- a/var/spack/repos/builtin.mock/packages/find-externals1/package.py +++ b/var/spack/repos/builtin.mock/packages/find-externals1/package.py @@ -2,12 +2,11 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - -from spack import * - import os import re +import spack.package + class FindExternals1(AutotoolsPackage): executables = ['find-externals1-exe'] @@ -31,4 +30,6 @@ class FindExternals1(AutotoolsPackage): match = re.search(r'find-externals1.*version\s+(\S+)', output) if match: version_str = match.group(1) - return Spec('find-externals1@{0}'.format(version_str)) + return Spec.from_detection( + 'find-externals1@{0}'.format(version_str) + ) diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py index d967611fdd..85e884c8d3 100644 --- a/var/spack/repos/builtin/packages/mpich/package.py +++ b/var/spack/repos/builtin/packages/mpich/package.py @@ -187,7 +187,7 @@ spack package at this time.''', # their run environments the code to make the compilers available. # For Cray MPIs, the regular compiler wrappers *are* the MPI wrappers. # Cray MPIs always have cray in the module name, e.g. "cray-mpich" - if self.spec.external_module and 'cray' in self.spec.external_module: + if self.spec.external_modules and 'cray' in self.spec.external_modules: env.set('MPICC', spack_cc) env.set('MPICXX', spack_cxx) env.set('MPIF77', spack_fc) @@ -210,7 +210,7 @@ spack package at this time.''', def setup_dependent_package(self, module, dependent_spec): # For Cray MPIs, the regular compiler wrappers *are* the MPI wrappers. # Cray MPIs always have cray in the module name, e.g. "cray-mpich" - if self.spec.external_module and 'cray' in self.spec.external_module: + if self.spec.external_modules and 'cray' in self.spec.external_modules: self.spec.mpicc = spack_cc self.spec.mpicxx = spack_cxx self.spec.mpifc = spack_fc diff --git a/var/spack/repos/builtin/packages/mvapich2/package.py b/var/spack/repos/builtin/packages/mvapich2/package.py index 0e0ab26a22..336f593806 100644 --- a/var/spack/repos/builtin/packages/mvapich2/package.py +++ b/var/spack/repos/builtin/packages/mvapich2/package.py @@ -235,7 +235,7 @@ class Mvapich2(AutotoolsPackage): def setup_compiler_environment(self, env): # For Cray MPIs, the regular compiler wrappers *are* the MPI wrappers. # Cray MPIs always have cray in the module name, e.g. "cray-mvapich" - if self.spec.external_module and 'cray' in self.spec.external_module: + if self.spec.external_modules and 'cray' in self.spec.external_modules: env.set('MPICC', spack_cc) env.set('MPICXX', spack_cxx) env.set('MPIF77', spack_fc) @@ -249,7 +249,7 @@ class Mvapich2(AutotoolsPackage): def setup_dependent_package(self, module, dependent_spec): # For Cray MPIs, the regular compiler wrappers *are* the MPI wrappers. # Cray MPIs always have cray in the module name, e.g. "cray-mvapich" - if self.spec.external_module and 'cray' in self.spec.external_module: + if self.spec.external_modules and 'cray' in self.spec.external_modules: self.spec.mpicc = spack_cc self.spec.mpicxx = spack_cxx self.spec.mpifc = spack_fc |