classification
Title: distutils: isinstance checks fail with setuptools-monkeypatched Extension/Distribution
Type: behavior Stage: patch review
Components: Distutils Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Jens.Timmerman, berker.peksag, cgrohmann, djc, dstufft, eric.araujo, gmt, jason.coombs, koobs, ralf.gommers, scoder
Priority: normal Keywords: patch

Created on 2014-12-23 06:36 by gmt, last changed 2017-07-18 09:50 by koobs.

Files
File name Uploaded Description Edit
distutils_accomodate_extension_ducktypes.patch gmt, 2014-12-23 06:36 patch: allow distutils.extension.Extension to be monkey-patched with a duck-type of itself. review
distutils_accomodate_distribution_ducktypes.patch gmt, 2014-12-29 11:26 patch: likewise for distutils.dist.Distribution
0001-distutils-type-checks-can-fail-issue-23102.patch gmt, 2016-09-13 04:56 patch: allow distutils.extension.Extension to be monkey-patched with a duck-type of itself (respin 1)
0002-distutils-type-checks-can-fail-issue-23102.patch gmt, 2016-09-13 05:01 patch: allow distutils.dist.Distribution to be monkey-patched with a duck-type of itself (respin 1)
Messages (12)
msg233034 - (view) Author: Greg Turner (gmt) * Date: 2014-12-23 06:36
Clinical Presentation:
Sometimes a developer consuming distutils will write what seems like a perfectly sensible setup.py, but something inscrutable happens: setup.py bombs out claiming "error: each element of 'ext_modules' option must be an Extension instance or 2-tuple".

Prognosis:
Once she manages to capture this error in pdb (takes some doing), intrepid developer may discover that, bizarrely, it IS an Extension instance, yet, somehow, (not isinstance(that_thing, Extension)) == True.  Sooner or later she'll likely throw up her hands and follow some far-seeing dude's advice on StackOverflow([1]), which will probably work.

If she undertakes a more thorough investigation, she may figure out the true etiology (see below), at which point, various minor tweaks will likely present themselves to her as obvious situation-specific solutions to the problem.

Etiology:
Developer likely employed code early in her module like:

  from distutils.extension import Extension

  .
  . (some other imports)
  .

  setup(..., ext_modules = [
      Extension(...), 
      ...,
  ], ...)

What happened was that setuptools got imported by (presumably) some third-party code, at which point, setuptools monkey-patched distutils.extension.Extension(*), as is setuptools' SOP.

However, in setup.py, a reference to the un-monkey-patched Extension was already saved off as __main__.Extension (along with, in all probability, other un-patched distutils things, as folks tend to consistently use one style or another of import).  So __main__ calls (an un-monkey-patched version of) distutils.core.setup, which ultimately iterates through the list of Extensions, checking isinstance(item, Extension), or so, which, as can now be seen, is not going to be true.

So, the error message is correct, it just fails to mention the possibility that there are multiple things called "Extension" floating around with identical repr's.

Epidemiological Outlook:
Seemingly this is a rare condition, but when a case develops, it can be costly, due to the likelihood of misdiagnosis and/or partial remission and relapse.

One possible vaccine has been developed and is enclosed.  It has not been subjected to clinical trial, nor peer-review (until now).  It is enclosed as a patch which applies to python 2.7-3.5 and seems to do the trick in the particular case that was buggin' me (wish I could say it will do the trick for any non-broken use-case, but I can't, as if I made such a claim, I'd clearly jinx it).

--
* Arguably, this is a bug or misfeature of setuptools, as here setuptools appears to too liberally assume that, if its modules were even casually imported, then it's a good time to monkey-patch distutils.  However, IME, fixing this putative bug, threatens to be a non-trivial undertaking and cause a bunch of regressions and contingent hassles.

Background URLS:

[1] http://stackoverflow.com/questions/21594925/error-each-element-of-ext-modules-option-must-be-an-extension-instance-or-2-t

https://bitbucket.org/pypa/setuptools/issue/309

https://bugs.gentoo.org/show_bug.cgi?id=532708
msg233109 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-12-26 17:16
IMO this would be best fixed in setuptools, but it may not be possible.
msg233113 - (view) Author: Stefan Behnel (scoder) * Date: 2014-12-26 18:55
The same applies to the error "dist must be a Distribution instance" in cmd.py. See issue 23114.
msg233172 - (view) Author: Greg Turner (gmt) * Date: 2014-12-29 11:26
Here's the same deal more-or-less for issue 23114... this time I expended considerably less effort verifying that it's not a bad idea.  At a glance, though, it looks fine, and solved an instance of the issue 23114 problem I was able to repro on my box.

Patch context reduced so as to be applicable to all three pythons in one go.
msg233173 - (view) Author: Greg Turner (gmt) * Date: 2014-12-29 11:55
Perhaps it is worth addressing, briefly, the following hypothetical question, as a litmus test against the faint dis-encapsulation code-smell some folks might be picking up from this:

In a hypothetcial world without setuptools, would these changes have merit?

I'd say, technically, yes.  In the Extension case, we are just really trying to ask, "is it a tuple", and in the Distribution case, "Can we use this thing to finalize/reinitialize Commands?", so, in theory, at least, these isinstance() checks are less pythonic than the hasattr checks in my patches.

