Remove outdated affected by starlark transition flag.

This is unused ever since `--experimental_output_directory_naming_scheme` defaulted to `diff_against_dynamic_baseline`.

Part of #27636 cleanup.

PiperOrigin-RevId: 832476588
Change-Id: I6f97181d29c65f89f84ff5113aad223f621ef6b9

Author: gregestren

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -1756,7 +1756,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis/config:core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis/config:fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis/config:optioninfo",
-        "//src/main/java/com/google/devtools/build/lib/analysis/config:options_diff",
         "//src/main/java/com/google/devtools/build/lib/analysis/config:scope",
         "//src/main/java/com/google/devtools/build/lib/analysis/config:starlark_defined_config_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis/config/transitions:configuration_transition",

src/main/java/com/google/devtools/build/lib/analysis/config/BUILD

@@ -282,7 +282,6 @@ java_library(
         ":core_options",
         ":fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
-        "//src/main/java/com/google/devtools/build/lib/analysis:starlark/function_transition_util",
         "//src/main/java/com/google/devtools/build/lib/analysis/config/transitions:configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis/config/transitions:no_config_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis/config/transitions:patch_transition",

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

@@ -24,7 +24,6 @@
 import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.LabelToStringEntryConverter;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.util.RegexFilter;
-import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.Converters.BooleanConverter;
 import com.google.devtools.common.options.Converters.CommaSeparatedOptionSetConverter;
@@ -33,7 +32,6 @@
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionMetadataTag;
-import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.TriState;
 import java.util.ArrayList;
 import java.util.Comparator;
@@ -432,40 +430,6 @@ If specified, the value of the cpu constraint (`@platforms//cpu:cpu`) of
           """)
   public boolean useAutoExecGroups;
 
-  /** Regardless of input, converts to an empty list. For use with affectedByStarlarkTransition */
-  public static class EmptyListConverter extends Converter.Contextless<List<String>> {
-    @Override
-    public List<String> convert(String input) throws OptionsParsingException {
-      return ImmutableList.of();
-    }
-
-    @Override
-    public String getTypeDescription() {
-      return "Regardless of input, converts to an empty list. For use with"
-          + " affectedByStarlarkTransition";
-    }
-  }
-
-  /**
-   * This internal option is a *set* of names of options that have been changed by starlark
-   * transitions at any point in the build at the time of accessing. It contains both native and
-   * starlark options in label form. e.g. "//command_line_option:cpu" for native options and
-   * "//myapp:foo" for starlark options. This is used to regenerate {@code
-   * transitionDirectoryNameFragment} after each starlark transition.
-   */
-  @Option(
-      name = "affected by starlark transition",
-      defaultValue = "",
-      converter = EmptyListConverter.class,
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {
-        OptionEffectTag.LOSES_INCREMENTAL_STATE,
-        OptionEffectTag.AFFECTS_OUTPUTS,
-        OptionEffectTag.LOADING_AND_ANALYSIS
-      },
-      metadataTags = {OptionMetadataTag.INTERNAL})
-  public List<String> affectedByStarlarkTransition;
-
   /* At the moment, EXPLICIT_IN_OUTPUT_PATH is not being set here because platform_suffix
    * is being used as a configuration distinguisher for the exec transition. */
   @Option(

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

@@ -30,7 +30,6 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.AttributeTransitionData;
-import com.google.devtools.build.lib.packages.DeclaredExecGroup;
 import com.google.devtools.build.lib.rules.config.FeatureFlagValue;
 import com.google.devtools.build.lib.starlarkbuildapi.config.StarlarkConfigApi.ExecTransitionFactoryApi;
 import com.google.devtools.build.lib.util.Pair;
@@ -228,8 +227,6 @@ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler)
       //   of exactly how the immutable state and mutable state of BuildOptions is interacting.
       //   Might be good to have an option to wipeout that state rather than cloning so much.
       coreOptions.platformSuffix = "exec";
-      coreOptions.affectedByStarlarkTransition =
-          options.underlying().get(CoreOptions.class).affectedByStarlarkTransition;
       coreOptions.executionInfoModifier =
           options.underlying().get(CoreOptions.class).executionInfoModifier;
       coreOptions.overridePlatformCpuName =

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

@@ -15,10 +15,10 @@
 package com.google.devtools.build.lib.analysis.config;
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.devtools.build.lib.cmdline.LabelConstants.COMMAND_LINE_OPTION_PREFIX;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
+import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -167,14 +167,9 @@ public ImmutableSet<String> getExplicitInOutputPathOptions() {
    *
    * <p>platform_suffix is omitted if empty.
    *
-   * <p>The exact ST-hash used depends on if baselineOptions is available:
-   *
-   * <p>If not, assume in legacy mode and use `affected by starlark transition` to see what options
-   * need to be hashed.
-   *
-   * <p>If available, the hash includes all options that are different between buildOptions and
-   * baselineOptions but were also not excluded from the output path by a call to {@link
-   * Fragment.OutputDirectoriesContext.markAsExplicitInOutputPathFor}
+   * <p>The exact ST-hash used depends on baselineOptions. The hash includes all options that are
+   * different between buildOptions and baselineOptions but were also not excluded from the output
+   * path by a call to {@link Fragment.OutputDirectoriesContext.markAsExplicitInOutputPathFor}
    */
   static final String computeMnemonic(
       BuildOptions buildOptions,
@@ -226,15 +221,10 @@ static final String computeMnemonic(
               + missingOptions);
     }
 
-    if (baselineOptions == null) {
-      ctx.checkedAddToMnemonic(
-          computeNameFragmentWithAffectedByStarlarkTransition(buildOptions),
-          "Transition directory name fragment");
-    } else {
-      ctx.checkedAddToMnemonic(
-          computeNameFragmentWithDiff(buildOptions, baselineOptions, explicitInOutputPathOptions),
-          "Transition directory name fragment");
-    }
+    ctx.checkedAddToMnemonic(
+        computeNameFragmentWithDiff(
+            buildOptions, Verify.verifyNotNull(baselineOptions), explicitInOutputPathOptions),
+        "Transition directory name fragment");
     return ctx.getMnemonic();
   }
 
@@ -319,43 +309,6 @@ public static String computeNameFragmentWithDiff(
     return hashChosenOptions(toOptions, chosenNativeOptions, chosenStarlarkOptions);
   }
 
-  /**
-   * Compute the output directory name fragment corresponding to the new BuildOptions based on the
-   * names and values of all options (both native and Starlark) previously transitioned anywhere in
-   * the build by Starlark transitions. Options only set on command line are not affecting the
-   * computation.
-   *
-   * @param toOptions the {@link BuildOptions} to use to calculate which we need to compute {@code
-   *     transitionDirectoryNameFragment}.
-   */
-  private static String computeNameFragmentWithAffectedByStarlarkTransition(
-      BuildOptions toOptions) {
-    CoreOptions buildConfigOptions = toOptions.get(CoreOptions.class);
-    if (buildConfigOptions.affectedByStarlarkTransition.isEmpty()) {
-      return "";
-    }
-
-    ImmutableList.Builder<String> affectedNativeOptions = ImmutableList.builder();
-    ImmutableList.Builder<String> affectedStarlarkOptions = ImmutableList.builder();
-
-    // Note that explicitInOutputPathOptions is not sent to this function.
-    // It is possible for two BuildOptions to differ only in `affected by Starlark transition`
-    //   where the only different is one includes a marked option and the other doesn't.
-    // Thus, must include all options so those cases get a different output path.
-    // This legacy is no longer the default and thus entire code path is slated for removal.
-    for (String optionName : buildConfigOptions.affectedByStarlarkTransition) {
-      if (optionName.startsWith(COMMAND_LINE_OPTION_PREFIX)) {
-        String nativeOptionName = optionName.substring(COMMAND_LINE_OPTION_PREFIX.length());
-        affectedNativeOptions.add(nativeOptionName);
-      } else {
-        affectedStarlarkOptions.add(optionName);
-      }
-    }
-
-    return hashChosenOptions(
-        toOptions, affectedNativeOptions.build(), affectedStarlarkOptions.build());
-  }
-
   /**
    * Compute a hash of the given BuildOptions by hashing only the options referenced in both
    * chosenNative and chosenStarlark. The order of the chosen order does not matter (as this

src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java

@@ -16,7 +16,6 @@
 
 import static com.google.common.base.Predicates.not;
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition.PATCH_TRANSITION_KEY;
 import static com.google.devtools.build.lib.cmdline.LabelConstants.COMMAND_LINE_OPTION_PREFIX;
 import static java.util.stream.Collectors.joining;
@@ -30,13 +29,11 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Sets.SetView;
-import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.CustomFlagConverter;
 import com.google.devtools.build.lib.analysis.config.CoreOptions;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.OptionInfo;
-import com.google.devtools.build.lib.analysis.config.OptionsDiff;
 import com.google.devtools.build.lib.analysis.config.Scope;
 import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
 import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition.ValidationException;
@@ -56,9 +53,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.NoneType;
 import net.starlark.java.eval.Starlark;
@@ -578,8 +573,6 @@ private static BuildOptions applyTransition(
     // toOptions being null means the transition hasn't changed anything. We avoid preemptively
     // cloning it from fromOptions since options cloning is an expensive operation.
     BuildOptions toOptions = null;
-    // The names of options (Starlark + native) that are different after this transition and must
-    //   be added to "affected by Starlark transition"
     // Starlark options that are different after this transition. We collect all of them, then clone
     // the build options once with all cumulative changes. Native option changes, in contrast, are
     // set directly in the BuildOptions instance. The former approach is preferred since it makes
@@ -717,7 +710,6 @@ private static BuildOptions applyTransition(
               toOptions = fromOptions.clone();
             }
             def.setValue(toOptions.get(optionInfo.getOptionClass()), convertedValue);
-
           }
 
         } catch (IllegalArgumentException e) {
@@ -741,48 +733,10 @@ private static BuildOptions applyTransition(
             .addStarlarkOptions(changedStarlarkOptions)
             .build();
     if (starlarkTransition.isForAnalysisTesting()) {
-      // We need to record every time we change a configuration option.
-      // see {@link #updateOutputDirectoryNameFragment} for usage.
       toOptions.get(CoreOptions.class).evaluatingForAnalysisTest = true;
     }
     return toOptions;
   }
 
-  /** Return different options in "affected by Starlark transition" form */
-  // TODO(blaze-configurability-team):This only exists for pseudo-legacy fixups of native
-  //   transitions. Remove once those fixups are removed in favor of the global fixup.
-  public static ImmutableSet<String> getAffectedByStarlarkTransitionViaDiff(
-      BuildOptions toOptions, BuildOptions baselineOptions) {
-    if (toOptions.equals(baselineOptions)) {
-      return ImmutableSet.of();
-    }
-
-    OptionsDiff diff = OptionsDiff.diff(toOptions, baselineOptions);
-    Stream<String> diffNative =
-        diff.getFirst().keySet().stream()
-            .map(option -> COMMAND_LINE_OPTION_PREFIX + option.getOptionName());
-    // Note: getChangedStarlarkOptions includes all changed options, added options and removed
-    //   options between baselineOptions and toOptions. This is necessary since there is no current
-    //   notion of trimming a Starlark option: 'null' or non-existent justs means set to default.
-    Stream<String> diffStarlark = diff.getChangedStarlarkOptions().stream().map(Label::toString);
-    return Streams.concat(diffNative, diffStarlark).collect(toImmutableSet());
-  }
-
-  /**
-   * Extend the global build config affectedByStarlarkTransition, by adding any new option names
-   * from changedOptions. Does nothing if output directory naming scheme is not in legacy mode.
-   */
-  public static void updateAffectedByStarlarkTransition(
-      CoreOptions buildConfigOptions, Set<String> changedOptions) {
-    if (changedOptions.isEmpty()) {
-      return;
-    }
-    Set<String> mutableCopyToUpdate =
-        new TreeSet<>(buildConfigOptions.affectedByStarlarkTransition);
-    mutableCopyToUpdate.addAll(changedOptions);
-    buildConfigOptions.affectedByStarlarkTransition =
-        ImmutableList.sortedCopyOf(mutableCopyToUpdate);
-  }
-
   private FunctionTransitionUtil() {}
 }

src/main/java/com/google/devtools/build/lib/rules/config/BUILD

@@ -34,7 +34,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:required_config_fragments_provider",
         "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
         "//src/main/java/com/google/devtools/build/lib/analysis:rule_error_consumer",
-        "//src/main/java/com/google/devtools/build/lib/analysis:starlark/function_transition_util",
         "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_config",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",

src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationFunctionTest.java

@@ -78,10 +78,6 @@ def _impl(ctx):
         """);
   }
 
