ensure external PythonPackages have python deps (#33777)

Currently, external `PythonPackage`s cause install failures because the logic in `PythonPackage` assumes that it can ask for `spec["python"]`.  Because we chop off externals' dependencies, an external Python extension may not have a `python` dependency.

This PR resolves the issue by guaranteeing that a `python` node is present in one of two ways:
1. If there is already a `python` node in the DAG, we wire the external up to it.
2. If there is no existing `python` node, we wire up a synthetic external `python` node, and we assume that it has the same prefix as the external.

The assumption in (2) isn't always valid, but it's better than leaving the user with a non-working `PythonPackage`.

The logic here is specific to `python`, but other types of extensions could take advantage of it.  Packages need only define `update_external_dependencies(self)`, and this method will be called on externals after concretization.  This likely needs to be fleshed out in the future so that any added nodes are included in concretization, but for now we only bolt on dependencies post-concretization.

Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>

5 files changed, 54 insertions, 0 deletions

diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index 0f84ac7f8a..afa36980f7 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/lib/spack/spack/build_systems/python.py
@@ -15,6 +15,7 @@ import llnl.util.tty as tty
 import spack.builder
 import spack.multimethod
 import spack.package_base
+import spack.spec
 from spack.directives import build_system, depends_on, extends
 from spack.error import NoHeadersError, NoLibrariesError, SpecError
 from spack.version import Version
@@ -218,6 +219,27 @@ class PythonPackage(PythonExtension):
             name = cls.pypi.split("/")[0]
             return "https://pypi.org/simple/" + name + "/"
 
+    def update_external_dependencies(self):
+        """
+        Ensure all external python packages have a python dependency
+
+        If another package in the DAG depends on python, we use that
+        python for the dependency of the external. If not, we assume
+        that the external PythonPackage is installed into the same
+        directory as the python it depends on.
+        """
+        # TODO: Include this in the solve, rather than instantiating post-concretization
+        if "python" not in self.spec:
+            if "python" in self.spec.root:
+                python = self.spec.root["python"]
+            else:
+                python = spack.spec.Spec("python")
+                repo = spack.repo.path.repo_for_pkg(python)
+                python.namespace = repo.namespace
+                python._mark_concrete()
+                python.external_path = self.prefix
+            self.spec.add_dependency_edge(python, ("build", "link", "run"))
+
     @property
     def headers(self):
         """Discover header files in platlib."""
diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index e0e250cf7c..48b036e8cf 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -919,6 +919,12 @@ class PackageBase(six.with_metaclass(PackageMeta, WindowsRPathMeta, PackageViewM
         """
         return self._implement_all_urls_for_version(version)[0]
 
+    def update_external_dependencies(self):
+        """
+        Method to override in package classes to handle external dependencies
+        """
+        pass
+
     def all_urls_for_version(self, version):
         """Return all URLs derived from version_urls(), url, urls, and
         list_url (if it contains a version) in a package in that order.
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 68c571e5bb..5f387636cc 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -2320,6 +2320,12 @@ class SpecBuilder(object):
                 if isinstance(spec.version, spack.version.GitVersion):
                     spec.version.generate_git_lookup(spec.fullname)
 
+        # Add synthetic edges for externals that are extensions
+        for root in self._specs.values():
+            for dep in root.traverse():
+                if dep.external:
+                    dep.package.update_external_dependencies()
+
         return self._specs
 
 
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index ef4c3b7ab0..96b137622a 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -2751,6 +2751,11 @@ class Spec(object):
         # If any spec in the DAG is deprecated, throw an error
         Spec.ensure_no_deprecated(self)
 
+        # Update externals as needed
+        for dep in self.traverse():
+            if dep.external:
+                dep.package.update_external_dependencies()
+
         # Now that the spec is concrete we should check if
         # there are declared conflicts
         #
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index ab3337da5e..f2d7edf126 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -1945,3 +1945,18 @@ class TestConcretize(object):
 
         for s in spec.traverse():
             assert s.satisfies("target=%s" % spack.platforms.test.Test.front_end)
+
+    def test_external_python_extensions_have_dependency(self):
+        """Test that python extensions have access to a python dependency"""
+        external_conf = {
+            "py-extension1": {
+                "buildable": False,
+                "externals": [{"spec": "py-extension1@2.0", "prefix": "/fake"}],
+            }
+        }
+        spack.config.set("packages", external_conf)
+
+        spec = Spec("py-extension2").concretized()
+
+        assert "python" in spec["py-extension1"]
+        assert spec["python"] == spec["py-extension1"]["python"]