Deprecate --incompatible_repo_env_ignores_action_env

Author: Silic0nS0ldier

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

@@ -463,9 +463,6 @@ If specified, the value of the cpu constraint (`@platforms//cpu:cpu`) of
           `=name`, which unsets the variable of that name. This option can be used
           multiple times; for options given for the same variable, the latest wins,
           options for different variables accumulate.
-
-          Note that unless `--incompatible_repo_env_ignores_action_env` is true, all `name=value`
-          pairs will be available to repository rules.
           """)
   public List<Converters.EnvVar> actionEnvironment;
 

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java

@@ -965,6 +965,16 @@ public static final class AllCommandGraveyardOptions extends OptionsBase {
         effectTags = {OptionEffectTag.NO_OP},
         help = "No-op.")
     public boolean legacyLocalFallback;
+
+    @Deprecated
+    @Option(
+      name = "incompatible_repo_env_ignores_action_env",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      metadataTags = {OptionMetadataTag.DEPRECATED},
+      help = "No-op.")
+  public boolean repoEnvIgnoresActionEnv;
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java

@@ -345,18 +345,12 @@ public void exit(AbruptExitException exception) {
         switch (envVar) {
           case Converters.EnvVar.Set(String name, String value) -> {
             visibleActionEnv.remove(name);
-            if (!options.getOptions(CommonCommandOptions.class).repoEnvIgnoresActionEnv) {
-              repoEnv.put(name, value);
-            }
           }
           case Converters.EnvVar.Inherit(String name) -> {
             visibleActionEnv.add(name);
           }
           case Converters.EnvVar.Unset(String name) -> {
             visibleActionEnv.remove(name);
-            if (!options.getOptions(CommonCommandOptions.class).repoEnvIgnoresActionEnv) {
-              repoEnv.remove(name);
-            }
           }
         }
       }
@@ -972,9 +966,7 @@ public String determineOutputFileSystem() {
   }
 
   /**
-   * Returns the repository environment created from the client environment, {@code --repo_env}, and
-   * {@code --action_env=NAME=VALUE} (when {@code
-   * --incompatible_repo_env_ignores_action_env=false}).
+   * Returns the repository environment created from the client environment and {@code --repo_env}.
    */
   public Map<String, String> getRepoEnv() {
     return Collections.unmodifiableMap(repoEnv);

src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java

@@ -621,19 +621,6 @@ public String getTypeDescription() {
           """)
   public List<Converters.EnvVar> repositoryEnvironment;
 
