classification
Title: distutils.command.install_lib.get_outputs() wrong with extensions built inplace
Type: behavior Stage:
Components: Distutils Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Elvis Stansvik, dstufft, estan, merwok
Priority: normal Keywords: patch

Created on 2017-01-10 15:37 by Elvis Stansvik, last changed 2017-02-15 08:20 by estan.

Files
File name Uploaded Description Edit
fix-install-lib-with-inplace-ext.patch Elvis Stansvik, 2017-01-10 15:42 Patch that fixes the issue review
Messages (3)
msg285127 - (view) Author: Elvis Stansvik (Elvis Stansvik) Date: 2017-01-10 15:37
I noticed the following strange behavior:

estan@newton:~/testdist$ find
.
./testext.c
./setup.cfg
./setup.py
estan@newton:~/testdist$ cat setup.py 
from distutils.core import setup, Extension

setup(
    name='testdist',
    version='1.0.0',
    ext_modules = [Extension('testext', ['testext.c'])]
)
estan@newton:~/testdist$ cat setup.cfg
[build_ext]
inplace=1
estan@newton:~/testdist$ ~/cpython/python setup.py install --prefix=~/prefix --record record.txt 
running install
running build
running build_ext
building 'testext' extension
creating build
creating build/temp.linux-x86_64-3.7-pydebug
gcc -pthread -Wno-unused-result -Wsign-compare -g -Og -Wall -Wstrict-prototypes -fPIC -I/home/estan/cpython/Include -I/home/estan/cpython -c testext.c -o build/temp.linux-x86_64-3.7-pydebug/testext.o
gcc -pthread -shared build/temp.linux-x86_64-3.7-pydebug/testext.o -o /home/estan/testdist/testext.cpython-37dm-x86_64-linux-gnu.so
running install_lib
warning: install_lib: 'build/lib.linux-x86_64-3.7-pydebug' does not exist -- no Python modules to install

running install_egg_info
Creating /home/estan/prefix/lib/python3.7/site-packages/
Writing /home/estan/prefix/lib/python3.7/site-packages/testdist-1.0.0-py3.7.egg-info
writing list of installed files to 'record.txt'
estan@newton:~/testdist$ cat record.txt 
/home/estan/prefix/lib/python3.7/site-packages/n-37dm-x86_64-linux-gnu.so
/home/estan/prefix/lib/python3.7/site-packages/testdist-1.0.0-py3.7.egg-info
estan@newton:~/testdist$ find ~/prefix
/home/estan/prefix
/home/estan/prefix/lib
/home/estan/prefix/lib/python3.7
/home/estan/prefix/lib/python3.7/site-packages
/home/estan/prefix/lib/python3.7/site-packages/testdist-1.0.0-py3.7.egg-info
estan@newton:~/testdist$

Notice how in the written record.txt, the path to the extension is wrong ("testext.cpytho" is missing), and the extension is not installed.

I did a little digging and found that the reason is install_lib.get_outputs() returns the wrong result for extensions that are built inplace: It assumes they are built in the build directory.

The attached patch fixes the problem. The failure of the included test case without the change to install_lib is:

======================================================================
FAIL: test_get_outputs_inplace_ext (distutils.tests.test_install_lib.InstallLibTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/estan/cpython/Lib/distutils/tests/test_install_lib.py", line 91, in test_get_outputs_inplace_ext
    self.assertEqual(expected, actual)
AssertionError: Lists differ: ['/tmp/tmptt4nhzgi/foo.cpython-37dm-x86_64-linux-gnu.so'] != ['/tmp/tmptt4nhzgi/dm-x86_64-linux-gnu.so']

First differing element 0:
'/tmp/tmptt4nhzgi/foo.cpython-37dm-x86_64-linux-gnu.so'
'/tmp/tmptt4nhzgi/dm-x86_64-linux-gnu.so'

- ['/tmp/tmptt4nhzgi/foo.cpython-37dm-x86_64-linux-gnu.so']
?                    --------------

+ ['/tmp/tmptt4nhzgi/dm-x86_64-linux-gnu.so']

----------------------------------------------------------------------

Notice the missing "foo.cpython-37". With the change to install_lib, the test passes.
msg285139 - (view) Author: Elvis Stansvik (Elvis Stansvik) Date: 2017-01-10 18:06
Not really related to this fix, but a colleague also prudently pointed out that perhaps an

   assert file[:prefix_len] == build_dir + os.sep

before the

   outputs.append(os.path.join(self.install_dir,
    file[prefix_len:]))

in the helper for pure modules (_pure_outputs after my patch is applied) would be in order, to assert that what is cut away is what was intended. 

That sounds like a good idea to me, but could it give false negatives on Windows (case-insensitive file systems? or file systems where both / and \ are allowed?).
msg287826 - (view) Author: Elvis Stansvik (estan) * Date: 2017-02-15 08:20
It's been over a month. It would be great if someone could at least acknowledge the bug. The current behavior is clearly wrong.

I realize changes to distutils is perhaps not the most fun thing to review, but I'd like some kind of fix for this to go in. I have a hard time seeing anyone is relying on this broken behavior or have put in workarounds for it, so provided the fix is good, it should have minimal impact.
History
Date User Action Args
2017-02-15 08:20:47estansetnosy: + estan
messages: + msg287826
2017-01-10 18:06:51Elvis Stansviksetmessages: + msg285139
2017-01-10 15:48:36Elvis Stansviksetversions: + Python 3.7
2017-01-10 15:42:17Elvis Stansviksetfiles: + fix-install-lib-with-inplace-ext.patch
keywords: + patch
2017-01-10 15:37:15Elvis Stansvikcreate