-  private CoreOptions getCoreOptions(ConfiguredTarget target) {
-    return getConfiguration(target).getOptions().get(CoreOptions.class);
-  }
-
   private String getMnemonic(ConfiguredTarget target) {
     return getConfiguration(target).getMnemonic();
   }
@@ -145,7 +141,6 @@ def _basic_impl(ctx):
     ConfiguredTarget test = getConfiguredTarget("//test");
 
     assertThat(getMnemonic(test)).doesNotContain("-ST-");
-    assertThat(getCoreOptions(test).affectedByStarlarkTransition).isEmpty();
 
     @SuppressWarnings("unchecked")
     ConfiguredTarget dep =
@@ -156,7 +151,6 @@ def _basic_impl(ctx):
         .endsWith(
             OutputPathMnemonicComputer.transitionDirectoryNameFragment(
                 ImmutableList.of("//test:foo=transitioned")));
-    assertThat(getCoreOptions(dep).affectedByStarlarkTransition).isEmpty();
   }
 
   @Test
@@ -213,7 +207,6 @@ def _basic_impl(ctx):
 
     assertThat(getConfiguration(test).getMnemonic()).contains("fastbuild");
     assertThat(getMnemonic(test)).doesNotContain("-ST-");
-    assertThat(getCoreOptions(test).affectedByStarlarkTransition).isEmpty();
 
     @SuppressWarnings("unchecked")
     ConfiguredTarget dep =
