Add the concept of a "universal" configuration fragment, which all
configurations contain regardless of whether their rules explicitly
require it.
This is used to ensure all rules have BazelConfiguration. That
fragment supplies the path to the shell, which powers
BuildConfiguration.getShExecutable(), which powers any rule that
generates a SpawnAction.
Since SpawnActions are such a ubiquitous pattern we only want to
accelerate going forward, there's no point not to make this
automatically available to every rule.
--
MOS_MIGRATED_REVID=107786879
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 7eb45f7..53c484c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -949,6 +949,7 @@
Order.STABLE_ORDER, PackageSpecification.EVERYTHING))
.setPrerequisites(getPrerequisiteMapForTesting(eventHandler, target, configurations))
.setConfigConditions(ImmutableSet.<ConfigMatchingProvider>of())
+ .setUniversalFragment(ruleClassProvider.getUniversalFragment())
.build();
}
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 616c60c..62d42c9 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
@@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
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;
import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
@@ -97,6 +98,7 @@
private final Digraph<Class<? extends RuleDefinition>> dependencyGraph =
new Digraph<>();
private ConfigurationCollectionFactory configurationCollectionFactory;
+ private Class<? extends BuildConfiguration.Fragment> universalFragment;
private PrerequisiteValidator prerequisiteValidator;
private ImmutableMap<String, SkylarkType> skylarkAccessibleJavaClasses = ImmutableMap.of();
@@ -163,6 +165,12 @@
return this;
}
+ public Builder setUniversalConfigurationFragment(
+ Class<? extends BuildConfiguration.Fragment> fragment) {
+ this.universalFragment = fragment;
+ return this;
+ }
+
public Builder setSkylarkAccessibleJavaClasses(ImmutableMap<String, SkylarkType> objects) {
this.skylarkAccessibleJavaClasses = objects;
return this;
@@ -238,6 +246,7 @@
ImmutableList.copyOf(configurationOptions),
ImmutableList.copyOf(configurationFragments),
configurationCollectionFactory,
+ universalFragment,
prerequisiteValidator,
skylarkAccessibleJavaClasses);
}
@@ -310,6 +319,12 @@
*/
private final ConfigurationCollectionFactory configurationCollectionFactory;
+ /**
+ * A configuration fragment that should be available to all rules even when they don't
+ * explicitly require it.
+ */
+ private final Class<? extends BuildConfiguration.Fragment> universalFragment;
+
private final ImmutableList<BuildInfoFactory> buildInfoFactories;
private final PrerequisiteValidator prerequisiteValidator;
@@ -327,6 +342,7 @@
ImmutableList<Class<? extends FragmentOptions>> configurationOptions,
ImmutableList<ConfigurationFragmentFactory> configurationFragments,
ConfigurationCollectionFactory configurationCollectionFactory,
+ Class<? extends BuildConfiguration.Fragment> universalFragment,
PrerequisiteValidator prerequisiteValidator,
ImmutableMap<String, SkylarkType> skylarkAccessibleJavaClasses) {
this.preludeLabel = preludeLabel;
@@ -339,6 +355,7 @@
this.configurationOptions = configurationOptions;
this.configurationFragments = configurationFragments;
this.configurationCollectionFactory = configurationCollectionFactory;
+ this.universalFragment = universalFragment;
this.prerequisiteValidator = prerequisiteValidator;
this.globals = createGlobals(skylarkAccessibleJavaClasses);
}
@@ -403,6 +420,14 @@
}
/**
+ * Returns the configuration fragment that should be available to all rules even when they
+ * don't explicitly require it.
+ */
+ public Class<? extends BuildConfiguration.Fragment> getUniversalFragment() {
+ return universalFragment;
+ }
+
+ /**
* Returns the defaults package for the default settings.
*/
public String getDefaultsPackageContent() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index e909714..1bc7dd2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -226,6 +226,7 @@
.setVisibility(convertVisibility(prerequisiteMap, env.getEventHandler(), rule, null))
.setPrerequisites(prerequisiteMap)
.setConfigConditions(configConditions)
+ .setUniversalFragment(ruleClassProvider.getUniversalFragment())
.build();
if (ruleContext.hasErrors()) {
return null;
@@ -307,6 +308,7 @@
.setPrerequisites(prerequisiteMap)
.setAspectAttributes(aspect.getDefinition().getAttributes())
.setConfigConditions(configConditions)
+ .setUniversalFragment(ruleClassProvider.getUniversalFragment())
.build();
if (ruleContext.hasErrors()) {
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index f94d22f..d95e95c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -139,6 +139,7 @@
private final Map<String, Attribute> aspectAttributes;
private final BuildConfiguration hostConfiguration;
private final ConfigurationFragmentPolicy configurationFragmentPolicy;
+ private final Class<? extends BuildConfiguration.Fragment> universalFragment;
private final ErrorReporter reporter;
private ActionOwner actionOwner;
@@ -148,11 +149,14 @@
private RuleContext(Builder builder, ListMultimap<String, ConfiguredTarget> targetMap,
ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap,
- Set<ConfigMatchingProvider> configConditions, Map<String, Attribute> aspectAttributes) {
+ Set<ConfigMatchingProvider> configConditions,
+ Class<? extends BuildConfiguration.Fragment> universalFragment,
+ Map<String, Attribute> aspectAttributes) {
super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null),
builder.visibility);
this.rule = builder.rule;
this.configurationFragmentPolicy = builder.configurationFragmentPolicy;
+ this.universalFragment = universalFragment;
this.targetMap = targetMap;
this.filesetEntryMap = filesetEntryMap;
this.configConditions = configConditions;
@@ -312,13 +316,10 @@
return getConfiguration(config).getSkylarkFragmentNames();
}
- public ConfigurationFragmentPolicy getConfigurationFragment() {
- return configurationFragmentPolicy;
- }
-
public <T extends Fragment> boolean isLegalFragment(
Class<T> fragment, ConfigurationTransition config) {
- return configurationFragmentPolicy.isLegalConfigurationFragment(fragment, config);
+ return fragment == universalFragment
+ || configurationFragmentPolicy.isLegalConfigurationFragment(fragment, config);
}
public <T extends Fragment> boolean isLegalFragment(Class<T> fragment) {
@@ -1227,6 +1228,7 @@
private final AnalysisEnvironment env;
private final Rule rule;
private final ConfigurationFragmentPolicy configurationFragmentPolicy;
+ private Class<? extends BuildConfiguration.Fragment> universalFragment;
private final BuildConfiguration configuration;
private final BuildConfiguration hostConfiguration;
private final PrerequisiteValidator prerequisiteValidator;
@@ -1255,7 +1257,7 @@
ListMultimap<String, ConfiguredTarget> targetMap = createTargetMap();
ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap =
createFilesetEntryMap(rule, configConditions);
- return new RuleContext(this, targetMap, filesetEntryMap, configConditions,
+ return new RuleContext(this, targetMap, filesetEntryMap, configConditions, universalFragment,
aspectAttributes != null ? aspectAttributes : ImmutableMap.<String, Attribute>of());
}
@@ -1290,6 +1292,18 @@
return this;
}
+ /**
+ * Sets the fragment that can be legally accessed even when not explicitly declared.
+ */
+ Builder setUniversalFragment(Class<? extends BuildConfiguration.Fragment> fragment) {
+ // TODO(bazel-team): Add this directly to ConfigurationFragmentPolicy, so we
+ // don't need separate logic specifically for checking this fragment. The challenge is
+ // that we need RuleClassProvider to figure out what this fragment is, and not every
+ // call state that creates ConfigurationFragmentPolicy has access to that.
+ this.universalFragment = fragment;
+ return this;
+ }
+
private boolean validateFilesetEntry(FilesetEntry filesetEntry, ConfiguredTarget src) {
if (src.getProvider(FilesetProvider.class) != null) {
return true;
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 03948e4..f66c151 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -382,5 +382,7 @@
builder.addConfigurationFragment(new AppleConfiguration.Loader());
builder.addConfigurationFragment(new J2ObjcConfiguration.Loader());
builder.addConfigurationFragment(new AndroidConfiguration.Loader());
+
+ builder.setUniversalConfigurationFragment(BazelConfiguration.class);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java
index 4f0dbca..820d5d7 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java
@@ -26,7 +26,6 @@
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
-import com.google.devtools.build.lib.bazel.rules.BazelConfiguration;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
@@ -49,8 +48,6 @@
<!-- #END_BLAZE_RULE.NAME --> */
return builder
.setOutputToGenfiles()
- // For BuildConfiguration.getShExecutable():
- .requiresConfigurationFragments(BazelConfiguration.class)
/* <!-- #BLAZE_RULE(genrule).ATTRIBUTE(srcs) -->
A list of inputs for this rule, such as source files to process.
${SYNOPSIS}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index 280e1a2..86a50b8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -145,6 +145,8 @@
fragment.asSubclass(BuildConfiguration.Fragment.class));
}
}
+ builder.getTransitiveConfigFragments().add(
+ ruleClassProvider.getUniversalFragment().asSubclass(BuildConfiguration.Fragment.class));
}
return builder.build(errorLoadingTarget);