diff options
author | Todd Gamblin <gamblin2@llnl.gov> | 2022-12-12 02:24:28 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-12 11:24:28 +0100 |
commit | 62da76cb5dca4d52c43bee06230cca6a5882f05d (patch) | |
tree | b4165c8019df683026f43249320cf3c733482527 /lib | |
parent | 65c914fff72861a44bdac20bc1f62fd5ff0879cc (diff) | |
download | spack-62da76cb5dca4d52c43bee06230cca6a5882f05d.tar.gz spack-62da76cb5dca4d52c43bee06230cca6a5882f05d.tar.bz2 spack-62da76cb5dca4d52c43bee06230cca6a5882f05d.tar.xz spack-62da76cb5dca4d52c43bee06230cca6a5882f05d.zip |
directives: depends_on should not admit anonymous specs (#34368)
Writing a long dependency like:
```python
depends_on(
"llvm"
"targets=amdgpu,bpf,nvptx,webassembly"
"version_suffix=jl +link_llvm_dylib ~internal_unwind"
)
```
when it should be formatted like this:
```python
depends_on(
"llvm"
" targets=amdgpu,bpf,nvptx,webassembly"
" version_suffix=jl +link_llvm_dylib ~internal_unwind"
)
```
can cause really subtle errors. Specifically, you'll get something like this in
the package sanity tests:
```
AttributeError: 'NoneType' object has no attribute 'rpartition'
```
because Spack happily constructs a class that has a dependency with name `None`.
We can catch this earlier by banning anonymous dependency specs directly in
`depends_on()`. This causes the package itself to fail to parse, and emits
a much better error message:
```
==> Error: Invalid dependency specification in package 'julia':
llvmtargets=amdgpu,bpf,nvptx,webassemblyversion_suffix=jl +link_llvm_dylib ~internal_unwind
```
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/directives.py | 8 | ||||
-rw-r--r-- | lib/spack/spack/test/directives.py | 8 |
2 files changed, 15 insertions, 1 deletions
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index b2058737e0..6e9dd819bb 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -361,6 +361,8 @@ def _depends_on(pkg, spec, when=None, type=default_deptype, patches=None): return dep_spec = spack.spec.Spec(spec) + if not dep_spec.name: + raise DependencyError("Invalid dependency specification in package '%s':" % pkg.name, spec) if pkg.name == dep_spec.name: raise CircularReferenceError("Package '%s' cannot depend on itself." % pkg.name) @@ -769,7 +771,11 @@ class DirectiveError(spack.error.SpackError): """This is raised when something is wrong with a package directive.""" -class CircularReferenceError(DirectiveError): +class DependencyError(DirectiveError): + """This is raised when a dependency specification is invalid.""" + + +class CircularReferenceError(DependencyError): """This is raised when something depends on itself.""" diff --git a/lib/spack/spack/test/directives.py b/lib/spack/spack/test/directives.py index 616d7ef5ee..d1fc31d09b 100644 --- a/lib/spack/spack/test/directives.py +++ b/lib/spack/spack/test/directives.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import pytest +import spack.directives import spack.repo import spack.spec @@ -60,3 +61,10 @@ def test_extends_spec(config, mock_packages): assert extender.dependencies assert extender.package.extends(extendee) + + +@pytest.mark.regression("34368") +def test_error_on_anonymous_dependency(config, mock_packages): + pkg = spack.repo.path.get_pkg_class("a") + with pytest.raises(spack.directives.DependencyError): + spack.directives._depends_on(pkg, "@4.5") |