-
Notifications
You must be signed in to change notification settings - Fork 179
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
Unexpected visibility limitations with config_setting visibility #404
Comments
cc @gregestren |
Fixes envoyproxy#23390 This is an edge case, upstream issue: bazelbuild/bazel-skylib#404 Signed-off-by: Keith Smiley <[email protected]>
Fixes #23390 This is an edge case, upstream issue: bazelbuild/bazel-skylib#404 Signed-off-by: Keith Smiley <[email protected]>
Interestingly adding |
shouldn't that be the same behavior as what we're getting with the default in this case? |
https://github.com/bazelbuild/bazel/blob/9ba1adab83703ca756ff05d1298e511dc8e09320/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java#L1751-L1760 shows the intention. So Unfortunately it's proving cumbersome to also land |
Fixes envoyproxy#23390 This is an edge case, upstream issue: bazelbuild/bazel-skylib#404 Signed-off-by: Keith Smiley <[email protected]> Signed-off-by: Jonh Wendell <[email protected]>
Looks like some recent progress on flipping I think it's marked for Bazel 7.0? (but no Github tags confirm that). If so, I think we can just sit back and wait. This doesn't help Bazel 6.0 users though. Maybe enough downstream projects will be updated that 6.0 users can just manually trigger |
As of bazelbuild/bazel#12932, using
selects.config_setting_group
produces unexpected results because of how it uses an underlying alias to surface the settings. For example if you have:And from another package you depend on this with:
You see this error:
The macro generates:
So this makes sense, since aliases propagate visibility, but I think it's a bit unexpected in this case. Maybe there's another solution to how this could be written to allow having private settings.
The text was updated successfully, but these errors were encountered: