Fix tf2_bullet dependency export#428
Merged
clalancette merged 2 commits intoros2:ros2from May 28, 2021
Merged
Conversation
clalancette
requested changes
May 28, 2021
Contributor
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
A few things to change, but this is definitely the right direction, thanks!
tf2_bullet/CMakeLists.txt
Outdated
| ament_export_include_directories(include) | ||
| ament_export_libraries(${PROJECT_NAME}) | ||
| ament_package() | ||
| ament_package(CONFIG_EXTRAS_POST bullet-extra.cmake) |
Contributor
There was a problem hiding this comment.
I think we can just use CONFIG_EXTRAS here:
Suggested change
| ament_package(CONFIG_EXTRAS_POST bullet-extra.cmake) | |
| ament_package(CONFIG_EXTRAS bullet-extra.cmake) |
Also, as convention, we almost always use "extras" as opposed to "extra", so rename this file to bullet-extras.cmake.
tf2_bullet/bullet-extra.cmake
Outdated
| # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE | ||
| # POSSIBILITY OF SUCH DAMAGE. | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
Nit: extra whitespace.
gleichdick
force-pushed
the
bullet_include
branch
from
55356b8 to
c069f3c
Compare
Contributor
Author
|
Thanks! I also converted this into a cmake header only library, but I think that's currently out of scope (also because I couldn't find a tutorial on how to make header only libraries in ROS2). |
clalancette
approved these changes
May 28, 2021
Contributor
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
Looks good to me, I'll run CI on this next.
Contributor
|
CI:
|
Contributor
|
@ahcorde This one looks good to me, but I'd appreciate a second look by you. |
ahcorde
approved these changes
May 28, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
tf2_bullet does not have the dependencies on the bullet library exported. The standard approach of ament does not work because of two reasons:
BULLET_ROOTseems to be needed on windowsFindBulletsets theINCLUDE_DIRSandLIBRARIESall uppercase, which is unexpected by ament.This was split out of #423.