Visibility failure on Bazel 6.0

[gregestren]

Title: Visibility failure on Bazel 6.0

Description:
Bazel 6.0 pre-release tests show envoy failing with --incompatible_enforce_config_setting_visibility.

This isn't a surprise. That flag fixes a long-standing omission in Bazel's visibility checking: config_setting has never been visibility-checked. That means that if a config_setting is defined in a package with private visibility, the config_setting is still public. The flag updates config_setting to properly honor visibility.

So flipping the flag makes it possible for existing config_setting references to fail. The fix is to update their visibility to reflect actual usage.

Repro steps:

$ git clone -v https://github.com/envoyproxy/envoy.git
$ USE_BAZEL_VERSION=latest bazel build --nobuild //test/common/common/... //test/integration/... //test/exe/... --incompatible_enforce_config_setting_visibility

ERROR: /usr/local/google/home/gregce/.cache/bazel/_bazel_gregce/160b5d944412957a853c062174d7f0d8/external/proxy_wasm_cpp_host/BUILD:49:11:
in cc_library rule @proxy_wasm_cpp_host//:base_lib: alias '@proxy_wasm_cpp_host//bazel:crypto_system' referring to target 
'@proxy_wasm_cpp_host//bazel:linux_s390x' is not visible from target '@proxy_wasm_cpp_host//:base_lib'. Check the visibility 
declaration of the former target if you think the dependency is legitimate
ERROR: /usr/local/google/home/gregce/.cache/bazel/_bazel_gregce/160b5d944412957a853c062174d7f0d8/external/proxy_wasm_cpp_host/BUILD:49:11:
 Analysis of target '@proxy_wasm_cpp_host//:base_lib' failed
ERROR: Analysis of target '@proxy_wasm_cpp_host//:base_lib' failed; build aborted: 

This leads to these config settings which need broader visibility. One option is adding this to their package:

package(default_visibility = ["//visibility:public"])

Other options here.

I'd write a PR, but I don't understand who owns proxy-wasm-cpp-host vs. envoy and what actual visibility levels the owners want.

[gregestren]

CC @meteorcloudy

[ggreenway]

cc @keith @lizan

[lizan]

Adding package(default_visibility = ["//visibility:public"]) there should be OK. cc @PiotrSikora

[gregestren]

FYI there's a small wrinkle with aliases (select() -> alias -> config-setting): https://bazel-review.googlesource.com/c/bazel/+/205211.

If you're comfortable with the solution from the above comment, it doesn't matter: ignore this comment entirely.

Just noting because I realize this case has an alias via selects.config_setting_group.

[PiotrSikora]

Adding default_visibility to Proxy-Wasm C++ Host seems like an easy solution. Please send a PR, thanks!

[keith]

proxy-wasm/proxy-wasm-cpp-host#311

[meteorcloudy]

Note that, the flag has been flipped at HEAD, and Envoy is currently failing in downstream: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2670#0183ca5c-4612-4980-88f5-86240a1f7c65

[keith]

#23460

[keith]

#23434

[keith]

everything should be fixed here, please lmk if anyone else spots more issues with this!

[SalmaSamy]

Hi @keith, It looks like Envoy is still failing in the downstream: https://buildkite.com/bazel/bazel-at-head-plus-disabled/builds/1461#0184143d-e174-4fa1-94a6-1b2337dd9476

[keith]

thanks, im interested to hear replies about this on bazelbuild/bazel-skylib#404