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

build: use correct platform suffix for Python native shared library #3678

Merged
21 changes: 15 additions & 6 deletions python/CMakeLists.txt
Expand Up @@ -44,16 +44,25 @@ endif()

# create the lib
add_library(pylibvw SHARED pylibvw.cc)

set_target_properties(pylibvw PROPERTIES PREFIX "")

# The python setup.py script should communicate what suffix it wants.
if(VW_PYTHON_SHARED_LIB_SUFFIX)
set_target_properties(pylibvw PROPERTIES SUFFIX ${VW_PYTHON_SHARED_LIB_SUFFIX})
else()
# fallback to previous suffix selection logic
if (NOT WIN32)
# OSX builds a .dylib and then attempts to find a .so. Just build a .so file and things work
set_target_properties(pylibvw PROPERTIES SUFFIX ".so")
else()
# Force Windows to build with a .pyd extension
set_target_properties(pylibvw PROPERTIES SUFFIX ".pyd")
endif()
endif()

if (NOT WIN32)
# Windows links dynamically
target_compile_definitions(pylibvw PUBLIC BOOST_PYTHON_STATIC_LIB)
# OSX builds a .dylib and then attempts to find a .so. Just build a .so file and things work
set_target_properties(pylibvw PROPERTIES SUFFIX ".so")
else()
# Force Windows to build with a .pyd extension
set_target_properties(pylibvw PROPERTIES SUFFIX ".pyd")
endif()

if(${CMAKE_VERSION} VERSION_LESS "3.12.0")
Expand Down
41 changes: 14 additions & 27 deletions setup.py
Expand Up @@ -55,41 +55,15 @@ def __init__(self, name):
Extension.__init__(self, name, sources=[])


def get_ext_filename_without_platform_suffix(filename):
from distutils.sysconfig import get_config_var

ext_suffix = get_config_var("EXT_SUFFIX")
name, ext = os.path.splitext(filename)

if not ext_suffix:
return filename

if ext_suffix == ext:
return filename

ext_suffix = ext_suffix.replace(ext, "")
idx = name.find(ext_suffix)

if idx == -1:
return filename
else:
return name[:idx] + ext


class BuildPyLibVWBindingsModule(_build_ext):
def get_ext_filename(self, ext_name):
# don't append the extension suffix to the binary name
# see https://stackoverflow.com/questions/38523941/change-cythons-naming-rules-for-so-files/40193040#40193040
filename = _build_ext.get_ext_filename(self, ext_name)
return get_ext_filename_without_platform_suffix(filename)

def run(self):
for ext in self.extensions:
self.build_cmake(ext)

_build_ext.run(self)

def build_cmake(self, ext):

# Make build directory
distutils.dir_util.mkpath(self.build_temp)

Expand All @@ -109,6 +83,19 @@ def build_cmake(self, ext):
"-DBUILD_TESTS=Off",
"-DWARNINGS=Off",
]

# This doesn't work as expected for Python3.6 and 3.7 on Windows.
# See bug: https://bugs.python.org/issue39825
if system == "Windows" and sys.version_info.minor < 8:
from distutils import sysconfig as distutils_sysconfig
required_shared_lib_suffix = distutils_sysconfig.get_config_var('EXT_SUFFIX')
else:
import sysconfig
required_shared_lib_suffix = sysconfig.get_config_var("EXT_SUFFIX")

if required_shared_lib_suffix is not None:
cmake_args += ["-DVW_PYTHON_SHARED_LIB_SUFFIX={}".format(required_shared_lib_suffix)]

if self.distribution.enable_boost_cmake is None:
# Add this flag as default since testing indicates its safe.
# But add a way to disable it in case it becomes a problem
Expand Down