That stated, I think isinstance was the sensible way to code this in a vacuum, and obviously, I would never have thought this was a fantastic and self-evident proposal, were it not for these setuptools problems.
msg260244 - (view) Author: Ralf Gommers (ralf.gommers) Date: 2016-02-13 17:32
Any chance to get this merged?
msg276045 - (view) Author: Jens Timmerman (Jens.Timmerman) Date: 2016-09-12 13:45
I'm also regularly running into this, it is really annoying, Can I do anything to help getting this merged in?
msg276148 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-09-13 02:13
The patch doesn't apply cleanly:

applying http://bugs.python.org/file37554/distutils_accomodate_distribution_ducktypes.patch
patching file Lib/distutils/cmd.py
Hunk #1 FAILED at 51
1 out of 1 hunks FAILED -- saving rejects to file Lib/distutils/cmd.py.rej
abort: patch failed to apply

So the next step would be to update the patch.

We also need a test case to make sure that the patch fixes the problem reported in msg233034.

+1 for relying on duck typing. Perhaps we could also look for methods that are called in Command (e.g. get_command_obj(), run_command())
msg276172 - (view) Author: Greg Turner (gmt) * Date: 2016-09-13 04:56
Huzzah
msg276175 - (view) Author: Greg Turner (gmt) * Date: 2016-09-13 05:01
Double Huzzah.

I can probably cook up a test and even look at those method invocations in considerably less than the existing 1.5-year lifecycle of this bug, but not necessarily immediately due to reasons.
msg276177 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-09-13 05:13
Thanks for the updated patches. Since they both intend to fix the same problem, we can combine them in one patch file.
msg276216 - (view) Author: Jens Timmerman (Jens.Timmerman) Date: 2016-09-13 09:01
small setup.py file to reproduce this problem if people still had trouble reproducing (this works with the attached d002-distutils-type-checks-can-fail-issue-23102.patch)

```
#!/usr/bin/env python
''' Installation script that breaks '''

from distutils.command import install
from distutils.core import setup

class installer(install.install):
    def run(self):
        import setuptools
        install.install.run(self)


setup(name='test',
      cmdclass = dict(install=installer)
     )

```
run with: 
python setup.py install --user
result:
```
running install
running build
running install_egg_info
Traceback (most recent call last):
  File "mysetup.py", line 14, in <module>
    cmdclass = dict(install=installer)
  File "/usr/lib64/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/usr/lib64/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/lib64/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "mysetup.py", line 10, in run
    install.install.run(self)
  File "/usr/lib64/python2.7/distutils/command/install.py", line 575, in run
    self.run_command(cmd_name)
  File "/usr/lib64/python2.7/distutils/cmd.py", line 326, in run_command
    self.distribution.run_command(command)
  File "/usr/lib64/python2.7/distutils/dist.py", line 970, in run_command
    cmd_obj = self.get_command_obj(command)
  File "/usr/lib64/python2.7/distutils/dist.py", line 846, in get_command_obj
    cmd_obj = self.command_obj[command] = klass(self)
  File "/usr/lib64/python2.7/distutils/cmd.py", line 59, in __init__
    raise TypeError, "dist must be a Distribution instance"
TypeError: dist must be a Distribution instance
```
expected result:
```
running install
running build
running install_egg_info
Writing /home/jens/.local/lib/python2.7/site-packages/test-0.0.0-py2.7.egg-info
```
History
Date User Action Args
2017-07-18 09:50:05koobssetnosy: + koobs
2016-09-13 09:01:57Jens.Timmermansetmessages: + msg276216
2016-09-13 05:13:57berker.peksagsetmessages: + msg276177
stage: needs patch -> patch review
2016-09-13 05:01:02gmtsetfiles: + 0002-distutils-type-checks-can-fail-issue-23102.patch

messages: + msg276175
2016-09-13 04:56:02gmtsetfiles: + 0001-distutils-type-checks-can-fail-issue-23102.patch

messages: + msg276172
2016-09-13 02:13:17berker.peksagsetversions: + Python 3.7, - Python 3.4
nosy: + berker.peksag, jason.coombs

messages: + msg276148

stage: needs patch
2016-09-12 13:45:55Jens.Timmermansetnosy: + Jens.Timmerman
messages: + msg276045
2016-02-13 17:32:22ralf.gommerssetmessages: + msg260244
versions: + Python 3.6
2016-02-13 17:19:57ralf.gommerssetnosy: + ralf.gommers
2014-12-29 11:55:03gmtsetmessages: + msg233173
2014-12-29 11:26:00gmtsetfiles: + distutils_accomodate_distribution_ducktypes.patch

messages: + msg233172
2014-12-27 20:35:04cgrohmannsetnosy: + cgrohmann
2014-12-27 20:31:25djcsetnosy: + djc
2014-12-26 18:56:49eric.araujosettitle: distutils: tip-toe around quirks owing to setuptools monkey-patching Extension -> distutils: isinstance checks fail with setuptools-monkeypatched Extension/Distribution
2014-12-26 18:55:44scodersetnosy: + scoder
messages: + msg233113
2014-12-26 18:54:56eric.araujolinkissue23114 superseder
2014-12-26 17:16:37eric.araujosetmessages: + msg233109
versions: - Python 3.2, Python 3.3, Python 3.6
2014-12-23 14:19:07Arfreversetnosy: + Arfrever
2014-12-23 06:36:28gmtcreate