iox-#2370 Fix Bzlmod dev_dependency setup

[lalten]

Notes for Reviewer

Fix Bzlmod dev_dependency setup

Pre-Review Checklist for the PR Author

1. [x] Code follows the coding style of CONTRIBUTING.md
2. [x] Tests follow the best practice for testing
3. [x] Changelog updated in the unreleased section including API breaking changes
4. [x] Branch follows the naming format (iox-123-this-is-a-branch)
5. [x] Commits messages are according to this guideline
   • [x] Commit messages have the issue ID (iox-#123 commit text)
   • [x] Commit author matches Eclipse Contributor Agreement (and ECA is signed)
6. [x] Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
7. [x] Relevant issues are linked
8. [x] Add sensible notes for the reviewer
9. [ ] All checks have passed (except task-list-completed)
10. [ ] Assign PR to reviewer

Checklist for the PR Reviewer

• [ ] Consider a second reviewer for complex new features or larger refactorings
• [ ] Commits are properly organized and messages are according to the guideline
• [ ] Code according to our coding style and naming conventions
• [ ] Unit tests have been written for new behavior
• [ ] Public API changes are documented via doxygen
• [ ] Copyright owner are updated in the changed files
• [ ] All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
• [ ] PR title describes the changes

Post-review Checklist for the PR Author

1. [ ] All open points are addressed and tracked via issues

References

• Closes #2370

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.70%. Comparing base (aa13b0a) to head (776e5b6).

Additional details and impacted files

[](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2371?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) ```diff @@ Coverage Diff @@ ## main #2371 +/- ## ======================================= Coverage 78.69% 78.70% ======================================= Files 440 440 Lines 16981 16981 Branches 2361 2361 ======================================= + Hits 13364 13365 +1 Misses 2736 2736 + Partials 881 880 -1 ``` | [Flag](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2371/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2371/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | `78.50% <ø> (+<0.01%)` | :arrow_up: | | [unittests_timing](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2371/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | `15.36% <ø> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#carryforward-flags-in-the-pull-request-comment) to find out more. [see 1 file with indirect coverage changes](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2371/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx)

[elfenpiff]

@lalten looks fine. The MinGW CI failure is a flaky test, which we can ignore, but the Ubuntu bazel target fails.

I assigned @elBoberido since he may has some more insights here.

[elBoberido]

@lalten the bzlmod code came from a community member and I'm not that familiar with the code. Can you specify which public targets are affected by the dev dependencies?

[lalten]

@lalten the bzlmod code came from a community member and I'm not that familiar with the code. Can you specify which public targets are affected by the dev dependencies?

Buildifier is unconditionally depended on in the root BUILD https://github.com/eclipse-iceoryx/iceoryx/blob/d628cb7111051ae1cd27620d9a621a366d5cac82/BUILD.bazel#L18 https://github.com/eclipse-iceoryx/iceoryx/blob/d628cb7111051ae1cd27620d9a621a366d5cac82/BUILD.bazel#L61-L65

Googletest is depended on in a regular cc_library with public visibility: https://github.com/eclipse-iceoryx/iceoryx/blob/d628cb7111051ae1cd27620d9a621a366d5cac82/iceoryx_hoofs/BUILD.bazel#L107

[elBoberido]

Okay, the buildifier is definitely a dev dependency. The iceoryx_hoofs_testing is actually also a dev dependency but I'm not sure how this could be modeled with bazel. It is basically meant to be used for tests which use iceoryx. For example it contains a logger which suppresses all the output but if a test fails, it prints all log messages to help debugging.

Is there another way to mark this library as a dev dependency but still make it available to users?

In the end, I guess it should not matter and as long as one does depend only on e.g. iceoryx_hoofs bazel will not require gTest to be available. Please correct me if I'm wrong.

[lalten]

dev dependencies can not be depended on outside the root module, i.e. as soon as iceoryx is used as Bzlmod dependency of another project (which of course is the intended use case), dev dependencies are not there anymore.

See https://bazel.build/rules/lib/globals/module#parameters_1

dev_dependency: If true, this dependency will be ignored if the current module is not the root module or --ignore_dev_dependency is enabled.

[lalten]

Note that Bazel will want to analyze the entire graph, so even if a user doesn't want to depend on iceoryx_hoofs_testing (or buildifier), these Bazel repos must be visible to the root module.

Looks like the buildifier usage is actually ok, here is another repo with the same setup: https://github.com/aspect-build/rules_ts/blob/v3.3.1/MODULE.bazel#L26 https://github.com/aspect-build/rules_ts/blob/v3.3.1/BUILD.bazel#L3

[lalten]

🤷

❯ cat MODULE.bazel
module(name = "test")

bazel_dep(name = "org_eclipse_iceoryx")
git_override(
    module_name = "org_eclipse_iceoryx",
    commit = "d628cb7111051ae1cd27620d9a621a366d5cac82",
    remote = "https://github.com/eclipse-iceoryx/iceoryx",
)
❯ bazel build @org_eclipse_iceoryx//:iceoryx_binding_c
WARNING: Target pattern parsing failed.
ERROR: Skipping '@org_eclipse_iceoryx//:iceoryx_binding_c': error loading package '@@org_eclipse_iceoryx~//': Unable to find package for @@[unknown repo 'buildifier_prebuilt' requested from @@org_eclipse_iceoryx~]//:rules.bzl: The repository '@@[unknown repo 'buildifier_prebuilt' requested from @@org_eclipse_iceoryx~]' could not be resolved: No repository visible as '@buildifier_prebuilt' from repository '@@org_eclipse_iceoryx~'.
ERROR: error loading package '@@org_eclipse_iceoryx~//': Unable to find package for @@[unknown repo 'buildifier_prebuilt' requested from @@org_eclipse_iceoryx~]//:rules.bzl: The repository '@@[unknown repo 'buildifier_prebuilt' requested from @@org_eclipse_iceoryx~]' could not be resolved: No repository visible as '@buildifier_prebuilt' from repository '@@org_eclipse_iceoryx~'.
INFO: Elapsed time: 1.897s
INFO: 0 processes.
ERROR: Build did NOT complete successfully

[elBoberido]

Okay, this is weird. Why should the buildifier be needed in order to build iceoryx_hoofs? Bazel has some weird dependency handling.

Do you have time to check it there is an obvious flaw in the bzlmod setup?