Repository: bazelbuild/bazel
Issue: `config_setting` incorrectly treated as implicit dependency by `--incompatible_visibility_private_attributes_at_definition`
Labels: type: bug
untriaged
team-Loading-API

### Description of the bug:


When explicitly depending on a `config_setting` and enabling `--incompatible_visibility_private_attributes_at_definition`, the setting is incorrectly treated as an implicit dependency which must be visible to the dependent's rule definition, which doesn't make logical sense. The full set of conditions from my understanding is:

1.  A target which depends on a `config_setting` from a different package through a `select` key.
1.  The target is an instance of a rule from a another package which does _not_ have visibility into the `config_setting`.
1.  `--incompatible_visibility_private_attributes_at_definition`.

```python
# BUILD.bazel

load("//build_defs:defs.bzl", "my_rule")

my_rule(
    name = "my_target",
    value = select({
        "//config_setting:my_setting": "foo",
        "//conditions:default": "bar",
    }),
)
```

When all three happen it throws an error like this:

```
$ bazel build //:my_target
ERROR: /home/doug/Source/bazel_visibility/BUILD.bazel:3:8: in my_rule rule //:my_target: target '//config_setting:my_setting' is not visible from target '//build_defs:defs.bzl'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: /home/doug/Source/bazel_visibility/BUILD.bazel:3:8: Analysis of target '//:my_target' failed
ERROR: Analysis of target '//:my_target' failed; build aborted: 
INFO: Elapsed time: 0.064s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (2 packages loaded, 2 targets configured)
```

This doesn't make sense: `target '//config_setting:my_setting' is not visible from target '//build_defs:defs.bzl'`. This should not be required, as the config setting is not an implicit dependency.

### What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.


Minimal reproduction here: https://github.com/dgp1130/bazel-config-setting-visibility-bug/

It does rely on Skylib which probably isn't necessary but I figured that wasn't too significant and I didn't want to try and figure out how to define flags manually.

### Which operating system are you running Bazel on?


Ubuntu on WSL

### What is the output of `bazel info release`?


release 6.3.0

### If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.


_No response_

### What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?


```text
$ git remote get-url origin; git rev-parse main; git rev-parse HEAD
git@github.com:dgp1130/bazel-config-setting-visibility-bug
d86833d09fb3bcb3eb0429d74eb80364db7e684a
d86833d09fb3bcb3eb0429d74eb80364db7e684a
```


### Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.


I don't know, however I initially reproduced on version 6.0.0 before upgrading to 6.3.0 to confirm it was still present on latest. The bug is at least that old.

### Have you found anything relevant by searching the web?

https://github.com/bazelbuild/bazel/issues/12932 feels like it might be relevant, however I found that `--incompatible_enforce_config_setting_visibility` had no effect, so it could be nothing.

### Any other information, logs, or outputs that you want to share?

_No response_

Issue comments:
@bazel-io flag
@bazel-io fork 6.4.0
A fix for this issue has been included in [Bazel 6.4.0 RC1](https://releases.bazel.build/6.4.0/rc1/index.html). Please test out the release candidate and report any issues as soon as possible. Thanks!
I gave 6.4.0rc1 a quick test and confirmed that it does fix the issue in my actual repository, so everything looks good here! 😁

Pull request review comments:

Patch that resolved the issue:
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..b5721702e1c3b5 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
@@ -26,6 +26,7 @@
 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;
@@ -89,6 +90,7 @@ private void validateDirectPrerequisiteVisibility(
 
       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())) {


Test patch:
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__'])");