diff options
10 files changed, 330 insertions, 171 deletions
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 0282489405..6e06ef1f91 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 @@ -176,7 +176,7 @@ public final class RuleContext extends TargetContext parseFeatures(getConfiguration().getDefaultFeatures(), globallyEnabled, globallyDisabled); for (ImmutableMap.Entry<Class<? extends Fragment>, Fragment> entry : getConfiguration().getAllFragments().entrySet()) { - if (configurationFragmentPolicy.isLegalConfigurationFragment(entry.getKey())) { + if (isLegalFragment(entry.getKey())) { globallyEnabled.addAll(entry.getValue().configurationEnabledFeatures(this)); } } 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 8ef66797ea..7e8c5b3486 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 @@ -56,7 +56,6 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.test.TestActionBuilder; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.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; @@ -1194,7 +1193,7 @@ public final class BuildConfiguration { this.actionsEnabled = !actionsDisabled; this.fragments = ImmutableSortedMap.copyOf(fragmentsMap, lexicalFragmentSorter); - this.skylarkVisibleFragments = buildIndexOfVisibleFragments(); + this.skylarkVisibleFragments = buildIndexOfSkylarkVisibleFragments(); this.buildOptions = buildOptions; this.options = buildOptions.get(Options.class); @@ -1306,12 +1305,11 @@ public final class BuildConfiguration { - private ImmutableMap<String, Class<? extends Fragment>> buildIndexOfVisibleFragments() { + private ImmutableMap<String, Class<? extends Fragment>> buildIndexOfSkylarkVisibleFragments() { ImmutableMap.Builder<String, Class<? extends Fragment>> builder = ImmutableMap.builder(); - SkylarkModuleNameResolver resolver = new SkylarkModuleNameResolver(); for (Class<? extends Fragment> fragmentClass : fragments.keySet()) { - String name = resolver.resolveName(fragmentClass); + String name = SkylarkModule.Resolver.resolveName(fragmentClass); if (name != null) { builder.put(name, fragmentClass); } 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 929ec0ee69..c75c01b088 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,19 +13,14 @@ // 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.common.collect.ImmutableSetMultimap; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; -import com.google.devtools.build.lib.syntax.FragmentClassNameResolver; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.Map; import java.util.Set; /** @@ -65,35 +60,40 @@ public final class ConfigurationFragmentPolicy { * Builder to construct a new ConfigurationFragmentPolicy. */ public static final class Builder { - private final Multimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments - = ArrayListMultimap.create(); - private final Map<ConfigurationTransition, ImmutableSet<String>> - requiredConfigurationFragmentNames = new LinkedHashMap<>(); + /** + * Sets of configuration fragment classes required by this rule, a set for each configuration. + * Duplicate entries will automatically be ignored by the SetMultimap. + */ + private final SetMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments + = LinkedHashMultimap.create(); + /** + * Sets of configuration fragment names required by this rule, a set for each configuration. + * Duplicate entries will automatically be ignored by the SetMultimap. + */ + private final SetMultimap<ConfigurationTransition, String> requiredConfigurationFragmentNames + = LinkedHashMultimap.create(); private MissingFragmentPolicy missingFragmentPolicy = MissingFragmentPolicy.FAIL_ANALYSIS; - private FragmentClassNameResolver fragmentNameResolver; /** * Declares that the implementation of the associated rule class requires the given - * fragments to be present in this rule's host and target configurations. + * fragments to be present in this rule's target configuration only. * * <p>The value is inherited by subclasses. */ public Builder requiresConfigurationFragments(Collection<Class<?>> configurationFragments) { - requiredConfigurationFragments.putAll(ConfigurationTransition.HOST, configurationFragments); - requiredConfigurationFragments.putAll(ConfigurationTransition.NONE, configurationFragments); + requiresConfigurationFragments(ConfigurationTransition.NONE, configurationFragments); return this; } /** * Declares that the implementation of the associated rule class requires the given - * fragments to be present in this rule's host and target configurations. + * fragments to be present in this rule's host configuration only. * * <p>The value is inherited by subclasses. */ - public Builder requiresConfigurationFragments(Class<?>... configurationFragments) { - Collection<Class<?>> asList = new ArrayList(); - Collections.addAll(asList, configurationFragments); - return requiresConfigurationFragments(asList); + public Builder requiresHostConfigurationFragments(Collection<Class<?>> configurationFragments) { + requiresConfigurationFragments(ConfigurationTransition.HOST, configurationFragments); + return this; } /** @@ -103,11 +103,42 @@ public final class ConfigurationFragmentPolicy { * * <p>The value is inherited by subclasses. */ - public Builder requiresConfigurationFragments(ConfigurationTransition transition, - Class<?>... configurationFragments) { - for (Class<?> fragment : configurationFragments) { - requiredConfigurationFragments.put(transition, fragment); - } + private void requiresConfigurationFragments(ConfigurationTransition transition, + Collection<Class<?>> configurationFragments) { + requiredConfigurationFragments.putAll(transition, configurationFragments); + } + + /** + * Declares that the implementation of the associated rule class requires the given + * fragments to be present in this rule's target configuration only. + * + * <p>In contrast to {@link #requiresConfigurationFragments(Collection)}, this method takes the + * names of fragments (as determined by {@link SkylarkModule.Resolver}) instead of their + * classes. + * + * <p>The value is inherited by subclasses. + */ + public Builder requiresConfigurationFragmentsBySkylarkModuleName( + Collection<String> configurationFragmentNames) { + requiresConfigurationFragmentsBySkylarkModuleName( + ConfigurationTransition.NONE, configurationFragmentNames); + return this; + } + + /** + * Declares that the implementation of the associated rule class requires the given + * fragments to be present in this rule's host configuration only. + * + * <p>In contrast to {@link #requiresHostConfigurationFragments(Collection)}, this method takes + * the names of fragments (as determined by {@link SkylarkModule.Resolver}) instead of their + * classes. + * + * <p>The value is inherited by subclasses. + */ + public Builder requiresHostConfigurationFragmentsBySkylarkModuleName( + Collection<String> configurationFragmentNames) { + requiresConfigurationFragmentsBySkylarkModuleName( + ConfigurationTransition.HOST, configurationFragmentNames); return this; } @@ -116,14 +147,23 @@ public final class ConfigurationFragmentPolicy { * 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. + * <p>In contrast to {@link #requiresConfigurationFragments(Collection)}, this method takes the + * names of fragments (as determined by {@link SkylarkModule.Resolver}) instead of their + * classes. + */ + private void requiresConfigurationFragmentsBySkylarkModuleName( + ConfigurationTransition transition, Collection<String> configurationFragmentNames) { + requiredConfigurationFragmentNames.putAll(transition, configurationFragmentNames); + } + + /** + * Adds the configuration fragments from the {@code other} policy to this Builder. + * + * <p>Does not change the missing fragment policy. */ - public Builder requiresConfigurationFragments( - FragmentClassNameResolver fragmentNameResolver, - Map<ConfigurationTransition, ImmutableSet<String>> configurationFragmentNames) { - requiredConfigurationFragmentNames.putAll(configurationFragmentNames); - this.fragmentNameResolver = fragmentNameResolver; + public Builder includeConfigurationFragmentsFrom(ConfigurationFragmentPolicy other) { + requiredConfigurationFragments.putAll(other.requiredConfigurationFragments); + requiredConfigurationFragmentNames.putAll(other.requiredConfigurationFragmentNames); return this; } @@ -138,9 +178,8 @@ public final class ConfigurationFragmentPolicy { public ConfigurationFragmentPolicy build() { return new ConfigurationFragmentPolicy( - ImmutableMultimap.copyOf(requiredConfigurationFragments), - ImmutableMap.copyOf(requiredConfigurationFragmentNames), - fragmentNameResolver, + ImmutableSetMultimap.copyOf(requiredConfigurationFragments), + ImmutableSetMultimap.copyOf(requiredConfigurationFragmentNames), missingFragmentPolicy); } } @@ -149,35 +188,27 @@ public final class ConfigurationFragmentPolicy { * A dictionary that maps configurations (NONE for target configuration, HOST for host * configuration) to required configuration fragments. */ - private final ImmutableMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments; + private final ImmutableSetMultimap<ConfigurationTransition, Class<?>> + requiredConfigurationFragments; /** * A dictionary that maps configurations (NONE for target configuration, HOST for host - * configuration) to lists of names of required configuration fragments. + * configuration) to lists of Skylark module names of required configuration fragments. */ - private final ImmutableMap<ConfigurationTransition, ImmutableSet<String>> + private final ImmutableSetMultimap<ConfigurationTransition, 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; private ConfigurationFragmentPolicy( - ImmutableMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments, - ImmutableMap<ConfigurationTransition, ImmutableSet<String>> - requiredConfigurationFragmentNames, - FragmentClassNameResolver fragmentNameResolver, + ImmutableSetMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments, + ImmutableSetMultimap<ConfigurationTransition, String> requiredConfigurationFragmentNames, MissingFragmentPolicy missingFragmentPolicy) { this.requiredConfigurationFragments = requiredConfigurationFragments; this.requiredConfigurationFragmentNames = requiredConfigurationFragmentNames; - this.fragmentNameResolver = fragmentNameResolver; this.missingFragmentPolicy = missingFragmentPolicy; } @@ -192,16 +223,23 @@ public final class ConfigurationFragmentPolicy { /** * Checks if the configuration fragment may be accessed (i.e., if it's declared) in the specified * configuration (target or host). + * + * <p>Note that, currently, all native fragments are included regardless of whether they were + * specified in the same configuration that was passed. */ public boolean isLegalConfigurationFragment( Class<?> configurationFragment, ConfigurationTransition config) { - return getRequiredConfigurationFragments().contains(configurationFragment) + return requiredConfigurationFragments.containsValue(configurationFragment) || hasLegalFragmentName(configurationFragment, config); } + /** + * Checks if the configuration fragment may be accessed (i.e., if it's declared) in any + * configuration. + */ public boolean isLegalConfigurationFragment(Class<?> configurationFragment) { - // NONE means target configuration. - return isLegalConfigurationFragment(configurationFragment, ConfigurationTransition.NONE); + return requiredConfigurationFragments.containsValue(configurationFragment) + || hasLegalFragmentName(configurationFragment); } /** @@ -210,13 +248,17 @@ public final class ConfigurationFragmentPolicy { */ private boolean hasLegalFragmentName( Class<?> configurationFragment, ConfigurationTransition config) { - if (fragmentNameResolver == null) { - return false; - } + return requiredConfigurationFragmentNames.containsEntry( + config, SkylarkModule.Resolver.resolveName(configurationFragment)); + } - String name = fragmentNameResolver.resolveName(configurationFragment); - ImmutableSet<String> fragmentNames = requiredConfigurationFragmentNames.get(config); - return (name != null && fragmentNames != null && fragmentNames.contains(name)); + /** + * Checks whether the name of the given fragment class was declared as required in any + * configuration. + */ + private boolean hasLegalFragmentName(Class<?> configurationFragment) { + return requiredConfigurationFragmentNames.containsValue( + SkylarkModule.Resolver.resolveName(configurationFragment)); } /** 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 9ce41f6cd4..92ea112216 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 @@ -32,7 +32,6 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.BuildType.SelectorList; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -40,7 +39,6 @@ import com.google.devtools.build.lib.packages.RuleFactory.AttributeValuesMap; 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.Runtime; @@ -508,8 +506,8 @@ public final class RuleClass { if (parent.preferredDependencyPredicate != Predicates.<String>alwaysFalse()) { setPreferredDependencyPredicate(parent.preferredDependencyPredicate); } - configurationFragmentPolicy.requiresConfigurationFragments( - parent.getConfigurationFragmentPolicy().getRequiredConfigurationFragments()); + configurationFragmentPolicy + .includeConfigurationFragmentsFrom(parent.getConfigurationFragmentPolicy()); configurationFragmentPolicy.setMissingFragmentPolicy( parent.getConfigurationFragmentPolicy().getMissingFragmentPolicy()); supportsConstraintChecking = parent.supportsConstraintChecking; @@ -580,7 +578,8 @@ public final class RuleClass { * <p>The value is inherited by subclasses. */ public Builder requiresConfigurationFragments(Class<?>... configurationFragments) { - configurationFragmentPolicy.requiresConfigurationFragments(configurationFragments); + configurationFragmentPolicy.requiresConfigurationFragments( + ImmutableSet.<Class<?>>copyOf(configurationFragments)); return this; } @@ -591,24 +590,36 @@ public final class RuleClass { * <p>The value is inherited by subclasses. */ public Builder requiresHostConfigurationFragments(Class<?>... configurationFragments) { - configurationFragmentPolicy - .requiresConfigurationFragments(ConfigurationTransition.HOST, configurationFragments); + configurationFragmentPolicy.requiresHostConfigurationFragments( + ImmutableSet.<Class<?>>copyOf(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. + * Declares the configuration fragments that are required by this rule for the target + * configuration. * * <p>In contrast to {@link #requiresConfigurationFragments(Class...)}, this method takes the - * names of fragments instead of their classes. + * Skylark module names of fragments instead of their classes. */ - public Builder requiresConfigurationFragments( - FragmentClassNameResolver fragmentNameResolver, - Map<ConfigurationTransition, ImmutableSet<String>> configurationFragmentNames) { - configurationFragmentPolicy.requiresConfigurationFragments( - fragmentNameResolver, configurationFragmentNames); + public Builder requiresConfigurationFragmentsBySkylarkModuleName( + Collection<String> configurationFragmentNames) { + configurationFragmentPolicy + .requiresConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames); + return this; + } + + /** + * Declares the configuration fragments that are required by this rule for the host + * configuration. + * + * <p>In contrast to {@link #requiresHostConfigurationFragments(Class...)}, this method takes + * Skylark module names of fragments instead of their classes. + */ + public Builder requiresHostConfigurationFragmentsBySkylarkModuleName( + Collection<String> configurationFragmentNames) { + configurationFragmentPolicy + .requiresHostConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames); return this; } 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 cc5898e721..11268945c9 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 @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules; import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.DATA; import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.NONE; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; @@ -33,7 +32,6 @@ import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.Constants; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.BaseRuleClasses; @@ -87,7 +85,6 @@ import com.google.devtools.build.lib.syntax.Printer; import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction; import com.google.devtools.build.lib.syntax.SkylarkList; -import com.google.devtools.build.lib.syntax.SkylarkModuleNameResolver; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor; import com.google.devtools.build.lib.syntax.Type; @@ -95,7 +92,6 @@ import com.google.devtools.build.lib.syntax.Type.ConversionException; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; -import java.util.HashMap; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -308,31 +304,14 @@ public class SkylarkRuleClassFunctions { builder.setOutputToGenfiles(); } - registerRequiredFragments(fragments, hostFragments, builder); + builder.requiresConfigurationFragmentsBySkylarkModuleName( + fragments.getContents(String.class, "fragments")); + builder.requiresHostConfigurationFragmentsBySkylarkModuleName( + hostFragments.getContents(String.class, "host_fragments")); builder.setConfiguredTargetFunction(implementation); builder.setRuleDefinitionEnvironment(funcallEnv); return new RuleFunction(builder, type, attributes, ast.getLocation()); } - - private void registerRequiredFragments( - SkylarkList fragments, SkylarkList hostFragments, RuleClass.Builder builder) - throws EvalException { - Map<ConfigurationTransition, ImmutableSet<String>> map = new HashMap<>(); - addFragmentsToMap(map, fragments, NONE); // NONE represents target configuration - addFragmentsToMap(map, hostFragments, HOST); - - builder.requiresConfigurationFragments(new SkylarkModuleNameResolver(), map); - } - - private void addFragmentsToMap( - Map<ConfigurationTransition, ImmutableSet<String>> map, - SkylarkList fragments, - ConfigurationTransition config) - throws EvalException { - if (!fragments.isEmpty()) { - map.put(config, ImmutableSet.copyOf(fragments.getContents(String.class, "fragments"))); - } - } }; protected static ImmutableList<Pair<String, Descriptor>> attrObjectToAttributesList( diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java index 92be3ba72b..0d9d63d1ac 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java @@ -18,6 +18,8 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import javax.annotation.Nullable; + /** * An annotation to mark Skylark modules or Skylark accessible Java data types. * A Skylark modules always corresponds to exactly one Java class. @@ -33,4 +35,20 @@ public @interface SkylarkModule { boolean documented() default true; boolean namespace() default false; + + /** Helper method to quickly get the SkylarkModule name of a class (if present). */ + public static final class Resolver { + /** + * Returns the Skylark name of the given class or null, if the SkylarkModule annotation is not + * present. + */ + @Nullable + public static String resolveName(Class<?> clazz) { + SkylarkModule annotation = clazz.getAnnotation(SkylarkModule.class); + return (annotation == null) ? null : annotation.name(); + } + + /** Utility method only. */ + private Resolver() {} + } } 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 deleted file mode 100644 index d3be61080f..0000000000 --- a/src/main/java/com/google/devtools/build/lib/syntax/FragmentClassNameResolver.java +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2015 The Bazel Authors. 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 deleted file mode 100644 index 0f7f23a101..0000000000 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModuleNameResolver.java +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2015 The Bazel Authors. 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 com.google.devtools.build.lib.skylarkinterface.SkylarkModule; - -/** - * 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(); - } -} diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index d2366813e9..4b622f3860 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -574,6 +574,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib:events", "//src/main/java/com/google/devtools/build/lib:packages", + "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/skyframe", diff --git a/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java b/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java new file mode 100644 index 0000000000..a3ea359781 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java @@ -0,0 +1,174 @@ +// Copyright 2016 The Bazel Authors. 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.packages; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; +import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Tests for the ConfigurationFragmentPolicy builder and methods. + */ +@RunWith(JUnit4.class) +public final class ConfigurationFragmentPolicyTest { + + @SkylarkModule(name = "test_fragment", doc = "first fragment") + private static final class TestFragment {} + + @SkylarkModule(name = "other_fragment", doc = "second fragment") + private static final class OtherFragment {} + + @SkylarkModule(name = "unknown_fragment", doc = "useless waste of permgen") + private static final class UnknownFragment {} + + @Test + public void testMissingFragmentPolicy() throws Exception { + ConfigurationFragmentPolicy policy = + new ConfigurationFragmentPolicy.Builder() + .setMissingFragmentPolicy(MissingFragmentPolicy.IGNORE) + .build(); + + assertThat(policy.getMissingFragmentPolicy()).isEqualTo(MissingFragmentPolicy.IGNORE); + + ConfigurationFragmentPolicy otherPolicy = + new ConfigurationFragmentPolicy.Builder() + .setMissingFragmentPolicy(MissingFragmentPolicy.CREATE_FAIL_ACTIONS) + .build(); + + assertThat(otherPolicy.getMissingFragmentPolicy()) + .isEqualTo(MissingFragmentPolicy.CREATE_FAIL_ACTIONS); + } + + @Test + public void testRequiresConfigurationFragments_AddsToRequiredSet() throws Exception { + // Although these aren't configuration fragments, there are no requirements as to what the class + // has to be, so... + ConfigurationFragmentPolicy policy = + new ConfigurationFragmentPolicy.Builder() + .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(Integer.class, String.class)) + .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(String.class, Long.class)) + .build(); + + assertThat(policy.getRequiredConfigurationFragments()) + .containsExactly(Integer.class, String.class, Long.class); + } + + @Test + public void testRequiresConfigurationFragments_RequiredAndLegalForSpecifiedConfiguration() + throws Exception { + ConfigurationFragmentPolicy policy = + new ConfigurationFragmentPolicy.Builder() + .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(Integer.class)) + .requiresHostConfigurationFragments(ImmutableSet.<Class<?>>of(Long.class)) + .build(); + + assertThat(policy.getRequiredConfigurationFragments()).containsAllOf(Integer.class, Long.class); + + assertThat(policy.isLegalConfigurationFragment(Integer.class)).isTrue(); + assertThat(policy.isLegalConfigurationFragment(Integer.class, ConfigurationTransition.NONE)) + .isTrue(); + // TODO(mstaib): .isFalse() when dynamic configurations care which configuration a fragment was + // specified for + assertThat(policy.isLegalConfigurationFragment(Integer.class, ConfigurationTransition.HOST)) + .isTrue(); + + assertThat(policy.isLegalConfigurationFragment(Long.class)).isTrue(); + // TODO(mstaib): .isFalse() when dynamic configurations care which configuration a fragment was + // specified for + assertThat(policy.isLegalConfigurationFragment(Long.class, ConfigurationTransition.NONE)) + .isTrue(); + assertThat(policy.isLegalConfigurationFragment(Long.class, ConfigurationTransition.HOST)) + .isTrue(); + + assertThat(policy.isLegalConfigurationFragment(String.class)).isFalse(); + assertThat(policy.isLegalConfigurationFragment(String.class, ConfigurationTransition.NONE)) + .isFalse(); + assertThat(policy.isLegalConfigurationFragment(String.class, ConfigurationTransition.HOST)) + .isFalse(); + } + + @Test + public void testRequiresConfigurationFragments_MapSetsLegalityBySkylarkModuleName_NoRequires() + throws Exception { + ConfigurationFragmentPolicy policy = + new ConfigurationFragmentPolicy.Builder() + .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableSet.of("test_fragment")) + .requiresHostConfigurationFragmentsBySkylarkModuleName( + ImmutableSet.of("other_fragment")) + .build(); + + assertThat(policy.getRequiredConfigurationFragments()).isEmpty(); + + assertThat(policy.isLegalConfigurationFragment(TestFragment.class)).isTrue(); + assertThat( + policy.isLegalConfigurationFragment(TestFragment.class, ConfigurationTransition.NONE)) + .isTrue(); + assertThat( + policy.isLegalConfigurationFragment(TestFragment.class, ConfigurationTransition.HOST)) + .isFalse(); + + assertThat(policy.isLegalConfigurationFragment(OtherFragment.class)).isTrue(); + assertThat( + policy.isLegalConfigurationFragment(OtherFragment.class, ConfigurationTransition.NONE)) + .isFalse(); + assertThat( + policy.isLegalConfigurationFragment(OtherFragment.class, ConfigurationTransition.HOST)) + .isTrue(); + + assertThat(policy.isLegalConfigurationFragment(UnknownFragment.class)).isFalse(); + assertThat( + policy.isLegalConfigurationFragment( + UnknownFragment.class, ConfigurationTransition.NONE)) + .isFalse(); + assertThat( + policy.isLegalConfigurationFragment( + UnknownFragment.class, ConfigurationTransition.HOST)) + .isFalse(); + } + + @Test + public void testIncludeConfigurationFragmentsFrom_MergesWithExistingFragmentSet() + throws Exception { + ConfigurationFragmentPolicy basePolicy = + new ConfigurationFragmentPolicy.Builder() + .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableSet.of("test_fragment")) + .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(Integer.class, Double.class)) + .build(); + ConfigurationFragmentPolicy addedPolicy = + new ConfigurationFragmentPolicy.Builder() + .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableSet.of("other_fragment")) + .requiresHostConfigurationFragmentsBySkylarkModuleName( + ImmutableSet.of("other_fragment")) + .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(Boolean.class)) + .requiresHostConfigurationFragments(ImmutableSet.<Class<?>>of(Character.class)) + .build(); + ConfigurationFragmentPolicy combinedPolicy = + new ConfigurationFragmentPolicy.Builder() + .includeConfigurationFragmentsFrom(basePolicy) + .includeConfigurationFragmentsFrom(addedPolicy) + .build(); + + assertThat(combinedPolicy.getRequiredConfigurationFragments()) + .containsExactly(Integer.class, Double.class, Boolean.class, Character.class); + assertThat(combinedPolicy.isLegalConfigurationFragment(TestFragment.class)).isTrue(); + assertThat(combinedPolicy.isLegalConfigurationFragment(OtherFragment.class)).isTrue(); + } +} |