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

Add pybind11 as a build dependency #7

Merged
merged 4 commits into from
Jan 13, 2022
Merged

Add pybind11 as a build dependency #7

merged 4 commits into from
Jan 13, 2022

Conversation

chapulina
Copy link
Contributor

The 6.9.3~pre1 pre-release failed. I think the first step is to add pybind11 as a dependency, but there may be other issues. @j-rivero , is it ok to add a build dependency to a stable version?

Test build: Build Status

@j-rivero
Copy link
Contributor

j-rivero commented Jan 4, 2022

The 6.9.3~pre1 pre-release failed. I think the first step is to add pybind11 as a dependency, but there may be other issues. @j-rivero , is it ok to add a build dependency to a stable version?

I would not recommend to do it since it potentially affect downstream packaging work in distributions if the dependency is not available on them. In this case, if we threat as an optional dep, the worst case would be the lack of python bindings in the package which is not nice but probably packagers can deal with it. Speaking about Debian I don't remember anything in the policy against adding new dependencies if they are available and ABI/API stability is guarantee.

@chapulina
Copy link
Contributor Author

Thanks for the feedback, I think I moved it to the right place in a423f03?

The build is still failing though, I opened gazebosim/gz-math#360 to help debug it

Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina merged commit 9bfcc9b into main Jan 13, 2022
@chapulina chapulina deleted the chapulina/pybind branch January 13, 2022 21:19
@chapulina
Copy link
Contributor Author

I think I moved it to the right place in a423f03?

Now while testing #9 I think I didn't. We need pybind11 to be available during build time. So I'll move it back to be a build dependency.

if we threat as an optional dep, the worst case would be the lack of python bindings in the package which is not nice but probably packagers can deal with it.

This will be the situation.

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