Replace CMake Python variables with new ones from FindPython3 module#402
Replace CMake Python variables with new ones from FindPython3 module#402azeey merged 14 commits intogazebosim:ign-math6from Bi0T1N:cmake_python3_fixes
Conversation
Codecov Report
@@ Coverage Diff @@
## ign-math6 #402 +/- ##
==========================================
Coverage 99.65% 99.65%
==========================================
Files 67 67
Lines 6392 6392
==========================================
Hits 6370 6370
Misses 22 22 Continue to review full report at Codecov.
|
j-rivero
left a comment
There was a problem hiding this comment.
Mostly fine, a couple of small things
|
Hi @Bi0T1N , thank you for the contribution. Are you still interested in moving on with this PR? |
Sure, but I don't have time for that now. I'll try to come back to this over the next few weeks as I should have more time soon. |
Codecov Report
@@ Coverage Diff @@
## ign-math6 #402 +/- ##
==========================================
Coverage 99.38% 99.38%
==========================================
Files 75 75
Lines 7029 7029
==========================================
Hits 6986 6986
Misses 43 43 |
there is no equivalent variable in the new FindPython3 module that provides the path to debug libraries Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
- cannot use IgnPython because it doesn't allow to specify the components to be found - old code probably don't work on old CMake versions < 3.12 as the required FindPython3 module doesn't exist Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
This reverts commit 76ffaee. Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
|
What is the status on this from your side? Should I update it to match the new |
|
I just resolved the conflicts and will continue review. Thanks for your patience and your help with pull request reviews! |
Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
| # the code below can be removed; e.g. pybind needs Interpreter and Development components | ||
| # see https://pybind11.readthedocs.io/en/stable/cmake/index.html#new-findpython-mode | ||
| if(${CMAKE_VERSION} VERSION_LESS "3.12.0") | ||
| set(IGN_PYTHON_VERSION "3") |
There was a problem hiding this comment.
I added IGN_PYTHON_VERSION "3" here to make it work. It turns out Bionic doesn't have pybind11 2.2 anyway, so this would only be useful for users that build pybind11 from source on Bionic.
|
@j-rivero I believe your requested changes have been resolved. Can you take a look? |
|
Removing |
🦟 Bug fix
Fixes #401
Summary
Replaces the old variables with the new ones used by the FindPython3 module. Right now
IgnPythoncannot be used as it only searches for the interpreter but pybind11 also needs the development components, thus it provides a workaround. The Win32PYTHON_LIBRARIES(PYTHON_DEBUG_LIBRARIES) variable was also removed as there is no equivalent in the new CMake module for getting the debug libraries.Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.