Remove the --experimental_dynamic_configs flag.

PiperOrigin-RevId: 370507866

Author: justinhorvitz

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

@@ -748,14 +748,6 @@ public List<Label> getActionListeners() {
     return options.actionListeners;
   }
 
-  /**
-   * Returns whether we should trim configurations to only include the fragments needed to correctly
-   * analyze a rule.
-   */
-  public boolean trimConfigurations() {
-    return options.configsMode == CoreOptions.ConfigsMode.ON;
-  }
-
   /**
    * <b>>Experimental feature:</b> if true, qualifying outputs use path prefixes based on their
    * content instead of the traditional <code>blaze-out/$CPU-$COMPILATION_MODE</code>.

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

@@ -16,12 +16,9 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Functions;
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.analysis.ConfigurationsCollector;
 import com.google.devtools.build.lib.analysis.ConfigurationsResult;
 import com.google.devtools.build.lib.analysis.Dependency;
@@ -37,7 +34,6 @@
 import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition.TransitionException;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -47,8 +43,6 @@
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.skyframe.PackageValue;
 import com.google.devtools.build.lib.skyframe.PlatformMappingValue;
-import com.google.devtools.build.lib.skyframe.TransitiveTargetKey;
-import com.google.devtools.build.lib.skyframe.TransitiveTargetValue;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -59,7 +53,6 @@
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -73,10 +66,8 @@
  * <p>This involves:
  *
  * <ol>
- *   <li>Patching a source configuration's options with the transition
- *   <li>If {@link BuildConfiguration#trimConfigurations} is true, trimming configuration fragments
- *       to only those needed by the destination target and its transitive dependencies
- *   <li>Getting the destination configuration from Skyframe
+ *   <li>Patching a source configuration's options with the transition.
+ *   <li>Getting the destination configuration from Skyframe.
  * </ol>
  *
  * <p>For the work of determining the transition requests themselves, see {@link
@@ -141,9 +132,7 @@ private BuildConfiguration getCurrentConfiguration() {
    * order, but that involves more runtime in performance-critical code, so we won't make that
    * change without a clear need.
    *
-   * <p>If {@link BuildConfiguration#trimConfigurations()} is true, these configurations only
-   * contain the fragments needed by the dep and its transitive closure. Else they unconditionally
-   * include all fragments.
+   * <p>These configurations unconditionally include all fragments.
    *
    * <p>This method is heavily performance-optimized. Because {@link
    * com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction} calls it over every edge in
@@ -185,18 +174,8 @@ private ImmutableList<Dependency> resolveConfiguration(
       return ImmutableList.of(resolveHostTransition(dependencyBuilder, dependencyKey));
     }
 
-    // Figure out the required fragments for this dep and its transitive closure.
-    Set<Class<? extends Fragment>> depFragments =
-        getTransitiveFragments(dependencyKey.getLabel(), getCurrentConfiguration());
-
-    // TODO(gregce): remove the below call once we have confidence trimmed configurations always
-    // provide needed fragments. This unnecessarily drags performance on the critical path (up
-    // to 0.5% of total analysis time as profiled over a simple cc_binary).
-    if (getCurrentConfiguration().trimConfigurations()) {
-      checkForMissingFragments(dependencyKind.getAttribute(), dependencyKey, depFragments);
-    }
-
-    return resolveGenericTransition(depFragments, dependencyBuilder, dependencyKey);
+    return resolveGenericTransition(
+        getCurrentConfiguration().getFragmentsMap().keySet(), dependencyBuilder, dependencyKey);
   }
 
   private Dependency resolveNullTransition(
@@ -352,30 +331,6 @@ private ImmutableList<String> collectTransitionKeys(Attribute attribute)
     return ImmutableList.of();
   }
 
-  /**
-   * Returns the configuration fragments required by a dep and its transitive closure. Returns null
-   * if Skyframe dependencies aren't yet available.
-   *
-   * @param dep label of the dep to check
-   * @param parentConfig configuration of the rule depending on the dep
-   */
-  private ImmutableSet<Class<? extends Fragment>> getTransitiveFragments(
-      Label dep, BuildConfiguration parentConfig)
-      throws InterruptedException, ValueMissingException {
-    if (!parentConfig.trimConfigurations()) {
-      return parentConfig.getFragmentsMap().keySet();
-    }
-    SkyKey fragmentsKey = TransitiveTargetKey.of(dep);
-    TransitiveTargetValue transitiveDepInfo = (TransitiveTargetValue) env.getValue(fragmentsKey);
-    if (transitiveDepInfo == null) {
-      // This should only be possible for tests. In actual runs, this was already called
-      // as a routine part of the loading phase.
-      // TODO(bazel-team): check this only occurs in a test context.
-      throw new ValueMissingException();
-    }
-    return transitiveDepInfo.getTransitiveConfigFragments().toSet();
-  }
-
   /**
    * Applies a configuration transition over a set of build options.
    *
@@ -438,52 +393,17 @@ private static BuildOptions addDefaultStarlarkOptions(
     return optionsWithDefaults == null ? fromOptions : optionsWithDefaults.build();
   }
 
-  /**
-   * Checks the config fragments required by a dep against the fragments in its actual
-   * configuration. If any are missing, triggers a descriptive "missing fragments" error.
-   */
-  private void checkForMissingFragments(
-      Attribute attribute, DependencyKey dep, Set<Class<? extends Fragment>> expectedDepFragments)
-      throws DependencyEvaluationException {
-    Set<String> ctgFragmentNames = new HashSet<>();
-    for (Fragment fragment : getCurrentConfiguration().getFragmentsMap().values()) {
-      ctgFragmentNames.add(fragment.getClass().getSimpleName());
-    }
-    Set<String> depFragmentNames = new HashSet<>();
-    for (Class<? extends Fragment> fragmentClass : expectedDepFragments) {
-      depFragmentNames.add(fragmentClass.getSimpleName());
-    }
-    Set<String> missing = Sets.difference(depFragmentNames, ctgFragmentNames);
-    if (!missing.isEmpty()) {
-      String msg =
-          String.format(
-              "%s: dependency %s from attribute \"%s\" is missing required config fragments: %s",
-              ctgValue.getLabel(),
-              dep.getLabel(),
-              attribute == null ? "(null)" : attribute.getName(),
-              Joiner.on(", ").join(missing));
-      env.getListener().handle(Event.error(msg));
-      throw new DependencyEvaluationException(new InvalidConfigurationException(msg));
-    }
-  }
-
   /**
    * This method allows resolution of configurations outside of a skyfunction call.
    *
    * <p>Unlike {@link #resolveConfigurations}, this doesn't expect the current context to be
    * evaluating dependencies of a parent target. So this method is also suitable for top-level
    * targets.
    *
-   * <p>Resolution consists of two steps:
-   *
-   * <ol>
-   *   <li>Apply the per-target transitions specified in {@code targetsToEvaluate}. This can be
-   *       used, e.g., to apply {@link
-   *       com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory}s over global
-   *       top-level configurations.
-   *   <li>(Optionally) trim configurations to only the fragments the targets actually need. This is
-   *       triggered by {@link BuildConfiguration#trimConfigurations}.
-   * </ol>
+   * <p>Resolution consists of applying the per-target transitions specified in {@code
+   * targetsToEvaluate}. This can be used, e.g., to apply {@link
+   * com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory}s over global
+   * top-level configurations.
    *
    * <p>Preserves the original input order (but merges duplicate nodes that might occur due to
    * top-level configuration transitions) . Uses original (untrimmed, pre-transition) configurations

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

@@ -611,41 +611,6 @@ public String getTypeDescription() {
               + "target_environment values.")
   public Label autoCpuEnvironmentGroup;
 
-  /** Values for --experimental_dynamic_configs. */
-  public enum ConfigsMode {
-    /**
-     * Deprecated mode: Each configured target is evaluated with only the configuration fragments it
-     * needs by loading the target graph and examining the transitive requirements for each target
-     * before analysis begins.
-     *
-     * <p>To become a no-op soon: b/129289764
-     */
-    ON,
-    /** Default mode: Each configured target is evaluated with all fragments known to Blaze. */
-    NOTRIM
-  }
-
-  /** Converter for --experimental_dynamic_configs. */
-  public static class ConfigsModeConverter extends EnumConverter<ConfigsMode> {
-    public ConfigsModeConverter() {
-      super(ConfigsMode.class, "configurations mode");
-    }
-  }
-
-  @Option(
-      name = "experimental_dynamic_configs",
-      defaultValue = "notrim",
-      converter = ConfigsModeConverter.class,
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {
-        OptionEffectTag.LOSES_INCREMENTAL_STATE,
-        OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION,
-        OptionEffectTag.LOADING_AND_ANALYSIS,
-      },
-      metadataTags = {OptionMetadataTag.EXPERIMENTAL},
-      help = "Instantiates build configurations with the specified properties")
-  public ConfigsMode configsMode;
-
   /** Values for --experimental_output_paths. */
   public enum OutputPathsMode {
     /** Use the production output path model. */
@@ -875,7 +840,7 @@ public enum IncludeConfigFragmentsEnum {
     /** Provide the fragments required <em>directly</em> by this rule. */
     DIRECT,
     /** Provide the fragments required by this rule and its transitive dependencies. */
-    TRANSITIVE;
+    TRANSITIVE
   }
 
   /** Enum converter for --include_config_fragments_provider. */
@@ -896,7 +861,6 @@ public FragmentOptions getHost() {
     host.compilationMode = hostCompilationMode;
     host.isHost = true;
     host.isExec = false;
-    host.configsMode = configsMode;
     host.outputPathsMode = outputPathsMode;
     host.enableRunfiles = enableRunfiles;
     host.executionInfoModifier = executionInfoModifier;

src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java

@@ -355,7 +355,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
         associatedConfiguredTargetAndData.fromConfiguredTarget(associatedTarget);
     aspectPathBuilder.add(aspect);
 
-    SkyframeDependencyResolver resolver = view.createDependencyResolver(env);
+    SkyframeDependencyResolver resolver = new SkyframeDependencyResolver(env);
     NestedSetBuilder<Package> transitivePackagesForPackageRootResolution =
         storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null;
 

src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java

@@ -245,20 +245,9 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
               : transitivePackagesForPackageRootResolution.build());
     }
 
-    // This line is only needed for accurate error messaging. Say this target has a circular
-    // dependency with one of its deps. With this line, loading this target fails so Bazel
-    // associates the corresponding error with this target, as expected. Without this line,
-    // the first TransitiveTargetValue call happens on its dep (in trimConfigurations), so Bazel
-    // associates the error with the dep, which is misleading.
-    if (configuration != null
-        && configuration.trimConfigurations()
-        && env.getValue(TransitiveTargetKey.of(label)) == null) {
-      return null;
-    }
-
     TargetAndConfiguration ctgValue = new TargetAndConfiguration(target, configuration);
 
-    SkyframeDependencyResolver resolver = view.createDependencyResolver(env);
+    SkyframeDependencyResolver resolver = new SkyframeDependencyResolver(env);
 
     ToolchainCollection<UnloadedToolchainContext> unloadedToolchainContexts = null;