From 90d50a0cee912fc1da43f617a166d0104c41939f Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 12 Sep 2017 21:10:31 +0200 Subject: Force reference consistency between Spec & Package The correct place to set the mutual references between spec and package objects is at the end of concretization. After a call to concretize we should now be ensured that spec is the same object as spec.package.spec. Code in `build_environment.py` that was performing the same operation has been turned into an assertion to be defensive on the new behavior. --- lib/spack/spack/build_environment.py | 6 +----- lib/spack/spack/spec.py | 30 ++++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index a27d8c68ab..66b7da9e55 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -465,12 +465,8 @@ def setup_package(pkg, dirty): # code ensures that all packages in the DAG have pieces of the # same spec object at build time. # - # This is safe for the build process, b/c the build process is a - # throwaway environment, but it is kind of dirty. - # - # TODO: Think about how to avoid this fix and do something cleaner. for s in pkg.spec.traverse(): - s.package.spec = s + assert s.package.spec is s # Trap spack-tracked compiler flags as appropriate. # Must be before set_compiler_environment_variables diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 1b5f41d4c4..5c082d90c8 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1782,15 +1782,24 @@ class Spec(object): def concretize(self): """A spec is concrete if it describes one build of a package uniquely. - This will ensure that this spec is concrete. + This will ensure that this spec is concrete. - If this spec could describe more than one version, variant, or build - of a package, this will add constraints to make it concrete. + If this spec could describe more than one version, variant, or build + of a package, this will add constraints to make it concrete. - Some rigorous validation and checks are also performed on the spec. - Concretizing ensures that it is self-consistent and that it's - consistent with requirements of its pacakges. See flatten() and - normalize() for more details on this. + Some rigorous validation and checks are also performed on the spec. + Concretizing ensures that it is self-consistent and that it's + consistent with requirements of its pacakges. See flatten() and + normalize() for more details on this. + + It also ensures that: + + .. code-block:: python + + for x in self.traverse(): + assert x.package.spec == x + + which may not be true *during* the concretization step. """ if not self.name: raise SpecError("Attempting to concretize anonymous spec") @@ -1844,6 +1853,11 @@ class Spec(object): if matches: raise ConflictsInSpecError(self, matches) + # At this point the spec-package mutual references should + # be self-consistent + for x in self.traverse(): + x.package.spec = x + def _mark_concrete(self, value=True): """Mark this spec and its dependencies as concrete. @@ -2463,7 +2477,7 @@ class Spec(object): def _dup(self, other, deps=True, cleardeps=True, caches=None): """Copy the spec other into self. This is an overwriting - copy. It does not copy any dependents (parents), but by default + copy. It does not copy any dependents (parents), but by default copies dependencies. To duplicate an entire DAG, call _dup() on the root of the DAG. -- cgit v1.2.3-70-g09d2