Distutils C++ support

[jaraco]

Upstreamed fix from nix, see patch here:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/setuptools/setuptools-distutils-C%2B%2B.patch

Resubmit of #221.

[jaraco]

@sciyoshi Would you be willing to investigate the two failures and suggest a fix?

[sciyoshi]

@jaraco sure. Not sure why the tests would have started failing after, but this patch fixes the tests for me:

diff --git a/distutils/sysconfig.py b/distutils/sysconfig.py
index bc579163..4e1cd3ac 100644
--- a/distutils/sysconfig.py
+++ b/distutils/sysconfig.py
@@ -335,7 +335,7 @@ def customize_compiler(compiler):  # noqa: C901
             ldshared = ldshared + ' ' + os.environ['LDFLAGS']
             ldcxxshared = ldcxxshared + ' ' + os.environ['LDFLAGS']
         if 'CFLAGS' in os.environ:
-            cflags = os.environ['CFLAGS']
+            cflags = cflags + ' ' + os.environ['CFLAGS']
             ldshared = ldshared + ' ' + os.environ['CFLAGS']
         if 'CXXFLAGS' in os.environ:
             cxxflags = os.environ['CXXFLAGS']
diff --git a/distutils/tests/test_sysconfig.py b/distutils/tests/test_sysconfig.py
index 6cbf5168..3177c6e1 100644
--- a/distutils/tests/test_sysconfig.py
+++ b/distutils/tests/test_sysconfig.py
@@ -133,7 +133,7 @@ class TestSysconfig:
         assert comp.exes['compiler_so'] == (
             'env_cc --sc-cflags ' '--env-cflags ' '--env-cppflags --sc-ccshared'
         )
-        assert comp.exes['compiler_cxx'] == 'env_cxx --env-cxx-flags'
+        assert comp.exes['compiler_cxx'] == 'env_cxx --env-cxx-flags --sc-cflags --env-cppflags'
         assert comp.exes['linker_exe'] == 'env_cc'
         assert comp.exes['linker_so'] == (
             'env_ldshared --env-ldflags --env-cflags' ' --env-cppflags'
@@ -161,7 +161,7 @@ class TestSysconfig:
         assert comp.exes['preprocessor'] == 'sc_cc -E'
         assert comp.exes['compiler'] == 'sc_cc --sc-cflags'
         assert comp.exes['compiler_so'] == 'sc_cc --sc-cflags --sc-ccshared'
-        assert comp.exes['compiler_cxx'] == 'sc_cxx'
+        assert comp.exes['compiler_cxx'] == 'sc_cxx --sc-cflags'
         assert comp.exes['linker_exe'] == 'sc_cc'
         assert comp.exes['linker_so'] == 'sc_ldshared'
         assert comp.shared_lib_extension == 'sc_shutil_suffix'
diff --git a/distutils/tests/test_unixccompiler.py b/distutils/tests/test_unixccompiler.py
index c1e57a01..165d9944 100644
--- a/distutils/tests/test_unixccompiler.py
+++ b/distutils/tests/test_unixccompiler.py
@@ -247,9 +247,13 @@ class TestUnixCCompiler(support.TempdirManager):
         def gcv(v):
             if v == 'LDSHARED':
                 return 'gcc-4.2 -bundle -undefined dynamic_lookup '
+            elif v == 'LDCXXSHARED':
+                return 'g++-4.2 -bundle -undefined dynamic_lookup '
             elif v == 'CXX':
                 return 'g++-4.2'
-            return 'gcc-4.2'
+            elif v == 'CC':
+                return 'gcc-4.2'
+            return ''

         def gcvs(*args, _orig=sysconfig.get_config_vars):
             if args:

Again, I'm not fully familiar with the intricacies involved, but the change in sysconfig.py seems to probably be an oversight, and the remaining changes in the tests are probably expected differences.

[jaraco]

Thanks! After applying that patch, there's one remaining failure in the pypy tests.

            if 'LDFLAGS' in os.environ:
                ldshared = ldshared + ' ' + os.environ['LDFLAGS']
>               ldcxxshared = ldcxxshared + ' ' + os.environ['LDFLAGS']
E               TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

distutils/sysconfig.py:336: TypeError

It appears that LDCXXSHARED is not defined in pypy:

>>>> import sysconfig
>>>> sysconfig.get_config_vars
<function get_config_vars at 0x000000011855d880>
>>>> sysconfig.get_config_vars('LDCXXSHARED')
[None]

[sciyoshi]

@jaraco yes I'm not sure how that would have worked before, but in any case it's probably easiest to change it like this:

diff --git a/distutils/sysconfig.py b/distutils/sysconfig.py
index 4e1cd3ac..937482a4 100644
--- a/distutils/sysconfig.py
+++ b/distutils/sysconfig.py
@@ -296,7 +296,6 @@ def customize_compiler(compiler):  # noqa: C901
             cflags,
             ccshared,
             ldshared,
-            ldcxxshared,
             shlib_suffix,
             ar,
             ar_flags,
@@ -306,13 +305,13 @@ def customize_compiler(compiler):  # noqa: C901
             'CFLAGS',
             'CCSHARED',
             'LDSHARED',
-            'LDCXXSHARED',
             'SHLIB_SUFFIX',
             'AR',
             'ARFLAGS',
         )

         cxxflags = cflags
