classification
Title: distutils fail to determine c++ linker with unixcompiler if using ccache
Type: compile error Stage: patch review
Components: Distutils Versions: Python 3.4, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Alexander.Sulfrian, Dan Mick, Joshua.J.Cogliati, Xuefer.x, dstufft, eric.araujo, erik.bray, jrincayc, mihaic, ned.deily, ronaldoussoren, tarek
Priority: normal Keywords: patch

Created on 2010-02-27 15:24 by Alexander.Sulfrian, last changed 2016-07-11 21:41 by mihaic.

Files
File name Uploaded Description Edit
fix-distutils-with-ccache.patch Alexander.Sulfrian, 2010-02-27 15:24
fix-distutils-279.patch Joshua.J.Cogliati, 2015-02-23 18:09 Patch to remove code in Python 2.7.9
fix-distutils-342.patch Joshua.J.Cogliati, 2015-02-23 18:18 Patch to remove code in Python 3.4.2
fix-distutils-350.patch Joshua.J.Cogliati, 2015-03-17 19:54 Patch to remove code in Python 3.5.0
small_example.tar.gz Joshua.J.Cogliati, 2015-03-18 16:47 short example of distutils example that will trigger the bug
Messages (19)
msg100186 - (view) Author: Alexander Sulfrian (Alexander.Sulfrian) Date: 2010-02-27 15:24
Hi,
if using ccache (CC="ccache gcc --flags", CXX="g++") distutils will try to execute something like "g++ gcc --flags" as linker for c++ libraries.

Patch attached.


Alex
msg122431 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-26 03:04
Hello Alexander, thanks for the report and patch.  Could you add a test for the fixed behavior?
msg232474 - (view) Author: Xuefer x (Xuefer.x) Date: 2014-12-11 12:53
please fix this bug. it seems the patch no longer applies to the current 2.7/3.4 code but the bug is still reproduce-able in another way

i have CXX="ccache_cxx -pthread -shared"
but what actually happen is:
ccache_cxx gcc -pthread -shared ...
x86_64-openwrt-linux-gnu-g++: error: gcc: No such file or directory
error: command 'ccache_cxx' failed with exit status 1
Makefile:511: recipe for target 'all-local' failed
make[5]: *** [all-local] Error 1

this is when building libtorrent-rasterbar-0.16.17 in openwrt build system, ccache_cxx is a wrapper script and distuils make a wrong build rule
msg232475 - (view) Author: Xuefer x (Xuefer.x) Date: 2014-12-11 13:11
even with
CXX="x86_64-openwrt-linux-gnu-g++ -pthread -shared" /usr/src/xuefer/openwrt/trunk/staging_dir/host/bin/python setup.py build

it's donig
x86_64-openwrt-linux-gnu-g++ gcc -pthread -shared ....
msg236454 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-02-23 18:09
I can confirm this bug on Python 2.7.8 and Python 3.4.1, and given that the code is still in later versions, I expect that it will fail in them as well.

From Python-3.4.2/Lib/distutils/unixcompiler.py, lines 179-189:
# skip over environment variable settings if /usr/bin/env
# is used to set up the linker's environment.
# This is needed on OSX. Note: this assumes that the
# normal and C++ compiler have the same environment
# settings.
i = 0
if os.path.basename(linker[0]) == "env":
    i = 1
    while '=' in linker[i]:
        i += 1
linker[i] = self.compiler_cxx[i]

First of all, as has been noted, this will fail if len(self.compiler_cxx) > 1.  

Second of all, I do not understand how this code was supposed to work correctly in the first place.  If env is in linker[0], then variable i will be greater than 0, so why are we then using self.compiler_cxx[i]?

As I understand the code, I should be able to do something like:
export CXX="env BAR=FOO g++"
and then have env work.  However, when I tried this, my python setup.py build failed.

However, if I do something like:
if target_lang == "c++" and self.compiler_cxx and False:
to the line in unixcompiler.py, then I can do:
export CXX="env BAR=FOO g++"
in the shell and have it compile.  I don't know if it would work if I actually needed BAR=FOO, but so far as I can tell this code is broken, and has been for a while, and therefore should be flat out removed.
msg236456 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-02-23 18:18
Patch for 3.4.2 that removes the code.
msg238333 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-03-17 19:54
This bug is still in Python 3.5.0a2 (but first issue 23644 needs to be fixed before g++ can be used at all)

