Propagate required fragments from config conditions as configured tar…

…gets.

As the added test case shows, this is necessary to pick up select keys within an alias that resolves to a config setting.

Not sure whether both this and the existing propagation through the config conditions as providers are both necessary. Left TODO.

PiperOrigin-RevId: 459512518
Change-Id: I4de1925ec3cc41bba6a7ceb4881c21e6280747be

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -312,7 +312,7 @@ private ConfiguredTarget createRule(
                     rule,
                     configuration,
                     ruleClassProvider.getFragmentRegistry().getUniversalFragments(),
-                    configConditions.asProviders(),
+                    configConditions,
                     prerequisiteMap.values()))
             .build();
 
@@ -520,7 +520,7 @@ public ConfiguredAspect createAspect(
                     associatedTarget.getTarget().getAssociatedRule(),
                     aspectConfiguration,
                     ruleClassProvider.getFragmentRegistry().getUniversalFragments(),
-                    configConditions.asProviders(),
+                    configConditions,
                     Iterables.concat(prerequisiteMap.values(), ImmutableList.of(associatedTarget))))
             .build();
 

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

@@ -17,15 +17,13 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.collect.Collections2;
-import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BuildSettingProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum;
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
-import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeTransitionData;
@@ -35,7 +33,6 @@
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleTransitionData;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
-import java.util.Collection;
 import java.util.Objects;
 import javax.annotation.Nullable;
 
@@ -85,22 +82,22 @@ public static RequiredConfigFragmentsProvider getRuleRequiredFragmentsIfEnabled(
       Rule target,
       BuildConfigurationValue configuration,
       FragmentClassSet universallyRequiredFragments,
-      ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+      ConfigConditions configConditions,
       Iterable<ConfiguredTargetAndData> prerequisites) {
     IncludeConfigFragmentsEnum mode = getRequiredFragmentsMode(configuration);
     if (mode == IncludeConfigFragmentsEnum.OFF) {
       return null;
     }
     RuleClass ruleClass = target.getRuleClassObject();
     ConfiguredAttributeMapper attributes =
-        ConfiguredAttributeMapper.of(target, configConditions, configuration);
+        ConfiguredAttributeMapper.of(target, configConditions.asProviders(), configuration);
     RequiredConfigFragmentsProvider.Builder requiredFragments =
         getRequiredFragments(
             mode,
             configuration,
             universallyRequiredFragments,
             ruleClass.getConfigurationFragmentPolicy(),
-            configConditions.values(),
+            configConditions,
             prerequisites);
     if (!ruleClass.isStarlark()) {
       ruleClass
@@ -139,7 +136,7 @@ public static RequiredConfigFragmentsProvider getAspectRequiredFragmentsIfEnable
       Rule associatedTarget,
       BuildConfigurationValue configuration,
       FragmentClassSet universallyRequiredFragments,
-      ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+      ConfigConditions configConditions,
       Iterable<ConfiguredTargetAndData> prerequisites) {
     IncludeConfigFragmentsEnum mode = getRequiredFragmentsMode(configuration);
     if (mode == IncludeConfigFragmentsEnum.OFF) {
@@ -151,13 +148,14 @@ public static RequiredConfigFragmentsProvider getAspectRequiredFragmentsIfEnable
             configuration,
             universallyRequiredFragments,
             aspect.getDefinition().getConfigurationFragmentPolicy(),
-            configConditions.values(),
+            configConditions,
             prerequisites);
     aspectFactory.addAspectImplSpecificRequiredConfigFragments(requiredFragments);
     addRequiredFragmentsFromAspectTransitions(
         requiredFragments,
         aspect,
-        ConfiguredAttributeMapper.of(associatedTarget, configConditions, configuration),
+        ConfiguredAttributeMapper.of(
+            associatedTarget, configConditions.asProviders(), configuration),
         configuration.getBuildOptionDetails());
     return requiredFragments.build();
   }
@@ -168,7 +166,7 @@ private static RequiredConfigFragmentsProvider.Builder getRequiredFragments(
       BuildConfigurationValue configuration,
       FragmentClassSet universallyRequiredFragments,
       ConfigurationFragmentPolicy configurationFragmentPolicy,
-      Collection<ConfigMatchingProvider> configConditions,
+      ConfigConditions configConditions,
       Iterable<ConfiguredTargetAndData> prerequisites) {
     RequiredConfigFragmentsProvider.Builder requiredFragments =
         RequiredConfigFragmentsProvider.builder();
@@ -195,10 +193,18 @@ private static RequiredConfigFragmentsProvider.Builder getRequiredFragments(
                 Objects::nonNull))
         // Fragments universally required by everything:
         .addFragmentClasses(universallyRequiredFragments);
-    // Fragments required by attached select()s.
-    configConditions.forEach(
-        configCondition -> requiredFragments.merge(configCondition.requiredFragmentOptions()));
-
+    // Fragments required by attached select()s. Propagating fragments from the config conditions as
+    // configured targets is necessary in case of a dependency on an alias that resolves to a config
+    // setting. The providers only contain the resolved settings, which won't include fragments
+    // required to resolve a select within the alias rule.
+    // TODO(jhorvitz): Are the CTs themselves sufficient? Tests pass without the providers.
+    for (ConfigMatchingProvider provider : configConditions.asProviders().values()) {
+      requiredFragments.merge(provider.requiredFragmentOptions());
+    }
+    for (ConfiguredTargetAndData targetAndData : configConditions.asConfiguredTargets().values()) {
+      requiredFragments.merge(
+          targetAndData.getConfiguredTarget().getProvider(RequiredConfigFragmentsProvider.class));
+    }
     return requiredFragments;
   }
 

src/test/java/com/google/devtools/build/lib/analysis/RequiredConfigFragmentsTest.java

@@ -511,4 +511,24 @@ public void configuredTargetInErrorWithAllowAnalysisFailures() throws Exception
 
     assertThat(requiredFragments.getDefines()).containsExactly("FAIL_MESSAGE");
   }
+
+  @Test
+  public void aliasWithSelectResolvesToConfigSetting() throws Exception {
+    scratch.file(
+        "a/BUILD",
+        "config_setting(name = 'define_x', define_values = {'x': '1'})",
+        "config_setting(name = 'k8', values = {'cpu': 'k8'})",
+        "alias(name = 'alias_to_setting', actual = select({':define_x': ':k8'}))",
+        "genrule(",
+        "  name = 'gen',",
+        "  outs = ['gen.out'],",
+        "  cmd = select({':alias_to_setting': 'touch $@'}),",
+        ")");
+
+    useConfiguration("--define=x=1", "--cpu=k8", "--include_config_fragments_provider=transitive");
+    RequiredConfigFragmentsProvider requiredFragments =
+        getConfiguredTarget("//a:gen").getProvider(RequiredConfigFragmentsProvider.class);
+
+    assertThat(requiredFragments.getDefines()).containsExactly("x");
+  }
 }