From 9067378c24d5fd0477285a973b7d441642f51da9 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 23 Jun 2020 11:26:15 -0500 Subject: [PATCH 01/13] fix compiler environment handling to reset environment after (#17204) bugfix: fix compiler environment handling to reset environment after --- lib/spack/spack/compiler.py | 39 ++++++++------- lib/spack/spack/test/compilers/basics.py | 60 ++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py index 784fd5544ea..f8c5ac65adc 100644 --- a/lib/spack/spack/compiler.py +++ b/lib/spack/spack/compiler.py @@ -355,11 +355,13 @@ def _get_compiler_link_paths(self, paths): for flag_type in flags: for flag in self.flags.get(flag_type, []): compiler_exe.add_default_arg(flag) + + output = '' with self._compiler_environment(): output = str(compiler_exe( self.verbose_flag, fin, '-o', fout, output=str, error=str)) # str for py2 - return _parse_non_system_link_dirs(output) + return _parse_non_system_link_dirs(output) except spack.util.executable.ProcessError as pe: tty.debug('ProcessError: Command exited with non-zero status: ' + pe.long_message) @@ -549,24 +551,27 @@ def _compiler_environment(self): # store environment to replace later backup_env = os.environ.copy() - # load modules and set env variables - for module in self.modules: - # On cray, mic-knl module cannot be loaded without cce module - # See: https://github.com/spack/spack/issues/3153 - if os.environ.get("CRAY_CPU_TARGET") == 'mic-knl': - spack.util.module_cmd.load_module('cce') - spack.util.module_cmd.load_module(module) + try: + # load modules and set env variables + for module in self.modules: + # On cray, mic-knl module cannot be loaded without cce module + # See: https://github.com/spack/spack/issues/3153 + if os.environ.get("CRAY_CPU_TARGET") == 'mic-knl': + spack.util.module_cmd.load_module('cce') + spack.util.module_cmd.load_module(module) - # apply other compiler environment changes - env = spack.util.environment.EnvironmentModifications() - env.extend(spack.schema.environment.parse(self.environment)) - env.apply_modifications() + # apply other compiler environment changes + env = spack.util.environment.EnvironmentModifications() + env.extend(spack.schema.environment.parse(self.environment)) + env.apply_modifications() - yield - - # Restore environment - os.environ.clear() - os.environ.update(backup_env) + yield + except BaseException: + raise + finally: + # Restore environment regardless of whether inner code succeeded + os.environ.clear() + os.environ.update(backup_env) class CompilerAccessError(spack.error.SpackError): diff --git a/lib/spack/spack/test/compilers/basics.py b/lib/spack/spack/test/compilers/basics.py index faf18e38715..32f298ae2b3 100644 --- a/lib/spack/spack/test/compilers/basics.py +++ b/lib/spack/spack/test/compilers/basics.py @@ -18,6 +18,7 @@ import spack.compilers as compilers from spack.compiler import Compiler +from spack.util.executable import ProcessError @pytest.fixture() @@ -653,3 +654,62 @@ def module(*args): compiler = compilers[0] version = compiler.get_real_version() assert version == test_version + + +def test_compiler_get_real_version_fails(working_env, monkeypatch, tmpdir): + # Test variables + test_version = '2.2.2' + + # Create compiler + gcc = str(tmpdir.join('gcc')) + with open(gcc, 'w') as f: + f.write("""#!/bin/bash +if [[ $CMP_ON == "1" ]]; then + echo "$CMP_VER" +fi +""") + fs.set_executable(gcc) + + # Add compiler to config + compiler_info = { + 'spec': 'gcc@foo', + 'paths': { + 'cc': gcc, + 'cxx': None, + 'f77': None, + 'fc': None, + }, + 'flags': {}, + 'operating_system': 'fake', + 'target': 'fake', + 'modules': ['turn_on'], + 'environment': { + 'set': {'CMP_VER': test_version}, + }, + 'extra_rpaths': [], + } + compiler_dict = {'compiler': compiler_info} + + # Set module load to turn compiler on + def module(*args): + if args[0] == 'show': + return '' + elif args[0] == 'load': + os.environ['SPACK_TEST_CMP_ON'] = "1" + monkeypatch.setattr(spack.util.module_cmd, 'module', module) + + # Make compiler fail when getting implicit rpaths + def _call(*args, **kwargs): + raise ProcessError("Failed intentionally") + monkeypatch.setattr(spack.util.executable.Executable, '__call__', _call) + + # Run and no change to environment + compilers = spack.compilers.get_compilers([compiler_dict]) + assert len(compilers) == 1 + compiler = compilers[0] + try: + _ = compiler.get_real_version() + assert False + except ProcessError: + # Confirm environment does not change after failed call + assert 'SPACK_TEST_CMP_ON' not in os.environ From 0493e133c5b83f87ebc04a4d15a1dc0e783cfbf0 Mon Sep 17 00:00:00 2001 From: Michio Ogawa Date: Wed, 24 Jun 2020 01:26:47 +0900 Subject: [PATCH 02/13] revocap-refiner: updated package (#17192) --- .../packages/revocap-refiner/package.py | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/var/spack/repos/builtin/packages/revocap-refiner/package.py b/var/spack/repos/builtin/packages/revocap-refiner/package.py index 4c563a91d83..f95e8f6e1d0 100644 --- a/var/spack/repos/builtin/packages/revocap-refiner/package.py +++ b/var/spack/repos/builtin/packages/revocap-refiner/package.py @@ -8,17 +8,15 @@ class RevocapRefiner(MakefilePackage): """The University of Tokyo, CISS Project: - Geometric processing, mesh processing, mesh generation""" + Library for refining of model meshes""" - homepage = "https://github.com/FrontISTR/REVOCAP_Refiner" - git = "https://github.com/FrontISTR/REVOCAP_Refiner.git" + homepage = "https://www.frontistr.com" + url = "https://www.frontistr.com/download/link.php?REVOCAP_Refiner-1.1.04.tar.gz" + # git = "https://gitlab.com/FrontISTR-Commons/REVOCAP_Refiner.git" - version('master', branch='master') + maintainers = ['k-tokunaga', 'kgoto', 'tuna' 'inagaki.kazuhisa'] - depends_on('ruby', type='build') - depends_on('mpi') - depends_on('doxygen', type='build') - depends_on('swig', type='build') + version('1.1.04', sha256='bf3d959f4c1ab08a7e99cd7e02e710c758af28d71500f4814eed8b4eb3fb2d13') parallel = False @@ -28,35 +26,32 @@ class RevocapRefiner(MakefilePackage): patch('delete_getIndices.patch') def edit(self, spec, prefix): - cflags = ['-O'] - cxxflags = ['-O', self.compiler.cxx_pic_flag] - fflags = [''] + cflags = ['-O3'] + cxxflags = ['-O3', self.compiler.cxx_pic_flag] + ldflags = [''] ldshare = [''] - libs = ['-lstdc++'] - if spec.satisfies('%gcc'): - ldshare.append('g++ -shared -s') - + libs = [''] m = FileFilter('MakefileConfig.in') - m.filter(r'CC\s=.*$', 'CC={0}'.format(spec['mpi'].mpicc)) - m.filter(r'CFLAGS\s=.*$', 'CFLAGS={0}'.format(' '.join(cflags))) - m.filter(r'CXX\s*=.*$', 'CXX={0}'.format(spec['mpi'].mpicxx)) + m.filter(r'ARCH\s*=.*$', 'ARCH=') + m.filter(r'CC\s*=.*$', 'CC={0}'.format(spack_cc)) + m.filter(r'CFLAGS\s*=.*$', 'CFLAGS={0}'.format(' '.join(cflags))) + m.filter(r'CXX\s*=.*$', 'CXX={0}'.format(spack_cxx)) m.filter(r'CXXFLAGS\s*=.*$', 'CXXFLAGS={0}'.format(' '.join(cxxflags))) m.filter(r'AR\s*=.*$', 'AR=ar') m.filter(r'ARFLAGS\s*=.*$', 'ARFLAGS=rsv') m.filter(r'LD\s*=.*$', 'LD={0}'.format(spack_fc)) m.filter(r'LDFLAGS\s*=.*$', - 'LDFLAGS={0}'.format(' '.join(fflags))) + 'LDFLAGS={0}'.format(' '.join(ldflags))) m.filter(r'LDSHARE\s*=.*$', 'LDSHARE={0}'.format(' '.join(ldshare))) m.filter(r'LIBS\s*=.*$', 'LIBS={0}'.format(' '.join(libs))) m.filter(r'LIBPATH\s*=.*$', 'LIBPATH= ') m.filter(r'RM\s*=.*$', 'RM=rm -f') - m.filter(r'DOXYGEN\s*=.*$', 'DOXYGEN=doxygen') m.filter(r'TAR\s*=.*$', 'TAR=tar') - m.filter(r'SWIG\s*=.*$', 'SWIG=swig') def install(self, spec, prefix): + make() install_tree('bin', prefix.bin) install_tree('lib', prefix.lib) - install_tree('Refiner', prefix.include.refine) + install_tree('Refiner', prefix.include) From f54a8a77b46dbbed97243a7d1d627d6f581f0b91 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren Date: Mon, 9 Mar 2020 15:14:27 -0700 Subject: [PATCH 03/13] Allow a single ctrl-c to terminate an install in progress --- lib/spack/spack/installer.py | 14 +++++++++++--- lib/spack/spack/test/installer.py | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 0eeec020bc1..786eec5383c 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -1530,12 +1530,20 @@ def install(self, **kwargs): self._update_installed(task) raise - except (Exception, KeyboardInterrupt, SystemExit) as exc: - # Assuming best effort installs so suppress the exception and - # mark as a failure UNLESS this is the explicit package. + except KeyboardInterrupt as exc: + # The build has been terminated with a Ctrl-C so terminate. err = 'Failed to install {0} due to {1}: {2}' tty.error(err.format(pkg.name, exc.__class__.__name__, str(exc))) + raise + + except (Exception, SystemExit) as exc: + # Best effort installs suppress the exception and mark the + # package as a failure UNLESS this is the explicit package. + err = 'Failed to install {0} due to {1}: {2}' + tty.error(err.format(pkg.name, exc.__class__.__name__, + str(exc))) + self._update_failed(task, True, exc) if pkg_id == self.pkg_id: diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 0be4bc78c02..96612c8f5f6 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -718,6 +718,26 @@ def test_install_failed(install_mockery, monkeypatch, capsys): assert 'Warning: b failed to install' in out +def test_install_fail_on_interrupt(install_mockery, monkeypatch, capsys): + """Test ctrl-c interrupted install.""" + err_msg = 'mock keyboard interrupt' + + def _interrupt(installer, task, **kwargs): + raise KeyboardInterrupt(err_msg) + + spec, installer = create_installer('a') + + # Raise a KeyboardInterrupt error to trigger early termination + monkeypatch.setattr(inst.PackageInstaller, '_install_task', _interrupt) + + with pytest.raises(KeyboardInterrupt, match=err_msg): + installer.install() + + out = str(capsys.readouterr()) + assert 'Failed to install' in out + assert err_msg in out + + def test_install_lock_failures(install_mockery, monkeypatch, capfd): """Cover basic install lock failure handling in a single pass.""" def _requeued(installer, task): From 96932d65a819a08fe40dd4120b0ed05ed8011e01 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren Date: Mon, 9 Mar 2020 15:52:13 -0700 Subject: [PATCH 04/13] Added support for --fail-fast install option to terminate on first failure --- lib/spack/docs/packaging_guide.rst | 11 ++++++-- lib/spack/spack/cmd/install.py | 4 +++ lib/spack/spack/installer.py | 16 ++++++++++++ lib/spack/spack/test/installer.py | 41 +++++++++++++++++++++++++++--- share/spack/spack-completion.bash | 2 +- 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 840c29454b9..1f246c0faad 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -4167,16 +4167,23 @@ want to clean up the temporary directory, or if the package isn't downloading properly, you might want to run *only* the ``fetch`` stage of the build. +Spack performs best-effort installation of package dependencies by default, +which means it will continue to install as many dependencies as possible +after detecting failures. If you are trying to install a package with a +lot of dependencies where one or more may fail to build, you might want to +try the ``--fail-fast`` option to stop the installation process on the first +failure. + A typical package workflow might look like this: .. code-block:: console $ spack edit mypackage - $ spack install mypackage + $ spack install --fail-fast mypackage ... build breaks! ... $ spack clean mypackage $ spack edit mypackage - $ spack install mypackage + $ spack install --fail-fast mypackage ... repeat clean/install until install works ... Below are some commands that will allow you some finer-grained diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index e2a6327f5f4..10eb3c327f8 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -32,6 +32,7 @@ def update_kwargs_from_args(args, kwargs): that will be passed to Package.do_install API""" kwargs.update({ + 'fail_fast': args.fail_fast, 'keep_prefix': args.keep_prefix, 'keep_stage': args.keep_stage, 'restage': not args.dont_restage, @@ -78,6 +79,9 @@ def setup_parser(subparser): subparser.add_argument( '--overwrite', action='store_true', help="reinstall an existing spec, even if it has dependents") + subparser.add_argument( + '--fail-fast', action='store_true', + help="stop all builds if any build fails (default is best effort)") subparser.add_argument( '--keep-prefix', action='store_true', help="don't remove the install prefix if installation fails") diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 786eec5383c..cd2aa00bd02 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -549,6 +549,9 @@ def package_id(pkg): dirty (bool): Don't clean the build environment before installing. explicit (bool): True if package was explicitly installed, False if package was implicitly installed (as a dependency). + fail_fast (bool): Fail if any dependency fails to install; + otherwise, the default is to install as many dependencies as + possible (i.e., best effort installation). fake (bool): Don't really build; install fake stub files instead. force (bool): Install again, even if already installed. install_deps (bool): Install dependencies before installing this @@ -1385,11 +1388,14 @@ def install(self, **kwargs): Args:""" + fail_fast = kwargs.get('fail_fast', False) install_deps = kwargs.get('install_deps', True) keep_prefix = kwargs.get('keep_prefix', False) keep_stage = kwargs.get('keep_stage', False) restage = kwargs.get('restage', False) + fail_fast_err = 'Terminating after first install failure' + # install_package defaults True and is popped so that dependencies are # always installed regardless of whether the root was installed install_package = kwargs.pop('install_package', True) @@ -1449,6 +1455,10 @@ def install(self, **kwargs): if pkg_id in self.failed or spack.store.db.prefix_failed(spec): tty.warn('{0} failed to install'.format(pkg_id)) self._update_failed(task) + + if fail_fast: + raise InstallError(fail_fast_err) + continue # Attempt to get a write lock. If we can't get the lock then @@ -1546,6 +1556,12 @@ def install(self, **kwargs): self._update_failed(task, True, exc) + if fail_fast: + # The user requested the installation to terminate on + # failure. + raise InstallError('{0}: {1}' + .format(fail_fast_err, str(exc))) + if pkg_id == self.pkg_id: raise diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 96612c8f5f6..89efbe4fbec 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -718,7 +718,7 @@ def test_install_failed(install_mockery, monkeypatch, capsys): assert 'Warning: b failed to install' in out -def test_install_fail_on_interrupt(install_mockery, monkeypatch, capsys): +def test_install_fail_on_interrupt(install_mockery, monkeypatch): """Test ctrl-c interrupted install.""" err_msg = 'mock keyboard interrupt' @@ -733,9 +733,44 @@ def _interrupt(installer, task, **kwargs): with pytest.raises(KeyboardInterrupt, match=err_msg): installer.install() + +def test_install_fail_fast_on_detect(install_mockery, monkeypatch, capsys): + """Test fail_fast install when an install failure is detected.""" + spec, installer = create_installer('a') + + # Make sure the package is identified as failed + # + # This will prevent b from installing, which will cause the build of a + # to be skipped. + monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true) + + with pytest.raises(spack.installer.InstallError): + installer.install(fail_fast=True) + out = str(capsys.readouterr()) - assert 'Failed to install' in out - assert err_msg in out + assert 'Skipping build of a' in out + + +def test_install_fail_fast_on_except(install_mockery, monkeypatch, capsys): + """Test fail_fast install when an install failure results from an error.""" + err_msg = 'mock patch failure' + + def _patch(installer, task, **kwargs): + raise RuntimeError(err_msg) + + spec, installer = create_installer('a') + + # Raise a non-KeyboardInterrupt exception to trigger fast failure. + # + # This will prevent b from installing, which will cause the build of a + # to be skipped. + monkeypatch.setattr(spack.package.PackageBase, 'do_patch', _patch) + + with pytest.raises(spack.installer.InstallError, matches=err_msg): + installer.install(fail_fast=True) + + out = str(capsys.readouterr()) + assert 'Skipping build of a' in out def test_install_lock_failures(install_mockery, monkeypatch, capfd): diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index ed42ad4eddb..620dd9bef2f 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -962,7 +962,7 @@ _spack_info() { _spack_install() { if $list_options then - SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --overwrite --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --no-check-signature --show-log-on-error --source -n --no-checksum -v --verbose --fake --only-concrete -f --file --clean --dirty --test --run-tests --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all" + SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --no-check-signature --show-log-on-error --source -n --no-checksum -v --verbose --fake --only-concrete -f --file --clean --dirty --test --run-tests --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all" else _all_packages fi From 94cce5f96379c10a24dbcf7bb02a2f242f72b2b1 Mon Sep 17 00:00:00 2001 From: victorusu Date: Tue, 23 Jun 2020 21:29:45 +0200 Subject: [PATCH 05/13] Enable mysql for macos (#17177) I get the following error message, if I do not use editline from the system. ``` >> 3090 Undefined symbols for architecture x86_64: 3091 "_tgetent", referenced from: 3092 _terminal_set in libedit.a(terminal.c.o) 3093 "_tgetflag", referenced from: 3094 _terminal_set in libedit.a(terminal.c.o) 3095 "_tgetnum", referenced from: 3096 _terminal_set in libedit.a(terminal.c.o) ... 3110 _terminal_insertwrite in libedit.a(terminal.c.o) 3111 _terminal_clear_EOL in libedit.a(terminal.c.o) 3112 _terminal_clear_screen in libedit.a(terminal.c.o) 3113 _terminal_beep in libedit.a(terminal.c.o) 3114 ... 3115 ld: symbol(s) not found for architecture x86_64 ``` --- var/spack/repos/builtin/packages/mysql/package.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/var/spack/repos/builtin/packages/mysql/package.py b/var/spack/repos/builtin/packages/mysql/package.py index 80b49865b6f..c84c311b567 100644 --- a/var/spack/repos/builtin/packages/mysql/package.py +++ b/var/spack/repos/builtin/packages/mysql/package.py @@ -112,7 +112,8 @@ class Mysql(CMakePackage): depends_on('rpcsvc-proto') depends_on('ncurses') depends_on('openssl') - depends_on('libtirpc', when='@5.7.0:') + depends_on('libtirpc', when='@5.7.0: platform=linux') + depends_on('libedit', type=['build', 'run']) depends_on('perl', type=['build', 'test'], when='@:7.99.99') depends_on('bison@2.1:', type='build') depends_on('m4', type='build', when='@develop platform=solaris') @@ -130,6 +131,11 @@ def cmake_args(self): options.append('-DBOOST_ROOT={0}'.format(spec['boost'].prefix)) if '+client_only' in self.spec: options.append('-DWITHOUT_SERVER:BOOL=ON') + options.append('-DWITH_EDITLINE=system') + options.append('-Dlibedit_INCLUDE_DIR={0}'.format( + spec['libedit'].prefix.include)) + options.append('-Dlibedit_LIBRARY={0}'.format( + spec['libedit'].libs.directories[0])) return options def _fix_dtrace_shebang(self, env): From 0b57567824d23159f011e34b1a841b832308903c Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Tue, 23 Jun 2020 22:38:04 +0200 Subject: [PATCH 06/13] Module index should not be unconditionally overwritten (#14837) * Module index should not be unconditionally overwritten Uncovered after we switched our CI to generate modules for packages one-by-one rather than in bulk. This overwrote a complete module index with an index with a single entry, and broke our downstream Spack instances that needed the upstream module index. --- lib/spack/spack/cmd/modules/__init__.py | 7 +++++-- lib/spack/spack/modules/common.py | 12 +++++++++--- lib/spack/spack/test/cmd/module.py | 9 +++++---- lib/spack/spack/test/modules/tcl.py | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py index 7d51224e146..7f3dd29ef1e 100644 --- a/lib/spack/spack/cmd/modules/__init__.py +++ b/lib/spack/spack/cmd/modules/__init__.py @@ -289,15 +289,18 @@ def refresh(module_type, specs, args): msg = 'Nothing to be done for {0} module files.' tty.msg(msg.format(module_type)) return - # If we arrived here we have at least one writer module_type_root = writers[0].layout.dirname() - spack.modules.common.generate_module_index(module_type_root, writers) + # Proceed regenerating module files tty.msg('Regenerating {name} module files'.format(name=module_type)) if os.path.isdir(module_type_root) and args.delete_tree: shutil.rmtree(module_type_root, ignore_errors=False) filesystem.mkdirp(module_type_root) + + # Dump module index after potentially removing module tree + spack.modules.common.generate_module_index( + module_type_root, writers, overwrite=args.delete_tree) for x in writers: try: x.write(overwrite=True) diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index 95ef81415ed..33e2c1a6d32 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -221,8 +221,15 @@ def root_path(name): return spack.util.path.canonicalize_path(path) -def generate_module_index(root, modules): - entries = syaml.syaml_dict() +def generate_module_index(root, modules, overwrite=False): + index_path = os.path.join(root, 'module-index.yaml') + if overwrite or not os.path.exists(index_path): + entries = syaml.syaml_dict() + else: + with open(index_path) as index_file: + yaml_content = syaml.load(index_file) + entries = yaml_content['module_index'] + for m in modules: entry = { 'path': m.layout.filename, @@ -230,7 +237,6 @@ def generate_module_index(root, modules): } entries[m.spec.dag_hash()] = entry index = {'module_index': entries} - index_path = os.path.join(root, 'module-index.yaml') llnl.util.filesystem.mkdirp(root) with open(index_path, 'w') as index_file: syaml.dump(index, default_flow_style=False, stream=index_file) diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py index f9aeb4af664..c3fe2793063 100644 --- a/lib/spack/spack/test/cmd/module.py +++ b/lib/spack/spack/test/cmd/module.py @@ -14,6 +14,7 @@ module = spack.main.SpackCommand('module') + #: make sure module files are generated for all the tests here @pytest.fixture(scope='module', autouse=True) def ensure_module_files_are_there( @@ -168,10 +169,10 @@ def test_loads_recursive_blacklisted(database, module_configuration): output = module('lmod', 'loads', '-r', 'mpileaks ^mpich') lines = output.split('\n') - assert any(re.match(r'[^#]*module load.*mpileaks', l) for l in lines) - assert not any(re.match(r'[^#]module load.*callpath', l) for l in lines) - assert any(re.match(r'## blacklisted or missing.*callpath', l) - for l in lines) + assert any(re.match(r'[^#]*module load.*mpileaks', ln) for ln in lines) + assert not any(re.match(r'[^#]module load.*callpath', ln) for ln in lines) + assert any(re.match(r'## blacklisted or missing.*callpath', ln) + for ln in lines) # TODO: currently there is no way to separate stdout and stderr when # invoking a SpackCommand. Supporting this requires refactoring diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index 7672cdc676d..41d83234564 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -236,6 +236,7 @@ def test_module_index( w1, s1 = factory('mpileaks') w2, s2 = factory('callpath') + w3, s3 = factory('openblas') test_root = str(tmpdir_factory.mktemp('module-root')) @@ -246,6 +247,22 @@ def test_module_index( assert index[s1.dag_hash()].use_name == w1.layout.use_name assert index[s2.dag_hash()].path == w2.layout.filename + spack.modules.common.generate_module_index(test_root, [w3]) + + index = spack.modules.common.read_module_index(test_root) + + assert len(index) == 3 + assert index[s1.dag_hash()].use_name == w1.layout.use_name + assert index[s2.dag_hash()].path == w2.layout.filename + + spack.modules.common.generate_module_index( + test_root, [w3], overwrite=True) + + index = spack.modules.common.read_module_index(test_root) + + assert len(index) == 1 + assert index[s3.dag_hash()].use_name == w3.layout.use_name + def test_suffixes(self, module_configuration, factory): """Tests adding suffixes to module file name.""" module_configuration('suffix') From e74c8e71ccfc86ea63446dff5517afcdd286bae0 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Tue, 23 Jun 2020 15:17:35 -0700 Subject: [PATCH 07/13] tests: check rpath presence not equality (#17216) --- lib/spack/spack/test/relocate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/test/relocate.py b/lib/spack/spack/test/relocate.py index 551f1596f7b..a1a8952c974 100644 --- a/lib/spack/spack/test/relocate.py +++ b/lib/spack/spack/test/relocate.py @@ -337,7 +337,8 @@ def test_make_elf_binaries_relative(hello_world, copy_binary, tmpdir): [str(new_binary)], [str(orig_binary)], str(orig_binary.dirpath()) ) - assert rpaths_for(new_binary) == '$ORIGIN/lib:$ORIGIN/lib64:/opt/local/lib' + # Some compilers add rpaths so ensure changes included in final result + assert '$ORIGIN/lib:$ORIGIN/lib64:/opt/local/lib' in rpaths_for(new_binary) def test_raise_if_not_relocatable(monkeypatch): From fd710fc93eee64e3ecd432d902a8bb6b61354451 Mon Sep 17 00:00:00 2001 From: Tom Payerle Date: Tue, 23 Jun 2020 20:50:19 -0400 Subject: [PATCH 08/13] Some minor fixes to set_permissions() in file_permissions.py (#17020) * Some minor fixes to set_permissions() in file_permissions.py The set_permissions() routine claims to prevent users from creating world writable suid binaries. However, it seems to only be checking for/preventing group writable suid binaries. This patch modifies the routine to check for both world and group writable suid binaries, and complain appropriately. * permissions.py: Add test to check blocks world writable SUID files The original test_chmod_rejects_group_writable_suid tested that the set_permissions() function in lib/spack/spack/util/file_permissions.py would raise an exception if changed permission on a file with both SUID and SGID plus sticky bits is chmod-ed to g+rwx and o+rwx. I have modified so that more narrowly tests a file with SUID (and no SGID or sticky bit) set is chmod-ed to g+w. I have added a second test test_chmod_rejects_world_writable_suid that checks that exception is raised if an SUID file is chmod-ed to o+w * file_permissions.py: Raise exception when try to make sgid file world writable Updated set_permissions() in file_permissions.py to also raise an exception if try to make an SGID file world writable. And added corresponding unit test as well. * Remove debugging prints from permissions.py --- lib/spack/spack/test/permissions.py | 24 ++++++++++++++++++++++-- lib/spack/spack/util/file_permissions.py | 17 +++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/spack/spack/test/permissions.py b/lib/spack/spack/test/permissions.py index fa83eb37ff3..26974c80960 100644 --- a/lib/spack/spack/test/permissions.py +++ b/lib/spack/spack/test/permissions.py @@ -27,9 +27,29 @@ def test_chmod_real_entries_ignores_suid_sgid(tmpdir): def test_chmod_rejects_group_writable_suid(tmpdir): path = str(tmpdir.join('file').ensure()) - mode = stat.S_ISUID | stat.S_ISGID | stat.S_ISVTX + mode = stat.S_ISUID fs.chmod_x(path, mode) - perms = stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO + perms = stat.S_IWGRP + with pytest.raises(InvalidPermissionsError): + set_permissions(path, perms) + + +def test_chmod_rejects_world_writable_suid(tmpdir): + path = str(tmpdir.join('file').ensure()) + mode = stat.S_ISUID + fs.chmod_x(path, mode) + + perms = stat.S_IWOTH + with pytest.raises(InvalidPermissionsError): + set_permissions(path, perms) + + +def test_chmod_rejects_world_writable_sgid(tmpdir): + path = str(tmpdir.join('file').ensure()) + mode = stat.S_ISGID + fs.chmod_x(path, mode) + + perms = stat.S_IWOTH with pytest.raises(InvalidPermissionsError): set_permissions(path, perms) diff --git a/lib/spack/spack/util/file_permissions.py b/lib/spack/spack/util/file_permissions.py index d94b74f51e9..b133d2569e9 100644 --- a/lib/spack/spack/util/file_permissions.py +++ b/lib/spack/spack/util/file_permissions.py @@ -25,10 +25,19 @@ def set_permissions(path, perms, group=None): # Preserve higher-order bits of file permissions perms |= os.stat(path).st_mode & (st.S_ISUID | st.S_ISGID | st.S_ISVTX) - # Do not let users create world writable suid binaries - if perms & st.S_ISUID and perms & st.S_IWGRP: - raise InvalidPermissionsError( - "Attepting to set suid with world writable") + # Do not let users create world/group writable suid binaries + if perms & st.S_ISUID: + if perms & st.S_IWOTH: + raise InvalidPermissionsError( + "Attempting to set suid with world writable") + if perms & st.S_IWGRP: + raise InvalidPermissionsError( + "Attempting to set suid with group writable") + # Or world writable sgid binaries + if perms & st.S_ISGID: + if perms & st.S_IWOTH: + raise InvalidPermissionsError( + "Attempting to set sgid with world writable") fs.chmod_x(path, perms) From 4c055630d545d3ec9c919d20485f6f85f8f8d6b3 Mon Sep 17 00:00:00 2001 From: "Nichols A. Romero" Date: Wed, 24 Jun 2020 02:52:27 -0500 Subject: [PATCH 09/13] quantum-espresso: correctly cross-compile code for Cray and BG/Q (#17180) --- .../builtin/packages/quantum-espresso/package.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/var/spack/repos/builtin/packages/quantum-espresso/package.py b/var/spack/repos/builtin/packages/quantum-espresso/package.py index 3c8b2c9d021..35454ae3ef3 100644 --- a/var/spack/repos/builtin/packages/quantum-espresso/package.py +++ b/var/spack/repos/builtin/packages/quantum-espresso/package.py @@ -224,6 +224,19 @@ def install(self, spec, prefix): prefix_path = prefix.bin if '@:5.4.0' in spec else prefix options = ['-prefix={0}'.format(prefix_path)] + # This additional flag is needed anytime the target architecture + # does not match the host architecture, which results in a binary that + # configure cannot execute on the login node. This is how we detect + # cross compilation: If the platform is NOT either Linux or Darwin + # and the target=backend, that we are in the cross-compile scenario + # scenario. This should cover Cray, BG/Q, and other custom platforms. + # The other option is to list out all the platform where you would be + # cross compiling explicitly. + if not (spec.satisfies('platform=linux') or + spec.satisfies('platform=darwin')): + if spec.satisfies('target=backend'): + options.append('--host') + # QE autoconf compiler variables has some limitations: # 1. There is no explicit MPICC variable so we must re-purpose # CC for the case of MPI. From fe0d3c22c6a02af6f9dde13aab3f4772c33aa59e Mon Sep 17 00:00:00 2001 From: Michael Kuhn Date: Wed, 24 Jun 2020 11:01:48 +0200 Subject: [PATCH 10/13] perl: added v5.30.3 and v5.32.0 (#17220) perl.org still recommends 5.30.3, so keep it as the preferred version. --- var/spack/repos/builtin/packages/perl/package.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/var/spack/repos/builtin/packages/perl/package.py b/var/spack/repos/builtin/packages/perl/package.py index 05cb13cff9a..ae47c8ce039 100644 --- a/var/spack/repos/builtin/packages/perl/package.py +++ b/var/spack/repos/builtin/packages/perl/package.py @@ -30,12 +30,16 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package # see http://www.cpan.org/src/README.html for # explanation of version numbering scheme + # Maintenance releases (even numbers, recommended) + version('5.32.0', sha256='efeb1ce1f10824190ad1cadbcccf6fdb8a5d37007d0100d2d9ae5f2b5900c0b4') + # Development releases (odd numbers) version('5.31.7', sha256='d05c4e72128f95ef6ffad42728ecbbd0d9437290bf0f88268b51af011f26b57d') version('5.31.4', sha256='418a7e6fe6485cc713a86d1227ef112f0bb3f80322e3b715ffe42851d97804a5') # Maintenance releases (even numbers, recommended) - version('5.30.2', sha256='66db7df8a91979eb576fac91743644da878244cf8ee152f02cd6f5cd7a731689', preferred=True) + version('5.30.3', sha256='32e04c8bb7b1aecb2742a7f7ac0eabac100f38247352a73ad7fa104e39e7406f', preferred=True) + version('5.30.2', sha256='66db7df8a91979eb576fac91743644da878244cf8ee152f02cd6f5cd7a731689') version('5.30.1', sha256='bf3d25571ff1ee94186177c2cdef87867fd6a14aa5a84f0b1fb7bf798f42f964') version('5.30.0', sha256='851213c754d98ccff042caa40ba7a796b2cee88c5325f121be5cbb61bbf975f2') From 577a88880ed63b832bc2b463cf0c5d5e36acd2c0 Mon Sep 17 00:00:00 2001 From: Gilles Gouaillardet Date: Wed, 24 Jun 2020 20:47:55 +0900 Subject: [PATCH 11/13] gromacs: fix fftw and update cmake dependencies (#17226) * gromacs: fix fftw dependency Only depend on fftw+mpi when gromacs is built with mpi, and depend on fftw~mpi otherwise. * gromacs: fix cmake dependency master branch depends on cmake 3.11 (as specified in CMakeLists.txt cmake dependency is also bumped to 3.11 when fj compilers are used in order to fix OpenMP detection. --- var/spack/repos/builtin/packages/gromacs/package.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/var/spack/repos/builtin/packages/gromacs/package.py b/var/spack/repos/builtin/packages/gromacs/package.py index 1aec4807239..31b5d701fe2 100644 --- a/var/spack/repos/builtin/packages/gromacs/package.py +++ b/var/spack/repos/builtin/packages/gromacs/package.py @@ -73,9 +73,12 @@ class Gromacs(CMakePackage): depends_on('mpi', when='+mpi') depends_on('plumed+mpi', when='+plumed+mpi') depends_on('plumed~mpi', when='+plumed~mpi') - depends_on('fftw') + depends_on('fftw+mpi', when='+mpi') + depends_on('fftw~mpi', when='~mpi') depends_on('cmake@2.8.8:3.99.99', type='build') depends_on('cmake@3.4.3:3.99.99', type='build', when='@2018:') + depends_on('cmake@3.13.0:3.99.99', type='build', when='@master') + depends_on('cmake@3.13.0:3.99.99', type='build', when='%fj') depends_on('cuda', when='+cuda') # TODO: openmpi constraint; remove when concretizer is fixed From ec58f28c209d1ffef51424b5d1423aa219c143a4 Mon Sep 17 00:00:00 2001 From: "Nichols A. Romero" Date: Wed, 24 Jun 2020 09:30:00 -0500 Subject: [PATCH 12/13] quantum-espresso: fix for scalapack with openmpi (#17179) --- .../repos/builtin/packages/quantum-espresso/package.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/var/spack/repos/builtin/packages/quantum-espresso/package.py b/var/spack/repos/builtin/packages/quantum-espresso/package.py index 35454ae3ef3..2fe01f864f8 100644 --- a/var/spack/repos/builtin/packages/quantum-espresso/package.py +++ b/var/spack/repos/builtin/packages/quantum-espresso/package.py @@ -307,7 +307,13 @@ def install(self, spec, prefix): options.append('BLAS_LIBS={0}'.format(lapack_blas.ld_flags)) if '+scalapack' in spec: - scalapack_option = 'intel' if '^mkl' in spec else 'yes' + if '^mkl' in spec: + if '^openmpi' in spec: + scalapack_option = 'yes' + else: # mpich, intel-mpi + scalapack_option = 'intel' + else: + scalapack_option = 'yes' options.append('--with-scalapack={0}'.format(scalapack_option)) if '+elpa' in spec: From bc53bb9b7cbf250c98cfe77d334ed30d6b958c21 Mon Sep 17 00:00:00 2001 From: Sergey Kosukhin Date: Wed, 24 Jun 2020 17:39:04 +0200 Subject: [PATCH 13/13] Unset environment variables that are most commonly used by Autotools packages. (#8623) --- lib/spack/spack/build_environment.py | 12 ++++++++++++ lib/spack/spack/util/environment.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index f997110b4c9..8a063377a7f 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -162,6 +162,18 @@ def clean_environment(): if 'PKGCONF' in varname: env.unset(varname) + # Unset the following variables because they can affect installation of + # Autotools and CMake packages. + build_system_vars = [ + 'CC', 'CFLAGS', 'CPP', 'CPPFLAGS', # C variables + 'CXX', 'CCC', 'CXXFLAGS', 'CXXCPP', # C++ variables + 'F77', 'FFLAGS', 'FLIBS', # Fortran77 variables + 'FC', 'FCFLAGS', 'FCLIBS', # Fortran variables + 'LDFLAGS', 'LIBS' # linker variables + ] + for v in build_system_vars: + env.unset(v) + build_lang = spack.config.get('config:build_language') if build_lang: # Override language-related variables. This can be used to force diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 8069f514319..3d69efa5ca5 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -376,7 +376,7 @@ def unset(self, name, **kwargs): """Stores a request to unset an environment variable. Args: - name: name of the environment variable to be set + name: name of the environment variable to be unset """ kwargs.update(self._get_outside_caller_attributes()) item = UnsetEnv(name, **kwargs)