+        ldcxxshared = ""

         if 'CC' in os.environ:
             newcc = os.environ['CC']

[sciyoshi]

More test failures? 😭

[jaraco]

I applied that diff, but unfortunately, that led to more failures in test_cc_overrides_ldshared_for_cxx_correctly... and while debugging the issue, I found that test is itself fragile - it fails differently depending on whether it's run in isolation or not (even if before the patch).

 distutils 765bbbd7 @ tox -e py311 -- -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly
.pkg-cpython311: _optional_hooks> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
.pkg-cpython311: get_requires_for_build_editable> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
.pkg-cpython311: build_editable> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
py311: install_package> python -I -m pip install --force-reinstall --no-deps /Users/jaraco/code/pypa/distutils/.tox/.tmp/package/20/distutils-0.1.dev3944+g765bbbd-0.editable-py3-none-any.whl
py311: commands[0]> pytest -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly
============================================================== test session starts ===============================================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
cachedir: .tox/py311/.pytest_cache
rootdir: /Users/jaraco/code/pypa/distutils
configfile: pytest.ini
plugins: enabler-3.0.0, checkdocs-2.10.1, pyfakefs-5.3.5, ruff-0.2.1
collected 478 items / 477 deselected / 1 selected                                                                                                

distutils/tests/test_unixccompiler.py F                                                                                                    [100%]

==================================================================== FAILURES ====================================================================
_________________________________________ TestUnixCCompiler.test_cc_overrides_ldshared_for_cxx_correctly _________________________________________

self = <distutils.tests.test_unixccompiler.TestUnixCCompiler object at 0x1031f8b50>

    @pytest.mark.skipif('platform.system == "Windows"')
    def test_cc_overrides_ldshared_for_cxx_correctly(self):
        """
        Ensure that setting CC env variable also changes default linker
        correctly when building C++ extensions.
    
        pypa/distutils#126
        """
    
        def gcv(v):
            if v == 'LDSHARED':
                return 'gcc-4.2 -bundle -undefined dynamic_lookup '
            elif v == 'LDCXXSHARED':
                return 'g++-4.2 -bundle -undefined dynamic_lookup '
            elif v == 'CXX':
                return 'g++-4.2'
            elif v == 'CC':
                return 'gcc-4.2'
            return ''
    
        def gcvs(*args, _orig=sysconfig.get_config_vars):
            if args:
                return list(map(sysconfig.get_config_var, args))
            return _orig()
    
        sysconfig.get_config_var = gcv
        sysconfig.get_config_vars = gcvs
        with mock.patch.object(
            self.cc, 'spawn', return_value=None
        ) as mock_spawn, mock.patch.object(
            self.cc, '_need_link', return_value=True
        ), mock.patch.object(
            self.cc, 'mkpath', return_value=None
        ), EnvironmentVarGuard() as env:
            env['CC'] = 'ccache my_cc'
            env['CXX'] = 'my_cxx'
            del env['LDSHARED']
>           sysconfig.customize_compiler(self.cc)

distutils/tests/test_unixccompiler.py:275: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