@@ -222,7 +215,6 @@ def _basic_impl(ctx):
 
     assertThat(getConfiguration(dep).getMnemonic()).contains("opt");
     assertThat(getMnemonic(dep)).doesNotContain("-ST-");
-    assertThat(getCoreOptions(dep).affectedByStarlarkTransition).isEmpty();
   }
 
   @Test
@@ -292,7 +284,6 @@ def _basic_impl(ctx):
     ConfiguredTarget test = getConfiguredTarget("//test");
 
     assertThat(getMnemonic(test)).doesNotContain("-ST-");
-    assertThat(getCoreOptions(test).affectedByStarlarkTransition).isEmpty();
 
     @SuppressWarnings("unchecked")
     ConfiguredTarget middle =
@@ -303,15 +294,13 @@ def _basic_impl(ctx):
         .endsWith(
             OutputPathMnemonicComputer.transitionDirectoryNameFragment(
                 ImmutableList.of("//test:foo=transitioned")));
-    assertThat(getCoreOptions(middle).affectedByStarlarkTransition).isEmpty();
 
     @SuppressWarnings("unchecked")
     ConfiguredTarget root =
         Iterables.getOnlyElement(
             (List<ConfiguredTarget>) getMyInfoFromTarget(middle).getValue("dep"));
 
     assertThat(getMnemonic(test)).doesNotContain("-ST-");
-    assertThat(getCoreOptions(test).affectedByStarlarkTransition).isEmpty();
 
     assertThat(getConfiguration(test)).isEqualTo(getConfiguration(root));
     assertThat(getConfiguration(test)).isNotEqualTo(getConfiguration(middle));

src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java

@@ -107,7 +107,6 @@ public void executionTransitionOutputPathDistinguisher() throws Exception {
             new BuildOptionsView(options, transition.requiresOptionFragments()),
             new StoredEventHandler());
 
-    assertThat(result.get(CoreOptions.class).affectedByStarlarkTransition).isEmpty();
     assertThat(result.get(CoreOptions.class).platformSuffix).isEqualTo("exec");
   }
 

src/test/java/com/google/devtools/build/lib/analysis/starlark/BUILD

@@ -55,7 +55,6 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:required_config_fragments_provider",
         "//src/main/java/com/google/devtools/build/lib/analysis/config:build_configuration",
-        "//src/main/java/com/google/devtools/build/lib/analysis/config:build_options",
         "//src/main/java/com/google/devtools/build/lib/analysis/config:core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis/config/transitions:configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/cmdline",