-
Notifications
You must be signed in to change notification settings - Fork 80
Description
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.
Originally posted by @jaraco in #228 (comment)
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.
Originally posted by @jaraco in #228 (comment)