compiler = <distutils.tests.test_unixccompiler.compiler_wrapper.<locals>.CompilerWrapper object at 0x1031f8f50>

    def customize_compiler(compiler):  # noqa: C901
        """Do any platform-specific customization of a CCompiler instance.
    
        Mainly needed on Unix, so we can plug in the information that
        varies across Unices and is stored in Python's Makefile.
        """
        if compiler.compiler_type == "unix":
            if sys.platform == "darwin":
                # Perform first-time customization of compiler-related
                # config vars on OS X now that we know we need a compiler.
                # This is primarily to support Pythons from binary
                # installers.  The kind and paths to build tools on
                # the user system may vary significantly from the system
                # that Python itself was built on.  Also the user OS
                # version and build tools may not support the same set
                # of CPU architectures for universal builds.
                global _config_vars
                # Use get_config_var() to ensure _config_vars is initialized.
                if not get_config_var('CUSTOMIZED_OSX_COMPILER'):
                    import _osx_support
    
                    _osx_support.customize_compiler(_config_vars)
>                   _config_vars['CUSTOMIZED_OSX_COMPILER'] = 'True'
E                   TypeError: 'NoneType' object does not support item assignment

distutils/sysconfig.py:291: TypeError
============================================================ short test summary info =============================================================
FAILED distutils/tests/test_unixccompiler.py::TestUnixCCompiler::test_cc_overrides_ldshared_for_cxx_correctly - TypeError: 'NoneType' object does not support item assignment
======================================================= 1 failed, 477 deselected in 0.16s ========================================================
py311: exit 1 (0.30 seconds) /Users/jaraco/code/pypa/distutils> pytest -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly pid=70507
.pkg-cpython311: _exit> python '/Users/jaraco/Library/Application Support/pipx/venvs/tox/lib/python3.12/site-packages/pyproject_api/_backend.py' True setuptools.build_meta
  py311: FAIL code 1 (0.86=setup[0.56]+cmd[0.30] seconds)
  evaluation failed :( (0.90 seconds)

Probably we should figure out why that test is reliant on other tests and fix that too.

[jaraco]

At main, that test runs fine in isolation.

 distutils main @ tox -e py311 -q -- -q -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly
.                                                                                                                                          [100%]
1 passed, 477 deselected in 0.18s
  py311: OK (0.98 seconds)
  congratulations :) (1.02 seconds)

[jaraco]

The issue seems to be surfaced by this change, where the fallback value is now '', which allows the darwin-specific behavior to be reached:

distutils/distutils/sysconfig.py

Lines 285 to 291 in e651e53

	global _config_vars
	# Use get_config_var() to ensure _config_vars is initialized.
	if not get_config_var('CUSTOMIZED_OSX_COMPILER'):
	import _osx_support
	_osx_support.customize_compiler(_config_vars)
	_config_vars['CUSTOMIZED_OSX_COMPILER'] = 'True'

That code attempts to mutate a global variable, which, if it wasn't initialized by another test, will lead to the error.

I believe the proposed diff is safer and more correct. We'll want to fix that undesirable entanglement.

[jaraco]

After fixing #231 and rebasing onto those changes, I'm now able to elicit the intrinsic error by invoking the test selectively:

 distutils feature/cpp-support @ tox -qq -- -q -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly
F                                                                                                                                          [100%]
==================================================================== FAILURES ====================================================================
_________________________________________ TestUnixCCompiler.test_cc_overrides_ldshared_for_cxx_correctly _________________________________________

self = <distutils.tests.test_unixccompiler.TestUnixCCompiler object at 0x1055eadb0>

    @pytest.mark.skipif('platform.system == "Windows"')
    @pytest.mark.usefixtures('disable_macos_customization')
    def test_cc_overrides_ldshared_for_cxx_correctly(self):
        """
        Ensure that setting CC env variable also changes default linker
        correctly when building C++ extensions.
    
        pypa/distutils#126
        """
    
        def gcv(v):
            if v == 'LDSHARED':
                return 'gcc-4.2 -bundle -undefined dynamic_lookup '
            elif v == 'LDCXXSHARED':
                return 'g++-4.2 -bundle -undefined dynamic_lookup '
            elif v == 'CXX':
                return 'g++-4.2'
            elif v == 'CC':
                return 'gcc-4.2'
            return ''
    
        def gcvs(*args, _orig=sysconfig.get_config_vars):
            if args:
                return list(map(sysconfig.get_config_var, args))
            return _orig()
    
        sysconfig.get_config_var = gcv
        sysconfig.get_config_vars = gcvs
        with mock.patch.object(
            self.cc, 'spawn', return_value=None
        ) as mock_spawn, mock.patch.object(
            self.cc, '_need_link', return_value=True
        ), mock.patch.object(
            self.cc, 'mkpath', return_value=None
        ), EnvironmentVarGuard() as env:
            env['CC'] = 'ccache my_cc'
            env['CXX'] = 'my_cxx'
            del env['LDSHARED']
            sysconfig.customize_compiler(self.cc)
            assert self.cc.linker_so[0:2] == ['ccache', 'my_cc']
>           self.cc.link(None, [], 'a.out', target_lang='c++')

distutils/tests/test_unixccompiler.py:290: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
distutils/unixccompiler.py:271: in link
    env, linker_ne = _split_env(linker)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cmd = []

    def _split_env(cmd):
        """
        For macOS, split command into 'env' portion (if any)
        and the rest of the linker command.
    
        >>> _split_env(['a', 'b', 'c'])
        ([], ['a', 'b', 'c'])
        >>> _split_env(['/usr/bin/env', 'A=3', 'gcc'])
        (['/usr/bin/env', 'A=3'], ['gcc'])
        """
        pivot = 0
>       if os.path.basename(cmd[0]) == "env":
E       IndexError: list index out of range

distutils/unixccompiler.py:56: IndexError
============================================================ short test summary info =============================================================
FAILED distutils/tests/test_unixccompiler.py::TestUnixCCompiler::test_cc_overrides_ldshared_for_cxx_correctly - IndexError: list index out of range
1 failed, 477 deselected in 0.18s
py: exit 1 (0.32 seconds) /Users/jaraco/code/pypa/distutils> pytest -q -p no:cov -k test_cc_overrides_ldshared_for_cxx_correctly pid=79617

[jaraco]

The issue seems to be that self.linker_so_cxx is not set on macOS here.

> /Users/jaraco/code/pypa/distutils/distutils/unixccompiler.py(56)_split_env()
-> if os.path.basename(cmd[0]) == "env":
(Pdb) u
> /Users/jaraco/code/pypa/distutils/distutils/unixccompiler.py(271)link()
-> env, linker_ne = _split_env(linker)
(Pdb) linker
[]
(Pdb) self.linker_exe
['ccache', 'my_cc']
(Pdb) building_exe
False
(Pdb) self.linker_so_cxx
[]

That doesn't explain why tests are failing on Linux, though. The error is the same on Linux.

[jaraco]

@sciyoshi Since the issue is in the proposed diff, can you investigate?

[sciyoshi]

Any thougths on the remaining failing test? I'll try to take a look but won't have much time.

[jaraco]

The issue seems to be that self.linker_so_cxx is not set on macOS here.

It looks like in test_cc_overrides_ldshared_for_cxx_correctly, when sysconfig.customize_compiler is called, it causes self.cc.linker_so_cxx to become [] so later when self.cc.link is called, the linker isn't valid and crashes when _split_env is expecting a non-empty list of command parameters.

I need someone to figure out what this test was asserting before and what it needs to be asserting now, and is it catching a legitimate bug, or does the test need to be changed to accommodate the new behavior?

[jaraco]

I do see an asymmetry in these lines:

distutils/distutils/sysconfig.py

Lines 301 to 322 in 95088c5

	(
	cc,
	cxx,
	cflags,
	ccshared,
	ldshared,
	shlib_suffix,
	ar,
	ar_flags,
	) = get_config_vars(
	'CC',
	'CXX',
	'CFLAGS',
	'CCSHARED',
	'LDSHARED',
	'SHLIB_SUFFIX',
	'AR',
	'ARFLAGS',
	)
	cxxflags = cflags
	ldcxxshared = ""

There, I see ldshared is initialized using get_config_vars('LDSHARED'), but ldcxxshared is initialized to "". Why the discrepancy?

[jaraco]

It looks like making the behavior symmetrical allows the tests to pass:

diff --git a/distutils/sysconfig.py b/distutils/sysconfig.py
index 31bdbec1b..847a26ba4 100644
--- a/distutils/sysconfig.py
+++ b/distutils/sysconfig.py
@@ -304,6 +304,7 @@ def customize_compiler(compiler):  # noqa: C901
             cflags,
             ccshared,
             ldshared,
+            ldcxxshared,
             shlib_suffix,
             ar,
             ar_flags,
