backport optional extension building for python 2.7 #107
Conversation
setup.py
Outdated
| @@ -2,12 +2,15 @@ | |||
|
|
|||
| try: | |||
| from setuptools import setup, Extension | |||
| from setuptolls.command.build_ext import build_ext as base_build_ext | |||
There was a problem hiding this comment.
This should be setuptools, no? (Not tolls...)
There was a problem hiding this comment.
yikes, yes, im going to put in a warning with the exception when it fails
|
|
||
| warnings.warn("import of setuptools failed %r" % val) |
There was a problem hiding this comment.
I don't think this warning is necessary -- it falls back to distutils anyway. It seems to me this was added to debug a spelling mistake in an earlier commit. Now that we're importing it correctly, let's just leave this out.
There was a problem hiding this comment.
i'd like to leave it in so user see that setuptools is missing
from a packaging standpoint i'd like to completely remove the support for plain distutils if this was my project
| @@ -25,8 +33,18 @@ | |||
| long_description = f.read() | |||
|
|
|||
|
|
|||
| # the extension is optional since in case of lack of c the api | |||
| # there is a ctypes fallback and a slow python fallback | |||
| class BuildExt(base_build_ext): | |||
There was a problem hiding this comment.
Is this code copied from somewhere, the Python 3.x optional=true code or something? Just curious.
There was a problem hiding this comment.
its a small reimplementation of the python3 mechanism, which has a context managers and a few more bits to sort it out
| except Exception: | ||
| exception = sys.exc_info()[0] | ||
| logging.warn("building the %s failed with %s", ext.name, exception) | ||
|
|
||
| extension = Extension('_scandir', ['_scandir.c'], optional=True) |
There was a problem hiding this comment.
Won't this still fail as we still always include the optional=True flag here? Or is it just a warning and so we're ignoring it?
There was a problem hiding this comment.
its just a warning on python2, i'm ignoring it here
the `build_ext command is what's interpreting those
|
Thanks @benhoyt for merging this. 👍 Would you mind pinging this issue when a new release comes out? Thanks! |
|
Done! It's version 1.9.0. |
|
Awesome, thank you! |
this sorts out the issues i caused on python2.7 by backporting the approach of the python3 optional parameter to python2
this fixes regressions introduced in #106