From 32a3d59bfab37ad0deec1e07d69d02a6ee92b89d Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Wed, 11 Mar 2020 03:24:38 -0700 Subject: fetch_strategy: remove vestigial code (#15431) --- lib/spack/spack/fetch_strategy.py | 18 ------------------ 1 file changed, 18 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 5ae01286c4..eaf9f130c5 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -29,7 +29,6 @@ import os.path import re import shutil import sys -import xml.etree.ElementTree import llnl.util.tty as tty import six @@ -760,13 +759,6 @@ class GitFetchStrategy(VCSFetchStrategy): result = os.path.sep.join(['git', repo_path, repo_ref]) return result - def get_source_id(self): - if not self.branch: - return - output = self.git('ls-remote', self.url, self.branch, output=str) - if output: - return output.split()[0] - def _repo_info(self): args = '' @@ -944,11 +936,6 @@ class SvnFetchStrategy(VCSFetchStrategy): def source_id(self): return self.revision - def get_source_id(self): - output = self.svn('info', '--xml', self.url, output=str) - info = xml.etree.ElementTree.fromstring(output) - return info.find('entry/commit').get('revision') - def mirror_id(self): if self.revision: repo_path = url_util.parse(self.url).path @@ -1064,11 +1051,6 @@ class HgFetchStrategy(VCSFetchStrategy): result = os.path.sep.join(['hg', repo_path, self.revision]) return result - def get_source_id(self): - output = self.hg('id', self.url, output=str) - if output: - return output.strip() - @_needs_stage def fetch(self): if self.stage.expanded: -- cgit v1.2.3-70-g09d2 From 3753424a87ebb86e9b6c972a8d136c8c2f2f604b Mon Sep 17 00:00:00 2001 From: Michael Kuhn Date: Mon, 24 Feb 2020 15:34:57 +0100 Subject: modules: store configure args during build (#11084) This change stores packages' configure arguments during build and makes use of them while refreshing module files. This fixes problems such as in #10716. --- lib/spack/spack/installer.py | 16 ++++++++++++++++ lib/spack/spack/modules/common.py | 13 +++---------- lib/spack/spack/package.py | 15 +++++++++++++++ lib/spack/spack/test/install.py | 8 +++++++- 4 files changed, 41 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index f4f39f2640..714dc0f31c 100755 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -463,6 +463,10 @@ def log(pkg): # Archive the environment used for the build install(pkg.env_path, pkg.install_env_path) + if os.path.exists(pkg.configure_args_path): + # Archive the args used for the build + install(pkg.configure_args_path, pkg.install_configure_args_path) + # Finally, archive files that are specific to each package with working_dir(pkg.stage.path): errors = six.StringIO() @@ -1013,6 +1017,18 @@ class PackageInstaller(object): # Save the build environment in a file before building. dump_environment(pkg.env_path) + for attr in ('configure_args', 'cmake_args'): + try: + configure_args = getattr(pkg, attr)() + configure_args = ' '.join(configure_args) + + with open(pkg.configure_args_path, 'w') as args_file: + args_file.write(configure_args) + + break + except Exception: + pass + # cache debug settings debug_enabled = tty.is_debug() diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index d9407a1bf6..d6fb536e8f 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -623,16 +623,9 @@ class BaseContext(tengine.Context): msg = 'unknown, software installed outside of Spack' return msg - # This is quite simple right now, but contains information on how - # to call different build system classes. - for attr in ('configure_args', 'cmake_args'): - try: - configure_args = getattr(pkg, attr)() - return ' '.join(configure_args) - except (AttributeError, IOError, KeyError): - # The method doesn't exist in the current spec, - # or it's not usable - pass + if os.path.exists(pkg.install_configure_args_path): + with open(pkg.install_configure_args_path, 'r') as args_file: + return args_file.read() # Returning a false-like value makes the default templates skip # the configure option section diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index d123f58cdf..b7efc3989b 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -68,6 +68,9 @@ _spack_build_logfile = 'spack-build-out.txt' # Filename for the Spack build/install environment file. _spack_build_envfile = 'spack-build-env.txt' +# Filename for the Spack configure args file. +_spack_configure_argsfile = 'spack-configure-args.txt' + class InstallPhase(object): """Manages a single phase of the installation. @@ -896,6 +899,18 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # Otherwise, return the current install log path name. return os.path.join(install_path, _spack_build_logfile) + @property + def configure_args_path(self): + """Return the configure args file path associated with staging.""" + return os.path.join(self.stage.path, _spack_configure_argsfile) + + @property + def install_configure_args_path(self): + """Return the configure args file path on successful installation.""" + install_path = spack.store.layout.metadata_path(self.spec) + + return os.path.join(install_path, _spack_configure_argsfile) + def _make_fetcher(self): # Construct a composite fetcher that always contains at least # one element (the root package). In case there are resources diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index f53a760f70..ff7f5a3355 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -15,7 +15,8 @@ import spack.patch import spack.repo import spack.store from spack.spec import Spec -from spack.package import _spack_build_envfile, _spack_build_logfile +from spack.package import (_spack_build_envfile, _spack_build_logfile, + _spack_configure_argsfile) def test_install_and_uninstall(install_mockery, mock_fetch, monkeypatch): @@ -410,6 +411,9 @@ def test_pkg_install_paths(install_mockery): env_path = os.path.join(spec.prefix, '.spack', _spack_build_envfile) assert spec.package.install_env_path == env_path + args_path = os.path.join(spec.prefix, '.spack', _spack_configure_argsfile) + assert spec.package.install_configure_args_path == args_path + # Backward compatibility checks log_dir = os.path.dirname(log_path) mkdirp(log_dir) @@ -448,6 +452,7 @@ def test_pkg_install_log(install_mockery): with working_dir(log_dir): touch(log_path) touch(spec.package.env_path) + touch(spec.package.configure_args_path) install_path = os.path.dirname(spec.package.install_log_path) mkdirp(install_path) @@ -456,6 +461,7 @@ def test_pkg_install_log(install_mockery): assert os.path.exists(spec.package.install_log_path) assert os.path.exists(spec.package.install_env_path) + assert os.path.exists(spec.package.install_configure_args_path) # Cleanup shutil.rmtree(log_dir) -- cgit v1.2.3-70-g09d2 From 654914d53e2bb578eb8d82ca02a0547cf7834f5d Mon Sep 17 00:00:00 2001 From: Andrew W Elble Date: Fri, 6 Mar 2020 19:29:01 -0500 Subject: Fix for being able to 'spack load' packages that have been renamed. (#14348) * Fix for being able to 'spack load' packages that have been renamed. * tests: add test for 'spack load' of a installed, but renamed/deleted package --- lib/spack/spack/modules/common.py | 6 +++++- lib/spack/spack/test/modules/common.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index d6fb536e8f..c09a225116 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -342,7 +342,11 @@ def get_module(module_type, spec, get_full_path, required=True): The module name or path. May return ``None`` if the module is not available. """ - if spec.package.installed_upstream: + try: + upstream = spec.package.installed_upstream + except spack.repo.UnknownPackageError: + upstream, record = spack.store.db.query_by_spec_hash(spec.dag_hash()) + if upstream: module = (spack.modules.common.upstream_module_index .upstream_module(spec, module_type)) if not module: diff --git a/lib/spack/spack/test/modules/common.py b/lib/spack/spack/test/modules/common.py index cc2eb61d0b..f2cb60e1db 100644 --- a/lib/spack/spack/test/modules/common.py +++ b/lib/spack/spack/test/modules/common.py @@ -11,6 +11,7 @@ import collections import spack.spec import spack.modules.tcl from spack.modules.common import UpstreamModuleIndex +from spack.spec import Spec import spack.error @@ -183,3 +184,33 @@ module_index: assert m1_path == '/path/to/a' finally: spack.modules.common.upstream_module_index = old_index + + +def test_load_installed_package_not_in_repo(install_mockery, mock_fetch, + monkeypatch): + # Get a basic concrete spec for the trivial install package. + spec = Spec('trivial-install-test-package') + spec.concretize() + assert spec.concrete + + # Get the package + pkg = spec.package + + def find_nothing(*args): + raise spack.repo.UnknownPackageError( + 'Repo package access is disabled for test') + + try: + pkg.do_install() + + spec._package = None + monkeypatch.setattr(spack.repo, 'get', find_nothing) + with pytest.raises(spack.repo.UnknownPackageError): + spec.package + + module_path = spack.modules.common.get_module('tcl', spec, True) + assert module_path + pkg.do_uninstall() + except Exception: + pkg.remove_prefix() + raise -- cgit v1.2.3-70-g09d2 From b02981f10ccfa336d6f777d0517468ba0429c42f Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Mon, 2 Mar 2020 11:16:20 -0800 Subject: bugfix: ensure proper dependency handling for package-only installs (#15197) The distributed build PR (#13100) -- did not check the install status of dependencies when using the `--only package` option so would refuse to install a package with the claim that it had uninstalled dependencies whether that was the case or not. - [x] add install status checks for the `--only package` case. - [x] add initial set of tests --- lib/spack/spack/installer.py | 77 ++++++++++++++++++++++++++++++++----- lib/spack/spack/test/cmd/install.py | 24 ++++++++++++ lib/spack/spack/test/installer.py | 65 +++++++++++++++++++++++++++---- 3 files changed, 149 insertions(+), 17 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 714dc0f31c..db82844cc5 100755 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -651,6 +651,66 @@ class PackageInstaller(object): if package_id(comp_pkg) not in self.build_tasks: self._push_task(comp_pkg, is_compiler, 0, 0, STATUS_ADDED) + def _check_db(self, spec): + """Determine if the spec is flagged as installed in the database + + Args: + spec (Spec): spec whose database install status is being checked + + Return: + (rec, installed_in_db) tuple where rec is the database record, or + None, if there is no matching spec, and installed_in_db is + ``True`` if the spec is considered installed and ``False`` + otherwise + """ + try: + rec = spack.store.db.get_record(spec) + installed_in_db = rec.installed if rec else False + except KeyError: + # KeyError is raised if there is no matching spec in the database + # (versus no matching specs that are installed). + rec = None + installed_in_db = False + return rec, installed_in_db + + def _check_deps_status(self): + """Check the install status of the explicit spec's dependencies""" + + err = 'Cannot proceed with {0}: {1}' + for dep in self.spec.traverse(order='post', root=False): + dep_pkg = dep.package + dep_id = package_id(dep_pkg) + + # Check for failure since a prefix lock is not required + if spack.store.db.prefix_failed(dep): + action = "'spack install' the dependency" + msg = '{0} is marked as an install failure: {1}' \ + .format(dep_id, action) + raise InstallError(err.format(self.pkg_id, msg)) + + # Attempt to get a write lock to ensure another process does not + # uninstall the dependency while the requested spec is being + # installed + ltype, lock = self._ensure_locked('write', dep_pkg) + if lock is None: + msg = '{0} is write locked by another process'.format(dep_id) + raise InstallError(err.format(self.pkg_id, msg)) + + # Flag external and upstream packages as being installed + if dep_pkg.spec.external or dep_pkg.installed_upstream: + form = 'external' if dep_pkg.spec.external else 'upstream' + tty.debug('Flagging {0} {1} as installed'.format(form, dep_id)) + self.installed.add(dep_id) + continue + + # Check the database to see if the dependency has been installed + # and flag as such if appropriate + rec, installed_in_db = self._check_db(dep) + if installed_in_db: + tty.debug('Flagging {0} as installed per the database' + .format(dep_id)) + self.installed.add(dep_id) + def _prepare_for_install(self, task, keep_prefix, keep_stage, restage=False): """ @@ -680,14 +740,7 @@ class PackageInstaller(object): return # Determine if the spec is flagged as installed in the database - try: - rec = spack.store.db.get_record(task.pkg.spec) - installed_in_db = rec.installed if rec else False - except KeyError: - # KeyError is raised if there is no matching spec in the database - # (versus no matching specs that are installed). - rec = None - installed_in_db = False + rec, installed_in_db = self._check_db(task.pkg.spec) # Make sure the installation directory is in the desired state # for uninstalled specs. @@ -929,6 +982,11 @@ class PackageInstaller(object): # Be sure to clear any previous failure spack.store.db.clear_failure(self.pkg.spec, force=True) + # If not installing dependencies, then determine their + # installation status before proceeding + if not install_deps: + self._check_deps_status() + # Now add the package itself, if appropriate self._push_task(self.pkg, False, 0, 0, STATUS_ADDED) @@ -1022,7 +1080,8 @@ class PackageInstaller(object): configure_args = getattr(pkg, attr)() configure_args = ' '.join(configure_args) - with open(pkg.configure_args_path, 'w') as args_file: + with open(pkg.configure_args_path, 'w') as \ + args_file: args_file.write(configure_args) break diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index 2697a539bf..1d4a0c6827 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -632,6 +632,30 @@ def test_install_only_dependencies(tmpdir, mock_fetch, install_mockery): assert not os.path.exists(root.prefix) +def test_install_only_package(tmpdir, mock_fetch, install_mockery, capfd): + msg = '' + with capfd.disabled(): + try: + install('--only', 'package', 'dependent-install') + except spack.installer.InstallError as e: + msg = str(e) + + assert 'Cannot proceed with dependent-install' in msg + assert '1 uninstalled dependency' in msg + + +def test_install_deps_then_package(tmpdir, mock_fetch, install_mockery): + dep = Spec('dependency-install').concretized() + root = Spec('dependent-install').concretized() + + install('--only', 'dependencies', 'dependent-install') + assert os.path.exists(dep.prefix) + assert not os.path.exists(root.prefix) + + install('--only', 'package', 'dependent-install') + assert os.path.exists(root.prefix) + + @pytest.mark.regression('12002') def test_install_only_dependencies_in_env(tmpdir, mock_fetch, install_mockery, mutable_mock_env_path): diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 7d2eec8caf..eadebfd3e4 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -27,6 +27,12 @@ def _none(*args, **kwargs): return None +def _not_locked(installer, lock_type, pkg): + """Generic monkeypatch function for _ensure_locked to return no lock""" + tty.msg('{0} locked {1}' .format(lock_type, pkg.spec.name)) + return lock_type, None + + def _true(*args, **kwargs): """Generic monkeypatch function that always returns True.""" return True @@ -285,6 +291,57 @@ def test_dump_packages_deps(install_mockery, tmpdir): inst.dump_packages(spec, '.') +@pytest.mark.tld +def test_check_deps_status_errs(install_mockery, monkeypatch): + """Test to cover _check_deps_status failures.""" + spec, installer = create_installer('a') + + # Make sure the package is identified as failed + orig_fn = spack.database.Database.prefix_failed + monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true) + + with pytest.raises(inst.InstallError, matches='install failure'): + installer._check_deps_status() + + monkeypatch.setattr(spack.database.Database, 'prefix_failed', orig_fn) + + # Ensure do not acquire the lock + monkeypatch.setattr(inst.PackageInstaller, '_ensure_locked', _not_locked) + + with pytest.raises(inst.InstallError, matches='write locked by another'): + installer._check_deps_status() + + +@pytest.mark.tld +def test_check_deps_status_external(install_mockery, monkeypatch): + """Test to cover _check_deps_status for external.""" + spec, installer = create_installer('a') + + deps = spec.dependencies() + assert len(deps) > 0 + dep_id = 'b' + + # Ensure the known dependent is installed if flagged as external + monkeypatch.setattr(spack.spec.Spec, 'external', True) + installer._check_deps_status() + assert dep_id in installer.installed + + +@pytest.mark.tld +def test_check_deps_status_upstream(install_mockery, monkeypatch): + """Test to cover _check_deps_status for upstream.""" + spec, installer = create_installer('a') + + deps = spec.dependencies() + assert len(deps) > 0 + dep_id = 'b' + + # Ensure the known dependent, b, is installed if flagged as upstream + monkeypatch.setattr(spack.package.PackageBase, 'installed_upstream', True) + installer._check_deps_status() + assert dep_id in installer.installed + + def test_add_bootstrap_compilers(install_mockery, monkeypatch): """Test to cover _add_bootstrap_compilers.""" def _pkgs(pkg): @@ -458,10 +515,6 @@ def test_install_lock_failures(install_mockery, monkeypatch, capfd): def _requeued(installer, task): tty.msg('requeued {0}' .format(task.pkg.spec.name)) - def _not_locked(installer, lock_type, pkg): - tty.msg('{0} locked {1}' .format(lock_type, pkg.spec.name)) - return lock_type, None - spec, installer = create_installer('b') # Ensure never acquire a lock @@ -485,10 +538,6 @@ def test_install_lock_installed_requeue(install_mockery, monkeypatch, capfd): def _install(installer, task, **kwargs): tty.msg('{0} installing'.format(task.pkg.spec.name)) - def _not_locked(installer, lock_type, pkg): - tty.msg('{0} locked {1}' .format(lock_type, pkg.spec.name)) - return lock_type, None - def _prep(installer, task, keep_prefix, keep_stage, restage): installer.installed.add('b') tty.msg('{0} is installed' .format(task.pkg.spec.name)) -- cgit v1.2.3-70-g09d2 From f3a1a8c6fe13b6f92a6c44e771aff5d1f9683a77 Mon Sep 17 00:00:00 2001 From: "Seth R. Johnson" Date: Wed, 26 Feb 2020 12:03:28 -0500 Subject: Uniquify suffixes added to module names (#14920) --- lib/spack/spack/modules/common.py | 1 + lib/spack/spack/test/data/modules/tcl/suffix.yaml | 1 + lib/spack/spack/test/modules/tcl.py | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index c09a225116..8dee443eb3 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -428,6 +428,7 @@ class BaseConfiguration(object): for constraint, suffix in self.conf.get('suffixes', {}).items(): if constraint in self.spec: suffixes.append(suffix) + suffixes = sorted(set(suffixes)) if self.hash: suffixes.append(self.hash) return suffixes diff --git a/lib/spack/spack/test/data/modules/tcl/suffix.yaml b/lib/spack/spack/test/data/modules/tcl/suffix.yaml index 015ac63b0f..57703e1733 100644 --- a/lib/spack/spack/test/data/modules/tcl/suffix.yaml +++ b/lib/spack/spack/test/data/modules/tcl/suffix.yaml @@ -5,3 +5,4 @@ tcl: suffixes: '+debug': foo '~debug': bar + '^mpich': foo diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index 70eee24461..40cb7be5ef 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -215,9 +215,10 @@ class TestTcl(object): writer, spec = factory('mpileaks+debug arch=x86-linux') assert 'foo' in writer.layout.use_name + assert 'foo-foo' not in writer.layout.use_name writer, spec = factory('mpileaks~debug arch=x86-linux') - assert 'bar' in writer.layout.use_name + assert 'bar-foo' in writer.layout.use_name def test_setup_environment(self, modulefile_content, module_configuration): """Tests the internal set-up of run-time environment.""" -- cgit v1.2.3-70-g09d2 From 5406e1f43ded0f124fe45b1a8568005c6166628b Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Wed, 26 Feb 2020 18:49:29 -0800 Subject: bugfix: Add dependents when initializing spec from yaml (#15220) The new build process, introduced in #13100 , relies on a spec's dependents in addition to their dependencies. Loading a spec from a yaml file was not initializing the dependents. - [x] populate dependents when loading from yaml --- lib/spack/spack/spec.py | 4 +--- lib/spack/spack/test/concretize_preferences.py | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 4b6bd67b6d..1493f0c973 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1919,9 +1919,7 @@ class Spec(object): yaml_deps = node[name]['dependencies'] for dname, dhash, dtypes in Spec.read_yaml_dep_specs(yaml_deps): - # Fill in dependencies by looking them up by name in deps dict - deps[name]._dependencies[dname] = DependencySpec( - deps[name], deps[dname], dtypes) + deps[name]._add_dependency(deps[dname], dtypes) return spec diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index 0d0d870506..81b5360869 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -15,15 +15,15 @@ from spack.version import Version @pytest.fixture() -def concretize_scope(config, tmpdir): +def concretize_scope(mutable_config, tmpdir): """Adds a scope for concretization preferences""" tmpdir.ensure_dir('concretize') - config.push_scope( + mutable_config.push_scope( ConfigScope('concretize', str(tmpdir.join('concretize')))) yield - config.pop_scope() + mutable_config.pop_scope() spack.repo.path._provider_index = None @@ -84,16 +84,24 @@ class TestConcretizePreferences(object): 'mpileaks', debug=True, opt=True, shared=False, static=False ) - def test_preferred_compilers(self, mutable_mock_repo): + def test_preferred_compilers(self): """Test preferred compilers are applied correctly """ - update_packages('mpileaks', 'compiler', ['clang@3.3']) + # Need to make sure the test uses an available compiler + compiler_list = spack.compilers.all_compiler_specs() + assert compiler_list + + # Try the first available compiler + compiler = str(compiler_list[0]) + update_packages('mpileaks', 'compiler', [compiler]) spec = concretize('mpileaks') - assert spec.compiler == spack.spec.CompilerSpec('clang@3.3') + assert spec.compiler == spack.spec.CompilerSpec(compiler) - update_packages('mpileaks', 'compiler', ['gcc@4.5.0']) + # Try the last available compiler + compiler = str(compiler_list[-1]) + update_packages('mpileaks', 'compiler', [compiler]) spec = concretize('mpileaks') - assert spec.compiler == spack.spec.CompilerSpec('gcc@4.5.0') + assert spec.compiler == spack.spec.CompilerSpec(compiler) def test_preferred_target(self, mutable_mock_repo): """Test preferred compilers are applied correctly -- cgit v1.2.3-70-g09d2 From fa0a5e44aa7616bdef4cc825436bfb207c03bf56 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 5 Mar 2020 11:16:02 +0100 Subject: Correct pytest.raises matches to match (#15346) --- lib/spack/spack/test/database.py | 2 +- lib/spack/spack/test/installer.py | 14 +++++++------- lib/spack/spack/test/llnl/util/lock.py | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py index 8f0f4dff6a..88e6c42693 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -755,7 +755,7 @@ def test_query_spec_with_non_conditional_virtual_dependency(database): def test_failed_spec_path_error(database): """Ensure spec not concrete check is covered.""" s = spack.spec.Spec('a') - with pytest.raises(ValueError, matches='Concrete spec required'): + with pytest.raises(ValueError, match='Concrete spec required'): spack.store.db._failed_spec_path(s) diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index eadebfd3e4..c261ca7a63 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -245,7 +245,7 @@ def test_ensure_locked_have(install_mockery, tmpdir): def test_package_id(install_mockery): """Test to cover package_id functionality.""" pkg = spack.repo.get('trivial-install-test-package') - with pytest.raises(ValueError, matches='spec is not concretized'): + with pytest.raises(ValueError, match='spec is not concretized'): inst.package_id(pkg) spec = spack.spec.Spec('trivial-install-test-package') @@ -280,7 +280,7 @@ def test_packages_needed_to_bootstrap_compiler(install_mockery, monkeypatch): # Test up to the dependency check monkeypatch.setattr(spack.compilers, 'compilers_for_spec', _no_compilers) - with pytest.raises(spack.repo.UnknownPackageError, matches='not found'): + with pytest.raises(spack.repo.UnknownPackageError, match='not found'): inst._packages_needed_to_bootstrap_compiler(spec.package) @@ -300,7 +300,7 @@ def test_check_deps_status_errs(install_mockery, monkeypatch): orig_fn = spack.database.Database.prefix_failed monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true) - with pytest.raises(inst.InstallError, matches='install failure'): + with pytest.raises(inst.InstallError, match='install failure'): installer._check_deps_status() monkeypatch.setattr(spack.database.Database, 'prefix_failed', orig_fn) @@ -308,7 +308,7 @@ def test_check_deps_status_errs(install_mockery, monkeypatch): # Ensure do not acquire the lock monkeypatch.setattr(inst.PackageInstaller, '_ensure_locked', _not_locked) - with pytest.raises(inst.InstallError, matches='write locked by another'): + with pytest.raises(inst.InstallError, match='write locked by another'): installer._check_deps_status() @@ -485,7 +485,7 @@ def test_install_uninstalled_deps(install_mockery, monkeypatch, capsys): monkeypatch.setattr(inst.PackageInstaller, '_update_failed', _noop) msg = 'Cannot proceed with dependent-install' - with pytest.raises(spack.installer.InstallError, matches=msg): + with pytest.raises(spack.installer.InstallError, match=msg): installer.install() out = str(capsys.readouterr()) @@ -503,7 +503,7 @@ def test_install_failed(install_mockery, monkeypatch, capsys): monkeypatch.setattr(inst.PackageInstaller, '_install_task', _noop) msg = 'Installation of b failed' - with pytest.raises(spack.installer.InstallError, matches=msg): + with pytest.raises(spack.installer.InstallError, match=msg): installer.install() out = str(capsys.readouterr()) @@ -622,7 +622,7 @@ def test_install_dir_exists(install_mockery, monkeypatch, capfd): spec, installer = create_installer('b') - with pytest.raises(dl.InstallDirectoryAlreadyExistsError, matches=err): + with pytest.raises(dl.InstallDirectoryAlreadyExistsError, match=err): installer.install() assert 'b' in installer.installed diff --git a/lib/spack/spack/test/llnl/util/lock.py b/lib/spack/spack/test/llnl/util/lock.py index 7236e1dbf9..b2b7cf85ac 100644 --- a/lib/spack/spack/test/llnl/util/lock.py +++ b/lib/spack/spack/test/llnl/util/lock.py @@ -1272,7 +1272,7 @@ def test_downgrade_write_fails(tmpdir): lock = lk.Lock('lockfile') lock.acquire_read() msg = 'Cannot downgrade lock from write to read on file: lockfile' - with pytest.raises(lk.LockDowngradeError, matches=msg): + with pytest.raises(lk.LockDowngradeError, match=msg): lock.downgrade_write_to_read() @@ -1292,5 +1292,5 @@ def test_upgrade_read_fails(tmpdir): lock = lk.Lock('lockfile') lock.acquire_write() msg = 'Cannot upgrade lock from read to write on file: lockfile' - with pytest.raises(lk.LockUpgradeError, matches=msg): + with pytest.raises(lk.LockUpgradeError, match=msg): lock.upgrade_read_to_write() -- cgit v1.2.3-70-g09d2 From 733f9f8cfa5814cd1df6c34cf7e70f460ce75ff4 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Thu, 5 Mar 2020 16:54:29 -0800 Subject: Recover coverage from subprocesses during unit tests (#15354) * Recover coverage from subprocesses during unit tests --- lib/spack/spack/cmd/test.py | 2 +- lib/spack/spack/test/cmd/test.py | 11 ++++++----- lib/spack/spack/test/llnl/util/log.py | 9 ++++++--- lib/spack/spack/test/pytest.ini | 10 ---------- pytest.ini | 10 ++++++++++ share/spack/qa/run-unit-tests | 8 ++------ 6 files changed, 25 insertions(+), 25 deletions(-) delete mode 100644 lib/spack/spack/test/pytest.ini create mode 100644 pytest.ini (limited to 'lib') diff --git a/lib/spack/spack/cmd/test.py b/lib/spack/spack/cmd/test.py index f2ca8fc93b..8cbc0fbccf 100644 --- a/lib/spack/spack/cmd/test.py +++ b/lib/spack/spack/cmd/test.py @@ -154,7 +154,7 @@ def test(parser, args, unknown_args): # The default is to test the core of Spack. If the option `--extension` # has been used, then test that extension. - pytest_root = spack.paths.test_path + pytest_root = spack.paths.spack_root if args.extension: target = args.extension extensions = spack.config.get('config:extensions') diff --git a/lib/spack/spack/test/cmd/test.py b/lib/spack/spack/test/cmd/test.py index 9a64209cfa..a9ef735afe 100644 --- a/lib/spack/spack/test/cmd/test.py +++ b/lib/spack/spack/test/cmd/test.py @@ -6,6 +6,7 @@ from spack.main import SpackCommand spack_test = SpackCommand('test') +cmd_test_py = 'lib/spack/spack/test/cmd/test.py' def test_list(): @@ -16,13 +17,13 @@ def test_list(): def test_list_with_pytest_arg(): - output = spack_test('--list', 'cmd/test.py') - assert output.strip() == "cmd/test.py" + output = spack_test('--list', cmd_test_py) + assert output.strip() == cmd_test_py def test_list_with_keywords(): output = spack_test('--list', '-k', 'cmd/test.py') - assert output.strip() == "cmd/test.py" + assert output.strip() == cmd_test_py def test_list_long(capsys): @@ -44,7 +45,7 @@ def test_list_long(capsys): def test_list_long_with_pytest_arg(capsys): with capsys.disabled(): - output = spack_test('--list-long', 'cmd/test.py') + output = spack_test('--list-long', cmd_test_py) assert "test.py::\n" in output assert "test_list" in output assert "test_list_with_pytest_arg" in output @@ -74,7 +75,7 @@ def test_list_names(): def test_list_names_with_pytest_arg(): - output = spack_test('--list-names', 'cmd/test.py') + output = spack_test('--list-names', cmd_test_py) assert "test.py::test_list\n" in output assert "test.py::test_list_with_pytest_arg\n" in output assert "test.py::test_list_with_keywords\n" in output diff --git a/lib/spack/spack/test/llnl/util/log.py b/lib/spack/spack/test/llnl/util/log.py index 1eae1ccf69..0c879c5a6e 100644 --- a/lib/spack/spack/test/llnl/util/log.py +++ b/lib/spack/spack/test/llnl/util/log.py @@ -32,7 +32,8 @@ def test_log_python_output_with_fd_stream(capfd, tmpdir): with open('foo.txt') as f: assert f.read() == 'logged\n' - assert capfd.readouterr() == ('', '') + # Coverage is cluttering stderr during tests + assert capfd.readouterr()[0] == '' def test_log_python_output_and_echo_output(capfd, tmpdir): @@ -42,7 +43,8 @@ def test_log_python_output_and_echo_output(capfd, tmpdir): print('echo') print('logged') - assert capfd.readouterr() == ('echo\n', '') + # Coverage is cluttering stderr during tests + assert capfd.readouterr()[0] == 'echo\n' with open('foo.txt') as f: assert f.read() == 'echo\nlogged\n' @@ -75,7 +77,8 @@ def test_log_subproc_and_echo_output(capfd, tmpdir): echo('echo') print('logged') - assert capfd.readouterr() == ('echo\n', '') + # Coverage is cluttering stderr during tests + assert capfd.readouterr()[0] == 'echo\n' with open('foo.txt') as f: assert f.read() == 'logged\n' diff --git a/lib/spack/spack/test/pytest.ini b/lib/spack/spack/test/pytest.ini deleted file mode 100644 index 59839c0005..0000000000 --- a/lib/spack/spack/test/pytest.ini +++ /dev/null @@ -1,10 +0,0 @@ -# content of pytest.ini -[pytest] -addopts = --durations=20 -ra -testpaths = . -python_files = *.py -markers = - db: tests that require creating a DB - network: tests that require access to the network - maybeslow: tests that may be slow (e.g. access a lot the filesystem, etc.) - regression: tests that fix a reported bug diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000000..baf163ef0b --- /dev/null +++ b/pytest.ini @@ -0,0 +1,10 @@ +# content of pytest.ini +[pytest] +addopts = --durations=20 -ra +testpaths = lib/spack/spack/test +python_files = *.py +markers = + db: tests that require creating a DB + network: tests that require access to the network + maybeslow: tests that may be slow (e.g. access a lot the filesystem, etc.) + regression: tests that fix a reported bug diff --git a/share/spack/qa/run-unit-tests b/share/spack/qa/run-unit-tests index 52748dacdf..01f564e5e1 100755 --- a/share/spack/qa/run-unit-tests +++ b/share/spack/qa/run-unit-tests @@ -37,16 +37,12 @@ bin/spack -h bin/spack help -a # Profile and print top 20 lines for a simple call to spack spec -bin/spack -p --lines 20 spec mpileaks%gcc ^elfutils@0.170 +spack -p --lines 20 spec mpileaks%gcc ^elfutils@0.170 #----------------------------------------------------------- # Run unit tests with code coverage #----------------------------------------------------------- -extra_args="" -if [[ -n "$@" ]]; then - extra_args="-k $@" -fi -$coverage_run bin/spack test -x --verbose "$extra_args" +$coverage_run $(which spack) test -x --verbose #----------------------------------------------------------- # Run tests for setup-env.sh -- cgit v1.2.3-70-g09d2 From 59a7963785b0e443d5682d497af373383876da20 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 5 Mar 2020 10:26:50 +0100 Subject: Bugfix: resolve StopIteration message attribute failure (#15341) Testing the install StopIteration exception resulted in an attribute error: AttributeError: 'StopIteration' object has no attribute 'message' This PR adds a unit test and resolves that error. --- lib/spack/spack/installer.py | 2 +- lib/spack/spack/test/installer.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index db82844cc5..bd38d11ea2 100755 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -1154,7 +1154,7 @@ class PackageInstaller(object): except StopIteration as e: # A StopIteration exception means that do_install was asked to # stop early from clients. - tty.msg('{0} {1}'.format(self.pid, e.message)) + tty.msg('{0} {1}'.format(self.pid, str(e))) tty.msg('Package stage directory : {0}' .format(pkg.stage.source_path)) diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index c261ca7a63..8c3a232f19 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -391,6 +391,27 @@ def test_install_task_use_cache(install_mockery, monkeypatch): assert spec.package.name in installer.installed +def test_install_task_stop_iter(install_mockery, monkeypatch, capfd): + """Test _install_task to cover the StopIteration exception.""" + mock_err_msg = 'mock stop iteration' + + def _raise(installer, pkg): + raise StopIteration(mock_err_msg) + + spec, installer = create_installer('a') + task = create_build_task(spec.package) + + monkeypatch.setattr(spack.package.PackageBase, 'unit_test_check', _true) + monkeypatch.setattr(inst.PackageInstaller, '_setup_install_dir', _raise) + + installer._install_task(task) + out = capfd.readouterr()[0] + + assert mock_err_msg in out + assert 'Package stage directory' in out + assert spec.package.stage.source_path in out + + def test_release_lock_write_n_exception(install_mockery, tmpdir, capsys): """Test _release_lock for supposed write lock with exception.""" spec, installer = create_installer('trivial-install-test-package') -- cgit v1.2.3-70-g09d2 From 9a1ce36e44fb5048952ae98209ddd60e0c137379 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Wed, 11 Mar 2020 03:39:23 -0700 Subject: bugfix: resolve undefined source_pkg_dir failure (#15339) --- lib/spack/spack/installer.py | 11 +++--- lib/spack/spack/test/installer.py | 71 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 74 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index bd38d11ea2..4559fd70ba 100755 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -407,7 +407,10 @@ def dump_packages(spec, path): source_repo = spack.repo.Repo(source_repo_root) source_pkg_dir = source_repo.dirname_for_package_name( node.name) - except spack.repo.RepoError: + except spack.repo.RepoError as err: + tty.debug('Failed to create source repo for {0}: {1}' + .format(node.name, str(err))) + source_pkg_dir = None tty.warn("Warning: Couldn't copy in provenance for {0}" .format(node.name)) @@ -419,10 +422,10 @@ def dump_packages(spec, path): # Get the location of the package in the dest repo. dest_pkg_dir = repo.dirname_for_package_name(node.name) - if node is not spec: - install_tree(source_pkg_dir, dest_pkg_dir) - else: + if node is spec: spack.repo.path.dump_provenance(node, dest_pkg_dir) + elif source_pkg_dir: + install_tree(source_pkg_dir, dest_pkg_dir) def install_msg(name, pid): diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 8c3a232f19..b9730c3165 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import os +import py import pytest import llnl.util.tty as tty @@ -17,6 +18,22 @@ import spack.repo import spack.spec +def _mock_repo(root, namespace): + """Create an empty repository at the specified root + + Args: + root (str): path to the mock repository root + namespace (str): mock repo's namespace + """ + repodir = py.path.local(root) if isinstance(root, str) else root + repodir.ensure(spack.repo.packages_dir_name, dir=True) + yaml = repodir.join('repo.yaml') + yaml.write(""" +repo: + namespace: {0} +""".format(namespace)) + + def _noop(*args, **kwargs): """Generic monkeypatch no-op routine.""" pass @@ -284,11 +301,57 @@ def test_packages_needed_to_bootstrap_compiler(install_mockery, monkeypatch): inst._packages_needed_to_bootstrap_compiler(spec.package) -def test_dump_packages_deps(install_mockery, tmpdir): - """Test to add coverage to dump_packages.""" +def test_dump_packages_deps_ok(install_mockery, tmpdir, mock_repo_path): + """Test to add coverage to dump_packages with dependencies happy path.""" + + spec_name = 'simple-inheritance' + spec = spack.spec.Spec(spec_name).concretized() + inst.dump_packages(spec, str(tmpdir)) + + repo = mock_repo_path.repos[0] + dest_pkg = repo.filename_for_package_name(spec_name) + assert os.path.isfile(dest_pkg) + + +def test_dump_packages_deps_errs(install_mockery, tmpdir, monkeypatch, capsys): + """Test to add coverage to dump_packages with dependencies.""" + orig_bpp = spack.store.layout.build_packages_path + orig_dirname = spack.repo.Repo.dirname_for_package_name + repo_err_msg = "Mock dirname_for_package_name" + + def bpp_path(spec): + # Perform the original function + source = orig_bpp(spec) + # Mock the required directory structure for the repository + _mock_repo(os.path.join(source, spec.namespace), spec.namespace) + return source + + def _repoerr(repo, name): + if name == 'cmake': + raise spack.repo.RepoError(repo_err_msg) + else: + return orig_dirname(repo, name) + + # Now mock the creation of the required directory structure to cover + # the try-except block + monkeypatch.setattr(spack.store.layout, 'build_packages_path', bpp_path) + spec = spack.spec.Spec('simple-inheritance').concretized() - with tmpdir.as_cwd(): - inst.dump_packages(spec, '.') + path = str(tmpdir) + + # The call to install_tree will raise the exception since not mocking + # creation of dependency package files within *install* directories. + with pytest.raises(IOError, match=path): + inst.dump_packages(spec, path) + + # Now try the error path, which requires the mock directory structure + # above + monkeypatch.setattr(spack.repo.Repo, 'dirname_for_package_name', _repoerr) + with pytest.raises(spack.repo.RepoError, match=repo_err_msg): + inst.dump_packages(spec, path) + + out = str(capsys.readouterr()[1]) + assert "Couldn't copy in provenance for cmake" in out @pytest.mark.tld -- cgit v1.2.3-70-g09d2 From 1c8f792bb5e3fd96d2cfb341d988e42923956f3f Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Thu, 12 Mar 2020 13:20:20 -0700 Subject: testing: increase installer coverage (#15237) --- lib/spack/spack/installer.py | 51 ++++---- lib/spack/spack/test/install.py | 69 +++++++--- lib/spack/spack/test/installer.py | 269 ++++++++++++++++++++++++++++---------- 3 files changed, 280 insertions(+), 109 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 4559fd70ba..4e8ec39733 100755 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -36,6 +36,7 @@ import six import sys import time +import llnl.util.filesystem as fs import llnl.util.lock as lk import llnl.util.tty as tty import spack.binary_distribution as binary_distribution @@ -43,14 +44,12 @@ import spack.compilers import spack.error import spack.hooks import spack.package +import spack.package_prefs as prefs import spack.repo import spack.store -from llnl.util.filesystem import \ - chgrp, install, install_tree, mkdirp, touch, working_dir from llnl.util.tty.color import colorize, cwrite from llnl.util.tty.log import log_output -from spack.package_prefs import get_package_dir_permissions, get_package_group from spack.util.environment import dump_environment from spack.util.executable import which @@ -133,21 +132,21 @@ def _do_fake_install(pkg): chmod = which('chmod') # Install fake command - mkdirp(pkg.prefix.bin) - touch(os.path.join(pkg.prefix.bin, command)) + fs.mkdirp(pkg.prefix.bin) + fs.touch(os.path.join(pkg.prefix.bin, command)) chmod('+x', os.path.join(pkg.prefix.bin, command)) # Install fake header file - mkdirp(pkg.prefix.include) - touch(os.path.join(pkg.prefix.include, header + '.h')) + fs.mkdirp(pkg.prefix.include) + fs.touch(os.path.join(pkg.prefix.include, header + '.h')) # Install fake shared and static libraries - mkdirp(pkg.prefix.lib) + fs.mkdirp(pkg.prefix.lib) for suffix in [dso_suffix, '.a']: - touch(os.path.join(pkg.prefix.lib, library + suffix)) + fs.touch(os.path.join(pkg.prefix.lib, library + suffix)) # Install fake man page - mkdirp(pkg.prefix.man.man1) + fs.mkdirp(pkg.prefix.man.man1) packages_dir = spack.store.layout.build_packages_path(pkg.spec) dump_packages(pkg.spec, packages_dir) @@ -377,14 +376,14 @@ def dump_packages(spec, path): Dump all package information for a spec and its dependencies. This creates a package repository within path for every namespace in the - spec DAG, and fills the repos wtih package files and patch files for every + spec DAG, and fills the repos with package files and patch files for every node in the DAG. Args: spec (Spec): the Spack spec whose package information is to be dumped path (str): the path to the build packages directory """ - mkdirp(path) + fs.mkdirp(path) # Copy in package.py files from any dependencies. # Note that we copy them in as they are in the *install* directory @@ -425,7 +424,7 @@ def dump_packages(spec, path): if node is spec: spack.repo.path.dump_provenance(node, dest_pkg_dir) elif source_pkg_dir: - install_tree(source_pkg_dir, dest_pkg_dir) + fs.install_tree(source_pkg_dir, dest_pkg_dir) def install_msg(name, pid): @@ -461,17 +460,17 @@ def log(pkg): tty.debug(e) # Archive the whole stdout + stderr for the package - install(pkg.log_path, pkg.install_log_path) + fs.install(pkg.log_path, pkg.install_log_path) # Archive the environment used for the build - install(pkg.env_path, pkg.install_env_path) + fs.install(pkg.env_path, pkg.install_env_path) if os.path.exists(pkg.configure_args_path): # Archive the args used for the build - install(pkg.configure_args_path, pkg.install_configure_args_path) + fs.install(pkg.configure_args_path, pkg.install_configure_args_path) # Finally, archive files that are specific to each package - with working_dir(pkg.stage.path): + with fs.working_dir(pkg.stage.path): errors = six.StringIO() target_dir = os.path.join( spack.store.layout.metadata_path(pkg.spec), 'archived-files') @@ -493,8 +492,8 @@ def log(pkg): target = os.path.join(target_dir, f) # We must ensure that the directory exists before # copying a file in - mkdirp(os.path.dirname(target)) - install(f, target) + fs.mkdirp(os.path.dirname(target)) + fs.install(f, target) except Exception as e: tty.debug(e) @@ -505,7 +504,7 @@ def log(pkg): if errors.getvalue(): error_file = os.path.join(target_dir, 'errors.txt') - mkdirp(target_dir) + fs.mkdirp(target_dir) with open(error_file, 'w') as err: err.write(errors.getvalue()) tty.warn('Errors occurred when archiving files.\n\t' @@ -1071,10 +1070,10 @@ class PackageInstaller(object): pkg.name, 'src') tty.msg('{0} Copying source to {1}' .format(pre, src_target)) - install_tree(pkg.stage.source_path, src_target) + fs.install_tree(pkg.stage.source_path, src_target) # Do the real install in the source directory. - with working_dir(pkg.stage.source_path): + with fs.working_dir(pkg.stage.source_path): # Save the build environment in a file before building. dump_environment(pkg.env_path) @@ -1286,20 +1285,20 @@ class PackageInstaller(object): spack.store.layout.create_install_directory(pkg.spec) else: # Set the proper group for the prefix - group = get_package_group(pkg.spec) + group = prefs.get_package_group(pkg.spec) if group: - chgrp(pkg.spec.prefix, group) + fs.chgrp(pkg.spec.prefix, group) # Set the proper permissions. # This has to be done after group because changing groups blows # away the sticky group bit on the directory mode = os.stat(pkg.spec.prefix).st_mode - perms = get_package_dir_permissions(pkg.spec) + perms = prefs.get_package_dir_permissions(pkg.spec) if mode != perms: os.chmod(pkg.spec.prefix, perms) # Ensure the metadata path exists as well - mkdirp(spack.store.layout.metadata_path(pkg.spec), mode=perms) + fs.mkdirp(spack.store.layout.metadata_path(pkg.spec), mode=perms) def _update_failed(self, task, mark=False, exc=None): """ diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index ff7f5a3355..2f779c6a5f 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -7,7 +7,7 @@ import os import pytest import shutil -from llnl.util.filesystem import mkdirp, touch, working_dir +import llnl.util.filesystem as fs from spack.package import InstallError, PackageBase, PackageStillNeededError import spack.error @@ -380,11 +380,11 @@ def test_pkg_build_paths(install_mockery): # Backward compatibility checks log_dir = os.path.dirname(log_path) - mkdirp(log_dir) - with working_dir(log_dir): + fs.mkdirp(log_dir) + with fs.working_dir(log_dir): # Start with the older of the previous log filenames older_log = 'spack-build.out' - touch(older_log) + fs.touch(older_log) assert spec.package.log_path.endswith(older_log) # Now check the newer log filename @@ -416,11 +416,11 @@ def test_pkg_install_paths(install_mockery): # Backward compatibility checks log_dir = os.path.dirname(log_path) - mkdirp(log_dir) - with working_dir(log_dir): + fs.mkdirp(log_dir) + with fs.working_dir(log_dir): # Start with the older of the previous install log filenames older_log = 'build.out' - touch(older_log) + fs.touch(older_log) assert spec.package.install_log_path.endswith(older_log) # Now check the newer install log filename @@ -437,7 +437,8 @@ def test_pkg_install_paths(install_mockery): shutil.rmtree(log_dir) -def test_pkg_install_log(install_mockery): +def test_log_install_without_build_files(install_mockery): + """Test the installer log function when no build files are present.""" # Get a basic concrete spec for the trivial install package. spec = Spec('trivial-install-test-package').concretized() @@ -445,17 +446,40 @@ def test_pkg_install_log(install_mockery): with pytest.raises(IOError, match="No such file or directory"): spack.installer.log(spec.package) - # Set up mock build files and try again + +def test_log_install_with_build_files(install_mockery, monkeypatch): + """Test the installer's log function when have build files.""" + config_log = 'config.log' + + # Retain the original function for use in the monkey patch that is used + # to raise an exception under the desired condition for test coverage. + orig_install_fn = fs.install + + def _install(src, dest): + orig_install_fn(src, dest) + if src.endswith(config_log): + raise Exception('Mock log install error') + + monkeypatch.setattr(fs, 'install', _install) + + spec = Spec('trivial-install-test-package').concretized() + + # Set up mock build files and try again to include archive failure log_path = spec.package.log_path log_dir = os.path.dirname(log_path) - mkdirp(log_dir) - with working_dir(log_dir): - touch(log_path) - touch(spec.package.env_path) - touch(spec.package.configure_args_path) + fs.mkdirp(log_dir) + with fs.working_dir(log_dir): + fs.touch(log_path) + fs.touch(spec.package.env_path) + fs.touch(spec.package.configure_args_path) install_path = os.path.dirname(spec.package.install_log_path) - mkdirp(install_path) + fs.mkdirp(install_path) + + source = spec.package.stage.source_path + config = os.path.join(source, 'config.log') + fs.touchp(config) + spec.package.archive_files = ['missing', '..', config] spack.installer.log(spec.package) @@ -463,6 +487,21 @@ def test_pkg_install_log(install_mockery): assert os.path.exists(spec.package.install_env_path) assert os.path.exists(spec.package.install_configure_args_path) + archive_dir = os.path.join(install_path, 'archived-files') + source_dir = os.path.dirname(source) + rel_config = os.path.relpath(config, source_dir) + + assert os.path.exists(os.path.join(archive_dir, rel_config)) + assert not os.path.exists(os.path.join(archive_dir, 'missing')) + + expected_errs = [ + 'OUTSIDE SOURCE PATH', # for '..' + 'FAILED TO ARCHIVE' # for rel_config + ] + with open(os.path.join(archive_dir, 'errors.txt'), 'r') as fd: + for ln, expected in zip(fd, expected_errs): + assert expected in ln + # Cleanup shutil.rmtree(log_dir) diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index b9730c3165..0be4bc78c0 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -7,15 +7,19 @@ import os import py import pytest +import llnl.util.filesystem as fs import llnl.util.tty as tty +import llnl.util.lock as ulk import spack.binary_distribution import spack.compilers import spack.directory_layout as dl import spack.installer as inst -import spack.util.lock as lk +import spack.package_prefs as prefs import spack.repo import spack.spec +import spack.store +import spack.util.lock as lk def _mock_repo(root, namespace): @@ -152,7 +156,7 @@ def test_process_external_package_module(install_mockery, monkeypatch, capfd): def test_process_binary_cache_tarball_none(install_mockery, monkeypatch, capfd): - """Tests to cover _process_binary_cache_tarball when no tarball.""" + """Tests of _process_binary_cache_tarball when no tarball.""" monkeypatch.setattr(spack.binary_distribution, 'download_tarball', _none) pkg = spack.repo.get('trivial-install-test-package') @@ -162,7 +166,7 @@ def test_process_binary_cache_tarball_none(install_mockery, monkeypatch, def test_process_binary_cache_tarball_tar(install_mockery, monkeypatch, capfd): - """Tests to cover _process_binary_cache_tarball with a tar file.""" + """Tests of _process_binary_cache_tarball with a tar file.""" def _spec(spec): return spec @@ -179,6 +183,25 @@ def test_process_binary_cache_tarball_tar(install_mockery, monkeypatch, capfd): assert 'Installing a from binary cache' in capfd.readouterr()[0] +def test_try_install_from_binary_cache(install_mockery, mock_packages, + monkeypatch, capsys): + """Tests SystemExit path for_try_install_from_binary_cache.""" + def _spec(spec, force): + spec = spack.spec.Spec('mpi').concretized() + return {spec: None} + + spec = spack.spec.Spec('mpich') + spec.concretize() + + monkeypatch.setattr(spack.binary_distribution, 'get_spec', _spec) + + with pytest.raises(SystemExit): + inst._try_install_from_binary_cache(spec.package, False, False) + + captured = capsys.readouterr() + assert 'add a spack mirror to allow download' in str(captured) + + def test_installer_init_errors(install_mockery): """Test to ensure cover installer constructor errors.""" with pytest.raises(ValueError, match='must be a package'): @@ -189,17 +212,18 @@ def test_installer_init_errors(install_mockery): inst.PackageInstaller(pkg) -def test_installer_strings(install_mockery): - """Tests of installer repr and str for coverage purposes.""" +def test_installer_repr(install_mockery): spec, installer = create_installer('trivial-install-test-package') - # Cover __repr__ irep = installer.__repr__() assert irep.startswith(installer.__class__.__name__) assert "installed=" in irep assert "failed=" in irep - # Cover __str__ + +def test_installer_str(install_mockery): + spec, installer = create_installer('trivial-install-test-package') + istr = str(installer) assert "#tasks=0" in istr assert "installed (0)" in istr @@ -207,7 +231,6 @@ def test_installer_strings(install_mockery): def test_installer_last_phase_error(install_mockery, capsys): - """Test to cover last phase error.""" spec = spack.spec.Spec('trivial-install-test-package') spec.concretize() assert spec.concrete @@ -220,7 +243,6 @@ def test_installer_last_phase_error(install_mockery, capsys): def test_installer_ensure_ready_errors(install_mockery): - """Test to cover _ensure_ready errors.""" spec, installer = create_installer('trivial-install-test-package') fmt = r'cannot be installed locally.*{0}' @@ -247,24 +269,102 @@ def test_installer_ensure_ready_errors(install_mockery): installer._ensure_install_ready(spec.package) -def test_ensure_locked_have(install_mockery, tmpdir): - """Test to cover _ensure_locked when already have lock.""" +def test_ensure_locked_err(install_mockery, monkeypatch, tmpdir, capsys): + """Test _ensure_locked when a non-lock exception is raised.""" + mock_err_msg = 'Mock exception error' + + def _raise(lock, timeout): + raise RuntimeError(mock_err_msg) + spec, installer = create_installer('trivial-install-test-package') + monkeypatch.setattr(ulk.Lock, 'acquire_read', _raise) with tmpdir.as_cwd(): + with pytest.raises(RuntimeError): + installer._ensure_locked('read', spec.package) + + out = str(capsys.readouterr()[1]) + assert 'Failed to acquire a read lock' in out + assert mock_err_msg in out + + +def test_ensure_locked_have(install_mockery, tmpdir, capsys): + """Test _ensure_locked when already have lock.""" + spec, installer = create_installer('trivial-install-test-package') + + with tmpdir.as_cwd(): + # Test "downgrade" of a read lock (to a read lock) lock = lk.Lock('./test', default_timeout=1e-9, desc='test') lock_type = 'read' tpl = (lock_type, lock) installer.locks[installer.pkg_id] = tpl assert installer._ensure_locked(lock_type, spec.package) == tpl + # Test "upgrade" of a read lock without read count to a write + lock_type = 'write' + err = 'Cannot upgrade lock' + with pytest.raises(ulk.LockUpgradeError, match=err): + installer._ensure_locked(lock_type, spec.package) + + out = str(capsys.readouterr()[1]) + assert 'Failed to upgrade to a write lock' in out + assert 'exception when releasing read lock' in out + + # Test "upgrade" of the read lock *with* read count to a write + lock._reads = 1 + tpl = (lock_type, lock) + assert installer._ensure_locked(lock_type, spec.package) == tpl + + # Test "downgrade" of the write lock to a read lock + lock_type = 'read' + tpl = (lock_type, lock) + assert installer._ensure_locked(lock_type, spec.package) == tpl + + +@pytest.mark.parametrize('lock_type,reads,writes', [ + ('read', 1, 0), + ('write', 0, 1)]) +def test_ensure_locked_new_lock( + install_mockery, tmpdir, lock_type, reads, writes): + pkg_id = 'a' + spec, installer = create_installer(pkg_id) + with tmpdir.as_cwd(): + ltype, lock = installer._ensure_locked(lock_type, spec.package) + assert ltype == lock_type + assert lock is not None + assert lock._reads == reads + assert lock._writes == writes + + +def test_ensure_locked_new_warn(install_mockery, monkeypatch, tmpdir, capsys): + orig_pl = spack.database.Database.prefix_lock + + def _pl(db, spec, timeout): + lock = orig_pl(db, spec, timeout) + lock.default_timeout = 1e-9 if timeout is None else None + return lock + + pkg_id = 'a' + spec, installer = create_installer(pkg_id) + + monkeypatch.setattr(spack.database.Database, 'prefix_lock', _pl) -def test_package_id(install_mockery): - """Test to cover package_id functionality.""" + lock_type = 'read' + ltype, lock = installer._ensure_locked(lock_type, spec.package) + assert ltype == lock_type + assert lock is not None + + out = str(capsys.readouterr()[1]) + assert 'Expected prefix lock timeout' in out + + +def test_package_id_err(install_mockery): pkg = spack.repo.get('trivial-install-test-package') with pytest.raises(ValueError, match='spec is not concretized'): inst.package_id(pkg) + +def test_package_id_ok(install_mockery): spec = spack.spec.Spec('trivial-install-test-package') spec.concretize() assert spec.concrete @@ -273,36 +373,44 @@ def test_package_id(install_mockery): def test_fake_install(install_mockery): - """Test to cover fake install basics.""" spec = spack.spec.Spec('trivial-install-test-package') spec.concretize() assert spec.concrete + pkg = spec.package inst._do_fake_install(pkg) assert os.path.isdir(pkg.prefix.lib) -def test_packages_needed_to_bootstrap_compiler(install_mockery, monkeypatch): - """Test to cover most of _packages_needed_to_boostrap_compiler.""" - # TODO: More work is needed to go beyond the dependency check - def _no_compilers(pkg, arch_spec): - return [] - - # Test path where no compiler packages returned +def test_packages_needed_to_bootstrap_compiler_none(install_mockery): spec = spack.spec.Spec('trivial-install-test-package') spec.concretize() assert spec.concrete + packages = inst._packages_needed_to_bootstrap_compiler(spec.package) assert not packages - # Test up to the dependency check - monkeypatch.setattr(spack.compilers, 'compilers_for_spec', _no_compilers) - with pytest.raises(spack.repo.UnknownPackageError, match='not found'): - inst._packages_needed_to_bootstrap_compiler(spec.package) + +def test_packages_needed_to_bootstrap_compiler_packages(install_mockery, + monkeypatch): + spec = spack.spec.Spec('trivial-install-test-package') + spec.concretize() + + def _conc_spec(compiler): + return spack.spec.Spec('a').concretized() + + # Ensure we can get past functions that are precluding obtaining + # packages. + monkeypatch.setattr(spack.compilers, 'compilers_for_spec', _none) + monkeypatch.setattr(spack.compilers, 'pkg_spec_for_compiler', _conc_spec) + monkeypatch.setattr(spack.spec.Spec, 'concretize', _noop) + + packages = inst._packages_needed_to_bootstrap_compiler(spec.package) + assert packages def test_dump_packages_deps_ok(install_mockery, tmpdir, mock_repo_path): - """Test to add coverage to dump_packages with dependencies happy path.""" + """Test happy path for dump_packages with dependencies.""" spec_name = 'simple-inheritance' spec = spack.spec.Spec(spec_name).concretized() @@ -314,7 +422,7 @@ def test_dump_packages_deps_ok(install_mockery, tmpdir, mock_repo_path): def test_dump_packages_deps_errs(install_mockery, tmpdir, monkeypatch, capsys): - """Test to add coverage to dump_packages with dependencies.""" + """Test error paths for dump_packages with dependencies.""" orig_bpp = spack.store.layout.build_packages_path orig_dirname = spack.repo.Repo.dirname_for_package_name repo_err_msg = "Mock dirname_for_package_name" @@ -354,59 +462,45 @@ def test_dump_packages_deps_errs(install_mockery, tmpdir, monkeypatch, capsys): assert "Couldn't copy in provenance for cmake" in out -@pytest.mark.tld -def test_check_deps_status_errs(install_mockery, monkeypatch): - """Test to cover _check_deps_status failures.""" +def test_check_deps_status_install_failure(install_mockery, monkeypatch): spec, installer = create_installer('a') # Make sure the package is identified as failed - orig_fn = spack.database.Database.prefix_failed monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true) with pytest.raises(inst.InstallError, match='install failure'): installer._check_deps_status() - monkeypatch.setattr(spack.database.Database, 'prefix_failed', orig_fn) - # Ensure do not acquire the lock +def test_check_deps_status_write_locked(install_mockery, monkeypatch): + spec, installer = create_installer('a') + + # Ensure the lock is not acquired monkeypatch.setattr(inst.PackageInstaller, '_ensure_locked', _not_locked) with pytest.raises(inst.InstallError, match='write locked by another'): installer._check_deps_status() -@pytest.mark.tld def test_check_deps_status_external(install_mockery, monkeypatch): - """Test to cover _check_deps_status for external.""" spec, installer = create_installer('a') - deps = spec.dependencies() - assert len(deps) > 0 - dep_id = 'b' - - # Ensure the known dependent is installed if flagged as external + # Mock the known dependent, b, as external so assumed to be installed monkeypatch.setattr(spack.spec.Spec, 'external', True) installer._check_deps_status() - assert dep_id in installer.installed + assert 'b' in installer.installed -@pytest.mark.tld def test_check_deps_status_upstream(install_mockery, monkeypatch): - """Test to cover _check_deps_status for upstream.""" spec, installer = create_installer('a') - deps = spec.dependencies() - assert len(deps) > 0 - dep_id = 'b' - - # Ensure the known dependent, b, is installed if flagged as upstream + # Mock the known dependent, b, as installed upstream monkeypatch.setattr(spack.package.PackageBase, 'installed_upstream', True) installer._check_deps_status() - assert dep_id in installer.installed + assert 'b' in installer.installed def test_add_bootstrap_compilers(install_mockery, monkeypatch): - """Test to cover _add_bootstrap_compilers.""" def _pkgs(pkg): spec = spack.spec.Spec('mpi').concretized() return [(spec.package, True)] @@ -445,7 +539,6 @@ def test_installer_init_queue(install_mockery): def test_install_task_use_cache(install_mockery, monkeypatch): - """Test _install_task to cover use_cache path.""" spec, installer = create_installer('trivial-install-test-package') task = create_build_task(spec.package) @@ -454,25 +547,27 @@ def test_install_task_use_cache(install_mockery, monkeypatch): assert spec.package.name in installer.installed -def test_install_task_stop_iter(install_mockery, monkeypatch, capfd): - """Test _install_task to cover the StopIteration exception.""" - mock_err_msg = 'mock stop iteration' +def test_install_task_add_compiler(install_mockery, monkeypatch, capfd): + config_msg = 'mock add_compilers_to_config' - def _raise(installer, pkg): - raise StopIteration(mock_err_msg) + def _add(_compilers): + tty.msg(config_msg) spec, installer = create_installer('a') task = create_build_task(spec.package) + task.compiler = True + # Preclude any meaningful side-effects monkeypatch.setattr(spack.package.PackageBase, 'unit_test_check', _true) - monkeypatch.setattr(inst.PackageInstaller, '_setup_install_dir', _raise) + monkeypatch.setattr(inst.PackageInstaller, '_setup_install_dir', _noop) + monkeypatch.setattr(spack.build_environment, 'fork', _noop) + monkeypatch.setattr(spack.database.Database, 'add', _noop) + monkeypatch.setattr(spack.compilers, 'add_compilers_to_config', _add) installer._install_task(task) - out = capfd.readouterr()[0] - assert mock_err_msg in out - assert 'Package stage directory' in out - assert spec.package.stage.source_path in out + out = capfd.readouterr()[0] + assert config_msg in out def test_release_lock_write_n_exception(install_mockery, tmpdir, capsys): @@ -529,8 +624,36 @@ def test_cleanup_all_tasks(install_mockery, monkeypatch): assert len(installer.build_tasks) == 1 -def test_cleanup_failed(install_mockery, tmpdir, monkeypatch, capsys): - """Test to increase coverage of _cleanup_failed.""" +def test_setup_install_dir_grp(install_mockery, monkeypatch, capfd): + """Test _setup_install_dir's group change.""" + mock_group = 'mockgroup' + mock_chgrp_msg = 'Changing group for {0} to {1}' + + def _get_group(spec): + return mock_group + + def _chgrp(path, group): + tty.msg(mock_chgrp_msg.format(path, group)) + + monkeypatch.setattr(prefs, 'get_package_group', _get_group) + monkeypatch.setattr(fs, 'chgrp', _chgrp) + + spec, installer = create_installer('trivial-install-test-package') + + fs.touchp(spec.prefix) + metadatadir = spack.store.layout.metadata_path(spec) + # Should fail with a "not a directory" error + with pytest.raises(OSError, match=metadatadir): + installer._setup_install_dir(spec.package) + + out = str(capfd.readouterr()[0]) + + expected_msg = mock_chgrp_msg.format(spec.prefix, mock_group) + assert expected_msg in out + + +def test_cleanup_failed_err(install_mockery, tmpdir, monkeypatch, capsys): + """Test _cleanup_failed exception path.""" msg = 'Fake release_write exception' def _raise_except(lock): @@ -550,13 +673,14 @@ def test_cleanup_failed(install_mockery, tmpdir, monkeypatch, capsys): assert msg in out -def test_update_failed_no_mark(install_mockery): - """Test of _update_failed sans mark and dependent build tasks.""" +def test_update_failed_no_dependent_task(install_mockery): + """Test _update_failed with missing dependent build tasks.""" spec, installer = create_installer('dependent-install') - task = create_build_task(spec.package) - installer._update_failed(task) - assert installer.failed['dependent-install'] is None + for dep in spec.traverse(root=False): + task = create_build_task(dep.package) + installer._update_failed(task, mark=False) + assert installer.failed[task.pkg_id] is None def test_install_uninstalled_deps(install_mockery, monkeypatch, capsys): @@ -710,3 +834,12 @@ def test_install_dir_exists(install_mockery, monkeypatch, capfd): installer.install() assert 'b' in installer.installed + + +def test_install_skip_patch(install_mockery, mock_fetch): + """Test the path skip_patch install path.""" + spec, installer = create_installer('b') + + installer.install(fake=False, skip_patch=True) + + assert 'b' in installer.installed -- cgit v1.2.3-70-g09d2 From 901bed48ec2d3e03d45014fdbfc936305897d2e2 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Sat, 7 Mar 2020 13:58:33 +0100 Subject: ArchSpec: fix semantics of satisfies when not concrete and strict is true (#15319) --- lib/spack/spack/spec.py | 4 ++++ lib/spack/spack/test/architecture.py | 13 +++++++++++++ 2 files changed, 17 insertions(+) (limited to 'lib') diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 1493f0c973..718b5ef14d 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -369,6 +369,10 @@ class ArchSpec(object): if not need_to_check: return True + # self is not concrete, but other_target is there and strict=True + if self.target is None: + return False + for target_range in str(other_target).split(','): t_min, sep, t_max = target_range.partition(':') diff --git a/lib/spack/spack/test/architecture.py b/lib/spack/spack/test/architecture.py index 1f5ff28900..552bc324bf 100644 --- a/lib/spack/spack/test/architecture.py +++ b/lib/spack/spack/test/architecture.py @@ -214,3 +214,16 @@ def test_optimization_flags_with_custom_versions( ) opt_flags = target.optimization_flags(compiler) assert opt_flags == expected_flags + + +@pytest.mark.regression('15306') +@pytest.mark.parametrize('architecture_tuple,constraint_tuple', [ + (('linux', 'ubuntu18.04', None), ('linux', None, 'x86_64')), + (('linux', 'ubuntu18.04', None), ('linux', None, 'x86_64:')), +]) +def test_satisfy_strict_constraint_when_not_concrete( + architecture_tuple, constraint_tuple +): + architecture = spack.spec.ArchSpec(architecture_tuple) + constraint = spack.spec.ArchSpec(constraint_tuple) + assert not architecture.satisfies(constraint, strict=True) -- cgit v1.2.3-70-g09d2 From 62683eb4bf2bcf155c460113be09915f452e9ce6 Mon Sep 17 00:00:00 2001 From: Patrick Gartung Date: Wed, 4 Mar 2020 19:21:15 -0600 Subject: Add function replace_prefix_nullterm for use on mach-o rpaths. (#15347) This recovers the old behavior of replace_prefix_bin that was modified to work with elf binaries by prefixing os.sep to new prefix until length is the same as old prefix. --- lib/spack/spack/relocate.py | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index fb4ff18fae..56fc993b5f 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -398,10 +398,43 @@ def replace_prefix_text(path_name, old_dir, new_dir): def replace_prefix_bin(path_name, old_dir, new_dir): + """ + Attempt to replace old install prefix with new install prefix + in binary files by prefixing new install prefix with os.sep + until the lengths of the prefixes are the same. + """ + + def replace(match): + occurances = match.group().count(old_dir.encode('utf-8')) + olen = len(old_dir.encode('utf-8')) + nlen = len(new_dir.encode('utf-8')) + padding = (olen - nlen) * occurances + if padding < 0: + return data + return match.group().replace(old_dir.encode('utf-8'), + new_dir.encode('utf-8')) + b'\0' * padding + + with open(path_name, 'rb+') as f: + data = f.read() + f.seek(0) + original_data_len = len(data) + pat = re.compile(old_dir.encode('utf-8') + b'([^\0]*?)\0') + if not pat.search(data): + return + ndata = pat.sub(replace, data) + if not len(ndata) == original_data_len: + raise BinaryStringReplacementException( + path_name, original_data_len, len(ndata)) + f.write(ndata) + f.truncate() + + +def replace_prefix_nullterm(path_name, old_dir, new_dir): """ Attempt to replace old install prefix with new install prefix in binary files by replacing with null terminated string that is the same length unless the old path is shorter + Used on linux to replace mach-o rpaths """ def replace(match): @@ -413,7 +446,6 @@ def replace_prefix_bin(path_name, old_dir, new_dir): return data return match.group().replace(old_dir.encode('utf-8'), new_dir.encode('utf-8')) + b'\0' * padding - with open(path_name, 'rb+') as f: data = f.read() f.seek(0) @@ -466,8 +498,7 @@ def relocate_macho_binaries(path_names, old_dir, new_dir, allow_root): modify_object_macholib(path_name, placeholder, new_dir) modify_object_macholib(path_name, old_dir, new_dir) if len(new_dir) <= len(old_dir): - replace_prefix_bin(path_name, old_dir, - new_dir) + replace_prefix_nullterm(path_name, old_dir, new_dir) else: tty.warn('Cannot do a binary string replacement' ' with padding for %s' -- cgit v1.2.3-70-g09d2 From 1e42f0a54589d73fab367de4389aa3318c087af0 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sat, 7 Mar 2020 23:48:18 -0800 Subject: bugfix: installer.py shouldn't be executable (#15386) This is a minor permission fix on the new installer.py introduced in #13100. --- lib/spack/spack/installer.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 lib/spack/spack/installer.py (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py old mode 100755 new mode 100644 -- cgit v1.2.3-70-g09d2 From ec720bf28d04bb5eadafaea5607832428d92d9d5 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 10 Mar 2020 10:35:20 -0700 Subject: bugfix: fix install_missing_compilers option bug from v0.14.0 (#15416) * bugfix: ensure bootstrapped compilers built before packages using the compiler --- lib/spack/spack/installer.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'lib') diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 4e8ec39733..a9965c9114 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -181,6 +181,9 @@ def _packages_needed_to_bootstrap_compiler(pkg): # concrete CompilerSpec has less info than concrete Spec # concretize as Spec to add that information dep.concretize() + # mark compiler as depended-on by the package that uses it + dep._dependents[pkg.name] = spack.spec.DependencySpec( + pkg.spec, dep, ('build',)) packages = [(s.package, False) for s in dep.traverse(order='post', root=False)] packages.append((dep.package, True)) @@ -1610,6 +1613,21 @@ class BuildTask(object): self.spec.dependencies() if package_id(d.package) != self.pkg_id) + # Handle bootstrapped compiler + # + # The bootstrapped compiler is not a dependency in the spec, but it is + # a dependency of the build task. Here we add it to self.dependencies + compiler_spec = self.spec.compiler + arch_spec = self.spec.architecture + if not spack.compilers.compilers_for_spec(compiler_spec, + arch_spec=arch_spec): + # The compiler is in the queue, identify it as dependency + dep = spack.compilers.pkg_spec_for_compiler(compiler_spec) + dep.architecture = arch_spec + dep.concretize() + dep_id = package_id(dep.package) + self.dependencies.add(dep_id) + # List of uninstalled dependencies, which is used to establish # the priority of the build task. # -- cgit v1.2.3-70-g09d2 From 296d58ef6bf07fa1440c988d6b2e1268d83e671f Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 12 Mar 2020 09:21:19 +0100 Subject: Creating versions from urls doesn't modify class attributes (#15452) fixes #15449 Before this PR a call to pkg.url_for_version was modifying class attributes determining different results for subsequents calls and an error when the urls was empty. --- lib/spack/spack/fetch_strategy.py | 2 +- lib/spack/spack/package.py | 2 +- lib/spack/spack/test/url_fetch.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index eaf9f130c5..a01bc143aa 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -1239,7 +1239,7 @@ def _from_merged_attrs(fetcher, pkg, version): # TODO: refactor this logic into its own method or function # TODO: to avoid duplication mirrors = [spack.url.substitute_version(u, version) - for u in getattr(pkg, 'urls', [])] + for u in getattr(pkg, 'urls', [])[1:]] attrs = {fetcher.url_attr: url, 'mirrors': mirrors} else: url = getattr(pkg, fetcher.url_attr) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index b7efc3989b..b2d841f145 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -763,7 +763,7 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # If no specific URL, use the default, class-level URL url = getattr(self, 'url', None) urls = getattr(self, 'urls', [None]) - default_url = url or urls.pop(0) + default_url = url or urls[0] # if no exact match AND no class-level default, use the nearest URL if not default_url: diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py index 71a122455f..679240049d 100644 --- a/lib/spack/spack/test/url_fetch.py +++ b/lib/spack/spack/test/url_fetch.py @@ -32,7 +32,7 @@ def pkg_factory(): def factory(url, urls): def fn(v): - main_url = url or urls.pop(0) + main_url = url or urls[0] return spack.url.substitute_version(main_url, v) return Pkg( -- cgit v1.2.3-70-g09d2 From 09e13cf7cf0327bb35a9b3d4e7c7ad64ea4a3076 Mon Sep 17 00:00:00 2001 From: Kai Germaschewski Date: Wed, 18 Mar 2020 19:41:18 -0400 Subject: Upstreams: don't write metadata directory to upstream DB (#15526) When trying to use an upstream Spack repository, as of f2aca86 Spack was attempting to write to the upstream DB based on a new metadata directory added in that commit. Upstream DBs are read-only, so this should not occur. This adds a check to prevent Spack from writing to the upstream DB --- lib/spack/spack/database.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index e04a1f292a..f3c88a75c3 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -324,7 +324,7 @@ class Database(object): if not os.path.exists(self._db_dir): mkdirp(self._db_dir) - if not os.path.exists(self._failure_dir): + if not os.path.exists(self._failure_dir) and not is_upstream: mkdirp(self._failure_dir) self.is_upstream = is_upstream -- cgit v1.2.3-70-g09d2 From 30b4704522316263fff07fc75146f0a7a6dc7952 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Thu, 19 Mar 2020 15:11:50 -0700 Subject: Cray bugfix: TERM missing while reading default target (#15381) Bug: Spack hangs on some Cray machines Reason: The TERM environment variable is necessary to run bash -lc "echo $CRAY_CPU_TARGET", but we run that command within env -i, which wipes the environment. Fix: Manually forward the TERM environment variable to env -i /bin/bash -lc "echo $CRAY_CPU_TARGET" --- lib/spack/spack/platforms/cray.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/platforms/cray.py b/lib/spack/spack/platforms/cray.py index cbeba96461..8eae51e66a 100644 --- a/lib/spack/spack/platforms/cray.py +++ b/lib/spack/spack/platforms/cray.py @@ -7,7 +7,7 @@ import os import re import llnl.util.tty as tty from spack.paths import build_env_path -from spack.util.executable import which +from spack.util.executable import Executable from spack.architecture import Platform, Target, NoPlatformError from spack.operating_systems.cray_frontend import CrayFrontend from spack.operating_systems.cnl import Cnl @@ -117,11 +117,13 @@ class Cray(Platform): ''' # env -i /bin/bash -lc echo $CRAY_CPU_TARGET 2> /dev/null if getattr(self, 'default', None) is None: - env = which('env') - output = env("-i", "/bin/bash", "-lc", "echo $CRAY_CPU_TARGET", - output=str, error=os.devnull) - self.default = output.strip() - tty.debug("Found default module:%s" % self.default) + output = Executable('/bin/bash')('-lc', 'echo $CRAY_CPU_TARGET', + env={'TERM': os.environ['TERM']}, + output=str, error=os.devnull) + output = ''.join(output.split()) # remove all whitespace + if output: + self.default = output + tty.debug("Found default module:%s" % self.default) return self.default def _avail_targets(self): -- cgit v1.2.3-70-g09d2 From 3826cdf139359de4c346bb72ef0064111b98c3c5 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 20 Mar 2020 12:22:32 -0700 Subject: multiprocessing: allow Spack to run uninterrupted in background (#14682) Spack currently cannot run as a background process uninterrupted because some of the logging functions used in the install method (especially to create the dynamic verbosity toggle with the v key) cause the OS to issue a SIGTTOU to Spack when it's backgrounded. This PR puts the necessary gatekeeping in place so that Spack doesn't do anything that will cause a signal to stop the process when operating as a background process. --- etc/spack/defaults/packages.yaml | 1 + lib/spack/llnl/util/tty/log.py | 130 +++++++++++++++++++++++++-------------- 2 files changed, 84 insertions(+), 47 deletions(-) (limited to 'lib') diff --git a/etc/spack/defaults/packages.yaml b/etc/spack/defaults/packages.yaml index fcc39f534e..46aa5ad9ad 100644 --- a/etc/spack/defaults/packages.yaml +++ b/etc/spack/defaults/packages.yaml @@ -40,6 +40,7 @@ packages: pil: [py-pillow] pkgconfig: [pkgconf, pkg-config] scalapack: [netlib-scalapack] + sycl: [hipsycl] szip: [libszip, libaec] tbb: [intel-tbb] unwind: [libunwind] diff --git a/lib/spack/llnl/util/tty/log.py b/lib/spack/llnl/util/tty/log.py index 10516e59d2..e608992642 100644 --- a/lib/spack/llnl/util/tty/log.py +++ b/lib/spack/llnl/util/tty/log.py @@ -13,12 +13,18 @@ import re import select import sys import traceback +import signal from contextlib import contextmanager from six import string_types from six import StringIO import llnl.util.tty as tty +try: + import termios +except ImportError: + termios = None + # Use this to strip escape sequences _escape = re.compile(r'\x1b[^m]*m|\x1b\[?1034h') @@ -31,12 +37,26 @@ xon, xoff = '\x11\n', '\x13\n' control = re.compile('(\x11\n|\x13\n)') +@contextmanager +def background_safe(): + signal.signal(signal.SIGTTOU, signal.SIG_IGN) + yield + signal.signal(signal.SIGTTOU, signal.SIG_DFL) + + +def _is_background_tty(): + """Return True iff this process is backgrounded and stdout is a tty""" + if sys.stdout.isatty(): + return os.getpgrp() != os.tcgetpgrp(sys.stdout.fileno()) + return False # not writing to tty, not background + + def _strip(line): """Strip color and control characters from a line.""" return _escape.sub('', line) -class keyboard_input(object): +class _keyboard_input(object): """Context manager to disable line editing and echoing. Use this with ``sys.stdin`` for keyboard input, e.g.:: @@ -81,32 +101,30 @@ class keyboard_input(object): if not self.stream or not self.stream.isatty(): return - try: - # If this fails, self.old_cfg will remain None - import termios - + # If this fails, self.old_cfg will remain None + if termios and not _is_background_tty(): # save old termios settings - fd = self.stream.fileno() - self.old_cfg = termios.tcgetattr(fd) + old_cfg = termios.tcgetattr(self.stream) - # create new settings with canonical input and echo - # disabled, so keypresses are immediate & don't echo. - self.new_cfg = termios.tcgetattr(fd) - self.new_cfg[3] &= ~termios.ICANON - self.new_cfg[3] &= ~termios.ECHO + try: + # create new settings with canonical input and echo + # disabled, so keypresses are immediate & don't echo. + self.new_cfg = termios.tcgetattr(self.stream) + self.new_cfg[3] &= ~termios.ICANON + self.new_cfg[3] &= ~termios.ECHO - # Apply new settings for terminal - termios.tcsetattr(fd, termios.TCSADRAIN, self.new_cfg) + # Apply new settings for terminal + termios.tcsetattr(self.stream, termios.TCSADRAIN, self.new_cfg) + self.old_cfg = old_cfg - except Exception: - pass # some OS's do not support termios, so ignore + except Exception: + pass # some OS's do not support termios, so ignore def __exit__(self, exc_type, exception, traceback): """If termios was avaialble, restore old settings.""" if self.old_cfg: - import termios - termios.tcsetattr( - self.stream.fileno(), termios.TCSADRAIN, self.old_cfg) + with background_safe(): # change it back even if backgrounded now + termios.tcsetattr(self.stream, termios.TCSADRAIN, self.old_cfg) class Unbuffered(object): @@ -426,45 +444,63 @@ class log_output(object): istreams = [in_pipe, stdin] if stdin else [in_pipe] log_file = self.log_file + + def handle_write(force_echo): + # Handle output from the with block process. + # If we arrive here it means that in_pipe was + # ready for reading : it should never happen that + # line is false-ish + line = in_pipe.readline() + if not line: + return (True, force_echo) # break while loop + + # find control characters and strip them. + controls = control.findall(line) + line = re.sub(control, '', line) + + # Echo to stdout if requested or forced + if echo or force_echo: + try: + if termios: + conf = termios.tcgetattr(sys.stdout) + tostop = conf[3] & termios.TOSTOP + else: + tostop = True + except Exception: + tostop = True + if not (tostop and _is_background_tty()): + sys.stdout.write(line) + sys.stdout.flush() + + # Stripped output to log file. + log_file.write(_strip(line)) + log_file.flush() + + if xon in controls: + force_echo = True + if xoff in controls: + force_echo = False + return (False, force_echo) + try: - with keyboard_input(stdin): + with _keyboard_input(stdin): while True: # No need to set any timeout for select.select # Wait until a key press or an event on in_pipe. rlist, _, _ = select.select(istreams, [], []) - # Allow user to toggle echo with 'v' key. # Currently ignores other chars. - if stdin in rlist: + # only read stdin if we're in the foreground + if stdin in rlist and not _is_background_tty(): if stdin.read(1) == 'v': echo = not echo - # Handle output from the with block process. if in_pipe in rlist: - # If we arrive here it means that in_pipe was - # ready for reading : it should never happen that - # line is false-ish - line = in_pipe.readline() - if not line: - break # EOF - - # find control characters and strip them. - controls = control.findall(line) - line = re.sub(control, '', line) - - # Echo to stdout if requested or forced - if echo or force_echo: - sys.stdout.write(line) - sys.stdout.flush() - - # Stripped output to log file. - log_file.write(_strip(line)) - log_file.flush() - - if xon in controls: - force_echo = True - if xoff in controls: - force_echo = False + br, fe = handle_write(force_echo) + force_echo = fe + if br: + break + except BaseException: tty.error("Exception occurred in writer daemon!") traceback.print_exc() -- cgit v1.2.3-70-g09d2 From ff0abb9838121522321df2a054d18e54b566b44a Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Fri, 20 Mar 2020 11:49:24 -0700 Subject: version bump: 0.14.1 --- lib/spack/spack/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py index b18acb2094..112a98c01c 100644 --- a/lib/spack/spack/__init__.py +++ b/lib/spack/spack/__init__.py @@ -5,7 +5,7 @@ #: major, minor, patch version for Spack, in a tuple -spack_version_info = (0, 14, 0) +spack_version_info = (0, 14, 1) #: String containing Spack version joined with .'s spack_version = '.'.join(str(v) for v in spack_version_info) -- cgit v1.2.3-70-g09d2