diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index c7d9a748372..b7aea1d5ef6 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -667,15 +667,22 @@ class DependencySpec: virtuals: virtual packages provided from child to parent node. """ - __slots__ = "parent", "spec", "depflag", "virtuals" + __slots__ = "parent", "spec", "depflag", "virtuals", "direct" def __init__( - self, parent: "Spec", spec: "Spec", *, depflag: dt.DepFlag, virtuals: Tuple[str, ...] + self, + parent: "Spec", + spec: "Spec", + *, + depflag: dt.DepFlag, + virtuals: Tuple[str, ...], + direct: bool = False, ): self.parent = parent self.spec = spec self.depflag = depflag self.virtuals = tuple(sorted(set(virtuals))) + self.direct = direct def update_deptypes(self, depflag: dt.DepFlag) -> bool: """Update the current dependency types""" @@ -1670,10 +1677,18 @@ def _set_architecture(self, **kwargs): else: setattr(self.architecture, new_attr, new_value) - def _add_dependency(self, spec: "Spec", *, depflag: dt.DepFlag, virtuals: Tuple[str, ...]): - """Called by the parser to add another spec as a dependency.""" + def _add_dependency( + self, spec: "Spec", *, depflag: dt.DepFlag, virtuals: Tuple[str, ...], direct: bool = False + ): + """Called by the parser to add another spec as a dependency. + + Args: + depflag: dependency type for this edge + virtuals: virtuals on this edge + direct: if True denotes a direct dependency (associated with the % sigil) + """ if spec.name not in self._dependencies or not spec.name: - self.add_dependency_edge(spec, depflag=depflag, virtuals=virtuals) + self.add_dependency_edge(spec, depflag=depflag, virtuals=virtuals, direct=direct) return # Keep the intersection of constraints when a dependency is added multiple times with @@ -1708,7 +1723,12 @@ def _add_dependency(self, spec: "Spec", *, depflag: dt.DepFlag, virtuals: Tuple[ ) def add_dependency_edge( - self, dependency_spec: "Spec", *, depflag: dt.DepFlag, virtuals: Tuple[str, ...] + self, + dependency_spec: "Spec", + *, + depflag: dt.DepFlag, + virtuals: Tuple[str, ...], + direct: bool = False, ): """Add a dependency edge to this spec. @@ -1716,6 +1736,7 @@ def add_dependency_edge( dependency_spec: spec of the dependency deptypes: dependency types for this edge virtuals: virtuals provided by this edge + direct: if True denotes a direct dependency """ # Check if we need to update edges that are already present selected = self._dependencies.select(child=dependency_spec.name) @@ -1754,7 +1775,9 @@ def add_dependency_edge( edge.update_virtuals(virtuals=virtuals) return - edge = DependencySpec(self, dependency_spec, depflag=depflag, virtuals=virtuals) + edge = DependencySpec( + self, dependency_spec, depflag=depflag, virtuals=virtuals, direct=direct + ) self._dependencies.add(edge) dependency_spec._dependents.add(edge) @@ -1981,15 +2004,28 @@ def traverse_edges( def long_spec(self): """Returns a string of the spec with the dependencies completely enumerated.""" - root_str = [self.format()] - sorted_dependencies = sorted( - self.traverse(root=False), key=lambda x: (x.name, x.abstract_hash) + name_conversion = { + "llvm": "clang", + "intel-oneapi-compilers": "oneapi", + "llvm-amdgpu": "rocmcc", + "intel-oneapi-compiler-classic": "intel", + "acfl": "arm", + } + parts = [self.format()] + direct, transitive = lang.stable_partition( + self.edges_to_dependencies(), predicate_fn=lambda x: x.direct ) - sorted_dependencies = [ - d.format("{edge_attributes} " + DEFAULT_FORMAT) for d in sorted_dependencies - ] - spec_str = " ^".join(root_str + sorted_dependencies) - return spec_str.strip() + for item in sorted(direct, key=lambda x: x.spec.name): + current_name = item.spec.name + new_name = name_conversion.get(current_name, current_name) + parts.append(f"%{item.spec.format()}".replace(current_name, new_name)) + for item in sorted(transitive, key=lambda x: x.spec.name): + # Recurse to attach build deps in order + edge_attributes = "" + if item.virtuals or item.depflag: + edge_attributes = item.spec.format("{edge_attributes}") + " " + parts.append(f"^{edge_attributes}{str(item.spec)}") + return " ".join(parts).strip() @property def short_spec(self): @@ -2227,9 +2263,6 @@ def to_node_dict(self, hash=ht.dag_hash): if self.architecture: d.update(self.architecture.to_dict()) - if self.compiler: - d.update(self.compiler.to_dict()) - if self.namespace: d["namespace"] = self.namespace @@ -2466,8 +2499,6 @@ def override(init_spec, change_spec): else: raise ValueError("{0} is not a variant of {1}".format(vname, new_spec.name)) - if change_spec.compiler: - new_spec.compiler = change_spec.compiler if change_spec.compiler_flags: for flagname, flagvals in change_spec.compiler_flags.items(): new_spec.compiler_flags[flagname] = flagvals @@ -3002,12 +3033,6 @@ def constrain(self, other, deps=True): self.namespace = other.namespace changed = True - if self.compiler is not None and other.compiler is not None: - changed |= self.compiler.constrain(other.compiler) - elif self.compiler is None and other.compiler is not None: - changed |= self.compiler != other.compiler - self.compiler = other.compiler - changed |= self.versions.intersect(other.versions) changed |= self.variants.constrain(other.variants) @@ -3028,10 +3053,8 @@ def constrain(self, other, deps=True): return changed - def _constrain_dependencies(self, other): + def _constrain_dependencies(self, other: "Spec") -> bool: """Apply constraints of other spec's dependencies to this spec.""" - other = self._autospec(other) - if not other._dependencies: return False @@ -3046,8 +3069,10 @@ def _constrain_dependencies(self, other): # Handle common first-order constraints directly changed = False - for name in self.common_dependencies(other): - changed |= self[name].constrain(other[name], deps=False) + common_dependencies = {x.name for x in self.dependencies()} + common_dependencies &= {x.name for x in other.dependencies()} + for name in common_dependencies: + changed |= self[name].constrain(other[name], deps=True) if name in self._dependencies: # WARNING: This function is an implementation detail of the # WARNING: original concretizer. Since with that greedy @@ -3076,7 +3101,7 @@ def _constrain_dependencies(self, other): return changed def common_dependencies(self, other): - """Return names of dependencies that self an other have in common.""" + """Return names of dependencies that self and other have in common.""" common = set(s.name for s in self.traverse(root=False)) common.intersection_update(s.name for s in other.traverse(root=False)) return common @@ -3178,10 +3203,6 @@ def intersects(self, other: Union[str, "Spec"], deps: bool = True) -> bool: if not self.versions.intersects(other.versions): return False - if self.compiler and other.compiler: - if not self.compiler.intersects(other.compiler): - return False - if not self.variants.intersects(other.variants): return False @@ -3204,8 +3225,10 @@ def _intersects_dependencies(self, other): return True # Handle first-order constraints directly - for name in self.common_dependencies(other): - if not self[name].intersects(other[name], deps=False): + common_dependencies = {x.name for x in self.dependencies()} + common_dependencies &= {x.name for x in other.dependencies()} + for name in common_dependencies: + if not self[name].intersects(other[name], deps=True): return False # For virtual dependencies, we need to dig a little deeper. @@ -3292,12 +3315,6 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool: if not self.versions.satisfies(other.versions): return False - if self.compiler and other.compiler: - if not self.compiler.satisfies(other.compiler): - return False - elif other.compiler and not self.compiler: - return False - if not self.variants.satisfies(other.variants): return False @@ -3425,7 +3442,6 @@ def _dup(self, other: "Spec", deps: Union[bool, dt.DepTypes, dt.DepFlag] = True) self.name != other.name and self.versions != other.versions and self.architecture != other.architecture - and self.compiler != other.compiler and self.variants != other.variants and self.concrete != other.concrete and self.external_path != other.external_path @@ -3440,8 +3456,6 @@ def _dup(self, other: "Spec", deps: Union[bool, dt.DepTypes, dt.DepFlag] = True) self.name = other.name self.versions = other.versions.copy() self.architecture = other.architecture.copy() if other.architecture else None - # FIXME (compiler as nodes): removing compiler attribute - # self.compiler = other.compiler.copy() if other.compiler else None self.compiler_flags = other.compiler_flags.copy() self.compiler_flags.spec = self self.variants = other.variants.copy() @@ -3652,7 +3666,6 @@ def _cmp_node(self): yield self.namespace yield self.versions yield self.variants - yield self.compiler yield self.compiler_flags yield self.architecture yield self.abstract_hash @@ -3713,9 +3726,6 @@ def format(self, format_string: str = DEFAULT_FORMAT, color: Optional[bool] = Fa name version - compiler - compiler.name - compiler.version compiler_flags variants architecture diff --git a/lib/spack/spack/spec_parser.py b/lib/spack/spack/spec_parser.py index d3e429ee305..7c2e4629c35 100644 --- a/lib/spack/spack/spec_parser.py +++ b/lib/spack/spack/spec_parser.py @@ -347,7 +347,7 @@ def add_flag(name: str, value: str, propagate: bool): build_dependency.name = name_conversion[build_dependency.name] initial_spec._add_dependency( - build_dependency, depflag=spack.deptypes.BUILD, virtuals=() + build_dependency, depflag=spack.deptypes.BUILD, virtuals=(), direct=True ) elif ( diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index cab91b5a428..73294bc6ed8 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -909,7 +909,9 @@ def test_simultaneous_concretization_of_specs(self, abstract_specs): concrete_specs = spack.concretize._concretize_specs_together(abstract_specs) # Check there's only one configuration of each package in the DAG - names = set(dep.name for spec in concrete_specs for dep in spec.traverse()) + names = set( + dep.name for spec in concrete_specs for dep in spec.traverse(deptype=("link", "run")) + ) for name in names: name_specs = set(spec[name] for spec in concrete_specs if name in spec) assert len(name_specs) == 1 diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 4f58be0b2c3..98071a91d60 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -13,6 +13,7 @@ import spack.concretize import spack.platforms.test import spack.repo +import spack.solver.asp import spack.spec from spack.spec_parser import ( UNIX_FILENAME, @@ -165,7 +166,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.COMPILER_AND_VERSION, value="%bar@1.0"), Token(SpecTokens.VERSION, value="@2.0"), ], - "foo@2.0%bar@1.0", + "foo@2.0 %bar@1.0", ), # Single dependency with version dependency_with_version("openmpi ^hwloc@1.2e6"), @@ -187,7 +188,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, value="stackwalker"), Token(SpecTokens.VERSION, value="@8.1_1e"), ], - "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1+debug~qt_4 ^stackwalker@8.1_1e", + "mvapich_foo ^_openmpi@1.2:1.4,1.6+debug~qt_4 %intel@12.1 ^stackwalker@8.1_1e", ), ( "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1~qt_4 debug=2 ^stackwalker@8.1_1e", @@ -203,7 +204,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, value="stackwalker"), Token(SpecTokens.VERSION, value="@8.1_1e"), ], - "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1~qt_4 debug=2 ^stackwalker@8.1_1e", + "mvapich_foo ^_openmpi@1.2:1.4,1.6~qt_4 debug=2 %intel@12.1 ^stackwalker@8.1_1e", ), ( "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags=-O3 +debug~qt_4 " @@ -221,8 +222,8 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, value="stackwalker"), Token(SpecTokens.VERSION, value="@8.1_1e"), ], - "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags=-O3 +debug~qt_4 " - "^stackwalker@8.1_1e", + "mvapich_foo ^_openmpi@1.2:1.4,1.6 cppflags=-O3 +debug~qt_4 %intel@12.1" + " ^stackwalker@8.1_1e", ), # Specs containing YAML or JSON in the package name ( @@ -235,7 +236,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, value="boost"), Token(SpecTokens.VERSION, value="@3.1.4"), ], - "yaml-cpp@0.1.8%intel@12.1 ^boost@3.1.4", + "yaml-cpp@0.1.8 %intel@12.1 ^boost@3.1.4", ), ( r"builtin.yaml-cpp%gcc", @@ -243,7 +244,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.FULLY_QUALIFIED_PACKAGE_NAME, value="builtin.yaml-cpp"), Token(SpecTokens.COMPILER, value="%gcc"), ], - "yaml-cpp%gcc", + "yaml-cpp %gcc", ), ( r"testrepo.yaml-cpp%gcc", @@ -251,7 +252,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.FULLY_QUALIFIED_PACKAGE_NAME, value="testrepo.yaml-cpp"), Token(SpecTokens.COMPILER, value="%gcc"), ], - "yaml-cpp%gcc", + "yaml-cpp %gcc", ), ( r"builtin.yaml-cpp@0.1.8%gcc@7.2.0 ^boost@3.1.4", @@ -263,7 +264,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.UNQUALIFIED_PACKAGE_NAME, value="boost"), Token(SpecTokens.VERSION, value="@3.1.4"), ], - "yaml-cpp@0.1.8%gcc@7.2.0 ^boost@3.1.4", + "yaml-cpp@0.1.8 %gcc@7.2.0 ^boost@3.1.4", ), ( r"builtin.yaml-cpp ^testrepo.boost ^zlib", @@ -490,7 +491,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.COMPILER_AND_VERSION, value="% intel @ 12.1:12.6"), Token(SpecTokens.BOOL_VARIANT, value="+ debug"), ], - "%intel@12.1:12.6+debug", + "+debug %intel@12.1:12.6", ), ( "@ 12.1:12.6 + debug - qt_4", @@ -515,7 +516,7 @@ def _specfile_for(spec_str, filename): Token(SpecTokens.VERSION, value="@:0.4"), Token(SpecTokens.COMPILER, value="% nvhpc"), ], - "@:0.4%nvhpc", + "@:0.4 %nvhpc", ), ( "^[virtuals=mpi] openmpi", @@ -823,7 +824,6 @@ def test_dep_spec_by_hash(database, config): mpileaks_hash_zmpi.replace_hash() assert "zmpi" in mpileaks_hash_zmpi assert mpileaks_hash_zmpi["zmpi"] == zmpi - assert mpileaks_zmpi.compiler.satisfies(mpileaks_hash_zmpi.compiler) mpileaks_hash_fake_and_zmpi = SpecParser( f"mpileaks ^/{fake.dag_hash()[:4]} ^ /{zmpi.dag_hash()[:5]}" @@ -986,13 +986,6 @@ def test_disambiguate_hash_by_spec(spec1, spec2, constraint, mock_packages, monk ("x@1.2%y@1.2@2.3:2.4", "version"), # Duplicate dependency ("x ^y@1 ^y@2", "Cannot depend on incompatible specs"), - # Duplicate compiler - ("x%intel%intel", "compiler"), - ("x%intel%gcc", "compiler"), - ("x%gcc%intel", "compiler"), - ("x ^y%intel%intel", "compiler"), - ("x ^y%intel%gcc", "compiler"), - ("x ^y%gcc%intel", "compiler"), # Duplicate Architectures ("x arch=linux-rhel7-x86_64 arch=linux-rhel7-x86_64", "two architectures"), ("x arch=linux-rhel7-x86_64 arch=linux-rhel7-ppc64le", "two architectures"), @@ -1147,7 +1140,7 @@ def test_parse_filename_missing_slash_as_spec(specfile_for, tmpdir, filename): ) # make sure that only happens when the spec ends in yaml - with pytest.raises(spack.repo.UnknownPackageError) as exc_info: + with pytest.raises(spack.solver.asp.UnsatisfiableSpecError) as exc_info: spack.concretize.concretize_one(SpecParser("builtin.mock.doesnotexist").next_spec()) assert not exc_info.value.long_message or ( "Did you mean to specify a filename with" not in exc_info.value.long_message