Attached is a patch for Python 3.5.0.
msg238458 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-03-18 16:47
Here is an example to use to test for this bug.

Proper use on my computer (PYTHONPATH may need adjusting on other systems):

Python2:
cd small_example
python setup.py build_ext build install --prefix=`pwd`
export PYTHONPATH=`pwd`/lib64/python2.7/site-packages
cd test
python a_test.py
# outputs 3

Python3:
cd small_example
python3 setup.py build_ext build install --prefix=`pwd`
export PYTHONPATH=`pwd`/lib64/python3.4/site-packages
cd test
python3 a_test.py
# outputs 3

Clean files:
rm -Rf build/ lib/ lib64/ example/example.py example/example_wrap.cpp *~ example/__pycache__/

Check for the bug:
export CXX="env BAR=FOO g++"
or
export CXX="ccache g++"
etc
Then run either the Python2 or Python3 test, and it will fail if this bug has not been fixed.
msg238462 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2015-03-18 17:18
Does the patch fix things on Mac OS X and Linux?
msg238463 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-03-18 17:25
> Does the patch fix things on Mac OS X and Linux?

So far as I can tell, the code in question is to do something so
that CXX variables like:
export CXX="env BAR=FOO g++"
work better.  However, it is broken for that case, and since it has
been broken since at least python 2.6, I think it would be better to
remove the code, as my patches do.

My patches do fix things for me on Linux on my case of:
export CXX="ccache g++"
and do not make things worse for the env case.

I can check on OSX if you like.
msg238467 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-03-18 19:16
On OSX 10.10.1,

With the small_example, and with python 3.4.3 and:
$ export CXX="env BAR=FOO g++"
$ python3 setup.py build_ext build install --prefix=`pwd`
running build_ext
building '_example' extension
swigging example/example.i to example/example_wrap.cpp
swig -python -c++ -py3 -Iexample -o example/example_wrap.cpp example/example.i
creating build
creating build/temp.macosx-10.10-x86_64-3.4
creating build/temp.macosx-10.10-x86_64-3.4/example
gcc -Wno-unused-result -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Iexample -I/Users/jjc/bug/py343/include/python3.4m -c example/example_wrap.cpp -o build/temp.macosx-10.10-x86_64-3.4/example/example_wrap.o
gcc -Wno-unused-result -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Iexample -I/Users/jjc/bug/py343/include/python3.4m -c example/example.cxx -o build/temp.macosx-10.10-x86_64-3.4/example/example.o
creating build/lib.macosx-10.10-x86_64-3.4
creating build/lib.macosx-10.10-x86_64-3.4/example
env -bundle -undefined dynamic_lookup build/temp.macosx-10.10-x86_64-3.4/example/example_wrap.o build/temp.macosx-10.10-x86_64-3.4/example/example.o -o build/lib.macosx-10.10-x86_64-3.4/example/_example.so
env: illegal option -- b
usage: env [-i] [name=value ...] [utility [argument ...]]
error: command 'env' failed with exit status 1


So, as is, python 3.4.3 fails.

