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

Missing Bazel alias //:iceoryx #2368

Open
lalten opened this issue Nov 15, 2024 · 5 comments · Fixed by #2369
Open

Missing Bazel alias //:iceoryx #2368

lalten opened this issue Nov 15, 2024 · 5 comments · Fixed by #2369

Comments

@lalten
Copy link
Contributor

lalten commented Nov 15, 2024

Brief feature description

To enable compatibility with rules_ros2 and other Bazel-built Iceoryx consumers, it would be good to have an :iceoryx alias so that e.g. https://github.com/mvukov/rules_ros2/blob/90e7e2d72e1dda8e76ae7ecb7d6e39fef6929270/repositories/cyclonedds.BUILD.bazel#L194 works (if you add the dependency like below)

bazel_dep(name = "org_eclipse_iceoryx", repo_name = "iceoryx")
git_override(
    module_name = "org_eclipse_iceoryx",
    commit = "aa13b0abdf6686fa9939fb2f85a828c1e59ce2f5",
    remote = "https://github.com/eclipse-iceoryx/iceoryx",
)

Detailed information

No cons, having the main target have the name of the main package is Bazel convention.

lalten added a commit to lalten/iceoryx that referenced this issue Nov 15, 2024
lalten added a commit to lalten/iceoryx that referenced this issue Nov 15, 2024
elfenpiff added a commit that referenced this issue Nov 18, 2024
@elBoberido elBoberido reopened this Nov 18, 2024
@elBoberido
Copy link
Member

@lalten sorry, but can we please revert this change. It does not make sense to have the C bindings as //:iceoryx target in bazel. If at all, iceoryx_posh would be the default target but I would also argue that we should not have an //:iceoryx target at all, since there is no blessed way on how to use iceoryx. For iceoryx2, for example, only iceoryx_hoofs is used.

In your linked example, why not just use @iceoryx//:iceoryx_binding_c?

@lalten
Copy link
Contributor Author

lalten commented Nov 18, 2024

tbh the main reason was to not have to patch how rules_ros2 uses iceoryx when substituting Bzlmod Iceoryx for its rules_foreign_cc version.
I'm fine with reverting the patch and doing the change in rules_ros2, it's just a convenience thing.
Cc @mvukov

@elBoberido
Copy link
Member

Just for the sake of understanding. What do you mean with patching rules_ros2?

We are currently working on rmw_iceoryx2 which might also have such issues, so I would like to understand the problem and find a solution that is not neglecting the non ROS 2 users of iceoryx and iceoryx2.

@lalten
Copy link
Contributor Author

lalten commented Nov 18, 2024

rules_ros2 is using @rules_foreign_cc//foreign_cc:cmake.bzl to provide @iceoryx in https://github.com/mvukov/rules_ros2/blob/90e7e2d72e1dda8e76ae7ecb7d6e39fef6929270/repositories/iceoryx.BUILD.bazel#L26
Which results in @iceoryx providing

        "libiceoryx_binding_c.a",
        "libiceoryx_posh_config.a",
        "libiceoryx_posh_roudi.a",
        "libiceoryx_posh.a",
        "libiceoryx_posh_gateway.a",
        "libiceoryx_hoofs.a",
        "libiceoryx_platform.a",

The idea in this ticket was to make any dependencies that already depend on "@iceoryx" continue to work in the exactly same way with Iceoryx's native Bazel build.

@elBoberido
Copy link
Member

Okay, but the current bazel setup should already provide all of this libraries. I don't understand why //:iceoryx needs to be an alias to the C bindings?

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 a pull request may close this issue.

2 participants