Flip incompatible_no_implicit_file_export#27674
Flip incompatible_no_implicit_file_export#27674hofbi wants to merge 3 commits intobazelbuild:masterfrom
Conversation
e5d190a to
0c7ff55
Compare
f560a43 to
257e440
Compare
|
@meteorcloudy Now that this is green, could you trigger https://buildkite.com/bazel/bazel-at-head-plus-downstream for me on this PR so that I can see if something downstream breaks. |
|
Hope that I did this right: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/5161#_ |
I think you triggered this job from I checked #26587 (comment) how we did it last time, and it looks like there is also https://buildkite.com/bazel/bcr-bazel-compatibility-test. Now I am not sure which one we need. And what to do exactly to trigger from this branch as both https://buildkite.com/bazel/bcr-bazel-compatibility-test/builds/492/steps/canvas and https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4824/steps/canvas were executed from Maybe we have to wait for @meteorcloudy. |
|
master is the base branch, but based on the errors this does include your commit. The BCR test is a separate check which runs on BCR modules rather than bazelbuild repos. |
I am working on bazelbuild/bazel#27674 towards bazelbuild/bazel#10225 to flip `incompatible_no_implicit_file_export`. I found that https://github.com/bazelbuild/bazel/blob/master/src/test/py/bazel/bzlmod/bazel_repo_mapping_test.py and https://github.com/bazelbuild/bazel/blob/master/src/test/py/bazel/bzlmod/bazel_module_test.py require `runtime_env_toolchain_interpreter.sh`. Since it is no longer implicitly exported after flipping the flag, we have to export it explicitly.
|
Added Also manually triggered https://buildkite.com/bazel/bcr-bazel-compatibility-test/builds/723 with env vars We have some docs at https://github.com/bazelbuild/continuous-integration/tree/master/buildkite/bazel-central-registry#bcr-bazel-compatibility-test, @fmeum feel free to trigger a build when needed. |
Identified by bazelbuild/bazel#27674 (comment), I want to see what else fails when flipping this flag on top of #3471.
30e23b8 to
73da3c3
Compare
|
@meteorcloudy Could you trigger another run for https://buildkite.com/bazel/bcr-bazel-compatibility-test to verify that cgrindel/bazel-starlib#566 and bazel-contrib/rules_bazel_integration_test#552 are fixed. bazel-contrib/rules_kotlin#1440 and bazelbuild/rules_android#440 are still open (maintainers wanted to fix this or I am waiting on a PR review). Are these blockers for getting this flag flipped? Speaking about blockers, is there anything we would have to change in this PR? E.g. waiting for the next rules_python release to clear out the TODOs added here? |
|
You can already see the result in the nightly run: https://buildkite.com/bazel/bcr-bazel-compatibility-test/builds/785 Indeed bazel-starlib and rules_bazel_integration_test have been fixed, but rules_android and rules_multitool are broken by this flag. |
|
I would say none of the broken modules should block the merge of this PR. |
There was a problem hiding this comment.
Code Review
This pull request flips the default value of the incompatible_no_implicit_file_export flag to true. This change makes source files package-private by default, requiring explicit exports_files declarations for cross-package visibility. The modifications correctly update the flag's definition and adjust various tests to align with the new default behavior. Some tests now include exports_files directives, while others temporarily disable the flag to accommodate dependencies that are not yet updated. The changes are consistent and look good.
|
@hofbi Looks like we cannot flip this flag inside the google monorepo. Can you use https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/packages/semantics/FlagConstants.java to refactor the change so that we only flip it for Bazel? |
73da3c3 to
6e7a76c
Compare
|
@meteorcloudy I added the flags constants as requested. Let me know if this works. |
|
@meteorcloudy kind reminder |
|
Sorry for the delay, this is in progress. FYI @deepalak56 |
Broken by bazelbuild/bazel#27674 (ignore-relnotes) PiperOrigin-RevId: 874542646 Change-Id: I4f1e8c9baae8330d703d6f7eb145e5a3ee2923fc
Emits an `exports_files` directive for the `.jar` files included in repositories generated by `scala_maven_import_external`. --- Follow-up from bazel-contrib#1812 that fixes `last_green` builds, which now incorporate the `--incompatible_no_implicit_file_export` flag flip from: - bazelbuild/bazel@0b8a518 - bazelbuild/bazel#27674
Emits an `exports_files` directive for the `.jar` files included in repositories generated by `scala_maven_import_external`. --- Follow-up from #1812 that fixes `last_green` builds, which now incorporate the `--incompatible_no_implicit_file_export` flag flip from: - bazelbuild/bazel@0b8a518 - bazelbuild/bazel#27674
I believe this happens when two things happen: 1. Bump Bazel past bazelbuild/bazel#27674 or add `--incompatible_no_implicit_file_export` 2. Use the deps declared in your main module in another module (for example a manually built wheel) Note that I couldn't even get close to getting `--incompatible_no_implicit_file_export` working on this repo overall, but this patch works for my repo.
Work towards bazel-contrib/SIG-rules-authors#41
Fixes #10225