-  @Option(
-      name = "incompatible_repo_env_ignores_action_env",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
-      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
-      help =
-          """
-          If true, <code>--action_env=NAME=VALUE</code> will no longer affect repository rule \
-          and module extension environments.
-          """)
-  public boolean repoEnvIgnoresActionEnv;
-
   @Option(
       name = "heuristically_drop_nodes",
       oldName = "experimental_heuristically_drop_nodes",

src/test/shell/bazel/starlark_repository_test.sh

@@ -582,37 +582,6 @@ function environ_invalidation_test_template() {
   assert_equals 6 $(cat "${execution_file}")
 }
 
-function environ_invalidation_action_env_test_template() {
-  local startup_flag="${1-}"
-  local command_flag="--noincompatible_repo_env_ignores_action_env"
-  setup_starlark_repository
-
-  # We use a counter to avoid other invalidation to hide repository
-  # invalidation (e.g., --action_env=K=V will cause all repositories to re-run).
-  local execution_file="$(setup_invalidation_test)"
-
-  # Set to FOO=BAZ BAR=FOO
-  FOO=BAZ BAR=FOO bazel ${startup_flag} build "${command_flag}" @foo//:bar >& $TEST_log \
-      || fail "Failed to build"
-  expect_log "<1> FOO=BAZ BAR=FOO BAZ=undefined"
-  assert_equals 1 $(cat "${execution_file}")
-
-  # Test with changing using --action_env
-  bazel ${startup_flag} build "${command_flag}" \
-      --action_env FOO=BAZ --action_env BAR=FOO  --action_env BEZ=BAR \
-      @foo//:bar >& $TEST_log || fail "Failed to build"
-  assert_equals 1 $(cat "${execution_file}")
-  bazel ${startup_flag} build "${command_flag}" \
-      --action_env FOO=BAZ --action_env BAR=FOO --action_env BAZ=BAR \
-      @foo//:bar >& $TEST_log || fail "Failed to build"
-  assert_equals 1 $(cat "${execution_file}")
-  bazel ${startup_flag} build "${command_flag}" \
-      --action_env FOO=BAR --action_env BAR=FOO --action_env BAZ=BAR \
-      @foo//:bar >& $TEST_log || fail "Failed to build"
-  expect_log "<2> FOO=BAR BAR=FOO BAZ=BAR"
-  assert_equals 2 $(cat "${execution_file}")
-}
-
 function test_starlark_repository_environ_invalidation() {
   environ_invalidation_test_template
 }
@@ -622,14 +591,6 @@ function test_starlark_repository_environ_invalidation_batch() {
   environ_invalidation_test_template --batch
 }
 
-function test_starlark_repository_environ_invalidation_action_env() {
-  environ_invalidation_action_env_test_template
-}
-
-function test_starlark_repository_environ_invalidation_action_env_batch() {
-  environ_invalidation_action_env_test_template --batch
-}
-
 # Test invalidation based on change to the bzl files
 function bzl_invalidation_test_template() {
   local startup_flag="${1-}"
@@ -3016,58 +2977,6 @@ EOF
     @repo//... &> $TEST_log || fail "expected Bazel to succeed"
 }
 
-function test_execute_environment_repo_env_ignores_action_env_off() {
-  cat >> $(setup_module_dot_bazel)  <<'EOF'
-my_repo = use_repo_rule("//:repo.bzl", "my_repo")
-my_repo(name="repo")
-EOF
-  touch BUILD
-  cat > repo.bzl <<'EOF'
-def _impl(ctx):
-  st = ctx.execute(
-    ["env"],
-  )
-  if st.return_code:
-    fail("Command did not succeed")
-  vars = {line.partition("=")[0]: line.partition("=")[-1] for line in st.stdout.strip().split("\n")}
-  if vars.get("ACTION_ENV_PRESENT") != "value1":
-    fail("ACTION_ENV_PRESENT has wrong value: " + vars.get("ACTION_ENV_PRESENT"))
-  ctx.file("BUILD", "exports_files(['data.txt'])")
-my_repo = repository_rule(_impl)
-EOF
-
-  bazel build \
-    --noincompatible_repo_env_ignores_action_env \
-    --action_env=ACTION_ENV_PRESENT=value1 \
-    @repo//... &> $TEST_log || fail "expected Bazel to succeed"
-}
-
-function test_execute_environment_repo_env_ignores_action_env_on() {
-  cat >> $(setup_module_dot_bazel)  <<'EOF'
-my_repo = use_repo_rule("//:repo.bzl", "my_repo")
-my_repo(name="repo")
-EOF
-  touch BUILD
-  cat > repo.bzl <<'EOF'
-def _impl(ctx):
-  st = ctx.execute(
-    ["env"],
-  )
-  if st.return_code:
-    fail("Command did not succeed")
-  vars = {line.partition("=")[0]: line.partition("=")[-1] for line in st.stdout.strip().split("\n")}
-  if "ACTION_ENV_REMOVED" in vars:
-    fail("ACTION_ENV_REMOVED should not be in the environment")
-  ctx.file("BUILD", "exports_files(['data.txt'])")
-my_repo = repository_rule(_impl)
-EOF
-
-  bazel build \
-    --incompatible_repo_env_ignores_action_env \
-    --action_env=ACTION_ENV_REMOVED=value1 \
-    @repo//... &> $TEST_log || fail "expected Bazel to succeed"
-}
-
 function test_dependency_on_repo_with_invalid_name() {
   cat >> $(setup_module_dot_bazel) <<'EOF'
 my_repo = use_repo_rule("//:repo.bzl", "my_repo")