From d14634e0e59c0799228e2eb1d9665a1a03a2a87a Mon Sep 17 00:00:00 2001 From: cyyever Date: Wed, 25 Sep 2024 09:30:32 +0800 Subject: [PATCH] Revert Python finding logic and some CMake fixes for Windows (#6382) ### Description Revert the python finding logic introduced by #6381 due to unable to handle Conda builds. However, the Python executable detection was improved. In addition, some Windows warnings were re-enabled, and the hidden visibility of onnx_cpp2py_export was fixed. ### Motivation and Context For better code. --------- Signed-off-by: cyy --- CMakeLists.txt | 47 +++++++++++++++++++---------------------------- setup.py | 20 +++++++++++++++++++- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8cdce4da6c6..7ffab8e0070 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -100,10 +100,16 @@ if(ONNX_DISABLE_EXCEPTIONS) endif() endif() +if(DEFINED PYTHON_EXECUTABLE) + message(STATUS "Use Python_EXECUTABLE ${Python_EXECUTABLE}") +endif() + +# find_package Python has replaced PythonInterp and PythonLibs since cmake 3.12 +# Use the following command in the future; now this is only compatible with the latest pybind11 +# find_package(Python ${PY_VERSION} COMPONENTS Interpreter Development REQUIRED) +find_package(PythonInterp ${PY_VERSION} REQUIRED) if(BUILD_ONNX_PYTHON) - find_package(Python ${PY_VERSION} COMPONENTS Interpreter Development REQUIRED) -else() - find_package(Python ${PY_VERSION} COMPONENTS Interpreter REQUIRED) + find_package(PythonLibs ${PY_VERSION}) endif() if(CMAKE_SYSTEM_NAME STREQUAL "AIX") @@ -358,7 +364,7 @@ function(RELATIVE_PROTOBUF_GENERATE_CPP NAME SRCS HDRS ROOT_DIR DEPEND) endif() add_custom_command(OUTPUT "${GENERATED_PROTO}" - COMMAND Python::Interpreter "${GEN_PROTO_PY}" + COMMAND "${PYTHON_EXECUTABLE}" "${GEN_PROTO_PY}" ARGS ${GEN_PROTO_ARGS} DEPENDS ${INFILE} COMMENT "Running gen_proto.py on ${INFILE}" @@ -541,8 +547,8 @@ if(BUILD_ONNX_PYTHON) add_library(onnx_cpp2py_export MODULE "${ONNX_ROOT}/onnx/cpp2py_export.cc") set_target_properties(onnx_cpp2py_export PROPERTIES PREFIX "") - set_target_properties(onnx_cpp2py_export - PROPERTIES COMPILE_FLAGS "-fvisibility=hidden") + set_target_properties(onnx_cpp2py_export PROPERTIES CXX_VISIBILITY_PRESET hidden) + set_target_properties(onnx_cpp2py_export PROPERTIES VISIBILITY_INLINES_HIDDEN 1) set_target_properties(onnx_cpp2py_export PROPERTIES SUFFIX ${PY_EXT_SUFFIX}) set_target_properties(onnx_cpp2py_export PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}) @@ -563,7 +569,7 @@ if(BUILD_ONNX_PYTHON) target_include_directories(onnx_cpp2py_export PUBLIC "${pybind11_INCLUDE_DIRS}" - "${PYTHON_INCLUDE_DIRS}") + "${PYTHON_INCLUDE_DIR}") if(APPLE) set_target_properties(onnx_cpp2py_export @@ -600,20 +606,14 @@ if(BUILD_ONNX_PYTHON) target_link_libraries(onnx_cpp2py_export PRIVATE ${PYTHON_LIBRARIES}) target_compile_options(onnx_cpp2py_export PRIVATE /MP + /wd4146 # unary minus operator applied to unsigned type, + # result still unsigned /wd4244 # 'argument': conversion from 'google:: # protobuf::uint64' to 'int', possible # loss of data /wd4267 # Conversion from 'size_t' to 'int', # possible loss of data - /wd4996 # The second parameter is ignored. ${EXTRA_FLAGS}) - if(ONNX_USE_PROTOBUF_SHARED_LIBS) - target_compile_options(onnx_cpp2py_export - PRIVATE /wd4251 # 'identifier' : class 'type1' needs to - # have dll-interface to be used by - # clients of class 'type2' - ) - endif() add_msvc_runtime_flag(onnx_cpp2py_export) add_onnx_global_defines(onnx_cpp2py_export) endif() @@ -629,6 +629,8 @@ endif() if(MSVC) target_compile_options(onnx_proto PRIVATE /MP + /wd4146 # unary minus operator applied to unsigned type, + # result still unsigned /wd4244 #'argument': conversion from 'google:: #protobuf::uint64' to 'int', possible # loss of data @@ -637,25 +639,14 @@ if(MSVC) ${EXTRA_FLAGS}) target_compile_options(onnx PRIVATE /MP + /wd4146 # unary minus operator applied to unsigned type, + # result still unsigned /wd4244 # 'argument': conversion from 'google:: # protobuf::uint64' to 'int', possible # loss of data /wd4267 # Conversion from 'size_t' to 'int', # possible loss of data - /wd4996 # The second parameter is ignored. ${EXTRA_FLAGS}) - if(ONNX_USE_PROTOBUF_SHARED_LIBS) - target_compile_options(onnx_proto - PRIVATE /wd4251 # 'identifier' : class 'type1' needs to - # have dll-interface to be used by - # clients of class 'type2' - ) - target_compile_options(onnx - PRIVATE /wd4251 # 'identifier' : class 'type1' needs to - # have dll-interface to be used by - # clients of class 'type2' - ) - endif() add_msvc_runtime_flag(onnx_proto) add_msvc_runtime_flag(onnx) set(onnx_static_library_flags diff --git a/setup.py b/setup.py index 7d2cb9e502c..53a1f2429ff 100644 --- a/setup.py +++ b/setup.py @@ -103,6 +103,24 @@ def get_ext_suffix(): return sysconfig.get_config_var("EXT_SUFFIX") +def get_python_execute(): + if WINDOWS: + return sys.executable + # Try to search more accurate path, because sys.executable may return a wrong one, + # as discussed in https://github.com/python/cpython/issues/84399 + python_dir = os.path.abspath( + os.path.join(sysconfig.get_path("include"), "..", "..") + ) + if os.path.isdir(python_dir): + python_bin = os.path.join(python_dir, "bin", "python3") + if os.path.isfile(python_bin): + return python_bin + python_bin = os.path.join(python_dir, "bin", "python") + if os.path.isfile(python_bin): + return python_bin + return sys.executable + + ################################################################################ # Customized commands ################################################################################ @@ -159,7 +177,7 @@ def run(self): cmake_args = [ CMAKE, f"-DPYTHON_INCLUDE_DIR={sysconfig.get_path('include')}", - f"-DPYTHON_EXECUTABLE={sys.executable}", + f"-DPYTHON_EXECUTABLE={get_python_execute()}", "-DBUILD_ONNX_PYTHON=ON", "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON", f"-DONNX_NAMESPACE={ONNX_NAMESPACE}",