From 54490c6053d51910f5f7c2160451ad4d36fe6946 Mon Sep 17 00:00:00 2001 From: makaimann Date: Wed, 28 Jul 2021 18:13:43 -0400 Subject: [PATCH] Fixes for building python wheels on manylinux2014 (#6947) This PR makes several fixes to the wheel building infrastructure for manylinux2014. don't statically link to Python (see: this section of PEP-513) use standard 'build' directory. I noticed that the wheel infrastructure uses directory 'build' to prepare the wheel anyway, so it doesn't make a lot of sense for us to use a separate working directory. I tried changing the name of the build directory in setuptools by setting member variables in initialize_options but that didn't seem to work and I decided it wasn't worth the effort. The wheel should be built in a clean environment anyway, i.e., docker. find the correct include directory and python library paths for building on manylinux2014 using skbuild. manylinux2014 has several versions of python installed in /opt/python. The CMake infrastructure can't find the necessary information (plus the libraries are deleted from the docker image to save space -- the library is just set to a generic libpython.a). Instead, this PR would manually set them via CMake options. to allow setting the python information, I extended the configure.sh script to allow directly setting CMake options. There are definitely other options here if you'd rather not change configure.sh. My reasoning was that it could be useful in other situations and I just marked it as an advanced feature. Another approach would be to use skbuild directly in the CMake infrastructure. I didn't like that solution because it would make the CMakeLists.txt more complicated and it wouldn't follow the standard CMake/Cython/Python approach which means we won't benefit from future improvements to that CMake infrastructure. Plus, this issue should only come up on systems with non-standard installations of Python, such as manylinux. This approach (setting CMake options) is basically what scikit-build does automatically if we had used that infrastructure directly. --- configure.sh | 8 ++++++-- src/api/python/CMakeLists.txt | 2 +- src/api/python/wheels/build_wheel.py | 14 +++++++++++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/configure.sh b/configure.sh index 4ba153a75..9338caf6d 100755 --- a/configure.sh +++ b/configure.sh @@ -74,6 +74,9 @@ Build limitations: --lib-only only build the library, but not the executable or the parser (default: off) +CMake Options (Advanced) + -DVAR=VALUE manually add CMake options + EOF exit 0 } @@ -146,6 +149,8 @@ lib_only=default #--------------------------------------------------------------------------# +cmake_opts="" + while [ $# -gt 0 ] do case $1 in @@ -290,6 +295,7 @@ do --dep-path=*) dep_path="${dep_path};${1##*=}" ;; --lib-only) lib_only=ON ;; + -D*) cmake_opts="${cmake_opts} $1" ;; -*) die "invalid option '$1' (try -h)";; @@ -313,8 +319,6 @@ if [ $werror != default ]; then export CXXFLAGS=-Werror fi -cmake_opts="" - [ $buildtype != default ] \ && cmake_opts="$cmake_opts -DCMAKE_BUILD_TYPE=$buildtype" diff --git a/src/api/python/CMakeLists.txt b/src/api/python/CMakeLists.txt index 27e36f538..7ee559a6a 100644 --- a/src/api/python/CMakeLists.txt +++ b/src/api/python/CMakeLists.txt @@ -92,7 +92,7 @@ target_include_directories(pycvc5 ${CMAKE_BINARY_DIR}/src # for cvc5_export.h ) -target_link_libraries(pycvc5 cvc5 ${PYTHON_LIBRARIES}) +target_link_libraries(pycvc5 cvc5) # Disable -Werror and other warnings for code generated by Cython. # Note: Visibility is reset to default here since otherwise the PyInit_... diff --git a/src/api/python/wheels/build_wheel.py b/src/api/python/wheels/build_wheel.py index 72b8f2468..f35dd5662 100644 --- a/src/api/python/wheels/build_wheel.py +++ b/src/api/python/wheels/build_wheel.py @@ -29,9 +29,10 @@ import shutil from setuptools import setup, Extension from setuptools.command.build_ext import build_ext +from skbuild.cmaker import CMaker from distutils.version import LooseVersion -WORKING_DIR="build-python-wheel" +WORKING_DIR="build" def get_project_src_path(): # expecting this script to be in src/api/python/wheels @@ -111,6 +112,17 @@ class CMakeBuild(build_ext): '--auto-download', '--lib-only', '--name='+WORKING_DIR] + # find correct Python include directory and library + # works even for nonstandard Python installations + # (e.g., on pypa/manylinux) + args.append('-DPYTHON_VERSION_STRING:STRING=' + \ + sys.version.split(' ')[0]) + python_version = CMaker.get_python_version() + args.append('-DPYTHON_INCLUDE_DIR:PATH=' + \ + CMaker.get_python_include_dir(python_version)) + args.append('-DPYTHON_LIBRARY:FILEPATH=' + \ + CMaker.get_python_library(python_version)) + config_filename = os.path.join(project_src_path, "configure.sh") subprocess.check_call([config_filename] + args) -- 2.30.2