diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java index cd47434d8b63fb..e8e419f0d3f0b0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java @@ -25,7 +25,7 @@ import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.RawAttributeMapper; import com.google.devtools.build.lib.packages.Rule; -import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; @@ -87,20 +87,26 @@ private void validateDirectPrerequisiteVisibility( .getBool( BuildLanguageOptions.INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION); - if (!toolCheckAtDefinition - || !attribute.isImplicit() - || rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel() == null) { - // Default check: The attribute must be visible from the target. - if (!context.isVisible(prerequisite.getConfiguredTarget())) { - handleVisibilityConflict(context, prerequisite, rule.getLabel()); - } - } else { - // For implicit attributes, check if the prerequisite is visible from the location of the - // rule definition - Label implicitDefinition = rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel(); - if (!RuleContext.isVisible(implicitDefinition, prerequisite.getConfiguredTarget())) { - handleVisibilityConflict(context, prerequisite, implicitDefinition); - } + // Determine if we should use the new visibility rules for tools. + boolean toolCheckAtDefinition = + context + .getStarlarkSemantics() + .getBool(BuildLanguageOptions.INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION); + + if (!toolCheckAtDefinition + || !attribute.isImplicit() + || attribute.getName().equals(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE) + || rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel() == null) { + // Default check: The attribute must be visible from the target. + if (!context.isVisible(prerequisite.getConfiguredTarget())) { + handleVisibilityConflict(context, prerequisite, rule.getLabel()); + } + } else { + // For implicit attributes, check if the prerequisite is visible from the location of the + // rule definition + Label implicitDefinition = rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel(); + if (!RuleContext.isVisible(implicitDefinition, prerequisite.getConfiguredTarget())) { + handleVisibilityConflict(context, prerequisite, implicitDefinition); } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/VisibilityTest.java b/src/test/java/com/google/devtools/build/lib/analysis/VisibilityTest.java index de19e50790cfc0..3a3e377974de0d 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/VisibilityTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/VisibilityTest.java @@ -166,6 +166,42 @@ public void testDataVisibilityPrivateCheckPrivateAtUse() throws Exception { assertThrows(ViewCreationFailedException.class, () -> update("//use:world")); } + @Test + public void testConfigSettingVisibilityAlwaysCheckedAtUse() throws Exception { + scratch.file( + "BUILD", + "load('//build_defs:defs.bzl', 'my_rule')", + "my_rule(", + " name = 'my_target',", + " value = select({", + " '//config_setting:my_setting': 'foo',", + " '//conditions:default': 'bar',", + " }),", + ")"); + scratch.file("build_defs/BUILD"); + scratch.file( + "build_defs/defs.bzl", + "def _my_rule_impl(ctx):", + " pass", + "my_rule = rule(", + " implementation = _my_rule_impl,", + " attrs = {", + " 'value': attr.string(mandatory = True),", + " },", + ")"); + scratch.file( + "config_setting/BUILD", + "config_setting(", + " name = 'my_setting',", + " values = {'cpu': 'does_not_matter'},", + " visibility = ['//:__pkg__'],", + ")"); + useConfiguration("--incompatible_visibility_private_attributes_at_definition"); + + update("//:my_target"); + assertThat(hasErrors(getConfiguredTarget("//:my_target"))).isFalse(); + } + void setupFilesScenario(String wantRead) throws Exception { scratch.file("src/source.txt", "source"); scratch.file("src/BUILD", "exports_files(['source.txt'], visibility=['//pkg:__pkg__'])");