With the same as above, but with the fix-distutils-342.patch applied:
$ export CXX="env BAR=FOO g++"
$ python3 setup.py build_ext build install --prefix=`pwd`
running build_ext
building '_example' extension
swigging example/example.i to example/example_wrap.cpp
swig -python -c++ -py3 -Iexample -o example/example_wrap.cpp example/example.i
creating build
creating build/temp.macosx-10.10-x86_64-3.4
creating build/temp.macosx-10.10-x86_64-3.4/example
gcc -Wno-unused-result -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Iexample -I/Users/jjc/bug/py343m/include/python3.4m -c example/example_wrap.cpp -o build/temp.macosx-10.10-x86_64-3.4/example/example_wrap.o
gcc -Wno-unused-result -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Iexample -I/Users/jjc/bug/py343m/include/python3.4m -c example/example.cxx -o build/temp.macosx-10.10-x86_64-3.4/example/example.o
creating build/lib.macosx-10.10-x86_64-3.4
creating build/lib.macosx-10.10-x86_64-3.4/example
gcc -bundle -undefined dynamic_lookup build/temp.macosx-10.10-x86_64-3.4/example/example_wrap.o build/temp.macosx-10.10-x86_64-3.4/example/example.o -o build/lib.macosx-10.10-x86_64-3.4/example/_example.so
running build
running build_py
copying example/example.py -> build/lib.macosx-10.10-x86_64-3.4/example
copying example/__init__.py -> build/lib.macosx-10.10-x86_64-3.4/example
running install
running install_lib
creating /Users/jjc/bug/small_example/lib
creating /Users/jjc/bug/small_example/lib/python3.4
creating /Users/jjc/bug/small_example/lib/python3.4/site-packages
creating /Users/jjc/bug/small_example/lib/python3.4/site-packages/example
copying build/lib.macosx-10.10-x86_64-3.4/example/__init__.py -> /Users/jjc/bug/small_example/lib/python3.4/site-packages/example
copying build/lib.macosx-10.10-x86_64-3.4/example/_example.so -> /Users/jjc/bug/small_example/lib/python3.4/site-packages/example
copying build/lib.macosx-10.10-x86_64-3.4/example/example.py -> /Users/jjc/bug/small_example/lib/python3.4/site-packages/example
byte-compiling /Users/jjc/bug/small_example/lib/python3.4/site-packages/example/__init__.py to __init__.cpython-34.pyc
byte-compiling /Users/jjc/bug/small_example/lib/python3.4/site-packages/example/example.py to example.cpython-34.pyc
running install_egg_info
Writing /Users/jjc/bug/small_example/lib/python3.4/site-packages/example-0.5-py3.4.egg-info
$ export PYTHONPATH=`pwd`/lib/python3.4/site-packages
$ cd test
$ python3 a_test.py 
3


So with the patch, it succeeds.  

> Does the patch fix things on Mac OS X and Linux?
Yes.
msg238473 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2015-03-18 21:37
Thanks for the patch and the testing.  I’m not in a position to commit the patch; if no core developer does it in the coming week, feel free to ping python-dev to get action.  You can also hop onto IRC at any time and see if someone here can finish this.
msg238488 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2015-03-19 02:58
The code in question was added a long time ago by Ronald as part of the initial OS X universal build support (3078fdb7cf3d for Issue1488098).  I'm not exactly sure what problem it was trying to solve ao I'm not sure that it's 100% safe to totally remove rather than try to fix it.  Ronald, do you recall?
msg238535 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-03-19 15:38
Looking at the patch (3078fdb7cf3d for Issue1488098) it has:
                 if target_lang == "c++" and self.compiler_cxx:
-                    linker[0] = self.compiler_cxx[0]

One possibility is that that the problem was:
 linker[0] = self.compiler_cxx[0]
which would also cause a compiler_cxx like:
export CXX="env BAR=FOO g++"
to fail.  Possibly the code:
 linker[i] = self.compiler_cxx[i]
managed to hit on an index value that worked better for the CXX that Ronald had on his system.

I think the root cause is the special casing of

if target_lang == "c++" and self.compiler_cxx:

and I think that few enough people use a c++ for extensions and of those, most use CXX="g++" or something similar where the complicated but incorrect version that is in Python works.

Basically, if you have something like CXX="g++" then both the version before 3078fdb7cf3d and after 3078fdb7cf3d work.
If you have something like CXX="env BAR=FOO g++"
then the version before 3078fdb7cf3d would not have worked.

I think it is likely that if 3078fdb7cf3d had completely eliminated:
-                if target_lang == "c++" and self.compiler_cxx:
-                    linker[0] = self.compiler_cxx[0]
instead of adding new code to try and work around the problem it also would have fixed the problem that Issue1488098 encountered.

I am guessing this problem really started with changeset 771b6f521b95 in 2002.  This codes purpose is:
 (UnixCCompiler.link): Included target_lang parameter, and made linker command use compiler_cxx, if target_lang is 'c++'. 
and that change is the one that added:
 linker[0] = self.compiler_cxx[0]
msg239934 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-04-02 17:49
Let me try and explain what is trying to be done in the original code, 
what the fix was, and then discuss some possible better solutions.

Original code:
if target_lang == "c++" and self.compiler_cxx:
    linker[0] = self.compiler_cxx[0]