@@ -313,13 +314,13 @@ def customize_compiler(compiler):  # noqa: C901
             'CFLAGS',
             'CCSHARED',
             'LDSHARED',
+            'LDCXXSHARED',
             'SHLIB_SUFFIX',
             'AR',
             'ARFLAGS',
         )
 
         cxxflags = cflags
-        ldcxxshared = ""
 
         if 'CC' in os.environ:
             newcc = os.environ['CC']

I'm unsure if that's the correct implementation, but since tests are passing, it feels more correct and at least makes the PR acceptable.

[sciyoshi]

Seems that's what the upstream patch does as well. This was probably a mistake on my end when I was updating it. Nice catch!

[sciyoshi]

Actually, I think the new error in test_customize_compiler is what I was attempting to fix by setting it to "".

[jaraco]

Actually, I think the new error in test_customize_compiler is what I was attempting to fix by setting it to "".

In PyPy only, it seems. Seems to be that PyPy has a different definition for LIBCXXSHARED LDCXXSHARED than other Pythons. We'll probably need to root cause why that is.

[gorloffslava]

@jaraco I may be wrong, but it looks like PyPy doesn't preserve LDCXXSHARED at all in its configuration.

CPython does that explicitely here: https://github.com/python/cpython/blob/5a7f7c48644baf82988f30bcb43e03dcfceb75dd/configure.ac#L3395.

[BwL1289]

@jaraco I also checked PyPy, and I don't see any reference to LIBCXXSHARED in the repo.

[jaraco]

My first instinct was to replace None with '' for ldcxxshared, but that also seems improper, as it still invokes compiler.set_executables(ldcxxshared=) for either the empty string or just the parameters with no executable, neither of which is valid.

My next instinct is not to set_executables for ldcxxshared when get_config_var('LDCXXSHARED') is None, but that requires touching several different places in the already messy customize_compiler function. This approach would, however, leave Compiler.linker_so_cxx set to the default. I'd really like to avoid special-casing this one variable.

My next thought was to just skip this test on PyPy, but that probably will cause compilers to break on PyPy on Unix.

Another thought was to inject LDCXXSHARED when it's missing, similar to how is done for add_ext_suffix_39. The problem is, what value to present? Again, an empty string is wrong. And the default value isn't a string at all but a list.

@mattip Do you have any insight into why PyPy doesn't have LDCXXSHARED set in sysconfig? Will PyPy users ever expect to be taking advantage of compiling C++ shared libraries under PyPy?

[jaraco]

In fcef963, I found a satisfactory workaround - don't attempt to add flags if the value is None. It first required a refactor in 3e4b457.

With this change, Compiler.set_executable(linker_so_cxx=None) will be called, which should trigger failures later on PyPy if something tries to compile C++. I believe that's an acceptable behavior... and could be corrected by PyPy exporting LDCXXSHARED.

[jaraco]

The only remaining failure is in diffcov, which is ignorable, since there's no coverage aggregation.

[mattip]

why PyPy doesn't have LDCXXSHARED set in sysconfig?

The values there have been added on an as-needed basis, in response to issues. PyPy does not use a Makefile or config.

On CPython windows it is not defined.

I'm not sure how PyPY would create that value, it appears to want to invoke a compiler. How does CPython know what compiler is available on the host platform? What happens if clang is installed instead of gcc?

[BwL1289]

Thank you @jaraco! Really appreciate all of the hard work.

[jaraco]

On CPython windows it is not defined.

This particular code path is only for Unix, so that's why the discrepancy hasn't been an issue.

I'm not sure how PyPY would create that value, it appears to want to invoke a compiler. How does CPython know what compiler is available on the host platform? What happens if clang is installed instead of gcc?

It looks like it's built from CXX. On my macOS Python 3.12 as compiled by homebrew, here's what I see:

 🐚 py -3.12 -c "import sysconfig; print(sysconfig.get_config_var('LDCXXSHARED'))"
clang++ -bundle -undefined dynamic_lookup

[mattip]

We don't use c++ in PyPy, so the best we could do would be to copy that value (and the one for linux) into the _sysconfigdata.py we ship with PyPy, assuming that is appropriate. Maybe distutils could also provide a default value per-platform, in a similar way.

[mattip]

Here are the hard-coded PyPy values https://github.com/pypy/pypy/blob/py3.9/lib_pypy/_sysconfigdata.py