Skip to content

Rotary collection: gz-math, gz-common#3368

Merged
scpeters merged 2 commits intomasterfrom
ci_matching_branch/rotary_part_2
Mar 4, 2026
Merged

Rotary collection: gz-math, gz-common#3368
scpeters merged 2 commits intomasterfrom
ci_matching_branch/rotary_part_2

Conversation

@scpeters
Copy link
Member

@scpeters scpeters commented Mar 4, 2026

This splits out the gz-rotary-math and gz-rotary-common formulae from #3363 since they are ready to merge. Also removes the freeimage dependency from gz-rotary-common in 9cb0813 since gazebosim/gz-common#725 has merged to main.

Follow-up to #3287, part of the following issues:

scpeters added 2 commits March 4, 2026 04:01
Signed-off-by: Steve Peters <scpeters@intrinsic.ai>
Signed-off-by: Steve Peters <scpeters@intrinsic.ai>
@scpeters
Copy link
Member Author

scpeters commented Mar 4, 2026

Copy link
Collaborator

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

Two minor comments, good to my eyes

target_link_libraries(test_cmake gz-common::gz-common)
EOS
system "pkg-config", "gz-common"
# cflags = `pkg-config --cflags gz-common`.split
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is fine to leave this commented here? re-enable or removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's been broken since #2774 due to a missing fmt dependency

it's probably fixable, but I think we aren't prioritizing pkg-config support, so we haven't tried to fix it. As long as we are installing .pc files, I have left these tests commented out rather than deleting them

pythons.each do |python|
system python.opt_libexec/"bin/python", "-c", "import gz.math"
end
system Formula["python3"].opt_libexec/"bin/python", "-c", "import gz.math"
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity, why is this python call different here than the ones inside the pythons.each loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

the pythons.each loop checks each python version declared as a dependency of this formula, which are all declared with explicit versions (like python@3.14). We also want to ensure that our bindings work with the default python3 formula in homebrew (which is defined by an alias to one specific version). I added this check after the default version was changed to python@3.14 to give us quicker notice when the default python version changes in homebrew-core if we weren't already aware

@scpeters
Copy link
Member Author

scpeters commented Mar 4, 2026

I'm going to merge to unblock version bump PRs but we can continue the discussion and address in follow-up PRs if needed

@scpeters scpeters merged commit 41acf04 into master Mar 4, 2026
1 check passed
@scpeters scpeters deleted the ci_matching_branch/rotary_part_2 branch March 4, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants