Adjust semantics for config_setting visibility migration.
Today, config_setting_visibility is completely ignored. This also means for select() -> alias -> config_setting, the alias visibility is ignored.
We'll migrate to full visibility checking in two steps:
1) Turn on --incompatible_enforce_config_setting_visibility. This turns on visibility checking but defaults config_setting visibility to public. This only causes depot failures for config_settings that are explicitly set otherwise, which we can clean up.
2) Turn on --incompatible_config_setting_private_default_visibility: this defaults config_setting visibility to private, which is standard visibility semantics. This will break more of the depot (i.e. any config_setting in a package that doesn't change its default visibility). We'll clean that up too.
This change fixes a small issue with aliases. The intention of step 1 is that we allow config_settings that aren't explicitly visibility-restricted. But that would fail for a select() -> alias -> config_setting_with_default_visibility. Visibility checking independently checks the select() -> alias edge, which has the usual private default.
This change skips that edge entirely and directly compares the select() against config_setting_with_default_visibility.
We don't want to ultimately skip alias visibility checking. But we can get that working at step 2. As it stands now, at step 0, this is already skipped. So this change isn't a regression. It just helps smooth migration.
For https://github.com/bazelbuild/bazel/issues/12932 and https://github.com/bazelbuild/bazel/issues/12933.
PiperOrigin-RevId: 479631810
Change-Id: I8b77f62bb1584cc74d182995afa19fe70a8d4875
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 959fc15..838f872 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1728,11 +1728,36 @@
// --config_setting_visibility_policy. This should be removed as soon as it's deemed safe
// to unconditionally check visibility. See
// https://github.com/bazelbuild/bazel/issues/12669.
- if (target.getPackage().getConfigSettingVisibilityPolicy()
- != ConfigSettingVisibilityPolicy.LEGACY_OFF) {
+ ConfigSettingVisibilityPolicy configSettingVisibilityPolicy =
+ target.getPackage().getConfigSettingVisibilityPolicy();
+ if (configSettingVisibilityPolicy != ConfigSettingVisibilityPolicy.LEGACY_OFF) {
Attribute configSettingAttr = attributes.getAttributeDefinition("$config_dependencies");
for (ConfiguredTargetAndData condition : configConditions.asConfiguredTargets().values()) {
- validateDirectPrerequisite(configSettingAttr, condition);
+ validateDirectPrerequisite(
+ configSettingAttr,
+ // Another nuance: when both --incompatible_enforce_config_setting_visibility and
+ // --incompatible_config_setting_private_default_visibility are disabled, both of
+ // these are ignored:
+ //
+ // - visibility settings on a select() -> config_setting dep
+ // - visibility settings on a select() -> alias -> config_setting dep chain
+ //
+ // In that scenario, both are ignored because the logic here that checks the
+ // select() -> ??? edge is completely skipped.
+ //
+ // When just --incompatible_enforce_config_setting_visibility is on, that means
+ // "enforce config_setting visibility with public default". That's a temporary state
+ // to support depot migration. In that case, we continue to ignore the alias'
+ // visibility in preference for the config_setting. So skip select() -> alias as
+ // before, but now enforce select() -> config_setting_the_alias_refers_to.
+ //
+ // When we also turn on --incompatible_config_setting_private_default_visibility, we
+ // expect full standard visibility compliance. In that case we directly evaluate the
+ // alias visibility, as is usual semantics. So two the following two edges are
+ // checked: 1: select() -> alias and 2: alias -> config_setting.
+ configSettingVisibilityPolicy == ConfigSettingVisibilityPolicy.DEFAULT_PUBLIC
+ ? condition.fromConfiguredTarget(condition.getConfiguredTarget().getActual())
+ : condition);
}
}
