-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport optional extension building for python 2.7 #107
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,20 @@ | |
|
||
try: | ||
from setuptools import setup, Extension | ||
from setuptools.command.build_ext import build_ext as base_build_ext | ||
except ImportError: | ||
from distutils.core import setup, Extension | ||
import warnings | ||
import sys | ||
val = sys.exc_info()[1] | ||
|
||
warnings.warn("import of setuptools failed %r" % val) | ||
from distutils.core import setup, Extension | ||
from distutils.command.build_ext import build_ext as base_build_ext | ||
|
||
import os | ||
import re | ||
import sys | ||
import logging | ||
|
||
# Get version without importing scandir because that will lock the | ||
# .pyd file (if scandir is already installed) so it can't be | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this code copied from somewhere, the Python 3.x optional=true code or something? Just curious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its a small reimplementation of the python3 mechanism, which has a context managers and a few more bits to sort it out |
||
|
||
# the extension is optional since in case of lack of c the api | ||
# there is a ctypes fallback and a slow python fallback | ||
|
||
def build_extension(self, ext): | ||
try: | ||
base_build_ext.build_extension(self, ext) | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this still fail as we still always include the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its just a warning on python2, i'm ignoring it here the `build_ext command is what's interpreting those |
||
|
||
|
||
|
@@ -58,5 +76,5 @@ | |
'Programming Language :: Python :: 3.5', | ||
'Programming Language :: Python :: 3.6', | ||
'Programming Language :: Python :: Implementation :: CPython', | ||
] | ||
], cmdclass={'build_ext': BuildExt}, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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