classification
Title: distutils test errors when CXX is not set
Type: behavior Stage: resolved
Components: Distutils Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Michael.Felt, dstufft, eric.araujo, ncoghlan, taleinat
Priority: normal Keywords: patch

Created on 2018-10-04 20:22 by Michael.Felt, last changed 2018-12-28 14:06 by ncoghlan. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 9706 merged Michael.Felt, 2018-10-04 23:21
PR 8709 Michael.Felt, 2018-12-26 04:08
PR 8709 Michael.Felt, 2018-12-26 04:08
Messages (7)
msg327083 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-10-04 20:22
while researching issue11191 I cam across 6 additional errors.

There is a test in Lib/test/support/__init__.py

def missing_compiler_executable(cmd_names=[]):
    """Check if the compiler components used to build the interpreter exist.

    Check for the existence of the compiler executables whose names are listed
    in 'cmd_names' or all the compiler executables when 'cmd_names' is empty
    and return the first missing executable or None when none is found
    missing.

    """
    from distutils import ccompiler, sysconfig, spawn
    compiler = ccompiler.new_compiler()
    sysconfig.customize_compiler(compiler)
    for name in compiler.executables:
        if cmd_names and name not in cmd_names:
            continue
        cmd = getattr(compiler, name)
        if cmd_names:
            assert cmd is not None, \
                    "the '%s' executable is not configured" % name
        elif cmd is None:
            continue
        if spawn.find_executable(cmd[0]) is None:
            return cmd[0]

The "elif cmd is None:" is not successful because cmd maybe '' (null string)

Initially I thought to change to
"elif cmd is None or (not cmd):" but I hope I found a better resolution!

In: Lib/distutils/sysconfig.py the final bits of customize_compiler is:

        compiler.set_executables(
            preprocessor=cpp,
            compiler=cc_cmd,
            compiler_so=cc_cmd + ' ' + ccshared,
            compiler_cxx=cxx,
            linker_so=ldshared,
            linker_exe=cc,
            archiver=archiver)

the value for cxx come from os.environ, if set, otherwise it comes from get_sys_vars()

        (cc, cxx, opt, cflags, ccshared, ldshared, shlib_suffix, ar, ar_flags) = \
            get_config_vars('CC', 'CXX', 'OPT', 'CFLAGS',
                            'CCSHARED', 'LDSHARED', 'SHLIB_SUFFIX', 'AR', 'ARFLAGS')

If, during the build, CXX was not set - this sets cxx to the null string ('').

So the fix is to assign cxx = None when len(cxx) == 0

        if 'CXX' in os.environ:
            cxx = os.environ['CXX']
        if not len(cxx):
            cxx = None

While this only seems to happen for cxx - maybe this should be extended to all the variables in (cc, cxx, opt, cflags, ccshared, ldshared, shlib_suffix, ar, ar_flags)?

So, ultimately - I choose to go with changing : compiler.set_executables()

diff --git a/Lib/distutils/ccompiler.py b/Lib/distutils/ccompiler.py
index b71d1d39bc..2e08c4abd2 100644
--- a/Lib/distutils/ccompiler.py
+++ b/Lib/distutils/ccompiler.py
@@ -148,7 +148,7 @@ class CCompiler:
             if key not in self.executables:
                 raise ValueError("unknown executable '%s' for class %s" %
                       (key, self.__class__.__name__))
-            self.set_executable(key, kwargs[key])
+            self.set_executable(key, kwargs[key] if len(kwargs[key]) else None)

     def set_executable(self, key, value):
         if isinstance(value, str):



