diff options
Diffstat (limited to 'lib')
24 files changed, 1058 insertions, 84 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 1bc177a68b..b8d0b4d2f1 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -47,6 +47,7 @@ __all__ = [ 'install_tree', 'is_exe', 'join_path', + 'last_modification_time_recursive', 'mkdirp', 'partition_path', 'prefixes', @@ -920,6 +921,15 @@ def set_executable(path): os.chmod(path, mode) +def last_modification_time_recursive(path): + path = os.path.abspath(path) + times = [os.stat(path).st_mtime] + times.extend(os.stat(os.path.join(root, name)).st_mtime + for root, dirs, files in os.walk(path) + for name in dirs + files) + return max(times) + + def remove_empty_directories(root): """Ascend up from the leaves accessible from `root` and remove empty directories. diff --git a/lib/spack/spack/cmd/dev_build.py b/lib/spack/spack/cmd/dev_build.py index d9ad0fb891..55f5d201a9 100644 --- a/lib/spack/spack/cmd/dev_build.py +++ b/lib/spack/spack/cmd/dev_build.py @@ -12,7 +12,6 @@ import spack.config import spack.cmd import spack.cmd.common.arguments as arguments import spack.repo -from spack.stage import DIYStage description = "developer build: build from code in current working directory" section = "build" @@ -72,6 +71,14 @@ def dev_build(self, args): "spack dev-build spec must have a single, concrete version. " "Did you forget a package version number?") + source_path = args.source_path + if source_path is None: + source_path = os.getcwd() + source_path = os.path.abspath(source_path) + + # Forces the build to run out of the source directory. + spec.constrain('dev_path=%s' % source_path) + spec.concretize() package = spack.repo.get(spec) @@ -80,14 +87,6 @@ def dev_build(self, args): tty.msg("Uninstall or try adding a version suffix for this dev build.") sys.exit(1) - source_path = args.source_path - if source_path is None: - source_path = os.getcwd() - source_path = os.path.abspath(source_path) - - # Forces the build to run out of the current directory. - package.stage = DIYStage(source_path) - # disable checksumming if requested if args.no_checksum: spack.config.set('config:checksum', False, scope='command_line') @@ -97,7 +96,6 @@ def dev_build(self, args): keep_prefix=args.keep_prefix, install_deps=not args.ignore_deps, verbose=not args.quiet, - keep_stage=True, # don't remove source dir for dev build. dirty=args.dirty, stop_before=args.before, stop_at=args.until) diff --git a/lib/spack/spack/cmd/develop.py b/lib/spack/spack/cmd/develop.py new file mode 100644 index 0000000000..957060536d --- /dev/null +++ b/lib/spack/spack/cmd/develop.py @@ -0,0 +1,102 @@ +# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) +import os +import shutil + +import llnl.util.tty as tty + +import spack.cmd +import spack.cmd.common.arguments as arguments +import spack.environment as ev + +from spack.error import SpackError + +description = "add a spec to an environment's dev-build information" +section = "environments" +level = "long" + + +def setup_parser(subparser): + subparser.add_argument( + '-p', '--path', help='Source location of package') + + clone_group = subparser.add_mutually_exclusive_group() + clone_group.add_argument( + '--no-clone', action='store_false', dest='clone', default=None, + help='Do not clone. The package already exists at the source path') + clone_group.add_argument( + '--clone', action='store_true', dest='clone', default=None, + help='Clone the package even if the path already exists') + + subparser.add_argument( + '-f', '--force', + help='Remove any files or directories that block cloning source code') + + arguments.add_common_arguments(subparser, ['spec']) + + +def develop(parser, args): + env = ev.get_env(args, 'develop', required=True) + + if not args.spec: + if args.clone is False: + raise SpackError("No spec provided to spack develop command") + + # download all dev specs + for name, entry in env.dev_specs.items(): + path = entry.get('path', name) + abspath = path if os.path.isabs(path) else os.path.join( + env.path, path) + + if os.path.exists(abspath): + msg = "Skipping developer download of %s" % entry['spec'] + msg += " because its path already exists." + tty.msg(msg) + continue + + stage = spack.spec.Spec(entry['spec']).package.stage + stage.steal_source(abspath) + + if not env.dev_specs: + tty.warn("No develop specs to download") + + return + + specs = spack.cmd.parse_specs(args.spec) + if len(specs) > 1: + raise SpackError("spack develop requires at most one named spec") + + spec = specs[0] + if not spec.versions.concrete: + raise SpackError("Packages to develop must have a concrete version") + + # default path is relative path to spec.name + path = args.path or spec.name + + # get absolute path to check + abspath = path + if not os.path.isabs(abspath): + abspath = os.path.join(env.path, path) + + # clone default: only if the path doesn't exist + clone = args.clone + if clone is None: + clone = not os.path.exists(abspath) + + if not clone and not os.path.exists(abspath): + raise SpackError("Provided path %s does not exist" % abspath) + + if clone and os.path.exists(abspath): + if args.force: + shutil.rmtree(abspath) + else: + msg = "Path %s already exists and cannot be cloned to." % abspath + msg += " Use `spack develop -f` to overwrite." + raise SpackError(msg) + + with env.write_transaction(): + changed = env.develop(spec, path, clone) + if changed: + env.write() diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 69158275f6..ef7433c5f7 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -392,13 +392,8 @@ environment variables: if not answer: tty.die('Reinstallation aborted.') - for abstract, concrete in zip(abstract_specs, specs): - if concrete in installed: - with fs.replace_directory_transaction(concrete.prefix): - install_spec(args, kwargs, abstract, concrete) - else: - install_spec(args, kwargs, abstract, concrete) + # overwrite all concrete explicit specs from this build + kwargs['overwrite'] = [spec.dag_hash() for spec in specs] - else: - for abstract, concrete in zip(abstract_specs, specs): - install_spec(args, kwargs, abstract, concrete) + for abstract, concrete in zip(abstract_specs, specs): + install_spec(args, kwargs, abstract, concrete) diff --git a/lib/spack/spack/cmd/undevelop.py b/lib/spack/spack/cmd/undevelop.py new file mode 100644 index 0000000000..6319b88e06 --- /dev/null +++ b/lib/spack/spack/cmd/undevelop.py @@ -0,0 +1,40 @@ +# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +import llnl.util.tty as tty + +import spack.cmd +import spack.cmd.common.arguments as arguments +import spack.environment as ev + + +description = 'remove specs from an environment' +section = "environments" +level = "long" + + +def setup_parser(subparser): + subparser.add_argument( + '-a', '--all', action='store_true', + help="remove all specs from (clear) the environment") + arguments.add_common_arguments(subparser, ['specs']) + + +def undevelop(parser, args): + env = ev.get_env(args, 'undevelop', required=True) + + if args.all: + specs = env.dev_specs.keys() + else: + specs = spack.cmd.parse_specs(args.specs) + + with env.write_transaction(): + changed = False + for spec in specs: + tty.msg('Removing %s from environment %s development specs' + % (spec, env.name)) + changed |= env.undevelop(spec) + if changed: + env.write() diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 7424b8a90d..928d35b956 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -36,6 +36,7 @@ import spack.compilers import spack.architecture import spack.error import spack.tengine +import spack.variant as vt from spack.config import config from spack.version import ver, Version, VersionList, VersionRange from spack.package_prefs import PackagePrefs, spec_externals, is_spec_buildable @@ -61,6 +62,29 @@ class Concretizer(object): self.abstract_spec = abstract_spec self._adjust_target_answer_generator = None + def concretize_develop(self, spec): + """ + Add ``dev_path=*`` variant to packages built from local source. + """ + env = spack.environment.get_env(None, None) + dev_info = env.dev_specs.get(spec.name, {}) if env else {} + if not dev_info: + return False + + path = dev_info['path'] + path = path if os.path.isabs(path) else os.path.join( + env.path, path) + + if 'dev_path' in spec.variants: + assert spec.variants['dev_path'].value == path + changed = False + else: + spec.variants.setdefault( + 'dev_path', vt.SingleValuedVariant('dev_path', path)) + changed = True + changed |= spec.constrain(dev_info['spec']) + return changed + def _valid_virtuals_and_externals(self, spec): """Returns a list of candidate virtual dep providers and external packages that coiuld be used to concretize a spec. @@ -328,6 +352,18 @@ class Concretizer(object): preferred_variants = PackagePrefs.preferred_variants(spec.name) pkg_cls = spec.package_class for name, variant in pkg_cls.variants.items(): + any_set = False + var = spec.variants.get(name, None) + if var and 'any' in var: + # remove 'any' variant before concretizing + # 'any' cannot be combined with other variables in a + # multivalue variant, a concrete variant cannot have the value + # 'any', and 'any' does not constrain a variant except to + # preclude the values 'none' and None. We track `any_set` to + # avoid replacing 'any' with None, and remove it to continue + # concretization. + spec.variants.pop(name) + any_set = True if name not in spec.variants: changed = True if name in preferred_variants: @@ -335,6 +371,14 @@ class Concretizer(object): else: spec.variants[name] = variant.make_default() + var = spec.variants[name] + if any_set and 'none' in var or None in var: + msg = "Attempted non-deterministic setting of variant" + msg += " '%s' set to 'any' and preference is." % name + msg += "'%s'. Set the variant to a non 'any'" % var.value + msg += " value or set a preference for variant '%s'." % name + raise NonDeterministicVariantError(msg) + return changed def concretize_compiler(self, spec): @@ -761,3 +805,7 @@ class NoBuildError(spack.error.SpackError): msg = ("The spec\n '%s'\n is configured as not buildable, " "and no matching external installs were found") super(NoBuildError, self).__init__(msg % spec) + + +class NonDeterministicVariantError(spack.error.SpecError): + """Raised when a spec variant is set to 'any' and concretizes to 'none'.""" diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index 01fb4fc7a5..4cc7ed6406 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -1114,8 +1114,10 @@ class Database(object): new_spec._hash = key else: - # If it is already there, mark it as installed. + # If it is already there, mark it as installed and update + # installation time self._data[key].installed = True + self._data[key].installation_time = _now() self._data[key].explicit = explicit diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index f6496bb62a..7b2084d229 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -50,7 +50,7 @@ from spack.version import Version, VersionChecksumError __all__ = [] #: These are variant names used by Spack internally; packages can't use them -reserved_names = ['patches'] +reserved_names = ['patches', 'dev_path'] _patch_order_index = 0 diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 9b9f8c7d17..e656e5e368 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -26,6 +26,7 @@ import spack.repo import spack.schema.env import spack.spec import spack.store +import spack.stage import spack.util.spack_json as sjson import spack.util.spack_yaml as syaml import spack.config @@ -707,6 +708,15 @@ class Environment(object): configuration = config_dict(self.yaml) self.concretization = configuration.get('concretization') + # Retrieve dev-build packages: + self.dev_specs = configuration['develop'] + for name, entry in self.dev_specs.items(): + # spec must include a concrete version + assert Spec(entry['spec']).version.concrete + # default path is the spec name + if 'path' not in entry: + self.dev_specs[name]['path'] = name + @property def user_specs(self): return self.spec_lists[user_speclist_name] @@ -722,6 +732,7 @@ class Environment(object): def clear(self): self.spec_lists = {user_speclist_name: SpecList()} # specs from yaml + self.dev_specs = {} # dev-build specs from yaml self.concretized_user_specs = [] # user specs from last concretize self.concretized_order = [] # roots of last concretize, in order self.specs_by_hash = {} # concretized specs by hash @@ -976,6 +987,71 @@ class Environment(object): del self.concretized_order[i] del self.specs_by_hash[dag_hash] + def develop(self, spec, path, clone=False): + """Add dev-build info for package + + Args: + spec (Spec): Set constraints on development specs. Must include a + concrete version. + path (string): Path to find code for developer builds. Relative + paths will be resolved relative to the environment. + clone (bool, default False): Clone the package code to the path. + If clone is False Spack will assume the code is already present + at ``path``. + + Return: + (bool): True iff the environment was changed. + """ + spec = spec.copy() # defensive copy since we access cached attributes + + if not spec.versions.concrete: + raise SpackEnvironmentError( + 'Cannot develop spec %s without a concrete version' % spec) + + for name, entry in self.dev_specs.items(): + if name == spec.name: + e_spec = Spec(entry['spec']) + e_path = entry['path'] + + if e_spec == spec: + if path == e_path: + tty.msg("Spec %s already configured for development" % + spec) + return False + else: + tty.msg("Updating development path for spec %s" % spec) + break + else: + msg = "Updating development spec for package " + msg += "%s with path %s" % (spec.name, path) + tty.msg(msg) + break + else: + tty.msg("Configuring spec %s for development at path %s" % + (spec, path)) + + if clone: + # "steal" the source code via staging API + abspath = path if os.path.isabs(path) else os.path.join( + self.path, path) + + stage = spec.package.stage + stage.steal_source(abspath) + + # If it wasn't already in the list, append it + self.dev_specs[spec.name] = {'path': path, 'spec': str(spec)} + return True + + def undevelop(self, spec): + """Remove develop info for abstract spec ``spec``. + + returns True on success, False if no entry existed.""" + spec = Spec(spec) # In case it's a spec object + if spec.name in self.dev_specs: + del self.dev_specs[spec.name] + return True + return False + def concretize(self, force=False): """Concretize user_specs in this environment. @@ -1248,6 +1324,53 @@ class Environment(object): self.concretized_order.append(h) self.specs_by_hash[h] = concrete + def _spec_needs_overwrite(self, spec): + # Overwrite the install if it's a dev build (non-transitive) + # and the code has been changed since the last install + # or one of the dependencies has been reinstalled since + # the last install + + # if it's not installed, we don't need to overwrite it + if not spec.package.installed: + return False + + # if spec and all deps aren't dev builds, we don't need to overwrite it + if not any(spec.satisfies(c) + for c in ('dev_path=any', '^dev_path=any')): + return False + + # if any dep needs overwrite, or any dep is missing and is a dev build + # then overwrite this package + if any( + self._spec_needs_overwrite(dep) or + ((not dep.package.installed) and dep.satisfies('dev_path=any')) + for dep in spec.traverse(root=False) + ): + return True + + # if it's not a direct dev build and its dependencies haven't + # changed, it hasn't changed. + # We don't merely check satisfaction (spec.satisfies('dev_path=any') + # because we need the value of the variant in the next block of code + dev_path_var = spec.variants.get('dev_path', None) + if not dev_path_var: + return False + + # if it is a direct dev build, check whether the code changed + # we already know it is installed + _, record = spack.store.db.query_by_spec_hash(spec.dag_hash()) + mtime = fs.last_modification_time_recursive(dev_path_var.value) + return mtime > record.installation_time + + def _get_overwrite_specs(self): + ret = [] + for dag_hash in self.concretized_order: + spec = self.specs_by_hash[dag_hash] + ret.extend([d.dag_hash() for d in spec.traverse(root=True) + if self._spec_needs_overwrite(d)]) + + return ret + def install(self, user_spec, concrete_spec=None, **install_args): """Install a single spec into an environment. @@ -1260,7 +1383,11 @@ class Environment(object): def _install(self, spec, **install_args): # "spec" must be concrete - spec.package.do_install(**install_args) + package = spec.package + + install_args['overwrite'] = install_args.get( + 'overwrite', []) + self._get_overwrite_specs() + package.do_install(**install_args) if not spec.external: # Make sure log directory exists @@ -1288,14 +1415,18 @@ class Environment(object): # a large amount of time due to repeatedly acquiring and releasing # locks, this does an initial check across all specs within a single # DB read transaction to reduce time spent in this case. - uninstalled_specs = [] + specs_to_install = [] with spack.store.db.read_transaction(): for concretized_hash in self.concretized_order: spec = self.specs_by_hash[concretized_hash] - if not spec.package.installed: - uninstalled_specs.append(spec) - - for spec in uninstalled_specs: + if not spec.package.installed or ( + spec.satisfies('dev_path=any') or + spec.satisfies('^dev_path=any') + ): + # If it's a dev build it could need to be reinstalled + specs_to_install.append(spec) + + for spec in specs_to_install: # Parse cli arguments and construct a dictionary # that will be passed to Package.do_install API kwargs = dict() @@ -1583,6 +1714,17 @@ class Environment(object): else: view = False yaml_dict['view'] = view + + if self.dev_specs: + # Remove entries that are mirroring defaults + write_dev_specs = copy.deepcopy(self.dev_specs) + for name, entry in write_dev_specs.items(): + if entry['path'] == name: + del entry['path'] + yaml_dict['develop'] = write_dev_specs + else: + yaml_dict.pop('develop', None) + # Remove yaml sections that are shadowing defaults # construct garbage path to ensure we don't find a manifest by accident with fs.temp_cwd() as env_dir: diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 401e2428b3..29fce5617a 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -565,7 +565,6 @@ install_args_docstring = """ 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 package install_source (bool): By default, source is not installed, but @@ -575,6 +574,8 @@ install_args_docstring = """ keep_stage (bool): By default, stage is destroyed only if there are no exceptions during build. Set to True to keep the stage even with exceptions. + overwrite (list): list of hashes for packages to do overwrite + installs. Default empty list. restage (bool): Force spack to restage the package source. skip_patch (bool): Skip patch stage of build if True. stop_before (InstallPhase): stop execution before this @@ -638,6 +639,10 @@ class PackageInstaller(object): # Cache of installed packages' unique ids self.installed = set() + # Cache of overwrite information + self.overwrite = set() + self.overwrite_time = time.time() + # Data store layout self.layout = spack.store.layout @@ -727,7 +732,9 @@ class PackageInstaller(object): # 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: + if installed_in_db and ( + dep.dag_hash() not in self.overwrite or + rec.installation_time > self.overwrite_time): tty.debug('Flagging {0} as installed per the database' .format(dep_id)) self.installed.add(dep_id) @@ -778,7 +785,10 @@ class PackageInstaller(object): if restage and task.pkg.stage.managed_by_spack: task.pkg.stage.destroy() - if not partial and self.layout.check_installed(task.pkg.spec): + if not partial and self.layout.check_installed(task.pkg.spec) and ( + rec.spec.dag_hash() not in self.overwrite or + rec.installation_time > self.overwrite_time + ): self._update_installed(task) # Only update the explicit entry once for the explicit package @@ -1417,6 +1427,12 @@ class PackageInstaller(object): # always installed regardless of whether the root was installed install_package = kwargs.pop('install_package', True) + # take a timestamp with the overwrite argument to check whether another + # process has already overridden the package. + self.overwrite = set(kwargs.get('overwrite', [])) + if self.overwrite: + self.overwrite_time = time.time() + # Ensure not attempting to perform an installation when user didn't # want to go that far. self._check_last_phase(**kwargs) @@ -1543,7 +1559,23 @@ class PackageInstaller(object): # Proceed with the installation since we have an exclusive write # lock on the package. try: - self._install_task(task, **kwargs) + if pkg.spec.dag_hash() in self.overwrite: + rec, _ = self._check_db(pkg.spec) + if rec and rec.installed: + if rec.installation_time < self.overwrite_time: + # If it's actually overwriting, do a fs transaction + if os.path.exists(rec.path): + with fs.replace_directory_transaction( + rec.path): + self._install_task(task, **kwargs) + else: + tty.debug("Missing installation to overwrite") + self._install_task(task, **kwargs) + else: + # overwriting nothing + self._install_task(task, **kwargs) + else: + self._install_task(task, **kwargs) self._update_installed(task) # If we installed then we should keep the prefix diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 3a0c17da88..14b1d1089a 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -916,6 +916,11 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): return stage def _make_stage(self): + # If it's a dev package (not transitively), use a DIY stage object + dev_path_var = self.spec.variants.get('dev_path', None) + if dev_path_var: + return spack.stage.DIYStage(dev_path_var.value) + # Construct a composite stage on top of the composite FetchStrategy composite_fetcher = self.fetcher composite_stage = StageComposite() @@ -1230,16 +1235,14 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): Creates a stage directory and downloads the tarball for this package. Working directory will be set to the stage directory. """ - if not self.spec.concrete: - raise ValueError("Can only fetch concrete packages.") - if not self.has_code: tty.debug('No fetch required for {0}: package has no code.' .format(self.name)) start_time = time.time() checksum = spack.config.get('config:checksum') - if checksum and self.version not in self.versions: + fetch = self.stage.managed_by_spack + if checksum and fetch and self.version not in self.versions: tty.warn("There is no checksum on file to fetch %s safely." % self.spec.cformat('{name}{@version}')) @@ -1275,9 +1278,6 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): def do_stage(self, mirror_only=False): """Unpacks and expands the fetched tarball.""" - if not self.spec.concrete: - raise ValueError("Can only stage concrete packages.") - # Always create the stage directory at this point. Why? A no-code # package may want to use the installation process to install metadata. self.stage.create() @@ -1587,6 +1587,12 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): their build process. Args:""" + # Non-transitive dev specs need to keep the dev stage and be built from + # source every time. Transitive ones just need to be built from source. + dev_path_var = self.spec.variants.get('dev_path', None) + if dev_path_var: + kwargs['keep_stage'] = True + builder = PackageInstaller(self) builder.install(**kwargs) diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 18a2048557..a7e3dc608f 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -73,6 +73,25 @@ schema = { 'type': 'string' }, }, + 'develop': { + 'type': 'object', + 'default': {}, + 'additionalProperties': False, + 'patternProperties': { + r'\w[\w-]*': { + 'type': 'object', + 'additionalProperties': False, + 'properties': { + 'spec': { + 'type': 'string' + }, + 'path': { + 'type': 'string' + }, + }, + }, + }, + }, 'definitions': { 'type': 'array', 'default': [], diff --git a/lib/spack/spack/schema/spec.py b/lib/spack/spack/schema/spec.py index 8ec17e7e76..c61447ef9b 100644 --- a/lib/spack/spack/schema/spec.py +++ b/lib/spack/spack/schema/spec.py @@ -96,6 +96,12 @@ properties = { 'version': {'type': 'string'}, }, }, + 'develop': { + 'anyOf': [ + {'type': 'boolean'}, + {'type': 'string'}, + ], + }, 'namespace': {'type': 'string'}, 'parameters': { 'type': 'object', diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 0b4e7dfacf..47ff0c5d06 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2063,7 +2063,8 @@ class Spec(object): # still need to select a concrete package later. if not self.virtual: changed |= any( - (concretizer.concretize_architecture(self), + (concretizer.concretize_develop(self), # special variant + concretizer.concretize_architecture(self), concretizer.concretize_compiler(self), concretizer.adjust_target(self), # flags must be concretized after compiler @@ -2698,6 +2699,10 @@ class Spec(object): if user_spec_deps: for name, spec in user_spec_deps.items(): + if not name: + msg = "Attempted to normalize anonymous dependency spec" + msg += " %s" % spec + raise InvalidSpecDetected(msg) if name not in all_spec_deps: all_spec_deps[name] = spec else: @@ -2850,6 +2855,9 @@ class Spec(object): if not other.satisfies_dependencies(self): raise UnsatisfiableDependencySpecError(other, self) + if any(not d.name for d in other.traverse(root=False)): + raise UnconstrainableDependencySpecError(other) + # Handle common first-order constraints directly changed = False for name in self.common_dependencies(other): @@ -4123,8 +4131,23 @@ class SpecParser(spack.parse.Parser): if not dep: # We're adding a dependency to the last spec - self.expect(ID) - dep = self.spec(self.token.value) + if self.accept(ID): + self.previous = self.token + if self.accept(EQ): + # This is an anonymous dep with a key=value + # push tokens to be parsed as part of the + # dep spec + self.push_tokens( + [self.previous, self.token]) + dep_name = None + else: + # named dep (standard) + dep_name = self.token.value + self.previous = None + else: + # anonymous dep + dep_name = None + dep = self.spec(dep_name) # Raise an error if the previous spec is already # concrete (assigned by hash) @@ -4509,6 +4532,14 @@ class UnsatisfiableDependencySpecError(spack.error.UnsatisfiableSpecError): provided, required, "dependency") +class UnconstrainableDependencySpecError(spack.error.SpecError): + """Raised when attempting to constrain by an anonymous dependency spec""" + def __init__(self, spec): + msg = "Cannot constrain by spec '%s'. Cannot constrain by a" % spec + msg += " spec containing anonymous dependencies" + super(UnconstrainableDependencySpecError, self).__init__(msg) + + class AmbiguousHashError(spack.error.SpecError): def __init__(self, msg, *specs): spec_fmt = '{namespace}.{name}{@version}{%compiler}{compiler_flags}' diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index cf32bc92f8..ca3781deba 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -5,13 +5,15 @@ from __future__ import print_function +import errno +import getpass +import glob +import hashlib import os +import shutil import stat import sys -import errno -import hashlib import tempfile -import getpass from six import string_types from six import iteritems @@ -491,6 +493,41 @@ class Stage(object): print_errors(errors) + def steal_source(self, dest): + """Copy the source_path directory in its entirety to directory dest + + This operation creates/fetches/expands the stage if it is not already, + and destroys the stage when it is done.""" + if not self.created: + self.create() + if not self.expanded and not self.archive_file: + self.fetch() + if not self.expanded: + self.expand_archive() + + if not os.path.isdir(dest): + mkdirp(dest) + + # glob all files and directories in the source path + hidden_entries = glob.glob(os.path.join(self.source_path, '.*')) + entries = glob.glob(os.path.join(self.source_path, '*')) + + # Move all files from stage to destination directory + # Include hidden files for VCS repo history + for entry in hidden_entries + entries: + if os.path.isdir(entry): + d = os.path.join(dest, os.path.basename(entry)) + shutil.copytree(entry, d) + else: + shutil.copy2(entry, dest) + + # copy archive file if we downloaded from url -- replaces for vcs + if self.archive_file and os.path.exists(self.archive_file): + shutil.copy2(self.archive_file, dest) + + # remove leftover stage + self.destroy() + def check(self): """Check the downloaded archive against a checksum digest. No-op if this stage checks code out of a repository.""" @@ -655,7 +692,8 @@ class ResourceStage(Stage): @pattern.composite(method_list=[ 'fetch', 'create', 'created', 'check', 'expand_archive', 'restage', - 'destroy', 'cache_local', 'cache_mirror', 'managed_by_spack']) + 'destroy', 'cache_local', 'cache_mirror', 'steal_source', + 'managed_by_spack']) class StageComposite: """Composite for Stage type objects. The first item in this composite is considered to be the root package, and operations that return a value are diff --git a/lib/spack/spack/test/cmd/dev_build.py b/lib/spack/spack/test/cmd/dev_build.py index 94c7690de6..2739bf1524 100644 --- a/lib/spack/spack/test/cmd/dev_build.py +++ b/lib/spack/spack/test/cmd/dev_build.py @@ -2,16 +2,22 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) + import os import pytest import spack.spec +import llnl.util.filesystem as fs +import spack.environment as ev from spack.main import SpackCommand, SpackCommandError dev_build = SpackCommand('dev-build') +install = SpackCommand('install') +env = SpackCommand('env') def test_dev_build_basics(tmpdir, mock_packages, install_mockery): - spec = spack.spec.Spec('dev-build-test-install@0.0.0').concretized() + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() with tmpdir.as_cwd(): with open(spec.package.filename, 'w') as f: @@ -23,9 +29,12 @@ def test_dev_build_basics(tmpdir, mock_packages, install_mockery): with open(os.path.join(spec.prefix, spec.package.filename), 'r') as f: assert f.read() == spec.package.replacement_string + assert os.path.exists(str(tmpdir)) + def test_dev_build_before(tmpdir, mock_packages, install_mockery): - spec = spack.spec.Spec('dev-build-test-install@0.0.0').concretized() + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() with tmpdir.as_cwd(): with open(spec.package.filename, 'w') as f: @@ -41,7 +50,8 @@ def test_dev_build_before(tmpdir, mock_packages, install_mockery): def test_dev_build_until(tmpdir, mock_packages, install_mockery): - spec = spack.spec.Spec('dev-build-test-install@0.0.0').concretized() + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() with tmpdir.as_cwd(): with open(spec.package.filename, 'w') as f: @@ -59,7 +69,8 @@ def test_dev_build_until(tmpdir, mock_packages, install_mockery): def test_dev_build_until_last_phase(tmpdir, mock_packages, install_mockery): # Test that we ignore the last_phase argument if it is already last - spec = spack.spec.Spec('dev-build-test-install@0.0.0').concretized() + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() with tmpdir.as_cwd(): with open(spec.package.filename, 'w') as f: @@ -73,10 +84,12 @@ def test_dev_build_until_last_phase(tmpdir, mock_packages, install_mockery): assert os.path.exists(spec.prefix) assert spack.store.db.query(spec, installed=True) + assert os.path.exists(str(tmpdir)) def test_dev_build_before_until(tmpdir, mock_packages, install_mockery): - spec = spack.spec.Spec('dev-build-test-install@0.0.0').concretized() + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() with tmpdir.as_cwd(): with open(spec.package.filename, 'w') as f: @@ -118,7 +131,8 @@ def test_dev_build_drop_in(tmpdir, mock_packages, monkeypatch, def test_dev_build_fails_already_installed(tmpdir, mock_packages, install_mockery): - spec = spack.spec.Spec('dev-build-test-install@0.0.0').concretized() + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() with tmpdir.as_cwd(): with open(spec.package.filename, 'w') as f: @@ -147,3 +161,226 @@ def test_dev_build_fails_nonexistent_package_name(mock_packages): def test_dev_build_fails_no_version(mock_packages): output = dev_build('dev-build-test-install', fail_on_error=False) assert 'dev-build spec must have a single, concrete version' in output + + +def test_dev_build_env(tmpdir, mock_packages, install_mockery, + mutable_mock_env_path): + """Test Spack does dev builds for packages in develop section of env.""" + # setup dev-build-test-install package for dev build + build_dir = tmpdir.mkdir('build') + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % + build_dir) + spec.concretize() + + with build_dir.as_cwd(): + with open(spec.package.filename, 'w') as f: + f.write(spec.package.original_string) + + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - dev-build-test-install@0.0.0 + + develop: + dev-build-test-install: + spec: dev-build-test-install@0.0.0 + path: %s +""" % build_dir) + + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + assert spec.package.filename in os.listdir(spec.prefix) + with open(os.path.join(spec.prefix, spec.package.filename), 'r') as f: + assert f.read() == spec.package.replacement_string + + +def test_dev_build_env_version_mismatch(tmpdir, mock_packages, install_mockery, + mutable_mock_env_path): + """Test Spack constraints concretization by develop specs.""" + # setup dev-build-test-install package for dev build + build_dir = tmpdir.mkdir('build') + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir) + spec.concretize() + + with build_dir.as_cwd(): + with open(spec.package.filename, 'w') as f: + f.write(spec.package.original_string) + + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - dev-build-test-install@0.0.0 + + develop: + dev-build-test-install: + spec: dev-build-test-install@1.1.1 + path: %s +""" % build_dir) + + env('create', 'test', './spack.yaml') + with ev.read('test'): + with pytest.raises(spack.spec.UnsatisfiableVersionSpecError): + install() + + +def test_dev_build_multiple(tmpdir, mock_packages, install_mockery, + mutable_mock_env_path, mock_fetch): + """Test spack install with multiple developer builds""" + # setup dev-build-test-install package for dev build + # Wait to concretize inside the environment to set dev_path on the specs; + # without the environment, the user would need to set dev_path for both the + # root and dependency if they wanted a dev build for both. + leaf_dir = tmpdir.mkdir('leaf') + leaf_spec = spack.spec.Spec('dev-build-test-install@0.0.0') + with leaf_dir.as_cwd(): + with open(leaf_spec.package.filename, 'w') as f: + f.write(leaf_spec.package.original_string) + + # setup dev-build-test-dependent package for dev build + # don't concretize outside environment -- dev info will be wrong + root_dir = tmpdir.mkdir('root') + root_spec = spack.spec.Spec('dev-build-test-dependent@0.0.0') + with root_dir.as_cwd(): + with open(root_spec.package.filename, 'w') as f: + f.write(root_spec.package.original_string) + + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - dev-build-test-install@0.0.0 + - dev-build-test-dependent@0.0.0 + + develop: + dev-build-test-install: + path: %s + spec: dev-build-test-install@0.0.0 + dev-build-test-dependent: + spec: dev-build-test-dependent@0.0.0 + path: %s +""" % (leaf_dir, root_dir)) + + env('create', 'test', './spack.yaml') + with ev.read('test'): + # Do concretization inside environment for dev info + leaf_spec.concretize() + root_spec.concretize() + + # Do install + install() + + for spec in (leaf_spec, root_spec): + assert spec.package.filename in os.listdir(spec.prefix) + with open(os.path.join(spec.prefix, spec.package.filename), 'r') as f: + assert f.read() == spec.package.replacement_string + + +def test_dev_build_env_dependency(tmpdir, mock_packages, install_mockery, + mock_fetch, mutable_mock_env_path): + """ + Test non-root specs in an environment are properly marked for dev builds. + """ + # setup dev-build-test-install package for dev build + build_dir = tmpdir.mkdir('build') + spec = spack.spec.Spec('dependent-of-dev-build@0.0.0') + dep_spec = spack.spec.Spec('dev-build-test-install') + + with build_dir.as_cwd(): + with open(dep_spec.package.filename, 'w') as f: + f.write(dep_spec.package.original_string) + + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - dependent-of-dev-build@0.0.0 + + develop: + dev-build-test-install: + spec: dev-build-test-install@0.0.0 + path: %s +""" % build_dir) + + env('create', 'test', './spack.yaml') + with ev.read('test'): + # concretize in the environment to get the dev build info + # equivalent to setting dev_build and dev_path variants + # on all specs above + spec.concretize() + dep_spec.concretize() + install() + + # Ensure that both specs installed properly + assert dep_spec.package.filename in os.listdir(dep_spec.prefix) + assert os.path.exists(spec.prefix) + + # Ensure variants set properly + for dep in (dep_spec, spec['dev-build-test-install']): + assert dep.satisfies('dev_path=%s' % build_dir) + assert spec.satisfies('^dev_path=any') + + +@pytest.mark.parametrize('test_spec', ['dev-build-test-install', + 'dependent-of-dev-build']) +def test_dev_build_rebuild_on_source_changes( + test_spec, tmpdir, mock_packages, install_mockery, + mutable_mock_env_path, mock_fetch): + """Test dev builds rebuild on changes to source code. + + ``test_spec = dev-build-test-install`` tests rebuild for changes to package + ``test_spec = dependent-of-dev-build`` tests rebuild for changes to dep + """ + # setup dev-build-test-install package for dev build + build_dir = tmpdir.mkdir('build') + spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % + build_dir) + spec.concretize() + + def reset_string(): + with build_dir.as_cwd(): + with open(spec.package.filename, 'w') as f: + f.write(spec.package.original_string) + + reset_string() + + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - %s@0.0.0 + + develop: + dev-build-test-install: + spec: dev-build-test-install@0.0.0 + path: %s +""" % (test_spec, build_dir)) + + env('create', 'test', './spack.yaml') + with ev.read('test'): + install() + + reset_string() # so the package will accept rebuilds + + fs.touch(os.path.join(str(build_dir), 'test')) + output = install() + + assert 'Installing %s' % test_spec in output diff --git a/lib/spack/spack/test/cmd/develop.py b/lib/spack/spack/test/cmd/develop.py new file mode 100644 index 0000000000..c539cd91c0 --- /dev/null +++ b/lib/spack/spack/test/cmd/develop.py @@ -0,0 +1,100 @@ +# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) +import pytest +import os +import shutil +import llnl.util.filesystem as fs + +import spack.spec +import spack.environment as ev +from spack.main import SpackCommand + +develop = SpackCommand('develop') +env = SpackCommand('env') + + +@pytest.mark.usefixtures( + 'mutable_mock_env_path', 'mock_packages', 'mock_fetch') +class TestDevelop(object): + def check_develop(self, env, spec, path=None): + path = path or spec.name + + # check in memory representation + assert spec.name in env.dev_specs + dev_specs_entry = env.dev_specs[spec.name] + assert dev_specs_entry['path'] == path + assert dev_specs_entry['spec'] == str(spec) + + # check yaml representation + yaml = ev.config_dict(env.yaml) + assert spec.name in yaml['develop'] + yaml_entry = yaml['develop'][spec.name] + assert yaml_entry['spec'] == str(spec) + if path == spec.name: + # default paths aren't written out + assert 'path' not in yaml_entry + else: + assert yaml_entry['path'] == path + + def test_develop_no_path_no_clone(self): + env('create', 'test') + with ev.read('test') as e: + # develop checks that the path exists + fs.mkdirp(os.path.join(e.path, 'mpich')) + develop('--no-clone', 'mpich@1.0') + self.check_develop(e, spack.spec.Spec('mpich@1.0')) + + def test_develop_no_clone(self, tmpdir): + env('create', 'test') + with ev.read('test') as e: + develop('--no-clone', '-p', str(tmpdir), 'mpich@1.0') + self.check_develop(e, spack.spec.Spec('mpich@1.0'), str(tmpdir)) + + def test_develop(self): + env('create', 'test') + with ev.read('test') as e: + develop('mpich@1.0') + self.check_develop(e, spack.spec.Spec('mpich@1.0')) + + def test_develop_no_args(self): + env('create', 'test') + with ev.read('test') as e: + # develop and remove it + develop('mpich@1.0') + shutil.rmtree(os.path.join(e.path, 'mpich')) + + # test develop with no args + develop() + self.check_develop(e, spack.spec.Spec('mpich@1.0')) + + def test_develop_twice(self): + env('create', 'test') + with ev.read('test') as e: + develop('mpich@1.0') + self.check_develop(e, spack.spec.Spec('mpich@1.0')) + + develop('mpich@1.0') + # disk representation isn't updated unless we write + # second develop command doesn't change it, so we don't write + # but we check disk representation + e.write() + self.check_develop(e, spack.spec.Spec('mpich@1.0')) + assert len(e.dev_specs) == 1 + + def test_develop_update_path(self, tmpdir): + env('create', 'test') + with ev.read('test') as e: + develop('mpich@1.0') + develop('-p', str(tmpdir), 'mpich@1.0') + self.check_develop(e, spack.spec.Spec('mpich@1.0'), str(tmpdir)) + assert len(e.dev_specs) == 1 + + def test_develop_update_spec(self): + env('create', 'test') + with ev.read('test') as e: + develop('mpich@1.0') + develop('mpich@2.0') + self.check_develop(e, spack.spec.Spec('mpich@2.0')) + assert len(e.dev_specs) == 1 diff --git a/lib/spack/spack/test/cmd/undevelop.py b/lib/spack/spack/test/cmd/undevelop.py new file mode 100644 index 0000000000..2ebc554f29 --- /dev/null +++ b/lib/spack/spack/test/cmd/undevelop.py @@ -0,0 +1,67 @@ +# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +import spack.spec +import spack.environment as ev +from spack.main import SpackCommand + +undevelop = SpackCommand('undevelop') +env = SpackCommand('env') +concretize = SpackCommand('concretize') + + +def test_undevelop(tmpdir, mock_packages, mutable_mock_env_path): + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - mpich + + develop: + mpich: + spec: mpich@1.0 + path: /fake/path +""") + + env('create', 'test', './spack.yaml') + with ev.read('test'): + before = spack.spec.Spec('mpich').concretized() + undevelop('mpich') + after = spack.spec.Spec('mpich').concretized() + + # Removing dev spec from environment changes concretization + assert before.satisfies('dev_path=any') + assert not after.satisfies('dev_path=any') + + +def test_undevelop_nonexistent(tmpdir, mock_packages, mutable_mock_env_path): + # setup environment + envdir = tmpdir.mkdir('env') + with envdir.as_cwd(): + with open('spack.yaml', 'w') as f: + f.write("""\ +env: + specs: + - mpich + + develop: + mpich: + spec: mpich@1.0 + path: /fake/path +""") + + env('create', 'test', './spack.yaml') + with ev.read('test') as e: + concretize() + before = e.specs_by_hash + undevelop('package-not-in-develop') # does nothing + concretize('-f') + after = e.specs_by_hash + + # nothing should have changed + assert before == after diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 6e86d7c914..65ac93105a 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -195,7 +195,7 @@ class TestConcretize(object): s.satisfies('mpich2') for s in repo.providers_for('mpi@3') ) - def test_provides_handles_multiple_providers_of_same_vesrion(self): + def test_provides_handles_multiple_providers_of_same_version(self): """ """ providers = spack.repo.path.providers_for('mpi@3.0') @@ -644,3 +644,12 @@ class TestConcretize(object): with pytest.raises(spack.error.SpecError): s = Spec('+variant') s.concretize() + + def test_concretize_anonymous_dep(self): + with pytest.raises(spack.error.SpecError): + s = Spec('mpileaks ^%gcc') + s.concretize() + + with pytest.raises(spack.error.SpecError): + s = Spec('mpileaks ^cflags=-g') + s.concretize() diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index c941ccad94..3910d528e9 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -9,6 +9,7 @@ import stat import spack.package_prefs import spack.repo import spack.util.spack_yaml as syaml +from spack.concretize import NonDeterministicVariantError from spack.config import ConfigScope, ConfigError from spack.spec import Spec from spack.version import Version @@ -84,6 +85,23 @@ class TestConcretizePreferences(object): 'mpileaks', debug=True, opt=True, shared=False, static=False ) + def test_preferred_variants_from_any(self): + """ + Test that 'foo=any' concretizes to any non-none value + + Test that concretization of variants raises an error attempting + non-deterministic concretization from 'any' when preferred value is + 'none'. + """ + update_packages('multivalue-variant', 'variants', 'foo=bar') + assert_variant_values( + 'multivalue-variant foo=any', foo=('bar',) + ) + + update_packages('multivalue-variant', 'variants', 'foo=none') + with pytest.raises(NonDeterministicVariantError): + concretize('multivalue-variant foo=any') + def test_preferred_compilers(self): """Test preferred compilers are applied correctly """ diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index 6d15dff7f6..47ec06bbc4 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -512,12 +512,6 @@ def test_unconcretized_install(install_mockery, mock_fetch, mock_packages): with pytest.raises(ValueError, match="only install concrete packages"): spec.package.do_install() - with pytest.raises(ValueError, match="only fetch concrete packages"): - spec.package.do_fetch() - - with pytest.raises(ValueError, match="only stage concrete packages"): - spec.package.do_stage() - with pytest.raises(ValueError, match="only patch concrete packages"): spec.package.do_patch() diff --git a/lib/spack/spack/test/modules/conftest.py b/lib/spack/spack/test/modules/conftest.py index 4ba25bd073..365a5a7092 100644 --- a/lib/spack/spack/test/modules/conftest.py +++ b/lib/spack/spack/test/modules/conftest.py @@ -34,7 +34,6 @@ def filename_dict(file_registry, monkeypatch): if not mode == 'w': raise RuntimeError('opening mode must be "w" [stringio_open]') - file_registry[filename] = StringIO() try: yield file_registry[filename] finally: @@ -63,6 +62,7 @@ def modulefile_content(filename_dict, request): # Get its filename filename = generator.layout.filename + # Retrieve the content content = filename_dict[filename].split('\n') generator.remove() diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index e4d111a64f..a258e45826 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -7,6 +7,7 @@ import sys import pytest from spack.error import SpecError, UnsatisfiableSpecError +from spack.spec import UnconstrainableDependencySpecError from spack.spec import Spec, SpecFormatSigilError, SpecFormatStringError from spack.variant import InvalidVariantValueError, UnknownVariantError from spack.variant import MultipleValuesInExclusiveVariantError @@ -80,7 +81,8 @@ def check_constrain_not_changed(spec, constraint): def check_invalid_constraint(spec, constraint): spec = Spec(spec) constraint = Spec(constraint) - with pytest.raises(UnsatisfiableSpecError): + with pytest.raises((UnsatisfiableSpecError, + UnconstrainableDependencySpecError)): spec.constrain(constraint) @@ -272,6 +274,8 @@ class TestSpecSematics(object): check_satisfies('mpich foo=true', 'mpich+foo') check_satisfies('mpich~foo', 'mpich foo=FALSE') check_satisfies('mpich foo=False', 'mpich~foo') + check_satisfies('mpich foo=any', 'mpich~foo') + check_satisfies('mpich +foo', 'mpich foo=any') def test_satisfies_multi_value_variant(self): # Check quoting @@ -284,6 +288,12 @@ class TestSpecSematics(object): # A more constrained spec satisfies a less constrained one check_satisfies('multivalue-variant foo="bar,baz"', + 'multivalue-variant foo=any') + + check_satisfies('multivalue-variant foo=any', + 'multivalue-variant foo="bar,baz"') + + check_satisfies('multivalue-variant foo="bar,baz"', 'multivalue-variant foo="bar"') check_satisfies('multivalue-variant foo="bar,baz"', @@ -307,6 +317,7 @@ class TestSpecSematics(object): a.concretize() assert a.satisfies('foobar=bar') + assert a.satisfies('foobar=any') # Assert that an autospec generated from a literal # gives the right result for a single valued variant @@ -441,6 +452,10 @@ class TestSpecSematics(object): check_unsatisfiable('mpich', 'mpich~foo', True) check_unsatisfiable('mpich', 'mpich foo=1', True) + # None and any do not satisfy each other + check_unsatisfiable('foo=none', 'foo=any') + check_unsatisfiable('foo=any', 'foo=none') + def test_unsatisfiable_variant_mismatch(self): # No matchi in specs check_unsatisfiable('mpich~foo', 'mpich+foo') @@ -608,6 +623,11 @@ class TestSpecSematics(object): 'multivalue-variant foo="baz"' ) + check_constrain( + 'libelf foo=bar,baz', 'libelf foo=bar,baz', 'libelf foo=any') + check_constrain( + 'libelf foo=bar,baz', 'libelf foo=any', 'libelf foo=bar,baz') + def test_constrain_compiler_flags(self): check_constrain( 'libelf cflags="-O3" cppflags="-Wall"', @@ -648,12 +668,15 @@ class TestSpecSematics(object): check_invalid_constraint('libelf+debug', 'libelf~debug') check_invalid_constraint('libelf+debug~foo', 'libelf+debug+foo') check_invalid_constraint('libelf debug=True', 'libelf debug=False') + check_invalid_constraint('libelf foo=none', 'libelf foo=any') + check_invalid_constraint('libelf foo=any', 'libelf foo=none') check_invalid_constraint( 'libelf cppflags="-O3"', 'libelf cppflags="-O2"') check_invalid_constraint( 'libelf platform=test target=be os=be', 'libelf target=fe os=fe' ) + check_invalid_constraint('libdwarf', '^%gcc') def test_constrain_changed(self): check_constrain_changed('libelf', '@1.0') @@ -661,6 +684,7 @@ class TestSpecSematics(object): check_constrain_changed('libelf', '%gcc') check_constrain_changed('libelf%gcc', '%gcc@4.5') check_constrain_changed('libelf', '+debug') + check_constrain_changed('libelf', 'debug=any') check_constrain_changed('libelf', '~debug') check_constrain_changed('libelf', 'debug=2') check_constrain_changed('libelf', 'cppflags="-O3"') @@ -680,6 +704,7 @@ class TestSpecSematics(object): check_constrain_not_changed('libelf+debug', '+debug') check_constrain_not_changed('libelf~debug', '~debug') check_constrain_not_changed('libelf debug=2', 'debug=2') + check_constrain_not_changed('libelf debug=2', 'debug=any') check_constrain_not_changed( 'libelf cppflags="-O3"', 'cppflags="-O3"') @@ -893,13 +918,14 @@ class TestSpecSematics(object): for x in ('cflags', 'cxxflags', 'fflags') ) - def test_any_combination_of(self): - # Test that using 'none' and another value raise during concretization - spec = Spec('multivalue-variant foo=none,bar') - with pytest.raises(spack.error.SpecError) as exc_info: - spec.concretize() + def test_combination_of_any_or_none(self): + # Test that using 'none' and another value raises + with pytest.raises(spack.variant.InvalidVariantValueCombinationError): + Spec('multivalue-variant foo=none,bar') - assert "is mutually exclusive with any of the" in str(exc_info.value) + # Test that using 'any' and another value raises + with pytest.raises(spack.variant.InvalidVariantValueCombinationError): + Spec('multivalue-variant foo=any,bar') @pytest.mark.skipif( sys.version_info[0] == 2, reason='__wrapped__ requires python 3' diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index e43a002182..d5d6acf6ec 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -25,6 +25,8 @@ try: except ImportError: from collections import Sequence +special_variant_values = [None, 'none', 'any'] + class Variant(object): """Represents a variant in a package, as declared in the @@ -119,13 +121,14 @@ class Variant(object): # Check and record the values that are not allowed not_allowed_values = [ - x for x in value if self.single_value_validator(x) is False + x for x in value + if x != 'any' and self.single_value_validator(x) is False ] if not_allowed_values: raise InvalidVariantValueError(self, not_allowed_values, pkg) # Validate the group of values if needed - if self.group_validator is not None: + if self.group_validator is not None and value != ('any',): self.group_validator(pkg.name, self.name, value) @property @@ -267,6 +270,14 @@ class AbstractVariant(object): # Tuple is necessary here instead of list because the # values need to be hashed value = re.split(r'\s*,\s*', str(value)) + value = list(map(lambda x: 'any' if str(x).upper() == 'ANY' else x, + value)) + + for val in special_variant_values: + if val in value and len(value) > 1: + msg = "'%s' cannot be combined with other variant" % val + msg += " values." + raise InvalidVariantValueCombinationError(msg) # With multi-value variants it is necessary # to remove duplicates and give an order @@ -302,7 +313,15 @@ class AbstractVariant(object): """ # If names are different then `self` does not satisfy `other` # (`foo=bar` will never satisfy `baz=bar`) - return other.name == self.name + if other.name != self.name: + return False + # If the variant is already set to none, it can't satisfy any + if ('none' in self or None in self) and 'any' in other: + return False + # If the variant is set to any, it can't be constrained by none + if 'any' in self and ('none' in other or None in other): + return False + return True @implicit_variant_conversion def compatible(self, other): @@ -317,8 +336,17 @@ class AbstractVariant(object): Returns: bool: True or False """ - # If names are different then they are not compatible - return other.name == self.name + # If names are different then `self` is not compatible with `other` + # (`foo=bar` is incompatible with `baz=bar`) + if other.name != self.name: + return False + # If the variant is already set to none, incompatible with any + if ('none' in self or None in self) and 'any' in other: + return False + # If the variant is set to any, it can't be compatible with none + if 'any' in self and ('none' in other or None in other): + return False + return True @implicit_variant_conversion def constrain(self, other): @@ -336,7 +364,13 @@ class AbstractVariant(object): raise ValueError('variants must have the same name') old_value = self.value - self.value = ','.join(sorted(set(self.value + other.value))) + + values = list(sorted(set(self.value + other.value))) + # If we constraint any by another value, just take value + if 'any' in values and len(values) > 1: + values.remove('any') + + self.value = ','.join(values) return old_value != self.value def __contains__(self, item): @@ -367,16 +401,16 @@ class MultiValuedVariant(AbstractVariant): Returns: bool: True or False """ - # If names are different then `self` does not satisfy `other` - # (`foo=bar` does not satisfy `baz=bar`) - if other.name != self.name: - return False + # If it doesn't satisfy as an AbstractVariant, it doesn't satisfy as a + # MultiValuedVariant this handles conflicts between none and any + super_sat = super(MultiValuedVariant, self).satisfies(other) # Otherwise we want all the values in `other` to be also in `self` - return all(v in self.value for v in other.value) + return super_sat and (all(v in self.value for v in other.value) or + 'any' in other or 'any' in self) -class SingleValuedVariant(MultiValuedVariant): +class SingleValuedVariant(AbstractVariant): """A variant that can hold multiple values, but one at a time.""" def _value_setter(self, value): @@ -393,12 +427,12 @@ class SingleValuedVariant(MultiValuedVariant): @implicit_variant_conversion def satisfies(self, other): - # If names are different then `self` does not satisfy `other` - # (`foo=bar` does not satisfy `baz=bar`) - if other.name != self.name: - return False + # If it doesn't satisfy as an AbstractVariant, it doesn't satisfy as a + # SingleValuedVariant this handles conflicts between none and any + abstract_sat = super(SingleValuedVariant, self).satisfies(other) - return self.value == other.value + return abstract_sat and (self.value == other.value or + other.value == 'any' or self.value == 'any') def compatible(self, other): return self.satisfies(other) @@ -408,6 +442,13 @@ class SingleValuedVariant(MultiValuedVariant): if self.name != other.name: raise ValueError('variants must have the same name') + if self.value == 'any': + self.value = other.value + return self.value != other.value + + if other.value == 'any' and self.value not in ('none', None): + return False + if self.value != other.value: raise UnsatisfiableVariantSpecError(other.value, self.value) return False @@ -420,7 +461,10 @@ class SingleValuedVariant(MultiValuedVariant): class BoolValuedVariant(SingleValuedVariant): - """A variant that can hold either True or False.""" + """A variant that can hold either True or False. + + BoolValuedVariant can also hold the value 'any', for coerced + comparisons between ``foo=any`` and ``+foo`` or ``~foo``.""" def _value_setter(self, value): # Check the string representation of the value and turn @@ -431,6 +475,9 @@ class BoolValuedVariant(SingleValuedVariant): elif str(value).upper() == 'FALSE': self._original_value = value self._value = False + elif str(value).upper() == 'ANY': + self._original_value = value + self._value = 'any' else: msg = 'cannot construct a BoolValuedVariant for "{0}" from ' msg += 'a value that does not represent a bool' @@ -604,6 +651,9 @@ def substitute_abstract_variants(spec): failed = [] for name, v in spec.variants.items(): if name in spack.directives.reserved_names: + if name == 'dev_path': + new_variant = SingleValuedVariant(name, v._original_value) + spec.variants.substitute(new_variant) continue pkg_variant = spec.package_class.variants.get(name, None) if not pkg_variant: @@ -823,6 +873,10 @@ class MultipleValuesInExclusiveVariantError(error.SpecError, ValueError): ) +class InvalidVariantValueCombinationError(error.SpecError): + """Raised when a variant has values 'any' or 'none' with other values.""" + + class InvalidVariantValueError(error.SpecError): """Raised when a valid variant has at least an invalid value.""" |