Skylark rules can now declare their required configuration fragments
--
MOS_MIGRATED_REVID=100163482
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 71f5175..b2e9f1a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.syntax.Label.SyntaxException;
import com.google.devtools.build.lib.syntax.SkylarkCallable;
import com.google.devtools.build.lib.syntax.SkylarkModule;
+import com.google.devtools.build.lib.syntax.SkylarkModuleNameResolver;
import com.google.devtools.build.lib.util.CPU;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.OS;
@@ -1121,11 +1122,12 @@
private ImmutableMap<String, Class<? extends Fragment>> buildIndexOfVisibleFragments() {
ImmutableMap.Builder<String, Class<? extends Fragment>> builder = ImmutableMap.builder();
+ SkylarkModuleNameResolver resolver = new SkylarkModuleNameResolver();
for (Class<? extends Fragment> fragmentClass : fragments.keySet()) {
- SkylarkModule annotation = fragmentClass.getAnnotation(SkylarkModule.class);
- if (annotation != null) {
- builder.put(annotation.name(), fragmentClass);
+ String name = resolver.resolveName(fragmentClass);
+ if (name != null) {
+ builder.put(name, fragmentClass);
}
}
return builder.build();
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 2a48d87..67cc935 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
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.syntax.Argument;
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.FragmentClassNameResolver;
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.GlobList;
import com.google.devtools.build.lib.syntax.Label;
@@ -489,6 +490,9 @@
private SkylarkEnvironment ruleDefinitionEnvironment = null;
private Set<Class<?>> configurationFragments = new LinkedHashSet<>();
private MissingFragmentPolicy missingFragmentPolicy = MissingFragmentPolicy.FAIL_ANALYSIS;
+ private Set<String> requiredFragmentNames = new LinkedHashSet<>();
+ private FragmentClassNameResolver fragmentNameResolver;
+
private boolean supportsConstraintChecking = true;
private final Map<String, Attribute> attributes = new LinkedHashMap<>();
@@ -574,8 +578,8 @@
workspaceOnly, outputsDefaultExecutable, implicitOutputsFunction, configurator,
configuredTargetFactory, validityPredicate, preferredDependencyPredicate,
ImmutableSet.copyOf(advertisedProviders), configuredTargetFunction,
- externalBindingsFunction,
- ruleDefinitionEnvironment, configurationFragments, missingFragmentPolicy,
+ externalBindingsFunction, ruleDefinitionEnvironment, configurationFragments,
+ requiredFragmentNames, fragmentNameResolver, missingFragmentPolicy,
supportsConstraintChecking, attributes.values().toArray(new Attribute[0]));
}
@@ -600,6 +604,17 @@
this.missingFragmentPolicy = missingFragmentPolicy;
return this;
}
+
+ /**
+ * Does the same as {@link #requiresConfigurationFragments(Class...)}, except for taking names
+ * of fragments instead of classes.
+ */
+ public Builder requiresConfigurationFragments(
+ FragmentClassNameResolver fragmentNameResolver, String... configurationFragmentNames) {
+ Collections.addAll(requiredFragmentNames, configurationFragmentNames);
+ this.fragmentNameResolver = fragmentNameResolver;
+ return this;
+ }
public Builder setUndocumented() {
documented = false;
@@ -948,6 +963,17 @@
private final ImmutableSet<Class<?>> requiredConfigurationFragments;
/**
+ * Same idea as requiredConfigurationFragments, but stores fragments by name instead of by class
+ */
+ private final ImmutableSet<String> requiredConfigurationFragmentNames;
+
+ /**
+ * Used to resolve the names of fragments in order to compare them to values in {@link
+ * #requiredConfigurationFragmentNames}
+ */
+ private final FragmentClassNameResolver fragmentNameResolver;
+
+ /**
* What to do during analysis if a configuration fragment is missing.
*/
private final MissingFragmentPolicy missingFragmentPolicy;
@@ -960,6 +986,53 @@
private final boolean supportsConstraintChecking;
/**
+ * Helper constructor that skips allowedConfigurationFragmentNames and fragmentNameResolver
+ */
+ @VisibleForTesting
+ RuleClass(String name,
+ boolean skylarkExecutable,
+ boolean documented,
+ boolean publicByDefault,
+ boolean binaryOutput,
+ boolean workspaceOnly,
+ boolean outputsDefaultExecutable,
+ ImplicitOutputsFunction implicitOutputsFunction,
+ Configurator<?, ?> configurator,
+ ConfiguredTargetFactory<?, ?> configuredTargetFactory,
+ PredicateWithMessage<Rule> validityPredicate,
+ Predicate<String> preferredDependencyPredicate,
+ ImmutableSet<Class<?>> advertisedProviders,
+ @Nullable BaseFunction configuredTargetFunction,
+ Function<? super Rule, Map<String, Label>> externalBindingsFunction,
+ @Nullable SkylarkEnvironment ruleDefinitionEnvironment,
+ Set<Class<?>> allowedConfigurationFragments,
+ MissingFragmentPolicy missingFragmentPolicy,
+ boolean supportsConstraintChecking,
+ Attribute... attributes) {
+ this(name,
+ skylarkExecutable,
+ documented,
+ publicByDefault,
+ binaryOutput,
+ workspaceOnly,
+ outputsDefaultExecutable,
+ implicitOutputsFunction,
+ configurator,
+ configuredTargetFactory,
+ validityPredicate,
+ preferredDependencyPredicate,
+ advertisedProviders,
+ configuredTargetFunction,
+ externalBindingsFunction,
+ ruleDefinitionEnvironment,
+ allowedConfigurationFragments,
+ ImmutableSet.<String>of(), null,
+ missingFragmentPolicy,
+ supportsConstraintChecking,
+ attributes);
+ }
+
+ /**
* Constructs an instance of RuleClass whose name is 'name', attributes
* are 'attributes'. The {@code srcsAllowedFiles} determines which types of
* files are allowed as parameters to the "srcs" attribute; rules are always
@@ -993,7 +1066,10 @@
@Nullable BaseFunction configuredTargetFunction,
Function<? super Rule, Map<String, Label>> externalBindingsFunction,
@Nullable SkylarkEnvironment ruleDefinitionEnvironment,
- Set<Class<?>> allowedConfigurationFragments, MissingFragmentPolicy missingFragmentPolicy,
+ Set<Class<?>> allowedConfigurationFragments,
+ Set<String> allowedConfigurationFragmentNames,
+ @Nullable FragmentClassNameResolver fragmentNameResolver,
+ MissingFragmentPolicy missingFragmentPolicy,
boolean supportsConstraintChecking,
Attribute... attributes) {
this.name = name;
@@ -1016,6 +1092,9 @@
this.workspaceOnly = workspaceOnly;
this.outputsDefaultExecutable = outputsDefaultExecutable;
this.requiredConfigurationFragments = ImmutableSet.copyOf(allowedConfigurationFragments);
+ this.requiredConfigurationFragmentNames =
+ ImmutableSet.copyOf(allowedConfigurationFragmentNames);
+ this.fragmentNameResolver = fragmentNameResolver;
this.missingFragmentPolicy = missingFragmentPolicy;
this.supportsConstraintChecking = supportsConstraintChecking;
@@ -1177,10 +1256,23 @@
public boolean isLegalConfigurationFragment(Class<?> configurationFragment) {
// For now, we allow all rules that don't declare allowed fragments to access any fragment.
// TODO(bazel-team): All built-in rules declare fragments, but Skylark rules don't.
- if (requiredConfigurationFragments.isEmpty()) {
+ if (requiredConfigurationFragments.isEmpty() && requiredConfigurationFragmentNames.isEmpty()) {
return true;
}
- return requiredConfigurationFragments.contains(configurationFragment);
+ return requiredConfigurationFragments.contains(configurationFragment)
+ || hasLegalFragmentName(configurationFragment);
+ }
+
+ /**
+ * Checks whether the name of the given fragment class was declared as required fragment
+ */
+ private boolean hasLegalFragmentName(Class<?> configurationFragment) {
+ if (fragmentNameResolver == null) {
+ return false;
+ }
+
+ String name = fragmentNameResolver.resolveName(configurationFragment);
+ return (name != null && requiredConfigurationFragmentNames.contains(name));
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index a224805..e128d7e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -33,6 +33,7 @@
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.RunUnder;
@@ -70,6 +71,7 @@
import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.SkylarkModuleNameResolver;
import com.google.devtools.build.lib.syntax.SkylarkSignature;
import com.google.devtools.build.lib.syntax.SkylarkSignature.Param;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
@@ -220,15 +222,18 @@
+ "there must be an action that generates <code>ctx.outputs.executable</code>."),
@Param(name = "output_to_genfiles", type = Boolean.class, defaultValue = "False",
doc = "If true, the files will be generated in the genfiles directory instead of the "
- + "bin directory. This is used for compatibility with existing rules.")},
+ + "bin directory. This is used for compatibility with existing rules."),
+ @Param(name = "fragments", type = SkylarkList.class, generic1 = String.class,
+ defaultValue = "[]",
+ doc = "List of names of configuration fragments that the rule requires.")},
useAst = true, useEnvironment = true)
private static final BuiltinFunction rule = new BuiltinFunction("rule") {
@SuppressWarnings({"rawtypes", "unchecked"}) // castMap produces
// an Attribute.Builder instead of a Attribute.Builder<?> but it's OK.
- public BaseFunction invoke(BaseFunction implementation, Boolean test,
- Object attrs, Object implicitOutputs, Boolean executable, Boolean outputToGenfiles,
- FuncallExpression ast, Environment funcallEnv)
- throws EvalException, ConversionException {
+ public BaseFunction invoke(BaseFunction implementation, Boolean test, Object attrs,
+ Object implicitOutputs, Boolean executable, Boolean outputToGenfiles,
+ SkylarkList fragments, FuncallExpression ast, Environment funcallEnv)
+ throws EvalException, ConversionException {
Preconditions.checkState(funcallEnv instanceof SkylarkEnvironment);
RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL;
@@ -273,6 +278,12 @@
builder.setOutputToGenfiles();
}
+ if (!fragments.isEmpty()) {
+ builder.requiresConfigurationFragments(
+ new SkylarkModuleNameResolver(),
+ Iterables.toArray(castList(fragments, String.class), String.class));
+ }
+
builder.setConfiguredTargetFunction(implementation);
builder.setRuleDefinitionEnvironment(
((SkylarkEnvironment) funcallEnv).getGlobalEnvironment());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FragmentClassNameResolver.java b/src/main/java/com/google/devtools/build/lib/syntax/FragmentClassNameResolver.java
new file mode 100644
index 0000000..c49a0a9
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FragmentClassNameResolver.java
@@ -0,0 +1,33 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.syntax;
+
+import javax.annotation.Nullable;
+
+/**
+ * Interface for retrieving the name of a {@link
+ * com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment} based on its
+ * class.
+ *
+ * <p>Classes implementing this specific interface are required when doing look up operations and
+ * comparisons of configuration fragments.
+ */
+public interface FragmentClassNameResolver {
+ /**
+ * Returns the name of the configuration fragment specified by the given class or null, if this is
+ * not possible.
+ */
+ @Nullable
+ String resolveName(Class<?> clazz);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModuleNameResolver.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModuleNameResolver.java
new file mode 100644
index 0000000..d5aa9bf
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModuleNameResolver.java
@@ -0,0 +1,29 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.syntax;
+
+/**
+ * Helper class that resolves the name of classes with SkylarkModule annotation.
+ */
+public final class SkylarkModuleNameResolver implements FragmentClassNameResolver {
+ /**
+ * Returns the Skylark name of the given class or null, if the SkylarkModule annotation is not
+ * present.
+ */
+ @Override
+ public String resolveName(Class<?> clazz) {
+ SkylarkModule annotation = clazz.getAnnotation(SkylarkModule.class);
+ return (annotation == null) ? null : annotation.name();
+ }
+}
