Python extensions: consistently set LDSHARED to get Spack RPATHs (#21149)

Python extensions use CC and LDSHARED from the sysconfig module to
build. When Spack installs Python, it replaces the Spack compiler
wrappers in these values with the underlying compilers (since these
wrappers are not useful outside of the context of running Spack).
In order to use the Spack compiler wrappers when building Python
extensions with Spack, Spack sets the LDSHARED environment variable
when running `Python.setup_py` (which overrides sysconfig). However,
many Python extensions use an alternative method to build (namely
PythonPackage.setup_py), which meant that LDSHARED was not set (and
RPATHs were not inserted for dependencies).

This commit makes the following changes:

* Sets LDSHARED in the environment: this applies to all commands
  executed during the build, rather than for a single command
  invocation
* Updates the logic to set LDSHARED: this replaces the compiler
  executable in LDSHARED with the Spack compiler wrapper. This
  means that for some externally-built instances of Python,
  Spack will now switch to using the Spack wrappers when building
  extensions. The behavior is expected to be the same for Spack-
  built instances of Python.
* Performs similar modifications for LDCXXSHARED (to ensure RPATHs
  are included for C++ codes)

1 files changed, 56 insertions, 99 deletions

diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 4ae9092d5a..e0ac0efd7a 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -3,18 +3,14 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import ast
 import os
 import platform
 import re
 
 import llnl.util.tty as tty
 from llnl.util.lang import match_predicate
-from llnl.util.filesystem import (force_remove, get_filetype,
-                                  path_contains_subdirectory)
+from llnl.util.filesystem import get_filetype, path_contains_subdirectory
 
-import spack.store
-import spack.util.spack_json as sjson
 from spack.util.environment import is_system_path
 from spack.util.prefix import Prefix
 from spack import *
@@ -221,10 +217,6 @@ class Python(AutotoolsPackage):
 
     conflicts('%nvhpc')
 
-    _DISTUTIL_VARS_TO_SAVE = ['LDSHARED']
-    _DISTUTIL_CACHE_FILENAME = 'sysconfig.json'
-    _distutil_vars = None
-
     # Used to cache home locations, since computing them might be expensive
     _homes = {}
 
@@ -482,90 +474,6 @@ class Python(AutotoolsPackage):
         return config_args
 
     @run_after('install')
-    def _save_distutil_vars(self):
-        """
-        Run before changing automatically generated contents of the
-        _sysconfigdata.py, which is used by distutils to figure out what
-        executables to use while compiling and linking extensions. If we build
-        extensions with spack those executables should be spack's wrappers.
-        Spack partially covers this by setting environment variables that
-        are also accounted for by distutils. Currently there is one more known
-        variable that must be set, which is LDSHARED, so the method saves its
-        autogenerated value to pass it to the dependent package's setup script.
-        """
-
-        self._distutil_vars = {}
-
-        input_filename = self.get_sysconfigdata_name()
-        input_dict = None
-        try:
-            with open(input_filename) as input_file:
-                match = re.search(r'build_time_vars\s*=\s*(?P<dict>{.*})',
-                                  input_file.read(),
-                                  flags=re.DOTALL)
-
-                if match:
-                    input_dict = ast.literal_eval(match.group('dict'))
-        except (IOError, SyntaxError):
-            pass
-
-        if not input_dict:
-            tty.warn("Failed to find 'build_time_vars' dictionary in file "
-                     "'%s'. This might cause the extensions that are "
-                     "installed with distutils to call compilers directly "
-                     "avoiding Spack's wrappers." % input_filename)
-            return
-
-        for var_name in Python._DISTUTIL_VARS_TO_SAVE:
-            if var_name in input_dict:
-                self._distutil_vars[var_name] = input_dict[var_name]
-            else:
-                tty.warn("Failed to find key '%s' in 'build_time_vars' "
-                         "dictionary in file '%s'. This might cause the "
-                         "extensions that are installed with distutils to "
-                         "call compilers directly avoiding Spack's wrappers."
-                         % (var_name, input_filename))
-
-        if len(self._distutil_vars) > 0:
-            output_filename = None
-            try:
-                output_filename = join_path(
-                    spack.store.layout.metadata_path(self.spec),
-                    Python._DISTUTIL_CACHE_FILENAME)
-                with open(output_filename, 'w') as output_file:
-                    sjson.dump(self._distutil_vars, output_file)
-            except Exception:
-                tty.warn("Failed to save metadata for distutils. This might "
-                         "cause the extensions that are installed with "
-                         "distutils to call compilers directly avoiding "
-                         "Spack's wrappers.")
-                # We make the cache empty if we failed to save it to file
-                # to provide the same behaviour as in the case when the cache
-                # is initialized by the method load_distutils_data().
-                self._distutil_vars = {}
-                if output_filename:
-                    force_remove(output_filename)
-
-    def _load_distutil_vars(self):
-        # We update and keep the cache unchanged only if the package is
-        # installed.
-        if not self._distutil_vars and self.installed:
-            try:
-                input_filename = join_path(
-                    spack.store.layout.metadata_path(self.spec),
-                    Python._DISTUTIL_CACHE_FILENAME)
-                if os.path.isfile(input_filename):
-                    with open(input_filename) as input_file:
-                        self._distutil_vars = sjson.load(input_file)
-            except Exception:
-                pass
-
-            if not self._distutil_vars:
-                self._distutil_vars = {}
-
-        return self._distutil_vars
-
-    @run_after('install')
     def filter_compilers(self):
         """Run after install to tell the configuration files and Makefiles
         to use the compilers that Spack built the package with.
@@ -946,6 +854,61 @@ class Python(AutotoolsPackage):
         pythonpath = ':'.join(python_paths)
         env.set('PYTHONPATH', pythonpath)
 
+        # We need to make sure that the extensions are compiled and linked with
+        # the Spack wrapper. Paths to the executables that are used for these
+        # operations are normally taken from the sysconfigdata file, which we
+        # modify after the installation (see method filter compilers). The
+        # modified file contains paths to the real compilers, not the wrappers.
+        # The values in the file, however, can be overridden with environment
+        # variables. The first variable, CC (CXX), which is used for
+        # compilation, is set by Spack for the dependent package by default.
+        # That is not 100% correct because the value for CC (CXX) in the
+        # sysconfigdata file often contains additional compiler flags (e.g.
+        # -pthread), which we lose by simply setting CC (CXX) to the path to the
+        # Spack wrapper. Moreover, the user might try to build an extension with
+        # a compiler that is different from the one that was used to build
+        # Python itself, which might have unexpected side effects. However, the
+        # experience shows that none of the above is a real issue and we will
+        # not try to change the default behaviour. Given that, we will simply
+        # try to modify LDSHARED (LDCXXSHARED), the second variable, which is
+        # used for linking, in a consistent manner.
+
+        for compile_var, link_var in [('CC', 'LDSHARED'),
+                                      ('CXX', 'LDCXXSHARED')]:
+            # First, we get the values from the sysconfigdata:
+            config_compile = self.get_config_var(compile_var)
+            config_link = self.get_config_var(link_var)
+
+            # The dependent environment will have the compilation command set to
+            # the following:
+            new_compile = join_path(
+                spack.paths.build_env_path,
+                dependent_spec.package.compiler.link_paths[compile_var.lower()])
+
+            # Normally, the link command starts with the compilation command:
+            if config_link.startswith(config_compile):
+                new_link = new_compile + config_link[len(config_compile):]
+            else:
+                # Otherwise, we try to replace the compiler command if it
+                # appears "in the middle" of the link command; to avoid
+                # mistaking some substring of a path for the compiler (e.g. to
+                # avoid replacing "gcc" in "-L/path/to/gcc/"), we require that
+                # the compiler command be surrounded by spaces. Note this may
+                # leave "config_link" unchanged if the compilation command does
+                # not appear in the link command at all, for example if "ld" is
+                # invoked directly (no change would be required in that case
+                # because Spack arranges for the Spack ld wrapper to be the
+                # first instance of "ld" in PATH).
+                new_link = config_link.replace(" {0} ".format(config_compile),
+                                               " {0} ".format(new_compile))
+
+            # There is logic in the sysconfig module that is sensitive to the
+            # fact that LDSHARED is set in the environment, therefore we export
+            # the variable only if the new value is different from what we got
+            # from the sysconfigdata file:
+            if config_link != new_link:
+                env.set(link_var, new_link)
+
     def setup_dependent_run_environment(self, env, dependent_spec):
         python_paths = []
         for d in dependent_spec.traverse(deptype='run'):
@@ -973,12 +936,6 @@ class Python(AutotoolsPackage):
         module.setup_py = Executable(
             self.command.path + ' setup.py --no-user-cfg')
 
-        distutil_vars = self._load_distutil_vars()
-
-        if distutil_vars:
-            for key, value in distutil_vars.items():
-                module.setup_py.add_default_env(key, value)
-
         # Add variables for lib/pythonX.Y and lib/pythonX.Y/site-packages dirs.
         module.python_lib_dir = join_path(dependent_spec.prefix,
                                           self.python_lib_dir)