diff options
15 files changed, 377 insertions, 117 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigAwareAspectBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigAwareAspectBuilder.java new file mode 100644 index 0000000000..0b6bb2a5a4 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigAwareAspectBuilder.java @@ -0,0 +1,89 @@ +// Copyright 2017 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.analysis.config; + +import com.google.devtools.build.lib.packages.AspectDefinition; +import java.util.Collection; + +/** + * A wrapper for {@link AspectDefinition.Builder} that supports access to {@link + * com.google.devtools.build.lib.analysis} classes. + * + * <p>{@link AspectDefinition.Builder} is in {@link com.google.devtools.build.lib.packages}, so it + * can't reference analysis-time objects. + */ +public class ConfigAwareAspectBuilder { + private final AspectDefinition.Builder aspectBuilder; + + /** + * Instantiates a builder wrapped around the given {@link AspectDefinition.Builder}. + */ + private ConfigAwareAspectBuilder(AspectDefinition.Builder aspectBuilder) { + this.aspectBuilder = aspectBuilder; + } + + /** + * Instantiates a builder wrapped around the given {@link AspectDefinition.Builder}. + */ + public static ConfigAwareAspectBuilder of(AspectDefinition.Builder aspectBuilder) { + return new ConfigAwareAspectBuilder(aspectBuilder); + } + + /** + * Returns the {@link AspectDefinition.Builder} this object wraps. + * + * <p>Call this when done with config-specific building to re-expose the builder methods in + * {@link AspectDefinition.Builder} + */ + public AspectDefinition.Builder originalBuilder() { + return aspectBuilder; + } + + /** + * Declares the fragments required by this aspect for the configuration of the host machine this + * aspect's actions execute on. + * + * <p>This is not the same as the aspect's configuration. The aspect's configuration is its + * <i>target</i> configuration, which determines the platform it builds outputs for. The actions + * created by this aspect run on a host machine, which is what the host configuration corresponds + * to. + * + * <p>The value is inherited by subclasses. + */ + public ConfigAwareAspectBuilder requiresHostConfigurationFragments( + Class<? extends BuildConfiguration.Fragment>... configurationFragments) { + aspectBuilder.requiresConfigurationFragments(HostTransition.INSTANCE, configurationFragments); + return this; + } + + /** + * Declares the fragments required by this aspect for the configuration of the host machine this + * aspect's actions execute on. + * + * <p>This is not the same as the aspect's configuration. The aspect's configuration is its + * <i>target</i> configuration, which determines the platform it builds outputs for. The actions + * created by this aspect run on a host machine, which is what the host configuration corresponds + * to. + * + * <p>In contrast to {@link #requiresHostConfigurationFragments(Class...)}, this method takes + * Skylark module names of fragments instead of their classes. + */ + public ConfigAwareAspectBuilder requiresHostConfigurationFragmentsBySkylarkModuleName( + Collection<String> configurationFragmentNames) { + aspectBuilder.requiresConfigurationFragmentsBySkylarkModuleName(HostTransition.INSTANCE, + configurationFragmentNames); + return this; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigAwareRuleClassBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigAwareRuleClassBuilder.java new file mode 100644 index 0000000000..bfa54147ea --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigAwareRuleClassBuilder.java @@ -0,0 +1,90 @@ +// Copyright 2017 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.analysis.config; + +import com.google.devtools.build.lib.packages.RuleClass; +import java.util.Collection; + +/** + * A wrapper for {@link RuleClass.Builder} that supports access to {@link + * com.google.devtools.build.lib.analysis} classes. + * + * <p>{@link RuleClass.Builder} is in {@link com.google.devtools.build.lib.packages}, so it can't + * reference analysis-time objects. + */ +public class ConfigAwareRuleClassBuilder { + private final RuleClass.Builder ruleClassBuilder; + + /** + * Instantiates a builder wrapped around the given {@link RuleClass.Builder}. + */ + private ConfigAwareRuleClassBuilder(RuleClass.Builder ruleClassBuilder) { + this.ruleClassBuilder = ruleClassBuilder; + } + + /** + * Instantiates a builder wrapped around the given {@link RuleClass.Builder}. + */ + public static ConfigAwareRuleClassBuilder of(RuleClass.Builder ruleClassBuilder) { + return new ConfigAwareRuleClassBuilder(ruleClassBuilder); + } + + /** + * Returns the {@link RuleClass.Builder} this object wraps. + * + * <p>Call this when done with config-specific building to re-expose the builder methods in + * {@link RuleClass.Builder} + */ + public RuleClass.Builder originalBuilder() { + return ruleClassBuilder; + } + + /** + * Declares the fragments required by this rule for the configuration of the host machine this + * rule's actions execute on. + * + * <p>This is not the same as the rule's configuration. The rule's configuration is its + * <i>target</i> configuration, which determines the platform it builds outputs for. The actions + * created by this rule run on a host machine, which is what the host configuration corresponds + * to. + * + * <p>The value is inherited by subclasses. + */ + public ConfigAwareRuleClassBuilder requiresHostConfigurationFragments( + Class<? extends BuildConfiguration.Fragment>... configurationFragments) { + ruleClassBuilder.requiresConfigurationFragments(HostTransition.INSTANCE, + configurationFragments); + return this; + } + + /** + * Declares the fragments required by this rule for the configuration of the host machine this + * rule's actions execute on. + * + * <p>This is not the same as the rule's configuration. The rule's configuration is its + * <i>target</i> configuration, which determines the platform it builds outputs for. The actions + * created by this rule run on a host machine, which is what the host configuration corresponds + * to. + * + * <p>In contrast to {@link #requiresHostConfigurationFragments(Class...)}, this method takes + * Skylark module names of fragments instead of their classes. + */ + public ConfigAwareRuleClassBuilder requiresHostConfigurationFragmentsBySkylarkModuleName( + Collection<String> configurationFragmentNames) { + ruleClassBuilder.requiresConfigurationFragmentsBySkylarkModuleName(HostTransition.INSTANCE, + configurationFragmentNames); + return this; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java index e593cd7621..099c8fdf2f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java @@ -22,6 +22,11 @@ public final class HostTransition implements PatchTransition { private HostTransition() {} @Override + public boolean isHostTransition() { + return true; + } + + @Override public BuildOptions apply(BuildOptions options) { if (options.get(BuildConfiguration.Options.class).isHost) { // If the input already comes from the host configuration, just return the existing values. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java index 48a0182a25..0bfcbc55bb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList; 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.analysis.config.HostTransition; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.syntax.Type; @@ -36,6 +37,7 @@ public class EnvironmentRule implements RuleDefinition { @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { return builder + .cfg(HostTransition.INSTANCE) .override(attr("tags", Type.STRING_LIST) // No need to show up in ":all", etc. target patterns. .value(ImmutableList.of("manual")) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index 350e98d41c..1e7d5fe5dc 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.analysis.skylark; import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER; 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.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; @@ -40,6 +39,8 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.DefaultInfo; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder; +import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.skylark.SkylarkAttr.Descriptor; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.cmdline.Label; @@ -184,19 +185,19 @@ public class SkylarkRuleClassFunctions { // Input files for every test action .add( attr("$test_runtime", LABEL_LIST) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .value( ImmutableList.of( labelCache.getUnchecked(toolsRepository + "//tools/test:runtime")))) // Input files for test actions collecting code coverage .add( attr("$coverage_support", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .value(labelCache.getUnchecked("//tools/defaults:coverage_support"))) // Used in the one-per-build coverage report generation action. .add( attr("$coverage_report_generator", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .value(labelCache.getUnchecked("//tools/defaults:coverage_report_generator")) .singleArtifact()) .add(attr(":run_under", LABEL).cfg(DATA).value(RUN_UNDER)) @@ -546,8 +547,9 @@ public class SkylarkRuleClassFunctions { builder.requiresConfigurationFragmentsBySkylarkModuleName( fragments.getContents(String.class, "fragments")); - builder.requiresHostConfigurationFragmentsBySkylarkModuleName( - hostFragments.getContents(String.class, "host_fragments")); + ConfigAwareRuleClassBuilder.of(builder) + .requiresHostConfigurationFragmentsBySkylarkModuleName( + hostFragments.getContents(String.class, "host_fragments")); builder.setConfiguredTargetFunction(implementation); builder.setRuleDefinitionEnvironment(funcallEnv); builder.addRequiredToolchains(collectToolchainLabels(toolchains, ast)); @@ -805,6 +807,7 @@ public class SkylarkRuleClassFunctions { SkylarkAttr.getSkylarkProviderIdentifiers(providesArg, ast.getLocation()), requiredParams.build(), ImmutableSet.copyOf(fragments.getContents(String.class, "fragments")), + HostTransition.INSTANCE, ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")), collectToolchainLabels(toolchains, ast), funcallEnv); 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 270238f015..f302d345af 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 @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.bazel.rules.java; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; @@ -22,6 +21,8 @@ import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder; +import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses.JavaRule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; @@ -40,9 +41,10 @@ public final class BazelJavaLibraryRule implements RuleDefinition { @Override public RuleClass build(Builder builder, final RuleDefinitionEnvironment env) { - return builder + return ConfigAwareRuleClassBuilder.of(builder) + .requiresHostConfigurationFragments(Jvm.class) // For getting the host Java executable. + .originalBuilder() .requiresConfigurationFragments(JavaConfiguration.class, CppConfiguration.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> @@ -149,7 +151,7 @@ public final class BazelJavaLibraryRule implements RuleDefinition { <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add( attr("exported_plugins", LABEL_LIST) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .allowedRuleClasses("java_plugin") .allowedFileTypes()) .advertiseProvider(JavaSourceInfoProvider.class) diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java index eea440e521..773001e004 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java @@ -24,6 +24,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.Attribute.Transition; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.syntax.Type.LabelClass; @@ -418,13 +419,19 @@ public final class AspectDefinition { /** * Declares that the implementation of the associated aspect definition requires the given - * fragments to be present in the host configuration. + * fragments to be present in the given configuration that isn't the aspect's configuration but + * is also readable by the aspect. + * + * <p>You probably don't want to use this, because aspects generally shouldn't read + * configurations other than their own. If you want to declare host config fragments, see + * {@link com.google.devtools.build.lib.analysis.config.ConfigAwareAspectBuilder}. * * <p>The value is inherited by subclasses. */ - public Builder requiresHostConfigurationFragments(Class<?>... configurationFragments) { - configurationFragmentPolicy - .requiresHostConfigurationFragments(ImmutableSet.copyOf(configurationFragments)); + public Builder requiresConfigurationFragments(Transition transition, + Class<?>... configurationFragments) { + configurationFragmentPolicy.requiresConfigurationFragments(transition, + ImmutableSet.copyOf(configurationFragments)); return this; } @@ -443,16 +450,21 @@ public final class AspectDefinition { } /** - * Declares the configuration fragments that are required by this rule for the host - * configuration. + * Declares that the implementation of the associated aspect definition requires the given + * fragments to be present in the given configuration that isn't the aspect's configuration but + * is also readable by the aspect. + * + * <p>In contrast to {@link #requiresConfigurationFragments(Transition, Class...)}, this method + * takes the Skylark module names of fragments instead of their classes. * - * <p>In contrast to {@link #requiresHostConfigurationFragments(Class...)}, this method takes - * the Skylark module names of fragments instead of their classes. + * <p>You probably don't want to use this, because aspects generally shouldn't read + * configurations other than their own. If you want to declare host config fragments, see + * {@link com.google.devtools.build.lib.analysis.config.ConfigAwareAspectBuilder}. */ - public Builder requiresHostConfigurationFragmentsBySkylarkModuleName( + public Builder requiresConfigurationFragmentsBySkylarkModuleName(Transition transition, Collection<String> configurationFragmentNames) { - configurationFragmentPolicy - .requiresHostConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames); + configurationFragmentPolicy.requiresConfigurationFragmentsBySkylarkModuleName(transition, + configurationFragmentNames); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 9877ea4054..d0ed2195f3 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -150,6 +150,12 @@ public final class Attribute implements Comparable<Attribute> { * A configuration transition. */ public interface Transition { + /** + * Does this transition switch to a "host" configuration? + */ + default boolean isHostTransition() { + return false; + } } /** @@ -171,13 +177,22 @@ public final class Attribute implements Comparable<Attribute> { /** * Declaration how the configuration should change when following a label or label list attribute. + * + * <p>Do not add to this. Use {@link + * com.google.devtools.build.lib.analysis.config.PatchTransition} or {@link SplitTransition} + * instead. */ public enum ConfigurationTransition implements Transition { /** No transition, i.e., the same configuration as the current. */ NONE, /** Transition to the host configuration. */ - HOST, + HOST { + @Override + public boolean isHostTransition() { + return true; + } + }, /** Transition to a null configuration (applies to, e.g., input files). */ NULL, @@ -1860,7 +1875,7 @@ public final class Attribute implements Comparable<Attribute> { if (isLateBound(name)) { LateBoundDefault<?, ?> lateBoundDefault = (LateBoundDefault<?, ?>) defaultValue; Preconditions.checkArgument(!lateBoundDefault.useHostConfiguration() - || (configTransition == ConfigurationTransition.HOST), + || (configTransition.isHostTransition()), "a late bound default value using the host configuration must use the host transition"); } 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 c75c01b088..793ec84eb2 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,14 +13,16 @@ // limitations under the License. package com.google.devtools.build.lib.packages; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; 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.packages.Attribute.Transition; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; - import java.util.Collection; +import java.util.Map; import java.util.Set; /** @@ -64,13 +66,13 @@ public final class ConfigurationFragmentPolicy { * 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 + private final SetMultimap<Transition, 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 + private final SetMultimap<Transition, String> requiredConfigurationFragmentNames = LinkedHashMultimap.create(); private MissingFragmentPolicy missingFragmentPolicy = MissingFragmentPolicy.FAIL_ANALYSIS; @@ -87,25 +89,19 @@ public final class ConfigurationFragmentPolicy { /** * Declares that the implementation of the associated rule class requires the given - * fragments to be present in this rule's host configuration only. - * - * <p>The value is inherited by subclasses. - */ - public Builder requiresHostConfigurationFragments(Collection<Class<?>> configurationFragments) { - requiresConfigurationFragments(ConfigurationTransition.HOST, configurationFragments); - return this; - } - - /** - * 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. */ - private void requiresConfigurationFragments(ConfigurationTransition transition, + public Builder requiresConfigurationFragments(Attribute.Transition transition, Collection<Class<?>> configurationFragments) { + // We can relax this assumption if needed. But it's already sketchy to let a rule see more + // than its own configuration. So we don't want to casually proliferate this pattern. + Preconditions.checkArgument( + transition == Attribute.ConfigurationTransition.NONE || transition.isHostTransition()); requiredConfigurationFragments.putAll(transition, configurationFragments); + return this; } /** @@ -120,25 +116,9 @@ public final class ConfigurationFragmentPolicy { */ 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); + Attribute.ConfigurationTransition.NONE, configurationFragmentNames); return this; } @@ -147,13 +127,19 @@ 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(Collection)}, this method takes the - * names of fragments (as determined by {@link SkylarkModule.Resolver}) instead of their + * <p>In contrast to {@link #requiresConfigurationFragments(Transition, + * 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) { + public Builder requiresConfigurationFragmentsBySkylarkModuleName(Transition transition, + Collection<String> configurationFragmentNames) { + // We can relax this assumption if needed. But it's already sketchy to let a rule see more + // than its own configuration. So we don't want to casually proliferate this pattern. + Preconditions.checkArgument( + transition == ConfigurationTransition.NONE || transition.isHostTransition()); requiredConfigurationFragmentNames.putAll(transition, configurationFragmentNames); + return this; } /** @@ -188,14 +174,14 @@ public final class ConfigurationFragmentPolicy { * A dictionary that maps configurations (NONE for target configuration, HOST for host * configuration) to required configuration fragments. */ - private final ImmutableSetMultimap<ConfigurationTransition, Class<?>> + private final ImmutableSetMultimap<Transition, Class<?>> requiredConfigurationFragments; /** * A dictionary that maps configurations (NONE for target configuration, HOST for host * configuration) to lists of Skylark module names of required configuration fragments. */ - private final ImmutableSetMultimap<ConfigurationTransition, String> + private final ImmutableSetMultimap<Transition, String> requiredConfigurationFragmentNames; /** @@ -204,8 +190,8 @@ public final class ConfigurationFragmentPolicy { private final MissingFragmentPolicy missingFragmentPolicy; private ConfigurationFragmentPolicy( - ImmutableSetMultimap<ConfigurationTransition, Class<?>> requiredConfigurationFragments, - ImmutableSetMultimap<ConfigurationTransition, String> requiredConfigurationFragmentNames, + ImmutableSetMultimap<Transition, Class<?>> requiredConfigurationFragments, + ImmutableSetMultimap<Transition, String> requiredConfigurationFragmentNames, MissingFragmentPolicy missingFragmentPolicy) { this.requiredConfigurationFragments = requiredConfigurationFragments; this.requiredConfigurationFragmentNames = requiredConfigurationFragmentNames; @@ -228,7 +214,7 @@ public final class ConfigurationFragmentPolicy { * specified in the same configuration that was passed. */ public boolean isLegalConfigurationFragment( - Class<?> configurationFragment, ConfigurationTransition config) { + Class<?> configurationFragment, Transition config) { return requiredConfigurationFragments.containsValue(configurationFragment) || hasLegalFragmentName(configurationFragment, config); } @@ -247,9 +233,20 @@ public final class ConfigurationFragmentPolicy { * specified configuration (target or host). */ private boolean hasLegalFragmentName( - Class<?> configurationFragment, ConfigurationTransition config) { - return requiredConfigurationFragmentNames.containsEntry( - config, SkylarkModule.Resolver.resolveName(configurationFragment)); + Class<?> configurationFragment, Transition transition) { + String name = SkylarkModule.Resolver.resolveName(configurationFragment); + if (requiredConfigurationFragmentNames.containsEntry(transition, name)) { + return true; + } else if (transition == ConfigurationTransition.HOST) { + // We can get rid of this check when we remove ConfigurationTransition.HOST. This essentially + // makes ConfigurationTransition.HOST and HostTransition.INSTANCE interchangeable. + for (Map.Entry<Transition, String> entry : requiredConfigurationFragmentNames.entries()) { + if (entry.getKey().isHostTransition() && entry.getValue().equals(name)) { + return true; + } + } + } + return false; } /** diff --git a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java index 88fe6d4157..f3d4971647 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java +++ b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.packages; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.DependencyFilter.AttributeInfoProvider; import com.google.devtools.build.lib.syntax.Type.LabelClass; import com.google.devtools.build.lib.util.BinaryPredicate; @@ -44,7 +43,7 @@ public abstract class DependencyFilter return true; } - return attribute.getConfigurationTransition() != ConfigurationTransition.HOST; + return !attribute.getConfigurationTransition().isHostTransition(); } }; /** Dependency predicate that excludes implicit dependencies */ 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 273745bf41..e76339dc7b 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 @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.packages; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; @@ -708,12 +707,19 @@ public class RuleClass { /** * Declares that the implementation of the associated rule class requires the given - * fragments to be present in the host configuration. + * fragments to be present in the given configuration that isn't the rule's configuration but + * is also readable by the rule. + * + * <p>You probably don't want to use this, because rules generally shouldn't read configurations + * other than their own. If you want to declare host config fragments, see + * {@link com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder}. * * <p>The value is inherited by subclasses. */ - public Builder requiresHostConfigurationFragments(Class<?>... configurationFragments) { - configurationFragmentPolicy.requiresHostConfigurationFragments( + public Builder requiresConfigurationFragments(Transition transition, + Class<?>... configurationFragments) { + configurationFragmentPolicy.requiresConfigurationFragments( + transition, ImmutableSet.<Class<?>>copyOf(configurationFragments)); return this; } @@ -736,13 +742,25 @@ public class RuleClass { * 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( + /** + * Declares that the implementation of the associated rule class requires the given + * fragments to be present in the given configuration that isn't the rule's configuration but + * is also readable by the rule. + * + * <p>In contrast to {@link #requiresConfigurationFragments(Transition, Class...)}, this method + * takes Skylark module names of fragments instead of their classes. + * * + * <p>You probably don't want to use this, because rules generally shouldn't read configurations + * other than their own. If you want to declare host config fragments, see + * {@link com.google.devtools.build.lib.analysis.config.ConfigAwareRuleClassBuilder}. + * + * <p>The value is inherited by subclasses. + */ + public Builder requiresConfigurationFragmentsBySkylarkModuleName(Transition transition, Collection<String> configurationFragmentNames) { - configurationFragmentPolicy - .requiresHostConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames); + configurationFragmentPolicy.requiresConfigurationFragmentsBySkylarkModuleName(transition, + configurationFragmentNames); return this; } @@ -1033,8 +1051,9 @@ public class RuleClass { * {@link com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics} for details. */ public <TYPE> Builder compatibleWith(Label... environments) { - add(attr(DEFAULT_COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST).cfg(HOST) - .value(ImmutableList.copyOf(environments))); + add( + attr(DEFAULT_COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST) + .value(ImmutableList.copyOf(environments))); return this; } @@ -1049,9 +1068,10 @@ public class RuleClass { public <TYPE> Builder restrictedTo(Label firstEnvironment, Label... otherEnvironments) { ImmutableList<Label> environments = ImmutableList.<Label>builder().add(firstEnvironment) .add(otherEnvironments).build(); - add(attr(DEFAULT_RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST).cfg(HOST).value(environments)); + add( + attr(DEFAULT_RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST) + .value(environments)); return this; - } /** diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java index 85aea03880..15851545ad 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java @@ -37,6 +37,7 @@ public class SkylarkDefinedAspect implements SkylarkExportable, SkylarkAspect { private final ImmutableSet<SkylarkProviderIdentifier> provides; private final ImmutableSet<String> paramAttributes; private final ImmutableSet<String> fragments; + private final Attribute.Transition hostTransition; private final ImmutableSet<String> hostFragments; private final ImmutableList<Label> requiredToolchains; @@ -51,6 +52,8 @@ public class SkylarkDefinedAspect implements SkylarkExportable, SkylarkAspect { ImmutableSet<SkylarkProviderIdentifier> provides, ImmutableSet<String> paramAttributes, ImmutableSet<String> fragments, + // The host transition is in lib.analysis, so we can't reference it directly here. + Attribute.Transition hostTransition, ImmutableSet<String> hostFragments, ImmutableList<Label> requiredToolchains, Environment funcallEnv) { @@ -61,6 +64,7 @@ public class SkylarkDefinedAspect implements SkylarkExportable, SkylarkAspect { this.provides = provides; this.paramAttributes = paramAttributes; this.fragments = fragments; + this.hostTransition = hostTransition; this.hostFragments = hostFragments; this.requiredToolchains = requiredToolchains; this.funcallEnv = funcallEnv; @@ -149,7 +153,7 @@ public class SkylarkDefinedAspect implements SkylarkExportable, SkylarkAspect { } builder.advertiseProvider(advertisedSkylarkProviders.build()); builder.requiresConfigurationFragmentsBySkylarkModuleName(fragments); - builder.requiresHostConfigurationFragmentsBySkylarkModuleName(hostFragments); + builder.requiresConfigurationFragmentsBySkylarkModuleName(hostTransition, hostFragments); builder.addRequiredToolchains(requiredToolchains); return builder.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java index 5b508c2366..7811b01c51 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.rules.objc; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static java.nio.charset.StandardCharsets.ISO_8859_1; @@ -37,6 +36,8 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorAr import com.google.devtools.build.lib.analysis.actions.ParamFileInfo; import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.config.ConfigAwareAspectBuilder; +import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -114,7 +115,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF protected AspectDefinition.Builder addAdditionalAttributes(AspectDefinition.Builder builder) { return builder.add( attr("$j2objc_plugin", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .value( Label.parseAbsoluteUnchecked( @@ -133,7 +134,9 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF @Override public AspectDefinition getDefinition(AspectParameters aspectParameters) { - return addAdditionalAttributes(new AspectDefinition.Builder(this)) + return ConfigAwareAspectBuilder.of(addAdditionalAttributes(new AspectDefinition.Builder(this))) + .requiresHostConfigurationFragments(Jvm.class) + .originalBuilder() .propagateAlongAttribute("deps") .propagateAlongAttribute("exports") .propagateAlongAttribute("runtime_deps") @@ -145,10 +148,9 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF J2ObjcConfiguration.class, ObjcConfiguration.class, ProtoConfiguration.class) - .requiresHostConfigurationFragments(Jvm.class) .add( attr("$j2objc", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .value( Label.parseAbsoluteUnchecked( @@ -156,7 +158,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF .add( attr("$j2objc_wrapper", LABEL) .allowedFileTypes(FileType.of(".py")) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .singleArtifact() .value( @@ -165,7 +167,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF .add( attr("$j2objc_header_map", LABEL) .allowedFileTypes(FileType.of(".py")) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .singleArtifact() .value( @@ -173,11 +175,11 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF toolsRepository + "//tools/j2objc:j2objc_header_map"))) .add( attr("$jre_emul_jar", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .value( Label.parseAbsoluteUnchecked( toolsRepository + "//third_party/java/j2objc:jre_emul.jar"))) - .add(attr(":dead_code_report", LABEL).cfg(HOST).value(DEAD_CODE_REPORT)) + .add(attr(":dead_code_report", LABEL).cfg(HostTransition.INSTANCE).value(DEAD_CODE_REPORT)) .add( attr("$jre_lib", LABEL) .value( @@ -190,12 +192,12 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF toolsRepository + "//third_party/java/j2objc:proto_runtime"))) .add( attr("$xcrunwrapper", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .value(Label.parseAbsoluteUnchecked(toolsRepository + "//tools/objc:xcrunwrapper"))) .add( attr(ObjcRuleClasses.LIBTOOL_ATTRIBUTE, LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .value(Label.parseAbsoluteUnchecked(toolsRepository + "//tools/objc:libtool"))) .add( @@ -206,7 +208,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF .value(AppleToolchain.getXcodeConfigLabel(toolsRepository))) .add( attr("$zipper", LABEL) - .cfg(HOST) + .cfg(HostTransition.INSTANCE) .exec() .value(Label.parseAbsoluteUnchecked(toolsRepository + "//tools/zip:zipper"))) .add( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java index 5738cb1923..5127120ab0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java @@ -19,6 +19,8 @@ import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.ConfigAwareAspectBuilder; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.AdvertisedProviderSet; import com.google.devtools.build.lib.packages.AspectDefinition; @@ -254,16 +256,21 @@ public class AspectDefinitionTest { .containsExactly(Integer.class, String.class); } + private static class FooFragment extends BuildConfiguration.Fragment {} + private static class BarFragment extends BuildConfiguration.Fragment {} + @Test public void testRequiresHostConfigurationFragments_PropagatedToConfigurationFragmentPolicy() throws Exception { - AspectDefinition requiresFragments = new AspectDefinition.Builder(TEST_ASPECT_CLASS) - .requiresHostConfigurationFragments(Integer.class, String.class) - .build(); + AspectDefinition requiresFragments = + ConfigAwareAspectBuilder.of(new AspectDefinition.Builder(TEST_ASPECT_CLASS)) + .requiresHostConfigurationFragments(FooFragment.class, BarFragment.class) + .originalBuilder() + .build(); assertThat(requiresFragments.getConfigurationFragmentPolicy()).isNotNull(); assertThat( requiresFragments.getConfigurationFragmentPolicy().getRequiredConfigurationFragments()) - .containsExactly(Integer.class, String.class); + .containsExactly(FooFragment.class, BarFragment.class); } @Test @@ -282,9 +289,12 @@ public class AspectDefinitionTest { @Test public void testRequiresHostConfigurationFragmentNames_PropagatedToConfigurationFragmentPolicy() throws Exception { - AspectDefinition requiresFragments = new AspectDefinition.Builder(TEST_ASPECT_CLASS) - .requiresHostConfigurationFragmentsBySkylarkModuleName(ImmutableList.of("test_fragment")) - .build(); + AspectDefinition requiresFragments = + ConfigAwareAspectBuilder.of(new AspectDefinition.Builder(TEST_ASPECT_CLASS)) + .requiresHostConfigurationFragmentsBySkylarkModuleName( + ImmutableList.of("test_fragment")) + .originalBuilder() + .build(); assertThat(requiresFragments.getConfigurationFragmentPolicy()).isNotNull(); assertThat( requiresFragments.getConfigurationFragmentPolicy() @@ -294,10 +304,12 @@ public class AspectDefinitionTest { @Test public void testEmptySkylarkConfigurationFragmentPolicySetup_HasNonNullPolicy() throws Exception { - AspectDefinition noPolicy = new AspectDefinition.Builder(TEST_ASPECT_CLASS) - .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableList.<String>of()) - .requiresHostConfigurationFragmentsBySkylarkModuleName(ImmutableList.<String>of()) - .build(); + AspectDefinition noPolicy = + ConfigAwareAspectBuilder.of(new AspectDefinition.Builder(TEST_ASPECT_CLASS)) + .requiresHostConfigurationFragmentsBySkylarkModuleName(ImmutableList.<String>of()) + .originalBuilder() + .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableList.<String>of()) + .build(); assertThat(noPolicy.getConfigurationFragmentPolicy()).isNotNull(); } 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 index a3ea359781..7ed005a3d1 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicyTest.java @@ -19,7 +19,6 @@ 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; @@ -71,13 +70,21 @@ public final class ConfigurationFragmentPolicyTest { .containsExactly(Integer.class, String.class, Long.class); } + private static final Attribute.Transition TEST_HOST_TRANSITION = new Attribute.Transition() { + @Override + public boolean isHostTransition() { + return true; + } + }; + @Test public void testRequiresConfigurationFragments_RequiredAndLegalForSpecifiedConfiguration() throws Exception { ConfigurationFragmentPolicy policy = new ConfigurationFragmentPolicy.Builder() .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(Integer.class)) - .requiresHostConfigurationFragments(ImmutableSet.<Class<?>>of(Long.class)) + .requiresConfigurationFragments(TEST_HOST_TRANSITION, + ImmutableSet.<Class<?>>of(Long.class)) .build(); assertThat(policy.getRequiredConfigurationFragments()).containsAllOf(Integer.class, Long.class); @@ -87,7 +94,7 @@ public final class ConfigurationFragmentPolicyTest { .isTrue(); // TODO(mstaib): .isFalse() when dynamic configurations care which configuration a fragment was // specified for - assertThat(policy.isLegalConfigurationFragment(Integer.class, ConfigurationTransition.HOST)) + assertThat(policy.isLegalConfigurationFragment(Integer.class, TEST_HOST_TRANSITION)) .isTrue(); assertThat(policy.isLegalConfigurationFragment(Long.class)).isTrue(); @@ -95,13 +102,13 @@ public final class ConfigurationFragmentPolicyTest { // specified for assertThat(policy.isLegalConfigurationFragment(Long.class, ConfigurationTransition.NONE)) .isTrue(); - assertThat(policy.isLegalConfigurationFragment(Long.class, ConfigurationTransition.HOST)) + assertThat(policy.isLegalConfigurationFragment(Long.class, TEST_HOST_TRANSITION)) .isTrue(); assertThat(policy.isLegalConfigurationFragment(String.class)).isFalse(); assertThat(policy.isLegalConfigurationFragment(String.class, ConfigurationTransition.NONE)) .isFalse(); - assertThat(policy.isLegalConfigurationFragment(String.class, ConfigurationTransition.HOST)) + assertThat(policy.isLegalConfigurationFragment(String.class, TEST_HOST_TRANSITION)) .isFalse(); } @@ -111,7 +118,7 @@ public final class ConfigurationFragmentPolicyTest { ConfigurationFragmentPolicy policy = new ConfigurationFragmentPolicy.Builder() .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableSet.of("test_fragment")) - .requiresHostConfigurationFragmentsBySkylarkModuleName( + .requiresConfigurationFragmentsBySkylarkModuleName(TEST_HOST_TRANSITION, ImmutableSet.of("other_fragment")) .build(); @@ -122,7 +129,7 @@ public final class ConfigurationFragmentPolicyTest { policy.isLegalConfigurationFragment(TestFragment.class, ConfigurationTransition.NONE)) .isTrue(); assertThat( - policy.isLegalConfigurationFragment(TestFragment.class, ConfigurationTransition.HOST)) + policy.isLegalConfigurationFragment(TestFragment.class, TEST_HOST_TRANSITION)) .isFalse(); assertThat(policy.isLegalConfigurationFragment(OtherFragment.class)).isTrue(); @@ -130,7 +137,7 @@ public final class ConfigurationFragmentPolicyTest { policy.isLegalConfigurationFragment(OtherFragment.class, ConfigurationTransition.NONE)) .isFalse(); assertThat( - policy.isLegalConfigurationFragment(OtherFragment.class, ConfigurationTransition.HOST)) + policy.isLegalConfigurationFragment(OtherFragment.class, TEST_HOST_TRANSITION)) .isTrue(); assertThat(policy.isLegalConfigurationFragment(UnknownFragment.class)).isFalse(); @@ -140,7 +147,7 @@ public final class ConfigurationFragmentPolicyTest { .isFalse(); assertThat( policy.isLegalConfigurationFragment( - UnknownFragment.class, ConfigurationTransition.HOST)) + UnknownFragment.class, TEST_HOST_TRANSITION)) .isFalse(); } @@ -155,10 +162,11 @@ public final class ConfigurationFragmentPolicyTest { ConfigurationFragmentPolicy addedPolicy = new ConfigurationFragmentPolicy.Builder() .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableSet.of("other_fragment")) - .requiresHostConfigurationFragmentsBySkylarkModuleName( + .requiresConfigurationFragmentsBySkylarkModuleName(TEST_HOST_TRANSITION, ImmutableSet.of("other_fragment")) .requiresConfigurationFragments(ImmutableSet.<Class<?>>of(Boolean.class)) - .requiresHostConfigurationFragments(ImmutableSet.<Class<?>>of(Character.class)) + .requiresConfigurationFragments(TEST_HOST_TRANSITION, + ImmutableSet.<Class<?>>of(Character.class)) .build(); ConfigurationFragmentPolicy combinedPolicy = new ConfigurationFragmentPolicy.Builder() |