Always check $config_dependencies visibility at use

[fmeum]

All dependencies for select expressions are collected in an implicit $config_dependencies attribute. Even with
--incompatible_visibility_private_attributes_at_definition, visibility for this attribute is now checked relative to the use rather than the definition of the rule.

Fixes #19126

[fmeum]

@gregestren I got the results of a downstream run with this flag flipped and it looks promising. Do you think I could attempt to flip it for Bazel 7?

[gregestren]

@brandjon what's the story for --incompatible_visibility_private_attributes_at_definition? I'm not familiar with this flag: it came from 752ffcc.

@fmeum this seems useful to me but I don't have much on-the-ground perspective. Are you finding real failures addressed by this? I pinged @brandjon since he's done a lot of work on clarifying visibility checking, including definition vs. use.

[gregestren]

Does this intersect at all with #12932 or #12933?

[fmeum]

It does, the visibility check is bypassed with --noincompatible_config_setting_private_default_visibility, which means that this issue has only been around since the flip of --incompatible_config_setting_private_default_visibility pre Bazel 6.

[gregestren]

Wait, isn't that still off by default?

bazel/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java

Lines 137 to 138 in 895969d

	name = "incompatible_config_setting_private_default_visibility",
	defaultValue = "false",

Or is this the connection?

bazel/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java

Lines 123 to 126 in 895969d

	name = "incompatible_enforce_config_setting_visibility",
	// TODO(b/179944632): set this and --incompatible_config_setting_private_default_visibility
	// to true, then make these no-ops, then remove.
	defaultValue = "true",

[fmeum]

Yes, that is the connection, the issue goes away with --noincompatible_enforce_config_setting_visibility. Without the explicit visibility on the config_setting, the test added in this PR passes without the changes to the production code.

[fmeum]

@fmeum this seems useful to me but I don't have much on-the-ground perspective. Are you finding real failures addressed by this?

I do. From my perspective as a ruleset author, the current behavior is pretty unintuitive and effectively forces all tools used by rules to be publicly visible, even when those tools are considered internal. Examples include rules_go and rules_jvm_external.

Since all tools in public rulesets need to be visible globally due to the current behavior, I would consider the risk of undetected breakages to be minimal.