Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix find Python3 logic and macOS workflow #1367

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Follow-up to gazebosim/gz-sim#2249 and osrf/homebrew-simulation#2545

Summary

The homebrew formula for sdformat15 was updated in osrf/homebrew-simulation#2545 to explicitly set the Python3_EXECUTABLE cmake variable. This works when building sdformat15 on its own (Build Status), but it fails when building sdformat15 from source as a dependency of gz-physics8 (Build Status) due to the presence of an additional python version on the system.

This pull request changes the logic for finding python and moves it earlier. It's mostly based on the approach in gz-sim with the following changes:

  • Python is required to generate the Embedded.cc file, so find the Python3 Interpreter as REQUIRED even if SKIP_PYBIND11 is set. When bindings are not skipped, find Development as an optional component to preserve existing behavior that just warns if it is unable to build bindings. I've tested this with osrf/homebrew-simulation@3a5b29e using ci_matching_branch/ and it fixes the sdformat15 build as a dependency of gz-physics8 (Build Status).

I've targeted main since it's easiest to confirm the failure and fix using these CI jobs, and I'll backport once this is merged.

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.

@scpeters scpeters requested a review from azeey as a code owner January 31, 2024 19:39
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Jan 31, 2024
Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b2ae616) 92.41% compared to head (e99b6c8) 92.41%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1367   +/-   ##
=======================================
  Coverage   92.41%   92.41%           
=======================================
  Files         134      134           
  Lines       17669    17669           
=======================================
  Hits        16328    16328           
  Misses       1341     1341           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scpeters scpeters merged commit 3c8c907 into main Jan 31, 2024
12 checks passed
@scpeters scpeters deleted the scpeters/fix_find_python branch January 31, 2024 21:34
scpeters added a commit that referenced this pull request Jan 31, 2024
* Find Python3 with find_package instead of GzPython,
  adapting the approach from gz-sim.
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Signed-off-by: Steve Peters <[email protected]>
scpeters added a commit that referenced this pull request Jan 31, 2024
* Find Python3 with find_package instead of GzPython,
  adapting the approach from gz-sim.
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Signed-off-by: Steve Peters <[email protected]>
scpeters added a commit that referenced this pull request Feb 2, 2024
* Find Python3 with find_package instead of GzPython,
  adapting the approach from gz-sim.
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Signed-off-by: Steve Peters <[email protected]>
@azeey azeey mentioned this pull request May 28, 2024
9 tasks
scpeters added a commit that referenced this pull request Jun 24, 2024
* Find Python3 with find_package instead of GzPython,
  adapting the approach from gz-sim.

Signed-off-by: Steve Peters <[email protected]>
azeey pushed a commit that referenced this pull request Jul 2, 2024
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Signed-off-by: Steve Peters <[email protected]>
azeey pushed a commit that referenced this pull request Jul 2, 2024
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Signed-off-by: Steve Peters <[email protected]>
scpeters added a commit that referenced this pull request Jul 2, 2024
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Partial backport of #1367.

Signed-off-by: Steve Peters <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
scpeters added a commit that referenced this pull request Jul 2, 2024
* macos workflow: use brew --prefix, not /usr/local
* macos workflow: set Python3_EXECUTABLE cmake var

Partial backport of #1367.

Signed-off-by: Steve Peters <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants