Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cmake extras: only find Python3 if needed #479

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

scpeters
Copy link
Member

@scpeters scpeters commented Jan 9, 2025

🦟 Bug fix

Fixes gz-transport python bindings

Summary

If Python3 has already been found, we shouldn't search again since it may find different components.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

If Python3 has already been found, we shouldn't search
again since it may find different components.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Member Author

scpeters commented Jan 9, 2025

testing gz-transport14 with this PR in gazebosim/gz-transport#566

checking that python bindings are built in Build Status https://build.osrfoundation.org/job/gz_transport-ci-pr_any-homebrew-amd64/217/

@scpeters
Copy link
Member Author

scpeters commented Jan 9, 2025

testing gz-transport14 with this PR in gazebosim/gz-transport#566

checking that python bindings are built in Build Status https://build.osrfoundation.org/job/gz_transport-ci-pr_any-homebrew-amd64/217/

I see python tests were run successfully in that build, so I think this fixes the issue

Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I ran the following and confirmed that Python3_Development_FOUND is set to false after the second call to find_package.

cmake_minimum_required(VERSION 3.5)
project(test_cmake)
include(CMakePrintHelpers)

find_package(Python3 REQUIRED COMPONENTS Development Interpreter)
cmake_print_variables(Python3_Development_FOUND)
cmake_print_variables(Python3_Interpreter_FOUND)
message("Now calling find_package again")
find_package(Python3 REQUIRED COMPONENTS Interpreter)
cmake_print_variables(Python3_Development_FOUND)
cmake_print_variables(Python3_Interpreter_FOUND)

@scpeters
Copy link
Member Author

LGTM!

I ran the following and confirmed that Python3_Development_FOUND is set to false after the second call to find_package.

thanks! I meant to clarify what I saw in the gz-transport build logs that made me suspect this was the issue. I'll post it here for posterity.

From Build Status https://build.osrfoundation.org/job/gz_transport-ci-gz-transport14-noble-amd64/45/:

-- ====== Finding Dependencies ======
-- Found Python3: /usr/bin/python3 (found version "3.12.3") found components: Interpreter Development Development.Module Development.Embed 
-- Found Protobuf: /usr/lib/x86_64-linux-gnu/libprotobuf.so (found version "3.21.12") 
...
-- Looking for gz-msgs11 -- found version 11.0.1
-- Searching for dependencies of gz-msgs11
-- Looking for gz-utils3 -- found version 3.1.0
-- Looking for gz-math8 -- found version 8.0.0
-- Searching for dependencies of gz-math8
-- Looking for gz-utils3 -- found version 3.1.0
-- Checking for module 'tinyxml2'
--   Found tinyxml2, version 10.0.0
-- Found Python3: /usr/bin/python3 (found version "3.12.3") found components: Interpreter 
-- Looking for gz-msgs11 - found

that second Found Python3 message occurs while finding gz-msgs, which is what pointed me in this direction

in the test build using this branch, I see the following when finding gz-msgs:

-- Looking for gz-msgs11 -- found version 11.0.1
-- Searching for dependencies of gz-msgs11
-- Looking for gz-utils3 -- found version 3.1.0
-- Looking for gz-math8 -- found version 8.1.0
-- Searching for dependencies of gz-math8
-- Looking for gz-utils3 -- found version 3.1.0
-- Checking for module 'tinyxml2'
--   Found tinyxml2, version 10.0.0
-- Looking for gz-msgs11 - found

@scpeters scpeters merged commit 373b6da into gz-msgs11 Jan 10, 2025
11 checks passed
@scpeters scpeters deleted the scpeters/extras_only_find_python_if_needed branch January 10, 2025 23:02
@scpeters
Copy link
Member Author

https://github.com/Mergifyio backport gz-msgs10

Copy link
Contributor

mergify bot commented Feb 10, 2025

backport gz-msgs10

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Feb 10, 2025
If Python3 has already been found, we shouldn't search
again since it may find different components.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
(cherry picked from commit 373b6da)
scpeters added a commit that referenced this pull request Feb 10, 2025
If Python3 has already been found, we shouldn't search
again since it may find different components.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
(cherry picked from commit 373b6da)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏛️ ionic Gazebo Ionic 🪵 jetty Gazebo Jetty
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants