Repository: bazelbuild/bazel
Issue: `common` does not work with `flag_alias`
Labels: type: feature request
P2
team-Configurability

### Description of the bug:


Using the new `common` in a .bazelrc with `flag_alias` returns the following error:

```sh

ERROR: /Users/<user>/Development/cash-ios/.bazelrc: "common --flag_alias=build_config=//:build_config" disallowed. --flag_alias only supports the "build" command.
```

### Which category does this issue belong to?


CLI, Configurability

### What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.


- Create a custom `config_setting`
- Create a .bazelrc
- Attempt to add a flag alias with the `common` command

```bzl
common --flag_alias=release_variant=//:release_variant
```

### Which operating system are you running Bazel on?


macOS

### What is the output of `bazel info release`?


release 6.3.2

### 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` ?


_No response_

### Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.


_No response_

### Have you found anything relevant by searching the web?

_No response_

### Any other information, logs, or outputs that you want to share?

Not entirely sure if this is expected but it feels like its not given how `common` works for other flags.

Issue comments:
Which commands besides `build` (and its logical descendents, like `test` and `cquery`) do you think need support for `--flag_alias`?
> Which commands besides `build` (and its logical descendents, like `test` and `cquery`) do you think need support for `--flag_alias`?

For our use case, none and I'm not sure where it might be useful in other commands. I was just basing this off of how `common` seems to be working for us with other flags. We've been able to replace usage of `build` in bazelrc with `common` except for this case.
The code that checks this is [in `BlazeOptionHandler`](https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java;drc=8833fd75d54c1763c221f1ed9d2e3f24ccfcc3d8;l=616), and the tests are in [`starlark_configurations_test.sh`](https://cs.opensource.google/bazel/bazel/+/master:src/test/shell/integration/starlark_configurations_test.sh;drc=4dc0d119a6f4901aa8353e75941c19205542743e;l=643).

The test makes it look like this is deliberate, and @gregestren added the check that it isn't supported for `test`, so I'm going to assign to him for further triage.
I'm also re-classifying this as a Feature Request instead of a Bug, since the behavior seems to be intentional.
Thats fair, I was considering this a bug based on this: https://github.com/bazelbuild/bazel/issues/3054
https://github.com/bazelbuild/bazel/commit/329b22a980ad7a0835c53938afe06a111f40ac96 is the original change.

The intention was to not allow flag alias definitions for subsets of `build` (like `test`). Motivated specifically by CI systems that start with a `$ bazel canonicalize-flags` to vet flags. `canonicalize-flags` itself is a subset of `build` and any setting has to be reflected there 

`common` goes the other way up the inheritance tree and sounds safe to me. 
This is basically a lowest common ancestor problem.

We can only apply flag alias definitions on nodes in `canonicalize-flags`'s ancestor path.
A fix for this issue has been included in [Bazel 7.0.0 RC5](https://releases.bazel.build/7.0.0/rc5/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/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
index 4756e023ff7387..9c862cb7019863 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
@@ -87,6 +87,10 @@ public final class BlazeOptionHandler {
   // being ignored as long as they are recognized by at least one (other) command.
   private static final String COMMON_PSEUDO_COMMAND = "common";
 
+  private static final String BUILD_COMMAND = "build";
+  private static final ImmutableSet<String> BUILD_COMMAND_ANCESTORS =
+      ImmutableSet.of(BUILD_COMMAND, COMMON_PSEUDO_COMMAND, ALWAYS_PSEUDO_COMMAND);
+
   // Marks an event to indicate a parsing error.
   static final String BAD_OPTION_TAG = "invalidOption";
   // Separates the invalid tag from the full error message for easier parsing.
@@ -611,12 +615,16 @@ static ListMultimap<String, RcChunkOfArgs> structureRcOptionsAndConfigs(
           // In production, "build" is always a valid command, but not necessarily in tests.
           // Particularly C0Command, which some tests use for low-level options parsing logic. We
           // don't want to interfere with those.
-          && validCommands.contains("build")
-          && !override.command.equals("build")) {
+          && validCommands.contains(BUILD_COMMAND)
+          && !BUILD_COMMAND_ANCESTORS.contains(override.command)) {
         throw new OptionsParsingException(
             String.format(
-                "%s: \"%s %s\" disallowed. --%s only supports the \"build\" command.",
-                rcFile, override.command, override.option, Converters.BLAZE_ALIASING_FLAG));
+                "%s: \"%s %s\" disallowed. --%s only supports these commands: %s",
+                rcFile,
+                override.command,
+                override.option,
+                Converters.BLAZE_ALIASING_FLAG,
+                String.join(", ", BUILD_COMMAND_ANCESTORS)));
       }
       String command = override.command;
       int index = command.indexOf(':');


Test patch:
diff --git a/src/test/shell/integration/starlark_configurations_test.sh b/src/test/shell/integration/starlark_configurations_test.sh
index b5ad195218098a..3cad6b08857c01 100755
--- a/src/test/shell/integration/starlark_configurations_test.sh
+++ b/src/test/shell/integration/starlark_configurations_test.sh
@@ -640,6 +640,19 @@ function test_rc_flag_alias_canonicalizes() {
   expect_log "--//$pkg:type=coffee"
 }
 
+function test_rc_flag_alias_supported_under_common_command() {
+  local -r pkg=$FUNCNAME
+  mkdir -p $pkg
+
+  add_to_bazelrc "common --flag_alias=drink=//$pkg:type"
+  write_build_setting_bzl
+
+  bazel canonicalize-flags -- --drink=coffee \
+    >& "$TEST_log" || fail "Expected success"
+
+  bazel build //$pkg:my_drink >& "$TEST_log" || fail "Expected success"
+}
+
 function test_rc_flag_alias_unsupported_under_test_command() {
   local -r pkg=$FUNCNAME
   mkdir -p $pkg
@@ -650,11 +663,11 @@ function test_rc_flag_alias_unsupported_under_test_command() {
   bazel canonicalize-flags -- --drink=coffee \
     >& "$TEST_log" && fail "Expected failure"
   expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
-"supports the \"build\" command."
+"supports these commands: build, common, always"
 
   bazel build //$pkg:my_drink >& "$TEST_log" && fail "Expected failure"
   expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
-"supports the \"build\" command."
+"supports these commands: build, common, always"
 
   # Post-test cleanup_workspace() calls "bazel clean", which would also fail
   # unless we reset the bazelrc.
@@ -671,11 +684,11 @@ function test_rc_flag_alias_unsupported_under_conditional_build_command() {
   bazel canonicalize-flags -- --drink=coffee \
 >& "$TEST_log" && fail "Expected failure"
   expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
-"supports the \"build\" command."
+"supports these commands: build, common, always"
 
   bazel build //$pkg:my_drink >& "$TEST_log" && fail "Expected failure"
   expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
-"supports the \"build\" command."
+"supports these commands: build, common, always"
 
   # Post-test cleanup_workspace() calls "bazel clean", which would also fail
   # unless we reset the bazelrc.
@@ -692,11 +705,11 @@ function test_rc_flag_alias_unsupported_with_space_assignment_syntax() {
   bazel canonicalize-flags -- --drink=coffee \
     >& "$TEST_log" && fail "Expected failure"
   expect_log "--flag_alias\" disallowed. --flag_alias only "\
-"supports the \"build\" command."
+"supports these commands: build, common, always"
 
   bazel build //$pkg:my_drink >& "$TEST_log" && fail "Expected failure"
   expect_log "--flag_alias\" disallowed. --flag_alias only "\
-"supports the \"build\" command."
+"supports these commands: build, common, always"
 
   # Post-test cleanup_workspace() calls "bazel clean", which would also fail
   # unless we reset the bazelrc.