Current code:
if target_lang == "c++" and self.compiler_cxx:
    i = 0
    if os.path.basename(linker[0]) == "env":
        i = 1
        while '=' in linker[i]:
            i += 1
    linker[i] = self.compiler_cxx[i]

Basically, what we have, is two variables, linker, and self.compiler_cxx.

By default on my linux system these are:
linker=['gcc', '-pthread', '-shared']
self.compiler_cxx=['g++', '-pthread']

So under the current code, since self.compiler_cxx[0] != "env"
i=0 so
linker[0] = self.compiler_cxx[0]
so linker=['g++', '-pthread', '-shared']

So the goal is to switch the linker to something that works better with c++.  

In the original code:
linker[0] = self.compiler_cxx[0]
improves things if the first element in the linker variable is the linker executable, and the first element in compiler_cxx is a compiler executable, and the compiler executable works better at linking c++ code than the c linker executable.

Next we switch to the current code.  

I am guessing that Ronald had something like this:
linker=['env','BAR=FOO','gcc', '-pthread', '-shared']
self.compiler_cxx=['env','BAR=FOO','g++', '-pthread']
and so with the current code we end up with i=2, and the linker variable ends up with:
linker=['env','BAR=FOO','g++', '-pthread', '-shared']

Now, what is the problem we are encountering with the current code?

Basically, people want to use things like CXX="ccache g++"
So if 
linker=['gcc', '-pthread', '-shared']
self.compiler_cxx=['ccache', 'g++']
we have i=0, 
linker[0]=self.compiler_cxx[0] = 'ccache'
resulting in
linker=['ccache', '-pthread', '-shared']
which will fail, because ccache expects the first argument to be the compiler executable (that is 'g++' not '-pthread')

So, how can we fix this?

If the linker can link c++ code, then the optimal solution is to do nothing, 
as in remove the entire 
if target_lang == "c++" and self.compiler_cxx:
   ...
(The fix-distutils-* patches I have created do this solution)

We could special case ccache:
if target_lang == "c++" and self.compiler_cxx:
    if os.path.basename(self.compiler_cxx[0]) == "ccache":
        linker[0:1] = self.compiler_cxx[0:2]
    elif os.path.basename(linker[0]) == "env":
    ...

We could hope that what we actually want is the entire compiler_cxx:
if target_lang == "c++" and self.compiler_cxx:
    linker[0:1] = self.compiler_cxx[:]


Maybe we could special case c++ a little earlier (since linker_exe is just cc by default):
if target_desc == CCompiler.EXECUTABLE and target_lang == "c++" and self.compiler_cxx:
    linker = self.compiler_cxx[:]
elif target_desc == CCompiler.EXECUTABLE:
    linker = self.linker_exe[:]
else:
    linker = self.linker_so[:]


None of these solutions make me feel awesome.  

The real problem is that we have no way to set the c++ linker.  I don't see any variable listed like this Make's implicit variables section.

A secondary problem is that we string concatenate the LDSHARED and LDFLAGS 
(in the line: ldshared = ldshared + ' ' + os.environ['LDFLAGS'] in sysconfig.py)

Another secondary problem is that we don't look for a CXXFLAGS varible, and so people stuff parameters into CXX instead.

If the two secondary issues did not exist we could reasonably reliably do something like
linker = cxx + ldflags
which might actually work for all the cases that are currently covered and work where cxx=['ccache','g++']

The current code is broken, how do we want to fix it?
msg240288 - (view) Author: Joshua J Cogliati (Joshua.J.Cogliati) * Date: 2015-04-08 22:08
I looked and the autoconf variable for the c++ linker is CXXLINK, so I think the proper way to fix this would be to change sysconfig.py to look at both CXXFLAGS and CXXLINK, and create those and use it to define a cxxlink variable, and only if they are missing should we actually try to do some of the magic that is currently used.

Also, my patches (fix-distutils-*.patch) do not always work, because sometimes the c compiler cannot link the c++ code.
msg242111 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2015-04-27 11:27
I don't recall exactly why the universal build support is done the way it is, adding it is too long ago.  

