From e0976963903ff938e1433e6b3c71d9ada1240c2d Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Thu, 16 Jul 2015 01:12:11 -0700 Subject: Update concretize to check for more changes and iterate further. --- lib/spack/spack/concretize.py | 21 ++++++++++---- lib/spack/spack/spec.py | 54 +++++++++++++++++++++-------------- lib/spack/spack/test/optional_deps.py | 19 ++++++++++-- 3 files changed, 65 insertions(+), 29 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 2e1d5d7f03..66002492cb 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -63,9 +63,9 @@ class DefaultConcretizer(object): """ # return if already concrete. if spec.versions.concrete: - return + return False - # If there are known avaialble versions, return the most recent + # If there are known available versions, return the most recent # version that satisfies the spec pkg = spec.package valid_versions = sorted( @@ -93,6 +93,8 @@ class DefaultConcretizer(object): else: spec.versions = ver([last]) + return True # Things changed + def concretize_architecture(self, spec): """If the spec already had an architecture, return. Otherwise if @@ -109,22 +111,27 @@ class DefaultConcretizer(object): they're not explicit. """ if spec.architecture is not None: - return + return False if spec.root.architecture: spec.architecture = spec.root.architecture else: spec.architecture = spack.architecture.sys_type() + assert(spec.architecture is not None) + return True # changed + def concretize_variants(self, spec): """If the spec already has variants filled in, return. Otherwise, add the default variants from the package specification. """ + changed = False for name, variant in spec.package.variants.items(): if name not in spec.variants: - spec.variants[name] = spack.spec.VariantSpec( - name, variant.default) + spec.variants[name] = spack.spec.VariantSpec(name, variant.default) + changed = True + return changed def concretize_compiler(self, spec): @@ -144,7 +151,7 @@ class DefaultConcretizer(object): if (spec.compiler and spec.compiler.concrete and spec.compiler in all_compilers): - return + return False try: nearest = next(p for p in spec.traverse(direction='parents') @@ -165,6 +172,8 @@ class DefaultConcretizer(object): except StopIteration: spec.compiler = spack.compilers.default_compiler().copy() + return True # things changed. + def choose_provider(self, spec, providers): """This is invoked for virtual specs. Given a spec with a virtual name, diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 5876fc6cf8..e1fbb84423 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -736,26 +736,31 @@ class Spec(object): if visited is None: visited = set() if self.name in visited: - return + return False + + changed = False # Concretize deps first -- this is a bottom-up process. for name in sorted(self.dependencies.keys()): - self.dependencies[name]._concretize_helper(presets, visited) + changed |= self.dependencies[name]._concretize_helper(presets, visited) if self.name in presets: - self.constrain(presets[self.name]) + changed |= self.constrain(presets[self.name]) + else: # Concretize virtual dependencies last. Because they're added # to presets below, their constraints will all be merged, but we'll # still need to select a concrete package later. if not self.virtual: - spack.concretizer.concretize_architecture(self) - spack.concretizer.concretize_compiler(self) - spack.concretizer.concretize_version(self) - spack.concretizer.concretize_variants(self) + changed |= any( + (spack.concretizer.concretize_architecture(self), + spack.concretizer.concretize_compiler(self), + spack.concretizer.concretize_version(self), + spack.concretizer.concretize_variants(self))) presets[self.name] = self visited.add(self.name) + return changed def _replace_with(self, concrete): @@ -783,20 +788,22 @@ class Spec(object): this are infrequent, but should implement this before it is a problem. """ + changed = False while True: virtuals =[v for v in self.traverse() if v.virtual] if not virtuals: - return + return changed for spec in virtuals: providers = spack.db.providers_for(spec) concrete = spack.concretizer.choose_provider(spec, providers) concrete = concrete.copy() spec._replace_with(concrete) + changed = True # If there are duplicate providers or duplicate provider deps, this # consolidates them and merge constraints. - self.normalize(force=True) + changed |= self.normalize(force=True) def concretize(self): @@ -814,9 +821,16 @@ class Spec(object): if self._concrete: return - self.normalize() - self._expand_virtual_packages() - self._concretize_helper() + changed = True + force = False + + while changed: + changes = (self.normalize(force=force), + self._expand_virtual_packages(), + self._concretize_helper()) + changed = any(changes) + force=True + self._concrete = True @@ -982,6 +996,7 @@ class Spec(object): # it from the package description. if dep.name not in spec_deps: spec_deps[dep.name] = dep.copy() + changed = True # Constrain package information with spec info try: @@ -998,7 +1013,6 @@ class Spec(object): dependency = spec_deps[dep.name] if dep.name not in self.dependencies: self._add_dependency(dependency) - changed = True changed |= dependency._normalize_helper(visited, spec_deps, provider_index) return changed @@ -1031,13 +1045,12 @@ class Spec(object): if pkg_dep: changed |= self._merge_dependency( pkg_dep, visited, spec_deps, provider_index) - any_change |= changed return any_change - def normalize(self, **kwargs): + def normalize(self, force=False): """When specs are parsed, any dependencies specified are hanging off the root, and ONLY the ones that were explicitly provided are there. Normalization turns a partial flat spec into a DAG, where: @@ -1050,13 +1063,13 @@ class Spec(object): package that provides a virtual package that is in the spec, then we replace the virtual package with the non-virtual one. - 4. The spec DAG matches package DAG. + 4. The spec DAG matches package DAG, including default variant values. TODO: normalize should probably implement some form of cycle detection, to ensure that the spec is actually a DAG. """ - if self._normal and not kwargs.get('force', False): - return + if self._normal and not force: + return False # Ensure first that all packages & compilers in the DAG exist. self.validate_names() @@ -1070,19 +1083,18 @@ class Spec(object): # traverse the package DAG and fill out dependencies according # to package files & their 'when' specs visited = set() - self._normalize_helper(visited, spec_deps, index) + any_change = self._normalize_helper(visited, spec_deps, index) # If there are deps specified but not visited, they're not # actually deps of this package. Raise an error. extra = set(spec_deps.keys()).difference(visited) - - # Anything left over is not a valid part of the spec. if extra: raise InvalidDependencyException( self.name + " does not depend on " + comma_or(extra)) # Mark the spec as normal once done. self._normal = True + return any_change def normalized(self): diff --git a/lib/spack/spack/test/optional_deps.py b/lib/spack/spack/test/optional_deps.py index 265a983f3f..fbee0cfa8f 100644 --- a/lib/spack/spack/test/optional_deps.py +++ b/lib/spack/spack/test/optional_deps.py @@ -86,9 +86,24 @@ class ConcretizeTest(MockPackagesTest): Spec('mpi')))) + def test_default_variant(self): + spec = Spec('optional-dep-test-3') + spec.concretize() + self.assertTrue('a' in spec) + + spec = Spec('optional-dep-test-3~var') + spec.concretize() + self.assertTrue('a' in spec) + + spec = Spec('optional-dep-test-3+var') + spec.concretize() + self.assertTrue('b' in spec) + + def test_transitive_chain(self): # Each of these dependencies comes from a conditional # dependency on another. This requires iterating to evaluate # the whole chain. - self.check_normalize('optional-dep-test+f', - Spec('optional-dep-test+f', Spec('f'), Spec('g'), Spec('mpi'))) + self.check_normalize( + 'optional-dep-test+f', + Spec('optional-dep-test+f', Spec('f'), Spec('g'), Spec('mpi'))) -- cgit v1.2.3-60-g2f50