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");
+ }
 }