Don't inherit the host configuration's fragments from the target config in --experimental_dynamic_configs=notrim mode.

--
MOS_MIGRATED_REVID=135126724

4 files changed, 27 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 2037dbbb38..2d3119c4ac 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -25,6 +25,7 @@ import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -63,6 +64,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 
 import javax.annotation.Nullable;
@@ -716,4 +718,17 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
   public String getDefaultWorkspaceSuffix() {
     return defaultWorkspaceFileSuffix;
   }
+
+  /**
+   * Returns all registered {@link BuildConfiguration.Fragment} classes.
+   */
+  public Set<Class<? extends BuildConfiguration.Fragment>> getAllFragments() {
+    ImmutableSet.Builder<Class<? extends BuildConfiguration.Fragment>> fragmentsBuilder =
+        ImmutableSet.builder();
+    for (ConfigurationFragmentFactory factory : getConfigurationFragments()) {
+      fragmentsBuilder.add(factory.creates());
+    }
+    fragmentsBuilder.add(getUniversalFragment());
+    return fragmentsBuilder.build();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
index e73b2b4cd5..007a5c129b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
@@ -17,6 +17,7 @@ import com.google.common.base.Supplier;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -116,13 +117,17 @@ public class ConfigurationCollectionFunction implements SkyFunction {
   }
 
   /** Returns the host configuration, or null on missing Skyframe deps. */
-  private static BuildConfiguration getHostConfiguration(
+  private BuildConfiguration getHostConfiguration(
       Environment env, BuildConfiguration targetConfiguration)
       throws InvalidConfigurationException, InterruptedException {
     if (targetConfiguration.useDynamicConfigurations()) {
       BuildOptions hostOptions = HostTransition.INSTANCE.apply(targetConfiguration.getOptions());
       SkyKey hostConfigKey =
-          BuildConfigurationValue.key(targetConfiguration.fragmentClasses(), hostOptions);
+          BuildConfigurationValue.key(
+              targetConfiguration.trimConfigurations()
+                  ? targetConfiguration.fragmentClasses()
+                  : ((ConfiguredRuleClassProvider) ruleClassProvider).getAllFragments(),
+              hostOptions);
       BuildConfigurationValue skyValHost = (BuildConfigurationValue)
           env.getValueOrThrow(hostConfigKey, InvalidConfigurationException.class);
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index e9ec6f417f..0c162b1783 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -517,7 +517,10 @@ public final class SkyframeBuildView {
     // trims a host configuration to the same scope as a target configuration. Since their options
     // are different, the host instance may actually be able to produce the fragment. So it's
     // wrong and potentially dangerous to unilaterally exclude it.
-    Set<Class<? extends BuildConfiguration.Fragment>> fragmentClasses = config.fragmentClasses();
+    Set<Class<? extends BuildConfiguration.Fragment>> fragmentClasses =
+        config.trimConfigurations()
+            ? config.fragmentClasses()
+            : ((ConfiguredRuleClassProvider) ruleClassProvider).getAllFragments();
     BuildConfiguration hostConfig = hostConfigurationCache.get(fragmentClasses);
     if (hostConfig != null) {
       return hostConfig;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 804b950d4d..c87db1e6b4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1247,7 +1247,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     // Check: if !Configuration.useDynamicConfigs then just return the original configs.
     Set<Class<? extends BuildConfiguration.Fragment>> allFragments = null;
     if (useUntrimmedDynamicConfigs(fromOptions)) {
-      allFragments = getAllFragments();
+      allFragments = ((ConfiguredRuleClassProvider) ruleClassProvider).getAllFragments();
     }
 
     // Get the fragments needed for dynamic configuration nodes.
@@ -1323,20 +1323,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   /**
-   * Returns all configuration fragments registered with Blaze.
-   */
-  private Set<Class<? extends BuildConfiguration.Fragment>> getAllFragments() {
-    ImmutableSet.Builder<Class<? extends BuildConfiguration.Fragment>> fragmentsBuilder =
-        ImmutableSet.builder();
-    for (ConfigurationFragmentFactory factory :
-        ((ConfiguredRuleClassProvider) ruleClassProvider).getConfigurationFragments()) {
-      fragmentsBuilder.add(factory.creates());
-    }
-    fragmentsBuilder.add(((ConfiguredRuleClassProvider) ruleClassProvider).getUniversalFragment());
-    return fragmentsBuilder.build();
-  }
-
-  /**
    * Computes the build options needed for the given key, accounting for transitions possibly
    * specified in the key.
    */