Was:
======================================================================
ERROR: test_run (distutils.tests.test_build_clib.BuildCLibTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_build_clib.py", line 121, in test_run
    ccmd = missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2730, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range

======================================================================
ERROR: test_build_ext (distutils.tests.test_build_ext.BuildExtTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_build_ext.py", line 62, in test_build_ext
    cmd = support.missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2730, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range

======================================================================
ERROR: test_get_outputs (distutils.tests.test_build_ext.BuildExtTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_build_ext.py", line 308, in test_get_outputs
    cmd = support.missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2730, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range

======================================================================
ERROR: test_build_ext (distutils.tests.test_build_ext.ParallelBuildExtTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_build_ext.py", line 62, in test_build_ext
    cmd = support.missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2730, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range

======================================================================
ERROR: test_get_outputs (distutils.tests.test_build_ext.ParallelBuildExtTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_build_ext.py", line 308, in test_get_outputs
    cmd = support.missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2730, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range

======================================================================
ERROR: test_record_extensions (distutils.tests.test_install.InstallTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_install.py", line 200, in test_record_extensions
    cmd = test_support.missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2730, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range
msg328553 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2018-10-26 14:36
I'm not sure that the resolution currently suggested, changing compiler.set_executables(), is the right way to go.

This change to distutils is a break of backwards compatibility. Though it is a minor change, it could still break existing code.

Fixing test.support seems just as good to me in terms of code design, and better in that it is only used internally for our tests.

(BTW, instead of `elif cmd is None or (not cmd):`, you can just use `elif not cmd:`.)
msg328851 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-10-29 18:32
> On 10/26/2018 5:36 PM, Tal Einat wrote:
> Tal Einat <taleinat@gmail.com> added the comment:
> 
> I'm not sure that the resolution currently suggested, changing compiler.set_executables(), is the right way to go.
> 
> This change to distutils is a break of backwards compatibility. Though it is a minor change, it could still break existing code.
> 
> Fixing test.support seems just as good to me in terms of code design, and better in that it is only used internally for our tests.
> 
> (BTW, instead of `elif cmd is None or (not cmd):`, you can just use `elif not cmd:`.)
Thx. One of the python bits I need to embrace.

Although - during my testing I saw that the null string "" was not being
accepted as None, but was accepted as "not cmd".

My reading error was taking None to mean a value was not initialized
while it seems to be None is a 'value' that is assigned, while ´not xxx'
can be either - never assigned or a null-length string.

This is the "ambiguity", at least for myself, that I feel I tracked down.

It is "unfortunate" if this breaks backward compatibility - IF the
backward compatibility has been based on an ambiguity. Or I again, miss
a fine-point of Python coding. Won´'t be the last time I expect.

Comments requested.

> 
> ----------
> nosy: +taleinat
> 
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue34897>
> _______________________________________
msg329155 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2018-11-02 19:39
`not cmd` will be true if cmd is None, so it is completely equivalent to `cmd is None or not cmd`.  This is a purely stylistic change which doesn't alter the logic.

To get a clear understanding of what's going on, I recommend reading the short "Truth Value Testing" section in the docs:
https://docs.python.org/library/stdtypes.html#truth-value-testing
msg329318 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-11-05 21:21
Thx.

So, while "" is not None (i.e., "" is not False), it does test as a Boolean expression as 'False' so the test for None or "" is the same as testing for "" (but not the same as testing for None).

I accept your logic - and shall make the change in the test in Lib/test/support/__init__.py rather than in the assignment in  Lib/distutils/ccompiler.py
msg329346 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2018-11-06 07:17
Michael, please read more about comparisons[1] and the None object[2].

[1] https://docs.python.org/3/library/stdtypes.html#comparisons
[2] https://docs.python.org/3/library/stdtypes.html#the-null-object
msg332532 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-12-26 05:45
New changeset 259c159fc1faab0dd631d20374842dc0d6a9f145 by Nick Coghlan (Michael Felt) in branch 'master':
bpo-34897: avoid distutils test error when CXX is not set (GH-9706)
https://github.com/python/cpython/commit/259c159fc1faab0dd631d20374842dc0d6a9f145
History
Date User Action Args
2018-12-28 14:06:32ncoghlansetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-12-26 05:45:21ncoghlansetnosy: + ncoghlan
messages: + msg332532
2018-12-26 04:09:00Michael.Feltsetpull_requests: + pull_request10577
2018-12-26 04:08:51Michael.Feltsetpull_requests: + pull_request10576
2018-11-06 07:17:48taleinatsetmessages: + msg329346
2018-11-05 21:21:26Michael.Feltsetmessages: + msg329318
2018-11-02 19:40:02taleinatsetmessages: - msg329154
2018-11-02 19:39:54taleinatsetmessages: + msg329155
2018-11-02 19:39:31taleinatsetmessages: + msg329154
2018-10-29 18:32:11Michael.Feltsetmessages: + msg328851
2018-10-26 14:36:11taleinatsetnosy: + taleinat
messages: + msg328553
2018-10-04 23:21:58Michael.Feltsetkeywords: + patch
stage: patch review
pull_requests: + pull_request9091
2018-10-04 20:22:26Michael.Feltcreate