Skip to content

iox-#2432: Fix bazel build for --incompatible_disallow_empty_glob #2433

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lalten
Copy link
Contributor

@lalten lalten commented Mar 14, 2025

Notes for Reviewer

Fix bazel build for --incompatible_disallow_empty_glob which is the default on Bazel 8.

Pre-Review Checklist for the PR Author

  1. Code follows the coding style of CONTRIBUTING.md
  2. Tests follow the best practice for testing
  3. Changelog updated in the unreleased section including API breaking changes
  4. Branch follows the naming format (iox-123-this-is-a-branch)
  5. Commits messages are according to this guideline
  6. Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  7. Relevant issues are linked
  8. 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

@lalten lalten force-pushed the iox-2432_fix-empty-globs branch 2 times, most recently from e127299 to 77028a4 Compare March 14, 2025 09:03
@lalten lalten changed the title iox-2432: Fix bazel build for --incompatible_disallow_empty_glob iox-#2432: Fix bazel build for --incompatible_disallow_empty_glob Mar 14, 2025
@lalten lalten force-pushed the iox-2432_fix-empty-globs branch from 77028a4 to 7531ca0 Compare March 14, 2025 09:04
@lalten lalten force-pushed the iox-2432_fix-empty-globs branch from 7531ca0 to 59eed8f Compare March 14, 2025 09:08
bazel-io pushed a commit to bazelbuild/bazel-central-registry that referenced this pull request Mar 14, 2025
Add https://github.com/eclipse-iceoryx/iceoryx/releases/tag/v2.95.4

Includes patches for these PRs:
* eclipse-iceoryx/iceoryx#2371
* eclipse-iceoryx/iceoryx#2433
* eclipse-iceoryx/iceoryx#2436
* eclipse-iceoryx/iceoryx#2437
* eclipse-iceoryx/iceoryx#2439

Added `test_targets` in presubmit as well, except for those that are
failing due to lack of resources in ci
@elBoberido
Copy link
Member

Do you know it this is compatible with bazel 6.2?

@lalten
Copy link
Contributor Author

lalten commented Mar 14, 2025

Do you know it this is compatible with bazel 6.2?

I don't see why not. Bazel 6 is quite old though, its EOL is only a few months away: https://bazel.build/release

@elBoberido
Copy link
Member

Are you able to check this?

It might be old, but we have users who are still using it, so we need to keep it working.

@lalten
Copy link
Contributor Author

lalten commented Mar 14, 2025

Are you able to check this?

Checking with Bazel 6 (via USE_BAZEL_VERSION, https://github.com/bazelbuild/bazelisk?tab=readme-ov-file#how-does-bazelisk-know-which-bazel-version-to-run) on main:

USE_BAZEL_VERSION=6.x bazel test //... --enable_bzlmod
ERROR: https://bcr.bazel.build/modules/googletest/1.15.2/MODULE.bazel:68:20: name 'use_repo_rule' is not defined
ERROR: Error computing the main repository mapping: in module dependency chain <root> -> [email protected]: error executing MODULE.bazel file for [email protected]

use_repo_rule was introduced in Bazel 7+, so there is no way to use Bzlmod with this googletest module.

USE_BAZEL_VERSION=6.x bazel test //... --noenable_bzlmod
INFO: Analyzed 122 targets (31 packages loaded, 2454 targets configured).
INFO: Found 112 targets and 10 test targets...
ERROR: /home/laltenmueller/.cache/bazel/_bazel_laltenmueller/740f0239debf861aadac7b86a3d4babd/external/ncurses/BUILD.bazel:24:15: Foreign Cc - Configure: Building ncurses failed: (Exit 2): bash failed: error executing command (from target @ncurses//:ncurses) /bin/bash -c bazel-out/k8-fastbuild/bin/external/ncurses/ncurses_foreign_cc/wrapper_build_script.sh
(...)
/usr/bin/ar: ../lib/libncurses.a: No such file or directory
make[1]: *** [Makefile:924: ../lib/libncurses.a] Error 1
make[1]: Leaving directory '/home/laltenmueller/.cache/bazel/_bazel_laltenmueller/740f0239debf861aadac7b86a3d4babd/sandbox/linux-sandbox/749/execroot/org_eclipse_iceoryx/bazel-out/k8-fastbuild/bin/external/ncurses/ncurses.build_tmpdir/ncurses'
make: *** [Makefile:133: all] Error 2
FAILED: Build did NOT complete successfully

So it seems like something about the Workspace build of ncurses also doesn't work

Note that

USE_BAZEL_VERSION=7.x bazel test //... --noenable_bzlmod
Starting local Bazel server and connecting to it...
INFO: Analyzed 122 targets (107 packages loaded, 3897 targets configured).
INFO: Found 112 targets and 10 test targets...
INFO: Elapsed time: 435.762s, Critical Path: 169.31s
INFO: 886 processes: 272 internal, 614 linux-sandbox.
INFO: Build completed successfully, 886 total actions
//iceoryx_binding_c/test:binding_c_integrationtests                      PASSED in 0.0s
//iceoryx_binding_c/test:binding_c_moduletests                           PASSED in 6.2s
//iceoryx_hoofs/test:hoofs_integrationtests                              PASSED in 0.0s
//iceoryx_hoofs/test:hoofs_moduletests                                   PASSED in 3.4s
//iceoryx_hoofs/test/stresstests:hoofs_stresstests                       PASSED in 163.5s
//iceoryx_hoofs/test/stresstests:test_stress_spsc_sofi                   PASSED in 6.0s
//iceoryx_platform/test:platform_integrationtests                        PASSED in 0.0s
//iceoryx_platform/test:platform_moduletests                             PASSED in 0.0s
//iceoryx_posh/test:posh_integrationtests                                PASSED in 56.7s
//iceoryx_posh/test:posh_moduletests                                     PASSED in 27.4s

Executed 10 out of 10 tests: 10 tests pass.

works just fine!

So I doubt that anybody uses this with Bazel 6 unless they have heavily patched things already.

@elBoberido
Copy link
Member

Hmm, it seems it is related to ncurses, so it might be that the 6.2 users are just not building the introspection. Everything else keeps building with these changes, so there is no reason not to merge them. I've review it the next days

feuerste pushed a commit to feuerste/bazel-central-registry that referenced this pull request Mar 18, 2025
deps = ["@googletest//:gtest"],
)

cc_library(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love seeing some of these targets split out 🙂

Copy link

@gpalmer-latai gpalmer-latai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elBoberido For what it is worth this LGTM

Can't tell just by staring at the respelled globs that they pick up exactly the same file set, but if things compile and tests pass we should be good, right?

@elBoberido
Copy link
Member

@gpalmer-latai @lalten sorry, got distracted. It's on my todo list to get back to all the bazel PRs. I try to refresh my memory on the weekend or beginning of next week.

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 this pull request may close these issues.

Broken Bazel build with --incompatible_disallow_empty_glob Fails to build with Bazel 8.0.0
3 participants