Skip to content
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

internal: Refactor setup.py c-extension building #1191

Merged
merged 18 commits into from
Feb 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ jobs:
resource_class: *resource_class
steps:
- checkout
- *restore_cache_step
# Create and activate a Python3.7 virtualenv
- run: virtualenv --python python3.7 .venv/build

Expand All @@ -86,7 +85,6 @@ jobs:
# Ensure package long description is valid and will render
# https://github.com/pypa/twine/tree/6c4d5ecf2596c72b89b969ccc37b82c160645df8#twine-check
- run: .venv/build/bin/twine check dist/*
- *save_cache_step

tracer:
docker:
Expand Down
26 changes: 26 additions & 0 deletions ddtrace/vendor/msgpack/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
__all__ = ["get_extensions"]

from setuptools import Extension
import sys


def get_extensions():
libraries = []
if sys.platform == "win32":
libraries.append("ws2_32")

macros = []
if sys.byteorder == "big":
macros = [("__BIG_ENDIAN__", "1")]
else:
macros = [("__LITTLE_ENDIAN__", "1")]

ext = Extension(
"ddtrace.vendor.msgpack._cmsgpack",
sources=["ddtrace/vendor/msgpack/_cmsgpack.cpp"],
libraries=libraries,
include_dirs=["ddtrace/vendor/"],
define_macros=macros,
)

return [ext]
7 changes: 7 additions & 0 deletions ddtrace/vendor/wrapt/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
__all__ = ["get_extensions"]

from setuptools import Extension


def get_extensions():
return [Extension("ddtrace.vendor.wrapt._wrappers", sources=["ddtrace/vendor/wrapt/_wrappers.c"],)]
87 changes: 56 additions & 31 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,40 @@

from distutils.command.build_ext import build_ext
from distutils.errors import CCompilerError, DistutilsExecError, DistutilsPlatformError
from setuptools import setup, find_packages, Extension
from setuptools import setup, find_packages
from setuptools.command.test import test as TestCommand


HERE = os.path.dirname(os.path.abspath(__file__))


def load_module_from_project_file(mod_name, fname):
"""
Helper used to load a module from a file in this project

DEV: Loading this way will by-pass loading all parent modules
e.g. importing `ddtrace.vendor.psutil.setup` will load `ddtrace/__init__.py`
which has side effects like loading the tracer
"""
fpath = os.path.join(HERE, fname)

if sys.version_info >= (3, 5):
import importlib.util

spec = importlib.util.spec_from_file_location(mod_name, fpath)
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
return mod
elif sys.version_info >= (3, 3):
from importlib.machinery import SourceFileLoader

return SourceFileLoader(mod_name, fpath).load_module()
else:
import imp

return imp.load_source(mod_name, fpath)


class Tox(TestCommand):

user_options = [("tox-args=", "a", "Arguments to pass to tox")]
Expand Down Expand Up @@ -56,7 +86,6 @@ def run_tests(self):
[visualization docs]: https://docs.datadoghq.com/tracing/visualization/
"""

# psutil used to generate runtime metrics for tracer
# enum34 is an enum backport for earlier versions of python
# funcsigs backport required for vendored debtcollector
install_requires = ["psutil>=5.0.0", "enum34; python_version<'3.4'", "funcsigs>=1.0.0;python_version=='2.7'"]
Expand Down Expand Up @@ -95,14 +124,7 @@ def run_tests(self):
)


# The following from here to the end of the file is borrowed from wrapt's and msgpack's `setup.py`:
# https://github.com/GrahamDumpleton/wrapt/blob/4ee35415a4b0d570ee6a9b3a14a6931441aeab4b/setup.py
# https://github.com/msgpack/msgpack-python/blob/381c2eff5f8ee0b8669fd6daf1fd1ecaffe7c931/setup.py
# These helpers are useful for attempting build a C-extension and then retrying without it if it fails

libraries = []
if sys.platform == "win32":
libraries.append("ws2_32")
build_ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError, IOError, OSError)
else:
build_ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
Expand All @@ -112,49 +134,52 @@ class BuildExtFailed(Exception):
pass


# Attempt to build a C-extension, catch and throw a common/custom error if there are any issues
# Attempt to build a C-extension, catch exceptions so failed building skips the extension
# DEV: This is basically what `distutils`'s' `Extension(optional=True)` does
class optional_build_ext(build_ext):
def run(self):
try:
build_ext.run(self)
except DistutilsPlatformError:
raise BuildExtFailed()
except DistutilsPlatformError as e:
extensions = [ext.name for ext in self.extensions]
print("WARNING: Failed to build extensions %r, skipping: %s" % (extensions, e))

def build_extension(self, ext):
try:
build_ext.build_extension(self, ext)
except build_ext_errors:
raise BuildExtFailed()
except build_ext_errors as e:
print("WARNING: Failed to build extension %s, skipping: %s" % (ext.name, e))


macros = []
if sys.byteorder == "big":
macros = [("__BIG_ENDIAN__", "1")]
else:
macros = [("__LITTLE_ENDIAN__", "1")]
def get_exts_for(name):
try:
mod = load_module_from_project_file(
"ddtrace.vendor.{}.setup".format(name), "ddtrace/vendor/{}/setup.py".format(name)
)
return mod.get_extensions()
except Exception as e:
print("WARNING: Failed to load %s extensions, skipping: %s" % (name, e))
return []


# Try to build with C extensions first, fallback to only pure-Python if building fails
try:
all_exts = []
for extname in ("msgpack", "wrapt"):
exts = get_exts_for(extname)
if exts:
all_exts.extend(exts)

kwargs = copy.deepcopy(setup_kwargs)
kwargs["ext_modules"] = [
Extension("ddtrace.vendor.wrapt._wrappers", sources=["ddtrace/vendor/wrapt/_wrappers.c"],),
Extension(
"ddtrace.vendor.msgpack._cmsgpack",
sources=["ddtrace/vendor/msgpack/_cmsgpack.cpp"],
libraries=libraries,
include_dirs=["ddtrace/vendor/"],
define_macros=macros,
),
]
kwargs["ext_modules"] = all_exts
# DEV: Make sure `cmdclass` exists
kwargs.setdefault("cmdclass", dict())
kwargs["cmdclass"]["build_ext"] = optional_build_ext
setup(**kwargs)
except BuildExtFailed:
except Exception as e:
# Set `DDTRACE_BUILD_TRACE=TRUE` in CI to raise any build errors
if os.environ.get("DDTRACE_BUILD_RAISE") == "TRUE":
raise

print("WARNING: Failed to install wrapt/msgpack C-extensions, using pure-Python wrapt/msgpack instead")
print("WARNING: Failed to install with ddtrace C-extensions, falling back to pure-Python only extensions: %s" % e)
setup(**setup_kwargs)
7 changes: 6 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ envlist =
benchmarks-{py27,py34,py35,py36,py37}

[testenv]
# Always re-run `setup.py develop` to ensure the proper C-extension .so files are created
# DEV: If we don't do this sometimes CircleCI gets messed up and only has the py27 .so
# meaning running on py3.x will fail
# https://stackoverflow.com/questions/57459123/why-do-i-need-to-run-tox-twice-to-test-a-python-package-with-c-extension
commands_pre={envpython} {toxinidir}/setup.py develop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure i understand that but ok

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly... same... this was super duper confusing to debug, this was the only thing that fixed it.

usedevelop = True
basepython =
py27: python2.7
Expand All @@ -142,7 +147,6 @@ deps =
pytest-django
pytest-mock
opentracing
psutil
# test dependencies installed in all envs
mock
# force the downgrade as a workaround
Expand Down Expand Up @@ -839,6 +843,7 @@ exclude=
.ddtox,.tox,
.git,__pycache__,
.eggs,*.egg,
build,
# We shouldn't lint our vendored dependencies
ddtrace/vendor/
# Ignore:
Expand Down