From 62da76cb5dca4d52c43bee06230cca6a5882f05d Mon Sep 17 00:00:00 2001
From: Todd Gamblin <gamblin2@llnl.gov>
Date: Mon, 12 Dec 2022 02:24:28 -0800
Subject: 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
```
---
 lib/spack/spack/directives.py      | 8 +++++++-
 lib/spack/spack/test/directives.py | 8 ++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

(limited to 'lib')

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")
-- 
cgit v1.2.3-70-g09d2