The reason is likely no longer relevant with any reasonably up-to-date toolset. The patch was created around the time x86 support was added to OSX and at the time there were numerous rough edges (and IIRC I worked on this using a Developer Transition Kit at work, that is before Apple shipped consumer hardware with x86 support, let alone non-beta software with x86 support)
msg263049 - (view) Author: Dan Mick (Dan Mick) Date: 2016-04-08 23:03
So the original author of the code says it's "likely no longer relevant", and it's clearly wrong, but no one has touched this in six years?

Even moving the "linker[i] = self.compiler_cxx[i]" inside the if that checks for "env" would *still* limit the exposure of this bug significantly without affecting the mystery-reason-for-the-workaround...

We probably spent a man-week on this bug.

Small wonder why Python packaging continues to be met with utter terror.
msg263663 - (view) Author: Erik Bray (erik.bray) * (Python triager) Date: 2016-04-18 13:07
I'm also inclined to agree that the buggy code (and it *is* buggy even if it was meant to fix something originally) should be removed outright.  Nobody can point to a real world issue that it fixes anymore, yet we can point to real world consequences caused by this bug still.

However, I could also agree that this should come with better support for CXXFLAGS and CXXLINK.  It's no wonder these variables are ignored by CPython--it doesn't use any c++.  That said, extension module compilation is tied, through distutils, to CPython. And there are plenty of c++ extension modules in the wild that are affected.  So it's worth addressing how these variables are used--or rather not used properly--by distutils.

I'd be happy to take a look at that if that sounds like a good approach?  Otherwise I'd push for accepting some versions of the existing patches by Joshua.J.Cogliati that remove the buggy code.
History
Date User Action Args
2016-07-11 21:41:47mihaicsetnosy: + mihaic
2016-04-18 13:08:00erik.braysetmessages: + msg263663
2016-04-18 12:40:37erik.braysetnosy: + erik.bray
2016-04-08 23:03:36Dan Micksetnosy: + Dan Mick
messages: + msg263049
2015-04-27 11:27:56ronaldoussorensetmessages: + msg242111
2015-04-08 22:08:13Joshua.J.Cogliatisetmessages: + msg240288
2015-04-02 17:49:29Joshua.J.Cogliatisetmessages: + msg239934
2015-03-19 15:38:35Joshua.J.Cogliatisetmessages: + msg238535
2015-03-19 09:23:12vstinnersetnosy: - vstinner
2015-03-19 02:58:11ned.deilysetnosy: + ned.deily, ronaldoussoren
messages: + msg238488
2015-03-18 21:37:46eric.araujosetnosy: + dstufft
messages: + msg238473
components: - Distutils2
2015-03-18 19:16:16Joshua.J.Cogliatisetmessages: + msg238467
2015-03-18 17:25:10Joshua.J.Cogliatisetmessages: + msg238463
2015-03-18 17:18:06eric.araujosetassignee: eric.araujo ->
messages: + msg238462
versions: - 3rd party, Python 3.1, Python 3.2
2015-03-18 16:47:07Joshua.J.Cogliatisetfiles: + small_example.tar.gz
nosy: + vstinner
messages: + msg238458

2015-03-17 19:54:01Joshua.J.Cogliatisetfiles: + fix-distutils-350.patch

messages: + msg238333
versions: + Python 3.5
2015-02-23 18:18:03Joshua.J.Cogliatisetfiles: + fix-distutils-342.patch

messages: + msg236456
2015-02-23 18:09:18Joshua.J.Cogliatisetfiles: + fix-distutils-279.patch
versions: + Python 3.4
nosy: + Joshua.J.Cogliati, jrincayc

messages: + msg236454
2014-12-11 13:11:25Xuefer.xsetmessages: + msg232475
2014-12-11 12:53:02Xuefer.xsetnosy: + Xuefer.x
messages: + msg232474
2010-11-26 03:05:04eric.araujosetmessages: - msg113050
2010-11-26 03:04:31eric.araujosetversions: + 3rd party
nosy: + eric.araujo, - terry.reedy
messages: + msg122431

assignee: tarek -> eric.araujo
components: + Distutils2
2010-08-05 20:46:36terry.reedysetnosy: + terry.reedy
messages: + msg113050
2010-08-05 03:32:59BreamoreBoysetstage: patch review
versions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6
2010-02-27 15:24:01Alexander.Sulfriancreate