Build executor Cython extension

[calebmarchent]

setup.py to create folly Cython bindings

Minimum required to get fbthrift to compile in OSS, so only includes executor

Cython requires that source files (pyx and pxd) are in a directory matching the extension name, to provide this a tree of symbolic links is created and the Cython compile happens in that directory.

FIXME: Handle systems that lack required tooling gracefully (don't cause build failure)

• Python3 > 3.6 available?
• Cython installed?

Test Plan:

Dimensions to consider in test plan:

• Distro:

  • Ubuntu 16.04

• Cython?

  • With / without Cython installed

• Python Version

  • Only test with Python3

• Install with alternative DESTDIR

• Configure CMake in source dir

• Configure CMake in _build dir

[calebmarchent]

Since creation of the original PR; I have added the following checks:

• Installation works with cmake from alternate dir
• Can install to an alternative location, (without root privileges)
• Gracefully handle environments lacking the tools to build the python extension
• Folly needs to be compiled with -fPIC (Position Independent Code), which it is not by default
• Resulting python extension tree contains many unexpected files, owning to overlap with pathname "folly/"

I have no further updates planned for this PR, pending review feedback

[simpkins]

Is this always false by default?

I think we should strongly discourage building libfolly as a shared library in most circumstances. When the cython build is enabled maybe we should compile the code twice: once as libfolly.a library to be used by downstream C++ dependencies, and a separate PIC build just for Cython users. Perhaps it would be good to have the Cython extension directly include all of the folly code rather than linking against folly as a separate shared library.

That said, I'm not really that familiar with the current python code in folly and how it gets used to know if bundling all of folly directly into it makes sense in practice. (This would potentially pose a challenge for dependencies that want to depend on folly both via the cython extension and directly as a C++ library.)

[calebmarchent]

I followed the example for singleton_thread_local_overload in top-level CMakeLists.txt.
I could look into, cherry-picking the objects required by the python extensions into code into the a new library; will see how far I get.

[simpkins]

I wouldn't bother with cherry-picking objects to create a new library, I was more just suggesting building two different versions of the existing library.

e.g., we could build libfolly.a and libfolly_pic.a, and the cython extension could link in libfolly_pic.a You could probably do this relatively easily with something like

add_library(folly_pic $<TARGET_OBJECTS:folly_base>)
set_property(TARGET folly_pic PROPERTY POSITION_INDEPENDENT_CODE 1)

[calebmarchent]

I have updated the diff to include a folly_pic library; but it pretty much means compiling all the sources twice. (I tested the suggested code:

add_library(folly_pic $<TARGET_OBJECTS:folly_base>)
set_property(TARGET folly_pic PROPERTY POSITION_INDEPENDENT_CODE 1)

but this resulted in a "folly_pic" which was compiled without -fPIC; because this option was not applied to the creation of the objects in folly_base.

I have also read quite a bit further on the subject of position independent code; while it may have a performance impact on the compiled code, as I understanding Darwin enable PIC by default and its mandatory for iOS.

https://eli.thegreenplace.net/2011/11/03/position-independent-code-pic-in-shared-libraries/
(specifically "The costs of PIC")

[simpkins]

Yeah, it will mean compiling everything twice, but I think this is probably the right thing to do. I would generally be inclined to:

• Add an option to control if the cython extension should be built or not, so that this can be disabled even if Cython is available on the system.
• If the building the Cython extension is enabled and BUILD_SHARED_LIBS was not explicitly enabled, do compile everything twice (once for the static libfolly.a library to be installed, and once for the libfolly_pic.a library to link into the extension). If PIC is unconditionally enabled one Mac or other systems we could potentially avoid double compiling there.

It's possible we might want/need another configuration option to control if the Cython extension links against folly statically (bundling all of the folly code directly into the extension) or dynamically. Linking against things dynamically is much more common for Python extensions. However, since folly provides no ABI compatibility guarantees across changes deploying folly as a shared library is somewhat dangerous. For a python extension we might be able to do something a little smarter by deploying folly as a shared library installed in the python extension directory, and including $ORIGIN in the extensions rpath, so that the python extension can find it but it would be found by normal C++ link commands. We might also want to stick the commit ID in the shared library's soname to make sure code can't run if it somehow ends up running with an unexpected version of the library.

[simpkins]

I'm assuming the dependencies on glog, double-conversion and iberty are only indirect dependencies through libfolly.

It would be nicer to pass this information in to the setup module, as the list of dependencies can be somewhat dynamic based on what was detected by CMake at folly's configure time.
In the CMake code, get_target_property(YOUR_VAR_NAME folly INTERFACE_LINK_LIBRARIES) will get the list of libraries that folly depends on.

[calebmarchent]

I shall experiment with it and see what I can do.

[calebmarchent]

I've spent quite a while looking into this: I cannot see an elegant way to pass the values form get_target_property(YOUR_VAR_NAME folly INTERFACE_LINK_LIBRARIES) through to setup.py. To give an example the output of:

Threads::Threads;/usr/lib/x86_64-linux-gnu/libboost_context.so;/usr/lib/x86_64-linux-gnu/libboost_chrono.so;/usr/lib/x86_64-linux-gnu/libboost_date_time.so;/usr/lib/x86_64-linux-gnu/libboost_filesystem.so;/usr/lib/x86_64-linux-gnu/libboost_program_options.so;/usr/lib/x86_64-linux-gnu/libboost_regex.so;/usr/lib/x86_64-linux-gnu/libboost_system.so;/usr/lib/x86_64-linux-gnu/libboost_thread.so;/usr/lib/x86_64-linux-gnu/libboost_atomic.so;/usr/lib/x86_64-linux-gnu/libpthread.so;/usr/lib/x86_64-linux-gnu/libdouble-conversion.so;/usr/lib/x86_64-linux-gnu/libglog.so;/usr/lib/x86_64-linux-gnu/libevent.so;/usr/lib/x86_64-linux-gnu/libssl.so;/usr/lib/x86_64-linux-gnu/libcrypto.so;/usr/lib/x86_64-linux-gnu/libz.so;/usr/lib/x86_64-linux-gnu/libbz2.so;/usr/lib/x86_64-linux-gnu/liblzma.so;/usr/lib/x86_64-linux-gnu/liblz4.so;/usr/lib/x86_64-linux-gnu/libzstd.so;/usr/lib/x86_64-linux-gnu/libsnappy.so;/usr/lib/x86_64-linux-gnu/libiberty.a;dl;gflags

In fact its probably easier to read the libfolly.pc (pkg-config) file back and use that from within setup.py.
This rubs against one of the most challenging parts of building with two systems, CMake and Cython's distutils.

[simpkins]

Getting the info from libfolly.pc would be nice if that isn't too hard.

Landing the current code as-is for now is also okay with me, and we can come back later to improve it if we need to. One thing I might check is if libelf and libdwarf need to be included here if they were found at configure time. If they are found FOLLY_USE_SYMBOLIZER will default to ON, and I think folly's init code may then depend on these libraries.

[facebook-github-bot]

@calebmarchent has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.