-
Notifications
You must be signed in to change notification settings - Fork 70
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
Permit building python bindings separately from gz-math library #636
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
646de60
Permit to build bindings against an external gz-math
traversaro d2bae80
Fix python install path
scpeters a670b65
Reference brew install instructions in tutorial
scpeters 54978b9
Add details about building python bindings
scpeters cb32516
Use same required cmake version
scpeters 63d16a8
Remove unneeded call to enable_testing()
scpeters b475bdc
Merge branch 'gz-math8' into scpeters/build_python_bindings_separately
scpeters 06b8ab8
Find pybind11 in just one place
scpeters File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid about keeping this number in sync with bumps. As a workaround we can put a comment in the main
CMakeLists.txt
pointing to here. In the mid-term, it would be nice to integrate our CMake code to grab the version/name from git directly, probably using something like https://github.com/LecrisUT/CMakeExtraUtils/blob/main/cmake/DynamicVersion.mdThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a big fan of using git for version information as fails for tarballs. Something a bit ugly but effective could be to grep the major version from the parent folder CMakeLists.txt .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we could extract the version numbers from package.xml, since we also have a workflow to keep those versions in sync
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prototype for extracting version numbers from package.xml: #639
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've also opened gazebosim/gz-cmake#456 adding the python script from #639 and a cmake helper function. If you don't mind approving this PR for now so we can start to fix CI, I will follow-up by using the gz-cmake helper once it has been merged and released