search

mvukov / rules_ros2

Build ROS 2 with Bazel
Apache License 2.0
85 stars 47 forks source link

Use versioned repo names to avoid conflict #12

Closed LiyouZhou closed 2 years ago

LiyouZhou commented 2 years ago

When importing rules_ros2 into another project fmt and libyaml causes name conflict. This patch uses versioned names to avoid chances of conflict.

mvukov commented 2 years ago

I intentionally defined http_archive targets under maybe macro such that overrides is possible. That said, if another fmt, libyaml, etc. are declared before the call to

load("//repositories:repositories.bzl", "ros2_repositories")

ros2_repositories()  # This one in particular

your project, rules_ros2 as well, are going to use those ftm and libyaml versions. I am curious to learn how your workspace setup looks like. Can you provide more info on the problems that you're facing?

mvukov commented 2 years ago

(I updated CI config and reopened the PR to trigger CI checks on your PR.)

LiyouZhou commented 2 years ago

We have monorepo and there are global 3rd party imports for fmt and libyaml already (for other part of the codebase unrelated to the robot), unfortunately the existing 3rd party imports appear to be incompatible versions. Hence to avoid further conflict, I want to specify the version exactly and use those solely for rules_ros2.

LiyouZhou commented 2 years ago

@mvukov do you think this can be merged?

mvukov commented 2 years ago

OK, some parts of your monorepo import (before rules_ros2 is imported) fmt&libyaml that don't work with rules_ros2. For that reason you want separate versions of those packages just for rules_ros2. That's OK, but, there is a practical chance that a dev can use a library that depends on fmt version x with a ros node that depends on fmt version y with your PR ---> linker would freak out, I think. I guess you prevent those clashes with proper target visibility settings. The shadowing deps approach you use is the way to go for you. I think it's a good solution for your use-case, but, not a generic one. For this reason, I am inclined not to merge this in the main branch. One day when this repo has a troubleshooting section, this PR should be referenced.