Repository: bazelbuild/bazel
Issue: bool_flag in config_setting via alias seems broken
Labels: type: bug
P3
team-Configurability
not stale

### Description of the problem / feature request:

When referencing a `bool_flag` in a `config_setting` via an `alias` indirection, the flag value set on the command line seems to be ignored.
NOTE: it isn't necessarily only broken for `bool_flags`, but that's just a simple example that illustrate the issue. I suspect other flag types would have the same problem.

### Feature requests: what underlying problem are you trying to solve with this feature?

I want to enable some sort of late-binding for enabling/disabling features, in a way that would 'point' to different workspaces based on a flag (boolean or otherwise). Since `select()` isn't possible in a `WORKSPACE`, it seems that a recommended alternative is to go through a `select`, `config_setting` + `alias` indirection. 
I have tried to simplify my example to a minimum size, so this doesn't exactly illustrate what I'm trying to archive, but it does illustrate the bug I see when following that approach

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

With the following files, run `run_all.sh` which will run 4 builds, one by one, and execute the result, printing `feature a: 0` or `feature a: 1` depending on the outcome of the `select` in the `BUILD` file.
As we can see, we get `0` and `1` (depending on the `--//:enable_feature_a=xxx` command line option) when the `select` is for a `config_setting` that references a `bool_flag` directly, but we always get `0` regardless of the command line option when using a `config_setting` that references a `bool_flag` via an alias.

```
❯ cat WORKSPACE 
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
    name = "bazel_skylib",
    urls = [
        "https://github.com/bazelbuild/bazel-skylib/releases/download/1.0.3/bazel-skylib-1.0.3.tar.gz",
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.0.3/bazel-skylib-1.0.3.tar.gz",
    ],
    sha256 = "1c531376ac7e5a180e0237938a2536de0c54d93f5c278634818e0efc952dd56c",
)
load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")
bazel_skylib_workspace()

❯ cat BUILD 
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")

config_setting(
    name = "feature_a_enabled",
    flag_values = {
        ":enable_feature_a": "true"
    }
)

config_setting(
    name = "feature_a_enabled_aliased",
    flag_values = {
        ":enable_feature_a_aliased": "true"
    }
)

bool_flag(
    name = "enable_feature_a",
    build_setting_default = False,
)

alias(
    name = "enable_feature_a_aliased",
    actual = "enable_feature_a"
)

cc_binary(
    name = "my_binary",
    srcs = ["main.c"] + select({
        ":feature_a_enabled": ["feature_a.c"],
        "//conditions:default": ["default_a.c"]
    })
)

cc_binary(
    name = "my_binary_aliased",
    srcs = ["main.c"] + select({
        ":feature_a_enabled_aliased": ["feature_a.c"],
        "//conditions:default": ["default_a.c"]
    })
)

❯ cat main.c 
#include <stdio.h>

extern int get_feature_a_value(void);

int main(int argc, char** argv) {
    printf("feature a: %d\n", get_feature_a_value());
}

❯ cat feature_a.c 
int get_feature_a_value(void) {
    return 1;
}

❯ cat default_a.c 
int get_feature_a_value(void) {
    return 0;
}

❯ cat run_all.sh 
bazel build --//:enable_feature_a=false :my_binary
bazel-bin/my_binary
bazel build --//:enable_feature_a=true :my_binary
bazel-bin/my_binary

bazel build --//:enable_feature_a=false :my_binary_aliased
bazel-bin/my_binary_aliased
bazel build --//:enable_feature_a=true :my_binary_aliased
bazel-bin/my_binary_aliased
```

### What operating system are you running Bazel on?

macOS Big Sur 11.3.1

### What's the output of `bazel info release`?

`release 4.0.0`

###  Have you found anything relevant by searching the web?

Looked online, couldn't find anything.
The original reference to using `alias` with `select` when selecting things in a WORKSPACE was found here:
https://docs.bazel.build/versions/master/configurable-attributes.html
(see "Why doesn’t select() work with bind()?")


Issue comments:
Interesting observation thanks. I'll dig in some to find the root cause and report back...
Some breakpoint debugging shows me this is what fails:

https://github.com/bazelbuild/bazel/blob/09c621e4cf5b968f4c6cdf905ab142d5961f9ddc/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java#L477-L480

