gazebosim / gz-transport

Transport library for component communication based on publication/subscription and service calls.
https://gazebosim.org
Apache License 2.0
29 stars 36 forks source link

Find Python3 directly, not with GzPython #472

Closed scpeters closed 5 months ago

scpeters commented 5 months ago

🦟 Bug fix

Motivated by https://github.com/gazebosim/gz-sim/issues/2249.

Summary

While testing the use of -DPython3_EXECUTABLE=$(which python3) to aid in finding the right version of python3 on macOS (see https://github.com/osrf/homebrew-simulation/pull/2543), I ran into some cmake configuration issues when testing locally (the bottle build seems to be progressing fine though). I was able to fix my local build by consolidating the logic for finding python3 by removing the use of gz-cmake's GzPython in favor of a single call to find_package(Python3). This fixes setting Python3_EXECUTABLE to specify which python version to use and is needed on macOS.

As an aside, I'm going to suggest that we deprecate GzPython on main since it's better and simpler to just find python directly with modern versions of cmake.

Check the diff without whitespace to see a simpler version of the changes without the indentation.

Checklist

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.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (764a04b) 87.80% compared to head (505537f) 87.94%. Report is 1 commits behind head on gz-transport13.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## gz-transport13 #472 +/- ## ================================================== + Coverage 87.80% 87.94% +0.14% ================================================== Files 59 59 Lines 5699 5974 +275 ================================================== + Hits 5004 5254 +250 - Misses 695 720 +25 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

scpeters commented 5 months ago

windows failed to find python development, as its search for gz-msgs found a version of python that may have derailed the later search. I'm going to try moving the python search logic before the search for gz-msgs

scpeters commented 5 months ago

I believe this find logic is fixed as of https://github.com/gazebosim/gz-transport/pull/472/commits/505537f5a99fe6422629f31390d0bb1fb9fdaa83

I used ci_matching_branch/ to run some CI jobs to reproduce the failure with gz-transport13 built from source and then to confirm the fix with this branch