-
Notifications
You must be signed in to change notification settings - Fork 16
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
Use system installed yaml-cpp 0.6 if available #8
Conversation
@mjbogusz Hi, I saw the original version you made a comment there about the ament limitation. Do you know is it still something true? And do you mind to share what's the original problem you are hitting? |
After short googling I've found the PR that this originated from - it was 2 years ago and happened in ros2-rviz: ros2/rviz#160 (I guess before yaml-cpp was split to separate vendor package) As I recall and based on the aforementioned PR the issue originated from having the required version of |
Thanks for the insights @mjbogusz. |
I may state the obvious, but make sure to check whether building packages depending on |
Yes, I did that. I first build depending packages without installing yaml_cpp with I will give it a shot in bionic now, with yaml_cpp 0.5 installed, to see if it still builds 0.6. |
I also checked that it was using the system library in that case, and not building it again. |
@seanyen Something went wrong on Windows, I don't have time to investigate right now. |
@ivanpauno hmm, on the Windows build agent, it found @brawner It is a long shot, but just curious, do you know if the Windows agent has
|
I'm not sure about the bare-metal machines, but the container builds will become the default today and I don't think it's installed there. How is it installed on windows? Can it be added to the dockerfile? https://github.com/ros2/ci/blob/master/windows_docker_resources/Dockerfile.msvc2019 |
@brawner, actually this is my question too. :) I don't know how does the bare-metal one get prepared and don't know what's the environment there. However, sounds like now we are switching to container build so the environment will be cleaner. Just curious, is @ivanpauno able to kick off a container build against this change or we just wait? I would like to see if this problem manifest or not on the new environment. |
The whole point is that it should find the system package, and work correctly when it does find it. |
@ivanpauno I am not sure what's the reference environment acceptable for this validation. However, I can provide some information on the In my settings, I have ros-eloquent-desktop built out on this sandbox channel https://anaconda.org/ros-playground: (and one of my goal is in favor of And for this validation, I can CLICK ME
And after doing CLICK ME
|
That's ok, but does |
@seanyen friendly ping |
@ivanpauno Sorry for the late on this. I did P.S., I am using https://github.com/ros2-gbp/rviz-release/tree/release/eloquent/rviz_common/7.0.4-1 and https://github.com/ros2-gbp/rosbag2-release/tree/release/eloquent/rosbag2_storage/0.2.4-1 as the targets. |
@ivanpauno friendly ping. |
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.
LGTM!
Sorry for the delay @seanyen
@seanyen can you rebase? I will run CI after that. Thanks! |
348cdcc
to
0430b8a
Compare
Signed-off-by: seanyen <[email protected]>
Signed-off-by: seanyen <[email protected]>
Signed-off-by: seanyen <[email protected]>
0430b8a
to
0725e94
Compare
@ivanpauno rebased! |
Thanks for the contribution @seanyen !! |
This PR seems to have introduced a regression on macOS: see #14. |
This reverts commit b11d00f.
This reverts commit b11d00f. Signed-off-by: Ivan Santiago Paunovic <[email protected]>
This reverts commit b11d00f. Signed-off-by: Ivan Santiago Paunovic <[email protected]>
…15)" This reverts commit 251be27. Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
On the environments (vcpkg, conda-forge, etc..) which already have the
yaml-cpp
, we can fallback to use the global one.