pip is a pythonextension not a pythonpackage, and it turns out we werent doing our external surgery on things that inherited pythonextension (#38186)

2 files changed, 46 insertions, 47 deletions

diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index 8df7144999..3a49639fdc 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/lib/spack/spack/build_systems/python.py
@@ -180,51 +180,6 @@ class PythonExtension(spack.package_base.PackageBase):
                 work_dir="spack-test",
             )
 
-
-class PythonPackage(PythonExtension):
-    """Specialized class for packages that are built using pip."""
-
-    #: Package name, version, and extension on PyPI
-    pypi: Optional[str] = None
-
-    # To be used in UI queries that require to know which
-    # build-system class we are using
-    build_system_class = "PythonPackage"
-    #: Legacy buildsystem attribute used to deserialize and install old specs
-    legacy_buildsystem = "python_pip"
-
-    #: Callback names for install-time test
-    install_time_test_callbacks = ["test"]
-
-    build_system("python_pip")
-
-    with spack.multimethod.when("build_system=python_pip"):
-        extends("python")
-        depends_on("py-pip", type="build")
-        # FIXME: technically wheel is only needed when building from source, not when
-        # installing a downloaded wheel, but I don't want to add wheel as a dep to every
-        # package manually
-        depends_on("py-wheel", type="build")
-
-    py_namespace: Optional[str] = None
-
-    @lang.classproperty
-    def homepage(cls):
-        if cls.pypi:
-            name = cls.pypi.split("/")[0]
-            return "https://pypi.org/project/" + name + "/"
-
-    @lang.classproperty
-    def url(cls):
-        if cls.pypi:
-            return "https://files.pythonhosted.org/packages/source/" + cls.pypi[0] + "/" + cls.pypi
-
-    @lang.classproperty
-    def list_url(cls):
-        if cls.pypi:
-            name = cls.pypi.split("/")[0]
-            return "https://pypi.org/simple/" + name + "/"
-
     def update_external_dependencies(self, extendee_spec=None):
         """
         Ensure all external python packages have a python dependency
@@ -270,6 +225,51 @@ class PythonPackage(PythonExtension):
                     python._mark_concrete()
             self.spec.add_dependency_edge(python, deptypes=("build", "link", "run"))
 
+
+class PythonPackage(PythonExtension):
+    """Specialized class for packages that are built using pip."""
+
+    #: Package name, version, and extension on PyPI
+    pypi: Optional[str] = None
+
+    # To be used in UI queries that require to know which
+    # build-system class we are using
+    build_system_class = "PythonPackage"
+    #: Legacy buildsystem attribute used to deserialize and install old specs
+    legacy_buildsystem = "python_pip"
+
+    #: Callback names for install-time test
+    install_time_test_callbacks = ["test"]
+
+    build_system("python_pip")
+
+    with spack.multimethod.when("build_system=python_pip"):
+        extends("python")
+        depends_on("py-pip", type="build")
+        # FIXME: technically wheel is only needed when building from source, not when
+        # installing a downloaded wheel, but I don't want to add wheel as a dep to every
+        # package manually
+        depends_on("py-wheel", type="build")
+
+    py_namespace: Optional[str] = None
+
+    @lang.classproperty
+    def homepage(cls):
+        if cls.pypi:
+            name = cls.pypi.split("/")[0]
+            return "https://pypi.org/project/" + name + "/"
+
+    @lang.classproperty
+    def url(cls):
+        if cls.pypi:
+            return "https://files.pythonhosted.org/packages/source/" + cls.pypi[0] + "/" + cls.pypi
+
+    @lang.classproperty
+    def list_url(cls):
+        if cls.pypi:
+            name = cls.pypi.split("/")[0]
+            return "https://pypi.org/simple/" + name + "/"
+
     def get_external_python_for_prefix(self):
         """
         For an external package that extends python, find the most likely spec for the python
diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index 3488b2624e..adc1c8c490 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -1231,6 +1231,7 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
             if any(dt in cls.dependencies[name][cond].type for cond in conds for dt in deptypes)
         )
 
+    # TODO: allow more than one active extendee.
     @property
     def extendee_spec(self):
         """
@@ -1246,7 +1247,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
             if dep.name in self.extendees:
                 deps.append(dep)
 
-        # TODO: allow more than one active extendee.
         if deps:
             assert len(deps) == 1
             return deps[0]
@@ -1256,7 +1256,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
         if self.spec._concrete:
             return None
         else:
-            # TODO: do something sane here with more than one extendee
             # If it's not concrete, then return the spec from the
             # extends() directive since that is all we know so far.
             spec_str, kwargs = next(iter(self.extendees.items()))