python: simplify `libs()` and `headers()` methods in `python/package.py`

- [x] Rework `headers` to search a sequence of directories and to display all searched
      locations on error, as opposed to handling each directory with a variable

- [x] Make `headers` and `libs` do the same type of search and raise the same sort of
      errors.

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

1 files changed, 46 insertions, 28 deletions

diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index a2bbf3865d..be353ab7df 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -1044,6 +1044,12 @@ config.update(get_paths())
         win_bin_dir = self.config_vars["BINDIR"]
 
         directories = [libdir, libpl, frameworkprefix, macos_developerdir, win_bin_dir]
+
+        # The Python shipped with Xcode command line tools isn't in any of these locations
+        for subdir in ["lib", "lib64"]:
+            directories.append(os.path.join(self.config_vars["base"], subdir))
+
+        directories = dedupe(directories)
         for directory in directories:
             path = os.path.join(directory, library)
             if os.path.exists(path):
@@ -1051,52 +1057,64 @@ config.update(get_paths())
 
     @property
     def libs(self):
-        # The +shared variant isn't always reliable, as `spack external find`
-        # currently can't detect it. If +shared, prefer the shared libraries, but check
-        # for static if those aren't found. Vice versa for ~shared.
+        py_version = self.version.up_to(2)
 
-        # The values of LDLIBRARY and LIBRARY also aren't reliable. Intel Python uses a
+        # The values of LDLIBRARY and LIBRARY aren't reliable. Intel Python uses a
         # static binary but installs shared libraries, so sysconfig reports
-        # libpythonX.Y.a but only libpythonX.Y.so exists.
+        # libpythonX.Y.a but only libpythonX.Y.so exists. So we add our own paths, too.
         shared_libs = [
             self.config_vars["LDLIBRARY"],
-            "libpython{}.{}".format(self.version.up_to(2), dso_suffix),
+            "libpython{}.{}".format(py_version, dso_suffix),
         ]
         static_libs = [
             self.config_vars["LIBRARY"],
-            "libpython{}.a".format(self.version.up_to(2)),
+            "libpython{}.a".format(py_version),
         ]
+
+        # The +shared variant isn't reliable, as `spack external find` currently can't
+        # detect it. If +shared, prefer the shared libraries, but check for static if
+        # those aren't found. Vice versa for ~shared.
         if "+shared" in self.spec:
-            libraries = shared_libs + static_libs
+            candidates = shared_libs + static_libs
         else:
-            libraries = static_libs + shared_libs
-        libraries = dedupe(libraries)
+            candidates = static_libs + shared_libs
+        candidates = dedupe(candidates)
 
-        for library in libraries:
-            lib = self.find_library(library)
+        for candidate in candidates:
+            lib = self.find_library(candidate)
             if lib:
                 return lib
 
-        msg = "Unable to locate {} libraries in {}"
-        libdir = self.config_vars["LIBDIR"]
-        raise spack.error.NoLibrariesError(msg.format(self.name, libdir))
+        raise spack.error.NoLibrariesError(
+            "Unable to find {} libraries with the following names:".format(self.name),
+            "\n".join(candidates),
+        )
 
     @property
     def headers(self):
-        # Location where pyconfig.h is _supposed_ to be
-        config_h = self.config_vars["config_h_filename"]
-        if os.path.exists(config_h):
-            headers = HeaderList(config_h)
+        # Locations where pyconfig.h could be
+        # This varies by system, especially on macOS where the command line tools are
+        # installed in a very different directory from the system python interpreter.
+        py_version = str(self.version.up_to(2))
+        candidates = [
+            os.path.dirname(self.config_vars["config_h_filename"]),
+            self.config_vars["INCLUDEPY"],
+            self.config_vars["CONFINCLUDEPY"],
+            os.path.join(self.config_vars["base"], "include", py_version),
+            os.path.join(self.config_vars["base"], "Headers"),
+        ]
+        candidates = list(dedupe(candidates))
+
+        for directory in candidates:
+            headers = find_headers("pyconfig", directory)
+            if headers:
+                config_h = headers[0]
+                break
         else:
-            # If not, one of these config vars should contain the right directory
-            for var in ["INCLUDEPY", "CONFINCLUDEPY"]:
-                headers = find_headers("pyconfig", self.config_vars[var])
-                if headers:
-                    config_h = headers[0]
-                    break
-            else:
-                msg = "Unable to locate {} headers in {}"
-                raise spack.error.NoHeadersError(msg.format(self.name, directory))
+            raise spack.error.NoHeadersError(
+                "Unable to locate {} headers in any of these locations:".format(self.name),
+                "\n".join(candidates),
+            )
 
         headers.directories = [os.path.dirname(config_h)]
         return headers