Windows python debug #153
Windows python debug #153
Conversation
| @@ -211,7 +222,11 @@ if(PYTHONINTERP_FOUND) | |||
| endif() | |||
|
|
|||
| if(WIN32) | |||
| set(PythonExtra_EXTENSION_EXTENSION ".pyd") | |||
| if("${CMAKE_BUILD_TYPE} " STREQUAL "Debug ") | |||
| set(PythonExtra_EXTENSION_EXTENSION "_d.pyd") | |||
There was a problem hiding this comment.
Should this also happen when building debug but not having a debug interpreter?
There was a problem hiding this comment.
Not necessarily, but if one doesn't have a debug interpreter, Visual Studio won't be able to link against it and the compilation will fail. So the suffix of the extensions would not matter in that case.
There was a problem hiding this comment.
If this can be detected at CMake time wouldn't it be better to print a clear error message then let it fail later with a not that clear error?
There was a problem hiding this comment.
PythonExtra now performs the existence test of the debug interpreter and throw an error if unable to find it. 52eb0f4
mikaelarguedas
added
in progress
mikaelarguedas
force-pushed
the
windows_python_debug
branch
from
e91fa48
to
5daba12
Compare
|
Round2: the goal here is to use the python_d.exe only when needed instead of for the entire build process.
CI passed with the expected result: http://ci.ros2.org/job/ci_windows/1481/ Note: people who want to build ament_python packages to generate executables (setuptools) will need to call |
mikaelarguedas
added
in review
|
The new variable should following the naming scheme of other CMake variables (which append the build type for customization). So I would suggest |
|
Makes sense. Renamed it in all packages using it. |
|
@ros2/ros2_team please review this and the associated PRs |
| @@ -128,6 +128,18 @@ if(PYTHONINTERP_FOUND) | |||
| message(STATUS "Using PythonExtra_LIBRARIES: ${PythonExtra_LIBRARIES}") | |||
| else() | |||
| find_package(PythonLibs 3.5 REQUIRED) | |||
| if(WIN32 AND "${CMAKE_BUILD_TYPE} " STREQUAL "Debug ") | |||
There was a problem hiding this comment.
Please remove the quotes and explicit variable expansion as well as the trailing space.
Same below.
|
Beside the comments +1. |
|
needs rebase and a round of CI before merging |
mikaelarguedas
force-pushed
the
windows_python_debug
branch
from
451ca5a
to
b8bb1a7
Compare
|
new CI job to validate after rebase: |
|
@dirk-thomas does the +1 covers the associated PRs? |
|
I'll fix the variable expansion/whitespace in the associated PRs too |
|
I just review the referenced PRs individually. |
|
thanks! |
|
Windows Debug job: |
|
CI passed, so I'll merge this now |
* added _d suffix to generated extensions * look for python debug interpreter * adding error message and PYTHON_DBG_EXECUTABLE variable * remove trailing whitespace * rename PYTHON_DBG_EXECUTABLE to PYTHON_EXECUTABLE_DEBUG * quote entire error message * cmake3.5 remove trailing whitespace
After discussion with @dirk-thomas, we will modify the python interpreter only for packages generating/using python extensions and not for the entire build process. If people want to generate executables in Debug mode they'll have to invoke ament_tools with
python_d.This adds the suffix and cmake logic to use the python debug interpreter when generating python extensions.
CI with change: http://ci.ros2.org/job/ci_windows/1455/#showFailuresLink
Nightly: http://ci.ros2.org/view/nightly/job/nightly_win_deb/146/#showFailuresLink