summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorTodd Gamblin <gamblin2@llnl.gov>2022-12-12 02:24:28 -0800
committerGitHub <noreply@github.com>2022-12-12 11:24:28 +0100
commit62da76cb5dca4d52c43bee06230cca6a5882f05d (patch)
treeb4165c8019df683026f43249320cf3c733482527 /lib
parent65c914fff72861a44bdac20bc1f62fd5ff0879cc (diff)
downloadspack-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.py8
-rw-r--r--lib/spack/spack/test/directives.py8
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")