Provide the ability to declare host config fragments for
native rules (which already exists for Skylark rules).
Don't actually distinguish between host and target fragments
on the consuming end yet, though. That'll be a subsequent
fine-tuning in constructing dynamic host / target configurations.
So this cl may overapproximate actual needs, but that puts us
in a better state than underapproximating, which just breaks builds.
Also add one instance: Jvm.class, which gets used by java_library
to find the java executable for compilation
(in BaseJavaCompilationHelper).
--
MOS_MIGRATED_REVID=107237470
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaLibraryRule.java
index 4144d46..23c684b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaLibraryRule.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;
import com.google.devtools.build.lib.rules.java.JavaSourceInfoProvider;
+import com.google.devtools.build.lib.rules.java.Jvm;
import com.google.devtools.build.lib.rules.java.ProguardLibraryRule;
/**
@@ -42,6 +43,7 @@
return builder
.requiresConfigurationFragments(
JavaConfiguration.class, CppConfiguration.class, J2ObjcConfiguration.class)
+ .requiresHostConfigurationFragments(Jvm.class) // For BaseJavaCompilationHelper
/* <!-- #BLAZE_RULE(java_library).IMPLICIT_OUTPUTS -->
<ul>
<li><code>lib<var>name</var>.jar</code>: A Java archive containing the class files.</li>
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
index 9833174..929ec0e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
@@ -13,15 +13,18 @@
// limitations under the License.
package com.google.devtools.build.lib.packages;
+import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
import com.google.devtools.build.lib.syntax.FragmentClassNameResolver;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
@@ -62,29 +65,65 @@
* Builder to construct a new ConfigurationFragmentPolicy.
*/
public static final class Builder {
- private Set<Class<?>> requiredConfigurationFragments = new LinkedHashSet<>();
- private Map<ConfigurationTransition, ImmutableSet<String>> requiredConfigurationFragmentNames =
- new LinkedHashMap<>();
+ private final Multimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments
+ = ArrayListMultimap.create();
+ private final Map<ConfigurationTransition, ImmutableSet<String>>
+ requiredConfigurationFragmentNames = new LinkedHashMap<>();
private MissingFragmentPolicy missingFragmentPolicy = MissingFragmentPolicy.FAIL_ANALYSIS;
private FragmentClassNameResolver fragmentNameResolver;
/**
* Declares that the implementation of the associated rule class requires the given
- * configuration fragments to be present in the configuration. The value is inherited by
- * subclasses.
+ * fragments to be present in this rule's host and target configurations.
+ *
+ * <p>The value is inherited by subclasses.
*/
public Builder requiresConfigurationFragments(Collection<Class<?>> configurationFragments) {
- requiredConfigurationFragments.addAll(configurationFragments);
+ requiredConfigurationFragments.putAll(ConfigurationTransition.HOST, configurationFragments);
+ requiredConfigurationFragments.putAll(ConfigurationTransition.NONE, configurationFragments);
return this;
}
/**
* Declares that the implementation of the associated rule class requires the given
- * configuration fragments to be present in the configuration. The value is inherited by
- * subclasses.
+ * fragments to be present in this rule's host and target configurations.
+ *
+ * <p>The value is inherited by subclasses.
*/
public Builder requiresConfigurationFragments(Class<?>... configurationFragments) {
- Collections.addAll(requiredConfigurationFragments, configurationFragments);
+ Collection<Class<?>> asList = new ArrayList();
+ Collections.addAll(asList, configurationFragments);
+ return requiresConfigurationFragments(asList);
+ }
+
+ /**
+ * Declares that the implementation of the associated rule class requires the given
+ * fragments to be present in the specified configuration. Valid transition values are
+ * HOST for the host configuration and NONE for the target configuration.
+ *
+ * <p>The value is inherited by subclasses.
+ */
+ public Builder requiresConfigurationFragments(ConfigurationTransition transition,
+ Class<?>... configurationFragments) {
+ for (Class<?> fragment : configurationFragments) {
+ requiredConfigurationFragments.put(transition, fragment);
+ }
+ return this;
+ }
+
+ /**
+ * Declares the configuration fragments that are required by this rule for the specified
+ * configuration. Valid transition values are HOST for the host configuration and NONE for
+ * the target configuration.
+ *
+ * <p>In contrast to {@link #requiresConfigurationFragments(Class...)}, this method takes the
+ * names of fragments instead of their classes.
+ */
+ public Builder requiresConfigurationFragments(
+ FragmentClassNameResolver fragmentNameResolver,
+ Map<ConfigurationTransition, ImmutableSet<String>> configurationFragmentNames) {
+ requiredConfigurationFragmentNames.putAll(configurationFragmentNames);
+ this.fragmentNameResolver = fragmentNameResolver;
return this;
}
@@ -97,24 +136,9 @@
return this;
}
- /**
- * Declares the configuration fragments that are required by this rule.
- *
- * <p>In contrast to {@link #requiresConfigurationFragments(Class...)}, this method a) takes the
- * names of fragments instead of their classes and b) distinguishes whether the fragments can be
- * accessed in host (HOST) or target (NONE) configuration.
- */
- public Builder requiresConfigurationFragments(
- FragmentClassNameResolver fragmentNameResolver,
- Map<ConfigurationTransition, ImmutableSet<String>> configurationFragmentNames) {
- requiredConfigurationFragmentNames.putAll(configurationFragmentNames);
- this.fragmentNameResolver = fragmentNameResolver;
- return this;
- }
-
public ConfigurationFragmentPolicy build() {
return new ConfigurationFragmentPolicy(
- ImmutableSet.copyOf(requiredConfigurationFragments),
+ ImmutableMultimap.copyOf(requiredConfigurationFragments),
ImmutableMap.copyOf(requiredConfigurationFragmentNames),
fragmentNameResolver,
missingFragmentPolicy);
@@ -122,10 +146,10 @@
}
/**
- * The set of required configuration fragments; this should list all fragments that can be
- * accessed by the rule implementation.
+ * A dictionary that maps configurations (NONE for target configuration, HOST for host
+ * configuration) to required configuration fragments.
*/
- private final ImmutableSet<Class<?>> requiredConfigurationFragments;
+ private final ImmutableMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments;
/**
* A dictionary that maps configurations (NONE for target configuration, HOST for host
@@ -146,7 +170,7 @@
private final MissingFragmentPolicy missingFragmentPolicy;
private ConfigurationFragmentPolicy(
- ImmutableSet<Class<?>> requiredConfigurationFragments,
+ ImmutableMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments,
ImmutableMap<ConfigurationTransition, ImmutableSet<String>>
requiredConfigurationFragmentNames,
FragmentClassNameResolver fragmentNameResolver,
@@ -159,10 +183,10 @@
/**
* The set of required configuration fragments; this contains all fragments that can be
- * accessed by the rule implementation.
+ * accessed by the rule implementation under any configuration.
*/
public Set<Class<?>> getRequiredConfigurationFragments() {
- return requiredConfigurationFragments;
+ return ImmutableSet.copyOf(requiredConfigurationFragments.values());
}
/**
@@ -171,7 +195,7 @@
*/
public boolean isLegalConfigurationFragment(
Class<?> configurationFragment, ConfigurationTransition config) {
- return requiredConfigurationFragments.contains(configurationFragment)
+ return getRequiredConfigurationFragments().contains(configurationFragment)
|| hasLegalFragmentName(configurationFragment, config);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 7846250..e3a673f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -572,12 +572,10 @@
}
/**
- * Declares that the implementation of this rule class requires the given configuration
- * fragments to be present in the configuration. The value is inherited by subclasses.
+ * Declares that the implementation of the associated rule class requires the given
+ * fragments to be present in this rule's host and target configurations.
*
- * <p>For backwards compatibility, if the set is empty, all fragments may be accessed. But note
- * that this is only enforced in the {@link com.google.devtools.build.lib.analysis.RuleContext}
- * class.
+ * <p>The value is inherited by subclasses.
*/
public Builder requiresConfigurationFragments(Class<?>... configurationFragments) {
configurationFragmentPolicy.requiresConfigurationFragments(configurationFragments);
@@ -585,6 +583,34 @@
}
/**
+ * Declares that the implementation of the associated rule class requires the given
+ * fragments to be present in the host configuration.
+ *
+ * <p>The value is inherited by subclasses.
+ */
+ public Builder requiresHostConfigurationFragments(Class<?>... configurationFragments) {
+ configurationFragmentPolicy
+ .requiresConfigurationFragments(ConfigurationTransition.HOST, configurationFragments);
+ return this;
+ }
+
+ /**
+ * Declares the configuration fragments that are required by this rule for the specified
+ * configuration. Valid transition values are HOST for the host configuration and NONE for
+ * the target configuration.
+ *
+ * <p>In contrast to {@link #requiresConfigurationFragments(Class...)}, this method takes the
+ * names of fragments instead of their classes.
+ */
+ public Builder requiresConfigurationFragments(
+ FragmentClassNameResolver fragmentNameResolver,
+ Map<ConfigurationTransition, ImmutableSet<String>> configurationFragmentNames) {
+ configurationFragmentPolicy.requiresConfigurationFragments(
+ fragmentNameResolver, configurationFragmentNames);
+ return this;
+ }
+
+ /**
* Sets the policy for the case where the configuration is missing required fragments (see
* {@link #requiresConfigurationFragments}).
*/
@@ -593,21 +619,6 @@
return this;
}
- /**
- * Declares the configuration fragments that are required by this rule.
- *
- * <p>In contrast to {@link #requiresConfigurationFragments(Class...)}, this method a) takes the
- * names of fragments instead of their classes and b) distinguishes whether the fragments can be
- * accessed in host (HOST) or target (NONE) configuration.
- */
- public Builder requiresConfigurationFragments(
- FragmentClassNameResolver fragmentNameResolver,
- Map<ConfigurationTransition, ImmutableSet<String>> configurationFragmentNames) {
- configurationFragmentPolicy.requiresConfigurationFragments(
- fragmentNameResolver, configurationFragmentNames);
- return this;
- }
-
public Builder setUndocumented() {
documented = false;
return this;