`optionDetails` has the registered value for `:enable_feature_a` (the original flag). But it's being called with `specifiedLabel=: enable_feature_a_aliased`. So `optionDetails.getOptionValue` returns null, which means it returns the flag's default value (`False`).

https://github.com/bazelbuild/bazel/commit/fd2c682a6a6bb0759f92476e533bffd2883b9c27 has some precedent. I think extending some of its setup logic to user-defined flags can fix this easily enough.
I prototyped the change, basically change to:

```
Object configurationValue =
    optionDetails.getOptionValue(actualLabel) != null
          ? optionDetails.getOptionValue(actualLabel)
          : provider.getDefaultValue(); 
```

It works, with one blocking caveat. There's already code to support `label_flag` in the opposite way:
https://github.com/bazelbuild/bazel/commit/83f8d1846938f19976c2f69f39941975303372b8

`label_flag` is implemented internally as an alias. And that code expects to be able to select() against its name vs. what it points to. Inside `ConfigSetting` there's no way to distinguish this from any other alias.

So we'd need either some way to distinguish them, and implement different semantics accordingly, or change the semantics so they're both interpreted the same way.
Re: `label_flag` implementation, see https://github.com/bazelbuild/bazel/blob/c9f9eedc313fc043755dd30ae7e01478eed7c417/src/main/java/com/google/devtools/build/lib/rules/LabelBuildSettings.java#L92.
Hi @gregestren , I'm new here and I'm looking through this ticket. Would it be fine if I pick this ticket up? 
Hi @nikhilpothuru - you're welcome to. Do you feel like you can navigate effectively the conceptual challenge above of

> So we'd need either some way to distinguish them, and implement different semantics accordingly, or change the semantics so they're both interpreted the same way.


?

The smallest user footprint way would probably be to tell whether an alias is a `label_flag` vs. a `bool_flag`, then implement each one's logic accordingly. 
Hi @gregestren, I take a look at this over the week and let you know if this is something I can handle. As I look through this ticket, I'll add any questions I have on this thread.
Hi @gregestren, Is there any documentation for setting up the development environment? (i.e. setting up the debugger)
Check out https://bazel.build/basics/getting_started.html.
Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (`@bazelbuild/triage`) if you think this issue is still relevant or you are interested in getting the issue resolved.
@bazelbuild/triage Not stale
Could you please prioritize this? Kleaf hits this issue too, so we just put in a wonky workaround here:

https://cs.android.com/android/kernel/superproject/+/common-android-mainline:build/kernel/kleaf/common_kernels.bzl;l=929;drc=8e7727c052aef0b328cd6dfa94121f7ef7d9f77f

It is not urgent, but definitely annoying.
@lberki noted this as a possible task for people looking for tasks. Still happily accepting contributors. This bug has some nice guidance in its history.
We also encountered this on ChromeOS. 

We are generating alias so developers don't have to type the full package paths. For example we have this alias set up:

```
% BOARD=amd64-generic bazel query @portage//target/sys-apps/attr:2.5.1 --output=build
(22:43:22) INFO: Current date is 2023-11-21
# /usr/local/google/home/aaronyu/.cache/bazel/_bazel_aaronyu/a5468eaf077d9e25965baa9c750fa428/external/_main~portage~portage/target/sys-apps/attr/BUILD.bazel:7:6
alias(
  name = "2.5.1",
  visibility = ["//bazel:internal"],
  actual = select({"//bazel/portage:stage1": "@_main~portage~portage//internal/packages/stage1/target/board/portage-stable/sys-apps/attr:2.5.1", "//bazel/portage:stage2": "@_main~portage~portage//internal/packages/stage2/target/board/portage-stable/sys-apps/attr:2.5.1"}),
)
```


We hit this problem when we want to generate some target-specific build flags, along with the alias where `@portage//target/sys-apps/attr:incremental` points to the `bool_flag` `"@_main~portage~portage//internal/packages/stage2/target/board/portage-stable/sys-apps/attr:incremental"`


`--@portage//target/sys-apps/attr:incremental` doesn't work in the command line.
I have an internal change out to fix this. Hilariously, @jin just started working on this 2.5 years old bug the exact moment I did :(
Any chance we get this in Bazel 7?
@bazel-io flag
@afq984 Bazel 7 has already been released, would Bazel 7.1 work?
Apologies, I confused this issue with #20582. We were aliasing on the flag, not config_setting.
I think we don't need this one.
@bazel-io fork 7.1.0
@iancha1992 Candidate for 7.0.1?
@brentleyjones I don't think so, unless @meteorcloudy pushes hard for it. 7.0.1 is to fix serious regressions that slipped through into 7.0.0 and this doesn't look like one.
A fix for this issue has been included in [Bazel 7.1.0 RC1](https://releases.bazel.build/7.1.0/rc1/index.html). Please test out the release candidate and report any issues as soon as possible. Thanks!

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
index cc06b86450274f..cb70244ed28eea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
@@ -497,10 +497,14 @@ static UserDefinedFlagMatch fromAttributeValueAndPrerequisites(
         } else if (target.satisfies(BuildSettingProvider.REQUIRE_BUILD_SETTING_PROVIDER)) {
           // build setting
           BuildSettingProvider provider = target.getProvider(BuildSettingProvider.class);
-          Object configurationValue =
-              optionDetails.getOptionValue(specifiedLabel) != null
-                  ? optionDetails.getOptionValue(specifiedLabel)
-                  : provider.getDefaultValue();
+
+          Object configurationValue;
+          if (optionDetails.getOptionValue(provider.getLabel()) != null) {
+            configurationValue = optionDetails.getOptionValue(provider.getLabel());
+          } else {
+            configurationValue = provider.getDefaultValue();
+          }
+
           Object convertedSpecifiedValue;
           try {
             // We don't need to supply a base package or repo mapping for the conversion here,


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java
index 5500ab253823fc..013cc9a02580d3 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java
@@ -116,6 +116,35 @@ public void testLabelFlag_set() throws Exception {
     assertThat(b.get("value")).isEqualTo("command_line_value");
   }
 
+  @Test
+  public void withSelectThroughAlias() throws Exception {
+    writeRulesBzl("flag");
+    scratch.file(
+        "test/BUILD",
+        "load('//test:rules.bzl', 'my_rule', 'simple_rule')",
+        "simple_rule(name = 'default', value = 'default_value')",
+        "simple_rule(name = 'command_line', value = 'command_line_value')",
+        "label_flag(name = 'my_label_flag', build_setting_default = ':default')",
+        "alias(name = 'my_label_flag_alias', actual = ':my_label_flag')",
+        "config_setting(",
+        "    name = 'is_default_label',",
+        "    flag_values = {':my_label_flag_alias': '//test:default'}",
+        ")",
+        "simple_rule(name = 'selector', value = select({':is_default_label': 'valid'}))");
+
+    useConfiguration();
+    getConfiguredTarget("//test:selector");
+    assertNoEvents();
+
+    reporter.removeHandler(failFastHandler);
+    useConfiguration(
+        ImmutableMap.of(
+            "//test:my_label_flag", Label.parseCanonicalUnchecked("//test:command_line")));
+    getConfiguredTarget("//test:selector");
+    assertContainsEvent(
+        "configurable attribute \"value\" in //test:selector doesn't match this configuration");
+  }
+
   @Test
   public void withSelect() throws Exception {
     writeRulesBzl("flag");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
index e96fb2f94d8e63..84ef07d33a286d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -2174,6 +2174,27 @@ public void ruleLicensesUsed() throws Exception {
     assertThat(getLicenses("//test:match")).containsExactly(LicenseType.NONE);
   }
 
+  @Test
+  public void aliasedStarlarkFlag() throws Exception {
+    scratch.file(
+        "test/flagdef.bzl",
+        "def _impl(ctx):",
+        "    return []",
+        "my_flag = rule(",
+        "    implementation = _impl,",
+        "    build_setting = config.string(flag = True))");
+
+    scratch.file(
+        "test/BUILD",
+        "load('//test:flagdef.bzl', 'my_flag')",
+        "my_flag(name = 'flag', build_setting_default = 'default')",
+        "alias(name = 'alias', actual = ':flag')",
+        "config_setting(name = 'alias_setting', flag_values = {':alias': 'specified'})");
+
+    useConfiguration(ImmutableMap.of("//test:flag", "specified"));
+    assertThat(getConfigMatchingProviderResultAsBoolean("//test:alias_setting")).isTrue();
+  }
+
   @Test
   public void simpleStarlarkFlag() throws Exception {
     scratch.file(