From a3ba00e61f81163f90b1edee4a076c7e7d877e4c Mon Sep 17 00:00:00 2001 From: asteinb Date: Wed, 21 Mar 2018 09:59:36 -0700 Subject: Remove support for dynamically configured resource filtering Dynamically configured resource filtering was never turned on, as dynamic configuration turned out to be too slow. While we're at it, do a bit of related cleanup (remove the unused flag that controlled resource filtering in analysis, and switch from a now-two-valued enum to a boolean to specify whether resources should be filtered in analysis). RELNOTES: none PiperOrigin-RevId: 189923588 --- .../lib/rules/android/AndroidConfiguration.java | 67 +----- .../lib/rules/android/AndroidRuleClasses.java | 23 -- .../lib/rules/android/ResourceFilterFactory.java | 245 ++----------------- .../build/lib/rules/android/AndroidBinaryTest.java | 93 -------- .../rules/android/ResourceFilterFactoryTest.java | 263 ++++----------------- 5 files changed, 81 insertions(+), 610 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index fe404b2947..845b540169 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -28,13 +28,10 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.skylark.annotations.SkylarkConfigurationField; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.packages.AggregatingAttributeMapper; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion.AndroidRobolectricTestDeprecationLevel; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; import com.google.devtools.build.lib.rules.cpp.CppOptions.DynamicModeConverter; @@ -491,8 +488,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { metadataTags = {OptionMetadataTag.EXPERIMENTAL}, documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - help = "Whether to use incremental dexing for proguarded Android binaries by default. " - + "Use incremental_dexing attribute to override default for a particular android_binary." + help = + "Whether to use incremental dexing for proguarded Android binaries by default. " + + "Use incremental_dexing attribute to override default for a particular " + + "android_binary." ) public boolean incrementalDexingAfterProguardByDefault; @@ -713,32 +712,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { ) public boolean useSingleJarApkBuilder; - @Option( - name = "experimental_android_resource_filtering_method", - converter = ResourceFilterFactory.Converter.class, - defaultValue = "filter_in_analysis", - documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION, - effectTags = { - OptionEffectTag.CHANGES_INPUTS, - OptionEffectTag.LOADING_AND_ANALYSIS, - OptionEffectTag.LOSES_INCREMENTAL_STATE, - }, - metadataTags = {OptionMetadataTag.EXPERIMENTAL}, - help = - "Determines when resource filtering attributes, such as the android_binary " - + "'resource_configuration_filters' and 'densities' attributes, are applied. " - + "By default, bazel will 'filter_in_analysis'. The experimental " - + "'filter_in_analysis_with_dynamic_configuration' option also passes these options " - + "to the android_binary's dependencies, which also filter their internal resources " - + "in analysis, possibly making the build even faster (especially in systems that " - + "do not cache the results of those dependencies). When using aapt2, filtering is " - + "only performed in execution, and this setting does nothing." - ) - // The ResourceFilterFactory object holds the filtering behavior as well as settings for which - // resources should be filtered. The filtering behavior is set from the command line, but the - // other settings default to empty and are set or modified via dynamic configuration. - public ResourceFilterFactory resourceFilterFactory; - @Option( name = "experimental_android_compress_java_resources", defaultValue = "false", @@ -893,7 +866,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final AndroidManifestMerger manifestMerger; private final ApkSigningMethod apkSigningMethod; private final boolean useSingleJarApkBuilder; - private final ResourceFilterFactory resourceFilterFactory; private final boolean compressJavaResources; private final boolean exportsManifestDefault; private final AndroidAaptVersion androidAaptVersion; @@ -931,7 +903,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.apkSigningMethod = options.apkSigningMethod; this.useSingleJarApkBuilder = options.useSingleJarApkBuilder; this.useRexToCompressDexFiles = options.useRexToCompressDexFiles; - this.resourceFilterFactory = options.resourceFilterFactory; this.compressJavaResources = options.compressJavaResources; this.exportsManifestDefault = options.exportsManifestDefault; this.androidAaptVersion = options.androidAaptVersion; @@ -949,7 +920,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { if (incrementalDexingAfterProguardByDefault && incrementalDexingShardsAfterProguard == 0) { throw new InvalidConfigurationException( "--experimental_incremental_dexing_after_proguard_by_default requires " - + "--experimental_incremental_dexing_after_proguard to be at least 1"); + + "--experimental_incremental_dexing_after_proguard to be at least 1"); } if (desugarJava8Libs && !desugarJava8) { throw new InvalidConfigurationException( @@ -982,7 +953,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { AndroidManifestMerger manifestMerger, ApkSigningMethod apkSigningMethod, boolean useSingleJarApkBuilder, - ResourceFilterFactory resourceFilterFactory, boolean compressJavaResources, boolean exportsManifestDefault, AndroidAaptVersion androidAaptVersion, @@ -1015,7 +985,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.manifestMerger = manifestMerger; this.apkSigningMethod = apkSigningMethod; this.useSingleJarApkBuilder = useSingleJarApkBuilder; - this.resourceFilterFactory = resourceFilterFactory; this.compressJavaResources = compressJavaResources; this.exportsManifestDefault = exportsManifestDefault; this.androidAaptVersion = androidAaptVersion; @@ -1140,10 +1109,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { return useSingleJarApkBuilder; } - public ResourceFilterFactory getResourceFilterFactory() { - return resourceFilterFactory; - } - public boolean useParallelDex2Oat() { return useParallelDex2Oat; } @@ -1183,26 +1148,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { @Override public String getOutputDirectoryName() { - // We expect this value to be null most of the time - it will only become non-null when a - // dynamically configured transition changes the configuration's resource filter object. - String resourceFilterSuffix = resourceFilterFactory.getOutputDirectorySuffix(); - - if (configurationDistinguisher.suffix == null) { - return resourceFilterSuffix; - } - - if (resourceFilterSuffix == null) { - return configurationDistinguisher.suffix; - } - - return configurationDistinguisher.suffix + "_" + resourceFilterSuffix; - } - - @Nullable - @Override - public PatchTransition topLevelConfigurationHook(Target toTarget) { - return resourceFilterFactory.getTopLevelPatchTransition( - toTarget.getAssociatedRule().getRuleClass(), - AggregatingAttributeMapper.of(toTarget.getAssociatedRule())); + return configurationDistinguisher.suffix; } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index bda2847eba..36ddef08e4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -28,7 +28,6 @@ import static com.google.devtools.build.lib.util.FileTypeSet.ANY_FILE; import static com.google.devtools.build.lib.util.FileTypeSet.NO_FILE; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Lists; import com.google.devtools.build.lib.analysis.BaseRuleClasses; @@ -37,7 +36,6 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.HostTransition; -import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.EventHandler; @@ -46,11 +44,9 @@ import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; -import com.google.devtools.build.lib.packages.RuleTransitionFactory; import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; @@ -289,25 +285,6 @@ public final class AndroidRuleClasses { } } - /** - * Turns off dynamic resource filtering for non-Android targets. This prevents unnecessary build - * graph bloat. For example, there's no point analyzing distinct cc_library targets for different - * resource filter configurations because cc_library semantics doesn't care about filters. - */ - public static final RuleTransitionFactory REMOVE_DYNAMIC_RESOURCE_FILTERING = - new RuleTransitionFactory() { - /** Dependencies of these rule class types need to keep resource filtering info. */ - private final ImmutableSet keepFilterRuleClasses = - ImmutableSet.of("android_binary", "android_library"); - - @Override - public ConfigurationTransition buildTransitionFor(Rule depRule) { - return keepFilterRuleClasses.contains(depRule.getRuleClass()) - ? null - : ResourceFilterFactory.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION; - } - }; - public static final FileType ANDROID_IDL = FileType.of(".aidl"); public static final String[] ALLOWED_DEPENDENCIES = { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java index 3b1261daca..881831baf8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java @@ -29,22 +29,16 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.RuleErrorConsumer; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.syntax.Type; -import com.google.devtools.common.options.EnumConverter; -import com.google.devtools.common.options.OptionsParsingException; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.annotation.Nullable; /** * Filters resources based on their qualifiers. @@ -56,58 +50,10 @@ import javax.annotation.Nullable; * and {@link #hashCode()} methods. Failure to do so isn't just bad practice; it could seriously * interfere with Bazel's caching performance. */ -@AutoCodec public class ResourceFilterFactory { public static final String RESOURCE_CONFIGURATION_FILTERS_NAME = "resource_configuration_filters"; public static final String DENSITIES_NAME = "densities"; - /** - * Locales used for pseudolocation. - * - *

These are special resources that can be used to test how apps handles special cases (such as - * particularly long text, accents, or left-to-right text). These resources are not provided like - * other resources; instead, when the appropriate filters are passed in, aapt generates them based - * on the default resources. - * - *

When these locales are specified in the configuration filters, even if we are filtering in - * analysis, we need to pass *all* configuration filters to aapt - the pseudolocalization filters - * themselves to trigger pseudolocalization, and the other filters to prevent aapt from filtering - * matching resources out. - */ - @VisibleForTesting - static enum FilterBehavior { - /** - * Resources will be filtered in execution. This class will just pass the filtering parameters - * to the appropriate resource processing actions. - */ - FILTER_IN_EXECUTION, - - /** - * Resources will be filtered in analysis. In android_binary targets, all resources will be - * filtered by this class, and only resources that are accepted will be passed to resource - * processing actions. Resources may be filtered again in execution to handle some edge cases, - * including resources contained in Filesets, Pseudolocalized resource generation, and stricter - * filtering implementations within aapt. - */ - FILTER_IN_ANALYSIS, - - /** - * Resources will be filtered in each android target in analysis. Filter settings will be - * extracted from android_binary targets and passed to all their dependencies using dynamic - * configuration. Only resources that are accepted by filtering will be passed to resource - * processing actions or to reverse dependencies. - */ - FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION; - - private static final class Converter extends EnumConverter { - Converter() { - super(FilterBehavior.class, "resource filter behavior"); - } - } - } - - static final FilterBehavior DEFAULT_BEHAVIOR = FilterBehavior.FILTER_IN_EXECUTION; - /** * The value of the {@link #RESOURCE_CONFIGURATION_FILTERS_NAME} attribute, as a list of qualifier * strings. @@ -120,22 +66,22 @@ public class ResourceFilterFactory { /** A builder for a set of strings representing resources that were filtered using this class. */ private final ImmutableSet.Builder filteredResources = ImmutableSet.builder(); - private final FilterBehavior filterBehavior; + private final boolean filterInAnalysis; /** * Constructor. * * @param configFilters the resource configuration filters, as a list of strings. * @param densities the density filters, as a list of strings. - * @param filterBehavior the behavior of this filter. + * @param filterInAnalysis whether this filter should filter resources in analysis */ ResourceFilterFactory( ImmutableList configFilters, ImmutableList densities, - FilterBehavior filterBehavior) { + boolean filterInAnalysis) { this.configFilters = configFilters; this.densities = densities; - this.filterBehavior = filterBehavior; + this.filterInAnalysis = filterInAnalysis; } private static boolean hasAttr(AttributeMap attrs, String attrName) { @@ -147,10 +93,6 @@ public class ResourceFilterFactory { return values != null && !values.isEmpty(); } - static boolean hasFilters(AttributeMap attrs) { - return hasAttr(attrs, RESOURCE_CONFIGURATION_FILTERS_NAME) || hasAttr(attrs, DENSITIES_NAME); - } - /** * Extracts filters from an AttributeMap, as a list of strings. * @@ -162,7 +104,7 @@ public class ResourceFilterFactory { */ private static ImmutableList extractFilters(AttributeMap attrs, String attrName) { if (!hasAttr(attrs, attrName)) { - return ImmutableList.of(); + return ImmutableList.of(); } /* @@ -204,47 +146,23 @@ public class ResourceFilterFactory { return empty(); } - ResourceFilterFactory base; - if (AndroidAaptVersion.chooseTargetAaptVersion(ruleContext) == AndroidAaptVersion.AAPT2) { - // aapt2 must have access to all of the resources in execution, so don't filter in analysis. - base = empty(FilterBehavior.FILTER_IN_EXECUTION); - } else { - base = ruleContext.getFragment(AndroidConfiguration.class).getResourceFilterFactory(); - } + // aapt2 must have access to all of the resources in execution, so don't filter in analysis. + boolean filterInAnalysis = + AndroidAaptVersion.chooseTargetAaptVersion(ruleContext) != AndroidAaptVersion.AAPT2; - return forBaseAndAttrs(base, ruleContext.attributes()); + return from(filterInAnalysis, ruleContext.attributes()); } @VisibleForTesting - static ResourceFilterFactory forBaseAndAttrs(ResourceFilterFactory base, AttributeMap attrs) { - return base.withAttrsFrom(attrs); - } - - /** - * Creates a new {@link ResourceFilterFactory} based on this object's properties, overridden by - * any filters specified in the passed {@link AttributeMap}. - * - *

A new object will always be returned, as returning the same object across multiple rules (as - * would be done with {@link FilterBehavior#FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION}) causes - * problems. - */ - ResourceFilterFactory withAttrsFrom(AttributeMap attrs) { - if (!hasFilters(attrs)) { - return this; + static ResourceFilterFactory from(boolean filterInAnalysis, AttributeMap attrs) { + if (!hasAttr(attrs, RESOURCE_CONFIGURATION_FILTERS_NAME) && !hasAttr(attrs, DENSITIES_NAME)) { + return empty(); } return new ResourceFilterFactory( extractFilters(attrs, RESOURCE_CONFIGURATION_FILTERS_NAME), extractFilters(attrs, DENSITIES_NAME), - filterBehavior); - } - - ResourceFilterFactory withoutDynamicConfiguration() { - if (!usesDynamicConfiguration()) { - return this; - } - - return empty(FilterBehavior.FILTER_IN_ANALYSIS); + filterInAnalysis); } private ImmutableList getConfigurationFilters( @@ -326,12 +244,12 @@ public class ResourceFilterFactory { /** * List of deprecated qualifiers that are not supported by {@link FolderConfiguration}. * - * For resources, we should warn if these qualifiers are encountered, since aapt supports the + *

For resources, we should warn if these qualifiers are encountered, since aapt supports the * fixed version (and aapt2 only supports that version). * - * For resource filters, however, aapt only supports this old version. Convert the qualifiers so - * that they can be parsed by FolderConfiguration, but do not warn (since they are, actually, what - * aapt expects) and save the original qualifier strings to be passed to aapt. + *

For resource filters, however, aapt only supports this old version. Convert the qualifiers + * so that they can be parsed by FolderConfiguration, but do not warn (since they are, actually, + * what aapt expects) and save the original qualifier strings to be passed to aapt. */ private final List deprecatedQualifierHandlers = ImmutableList.of( @@ -383,14 +301,10 @@ public class ResourceFilterFactory { } } - static ResourceFilterFactory empty() { - return empty(DEFAULT_BEHAVIOR); - } - @VisibleForTesting - static ResourceFilterFactory empty(FilterBehavior filterBehavior) { + static ResourceFilterFactory empty() { return new ResourceFilterFactory( - ImmutableList.of(), ImmutableList.of(), filterBehavior); + ImmutableList.of(), ImmutableList.of(), /* filterInAnalysis = */ false); } /** @@ -402,8 +316,7 @@ public class ResourceFilterFactory { ResourceFilter getResourceFilter( RuleErrorConsumer ruleErrorConsumer, ResourceDependencies resourceDeps, - LocalResourceContainer localResources) - throws RuleErrorException { + LocalResourceContainer localResources) { if (!isPrefiltering()) { return ResourceFilter.empty(); } @@ -470,7 +383,7 @@ public class ResourceFilterFactory { private final Multimap nameAndConfigurationToBestArtifacts = HashMultimap.create(); - public BestArtifactsForDensity(RuleErrorConsumer ruleErrorConsumer, Density density) { + BestArtifactsForDensity(RuleErrorConsumer ruleErrorConsumer, Density density) { this.ruleErrorConsumer = ruleErrorConsumer; desiredDensity = density; } @@ -479,7 +392,7 @@ public class ResourceFilterFactory { * @param artifact if this artifact is a better match for this object's desired density than any * other artifacts with the same name and non-density configuration, adds it to this object. */ - public void maybeAddArtifact(Artifact artifact) { + void maybeAddArtifact(Artifact artifact) { FolderConfiguration config = getConfigForArtifact(ruleErrorConsumer, artifact); // We want to find a single best artifact for each combination of non-density qualifiers and @@ -655,10 +568,6 @@ public class ResourceFilterFactory { return Joiner.on(',').join(configFilters); } - ImmutableList getConfigFilters() { - return configFilters; - } - /** * Returns if this object contains a non-empty density filter. * @@ -678,17 +587,13 @@ public class ResourceFilterFactory { } boolean isPrefiltering() { - return hasFilters() && filterBehavior != FilterBehavior.FILTER_IN_EXECUTION; + return filterInAnalysis; } boolean hasFilters() { return hasConfigurationFilters() || hasDensities(); } - FilterBehavior getFilterBehavior() { - return filterBehavior; - } - public String getOutputDirectorySuffix() { if (!hasFilters()) { return null; @@ -697,10 +602,6 @@ public class ResourceFilterFactory { return getConfigurationFilterString() + "_" + getDensityString(); } - boolean usesDynamicConfiguration() { - return filterBehavior == FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION; - } - /** * Gets a list of resource names that should be ignored by resource processing if they don't * exist. @@ -730,7 +631,7 @@ public class ResourceFilterFactory { ResourceFilterFactory other = (ResourceFilterFactory) object; - return filterBehavior == other.filterBehavior + return filterInAnalysis == other.filterInAnalysis && configFilters.equals(other.configFilters) && densities.equals(other.densities) && filteredResources.build().equals(other.filteredResources.build()); @@ -738,102 +639,6 @@ public class ResourceFilterFactory { @Override public int hashCode() { - return Objects.hashCode(filterBehavior, configFilters, densities, filteredResources.build()); - } - - /** - * Converts command line settings for the filter behavior into an empty {@link - * ResourceFilterFactory} object. - */ - public static final class Converter - implements com.google.devtools.common.options.Converter { - private final FilterBehavior.Converter filterEnumConverter = new FilterBehavior.Converter(); - - @Override - public ResourceFilterFactory convert(String input) throws OptionsParsingException { - - return empty(filterEnumConverter.convert(input)); - } - - @Override - public String getTypeDescription() { - return filterEnumConverter.getTypeDescription(); - } - } - - // Transitions for dealing with dynamically configured resource filtering: - - @Nullable - PatchTransition getTopLevelPatchTransition(String ruleClass, AttributeMap attrs) { - if (!usesDynamicConfiguration()) { - // We're not using dynamic configuration, so we don't need to make a transition - return null; - } - - if (!ruleClass.equals("android_binary") || !hasFilters(attrs)) { - // This target doesn't specify any filtering settings, so dynamically configured resource - // filtering would be a waste of time. - // If the target's dependencies include android_binary targets, those dependencies might - // specify filtering settings, but we don't apply them dynamically since the chances of - // encountering differing settings (leading to splitting the build graph and poor overall - // performance) are high. - return REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION; - } - - // Continue using dynamically configured resource filtering, and propagate this target's - // filtering settings. - return new AddDynamicallyConfiguredResourceFilteringTransition(attrs); - } - - @AutoCodec - public static final PatchTransition REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION = - new RemoveDynamicallyConfiguredResourceFilteringTransition(); - - private static final class RemoveDynamicallyConfiguredResourceFilteringTransition - extends BaseDynamicallyConfiguredResourceFilteringTransition { - @Override - ResourceFilterFactory getNewResourceFilter(ResourceFilterFactory oldResourceFilterFactory) { - return oldResourceFilterFactory.withoutDynamicConfiguration(); - } - } - - // There is no codec for this class because AttributeMap can contain extremely heavyweight - // objects like Package. - @VisibleForTesting - static final class AddDynamicallyConfiguredResourceFilteringTransition - extends BaseDynamicallyConfiguredResourceFilteringTransition { - private final AttributeMap attrs; - - AddDynamicallyConfiguredResourceFilteringTransition(AttributeMap attrs) { - this.attrs = attrs; - } - - @Override - ResourceFilterFactory getNewResourceFilter(ResourceFilterFactory oldResourceFilterFactory) { - return oldResourceFilterFactory.withAttrsFrom(attrs); - } - - @VisibleForTesting - AttributeMap getAttrs() { - return attrs; - } - } - - private abstract static class BaseDynamicallyConfiguredResourceFilteringTransition - implements PatchTransition { - @Override - public BuildOptions apply(BuildOptions options) { - BuildOptions newOptions = options.clone(); - - AndroidConfiguration.Options androidOptions = - newOptions.get(AndroidConfiguration.Options.class); - androidOptions.resourceFilterFactory = - getNewResourceFilter(androidOptions.resourceFilterFactory); - - return newOptions; - } - - abstract ResourceFilterFactory getNewResourceFilter( - ResourceFilterFactory oldResourceFilterFactory); + return Objects.hashCode(filterInAnalysis, configFilters, densities, filteredResources.build()); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index 5c292fb7fe..d25b28db01 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -29,7 +29,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; -import com.google.common.eventbus.EventBus; import com.google.common.truth.Truth; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; @@ -1227,8 +1226,6 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { public void testFilteredResourcesInvalidResourceDir() throws Exception { String badQualifierDir = "values-invalid-qualifier"; - useConfiguration("--experimental_android_resource_filtering_method", "filter_in_execution"); - checkError( "java/r/android", "r", @@ -1240,37 +1237,6 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " resource_configuration_filters = ['en'])"); } - @Test - public void testFilteredResourcesFilteringDisabled() throws Exception { - List resources = - ImmutableList.of("res/values/foo.xml", "res/values-en/foo.xml", "res/values-fr/foo.xml"); - String dir = "java/r/android"; - - useConfiguration("--experimental_android_resource_filtering_method", "filter_in_execution"); - - ConfiguredTarget binary = - scratchConfiguredTarget( - dir, - "r", - "android_binary(name = 'r',", - " manifest = 'AndroidManifest.xml',", - " resource_configuration_filters = ['', 'en, es, '],", - " densities = ['hdpi, , ', 'xhdpi'],", - " resource_files = ['" + Joiner.on("', '").join(resources) + "'])"); - ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false); - - // Validate that the AndroidResourceProvider for this binary contains all values. - assertThat(resourceContentsPaths(dir, directResources)).containsExactlyElementsIn(resources); - - // Validate that the input to resource processing contains all values. - assertThat(resourceInputPaths(dir, directResources)).containsAllIn(resources); - - // Validate that the filters are correctly passed to the resource processing action - // This includes trimming whitespace and ignoring empty filters. - assertThat(resourceArguments(directResources)).contains("en,es"); - assertThat(resourceArguments(directResources)).contains("hdpi,xhdpi"); - } - /** Test that resources are not filtered in analysis under aapt2. */ @Test public void testFilteredResourcesFilteringAapt2() throws Exception { @@ -1855,65 +1821,6 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { assertThat(resourceProcessingArgs).contains("--throwOnResourceConflict"); } - @Test - public void testFilteredTransitiveResourcesDynamicConfiguration() throws Exception { - String enResource = "res/values-en/foo.xml"; - String unqualifiedResource = "res/values/foo.xml"; - String frResource = "res/values-fr/foo.xml"; - - String dir = "java/r/android"; - scratch.file( - dir + "/BUILD", - "android_library(name = 'lib',", - " manifest = 'AndroidManifest.xml',", - " resource_files = [", - " '" + enResource + "',", - " '" + frResource + "',", - " '" + unqualifiedResource + "',", - "])", - "android_binary(name = 'en',", - " manifest = 'AndroidManifest.xml',", - " resource_configuration_filters = ['en'],", - " deps = [':lib'])"); - - useConfiguration( - "--experimental_android_resource_filtering_method", - "filter_in_analysis_with_dynamic_configuration"); - - ConfiguredTarget binary = - Iterables.getOnlyElement( - update( - ImmutableList.of("//" + dir + ":en"), - /* keepGoing= */ false, - /* loadingPhaseThreads= */ 1, - /* doAnalysis= */ true, - new EventBus()) - .getTargetsToBuild()); - - // Assert the resources were still filtered in analysis in the binary. - String expectedQualifiedResource = - binary.getLabel().toString().endsWith("en") ? enResource : frResource; - - assertThat(resourceContentsPaths(dir, getResourceContainer(binary, /* transitive=*/ true))) - .containsExactly(expectedQualifiedResource, unqualifiedResource); - - ConfiguredTarget library = getDirectPrerequisite(binary, "//" + dir + ":lib"); - - // Assert the resources were filtered in the library. - // This is only possible if the filters are correctly being passed using dynamic - // configuration. - assertThat(resourceContentsPaths(dir, getResourceContainer(library))) - .containsExactly(expectedQualifiedResource, unqualifiedResource); - - // assert the correct prefix is used for library outputs - assertThat( - library - .getConfiguration() - .getFragment(AndroidConfiguration.class) - .getOutputDirectoryName()) - .contains("en_"); - } - /** * Gets the paths of matching artifacts contained within a resource container * diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java index 7564c5d376..e80c837259 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java @@ -19,16 +19,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.truth.BooleanSubject; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.rules.android.ResourceFilterFactory.AddDynamicallyConfiguredResourceFilteringTransition; -import com.google.devtools.build.lib.rules.android.ResourceFilterFactory.FilterBehavior; import com.google.devtools.build.lib.testutil.FakeAttributeMapper; import java.util.ArrayList; import java.util.List; @@ -78,14 +74,14 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testNoopFilter( "en", "hdpi", - FilterBehavior.FILTER_IN_EXECUTION, + /* filterInAnalysis = */ false, ImmutableList.of( "values-en/foo.xml", "values/foo.xml", "values-hdpi/foo.png", "values-ldpi/foo.png")); } @Test public void testFilterEmpty() throws Exception { - testNoopFilter("", "", FilterBehavior.FILTER_IN_ANALYSIS, ImmutableList.of()); + testNoopFilter("", "", /* filterInAnalysis = */ true, ImmutableList.of()); } @Test @@ -93,7 +89,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testNoopFilter( "en", "xhdpi,xxhdpi", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("drawable/ic_clear.xml", "drawable-v21/ic_clear.xml")); } @@ -105,7 +101,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testNoopFilter( "v4", "hdpi", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("drawable-hdpi-v4/foo.png", "drawable-hdpi-v11/foo.png")); } @@ -114,7 +110,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "", "hdpi,ldpi", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, // If we add resources to the output list in density order, these resources will be // rearranged. ImmutableList.of( @@ -133,7 +129,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "en_US", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"), ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml")); @@ -146,7 +142,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "en_US-ldrtl", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"), ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml")); @@ -159,7 +155,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "mcc111-en_US", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"), ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml")); @@ -172,7 +168,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "mcc111-mnc111-en_US", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("values-en/foo.xml", "values-en-rUS/foo.xml"), ImmutableList.of("values-fr/foo.xml", "values-en-rCA/foo.xml")); @@ -185,7 +181,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "sr-Latn,sr-rLatn,sr_Latn,b+sr+Latn", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of( "values-sr/foo.xml", "values-b+sr+Latn/foo.xml", @@ -203,7 +199,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testNoopFilter( "sr", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of( "values-sr/foo.xml", "values-b+sr+Latn/foo.xml", @@ -220,7 +216,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testFilter( "es-419,es_419,b+es+419", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of("values-es/foo.xml", "values-b+es+419/foo.xml", "values-es-419/foo.xml"), // Spanish with another region specified should be filtered out. ImmutableList.of("values-es-rUS/foo.xml")); @@ -234,7 +230,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testNoopFilter( "es", "", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of( "values-es/foo.xml", "values-b+es+419/foo.xml", @@ -267,7 +263,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { ResourceFilterFactory filter = new ResourceFilterFactory( - ImmutableList.of("en"), ImmutableList.of(), FilterBehavior.FILTER_IN_ANALYSIS); + ImmutableList.of("en"), ImmutableList.of(), /* filterInAnalysis = */ true); doFilter(filter, badResources); assertThat(errorConsumer.getAndClearRuleWarnings()).containsExactlyElementsIn(expectedWarnings); @@ -284,7 +280,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { // target filter = new ResourceFilterFactory( - ImmutableList.of("en"), ImmutableList.of(), FilterBehavior.FILTER_IN_ANALYSIS); + ImmutableList.of("en"), ImmutableList.of(), /* filterInAnalysis = */ true); doFilter(filter, badResources); assertThat(errorConsumer.getAndClearRuleWarnings()).containsExactlyElementsIn(expectedWarnings); errorConsumer.assertNoAttributeWarnings( @@ -296,32 +292,22 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { testNoopFilter( "en", "hdpi", - FilterBehavior.FILTER_IN_ANALYSIS, + /* filterInAnalysis = */ true, ImmutableList.of( "first-subdir/res/drawable-en-hdpi/foo.png", "second-subdir/res/drawable-en-hdpi/foo.png")); } - @Test - public void testFilterWithDynamicConfiguration() throws Exception { - testFilter( - "en", - "hdpi", - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION, - ImmutableList.of("drawable-en-hdpi/foo.png"), - ImmutableList.of("drawable-en-ldpi/foo.png", "drawable-fr-hdpi/foo.png")); - } - private void testNoopFilter( String resourceConfigurationFilters, String densities, - FilterBehavior filterBehavior, + boolean filterInAnalysis, List resources) throws Exception { testFilter( resourceConfigurationFilters, densities, - filterBehavior, + filterInAnalysis, resources, ImmutableList.of()); } @@ -329,7 +315,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { private void testFilter( String resourceConfigurationFilters, String densities, - FilterBehavior filterBehavior, + boolean filterInAnalysis, List resourcesToKeep, List resourcesToDiscard) throws Exception { @@ -346,7 +332,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { ImmutableList allArtifacts = ImmutableList.copyOf(Iterables.concat(expectedResources, unexpectedResources)); ResourceFilterFactory resourceFilterFactory = - makeResourceFilter(resourceConfigurationFilters, densities, filterBehavior); + makeResourceFilter(resourceConfigurationFilters, densities, filterInAnalysis); ImmutableList filtered = doFilter(resourceFilterFactory, allArtifacts); assertThat(filtered).containsExactlyElementsIn(expectedResources).inOrder(); @@ -392,7 +378,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { .build()); ResourceFilterFactory resourceFilterFactory = - makeResourceFilter("en", "hdpi", FilterBehavior.FILTER_IN_ANALYSIS); + makeResourceFilter("en", "hdpi", /* filterInAnalysis = */ true); ResourceFilter filter = resourceFilterFactory.getResourceFilter( errorConsumer, resourceDependencies, localResources); @@ -447,44 +433,37 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { @Test public void testIsPrefilteringFilterInExecution() throws Exception { - assertIsPrefiltering(FilterBehavior.FILTER_IN_EXECUTION, false); + assertIsPrefiltering(/* filterInAnalysis = */ false, false); } @Test public void testIsPrefilteringFilterInAnalysis() throws Exception { - assertIsPrefiltering(FilterBehavior.FILTER_IN_ANALYSIS, true); + assertIsPrefiltering(/* filterInAnalysis = */ true, true); } - @Test - public void testIsPrefilteringFilterInAnalysisWithDynamicConfiguration() throws Exception { - assertIsPrefiltering(FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION, true); - } - - private void assertIsPrefiltering(FilterBehavior behavior, boolean expectWhenNonEmpty) + private void assertIsPrefiltering(boolean filterInAnalysis, boolean expectWhenNonEmpty) throws Exception { // Empty filters should never prefilter - assertIsPrefiltering(false, false, behavior).isFalse(); + assertIsPrefiltering(false, false, filterInAnalysis).isFalse(); // Prefiltering behavior should be the same regardless of which setting is set - assertIsPrefiltering(true, false, behavior).isEqualTo(expectWhenNonEmpty); - assertIsPrefiltering(false, true, behavior).isEqualTo(expectWhenNonEmpty); - assertIsPrefiltering(true, true, behavior).isEqualTo(expectWhenNonEmpty); + assertIsPrefiltering(true, false, filterInAnalysis).isEqualTo(expectWhenNonEmpty); + assertIsPrefiltering(false, true, filterInAnalysis).isEqualTo(expectWhenNonEmpty); + assertIsPrefiltering(true, true, filterInAnalysis).isEqualTo(expectWhenNonEmpty); } private BooleanSubject assertIsPrefiltering( - boolean hasConfigurationFilters, boolean hasDensities, FilterBehavior behavior) + boolean hasConfigurationFilters, boolean hasDensities, boolean filterInAnalysis) throws Exception { return assertThat( makeResourceFilter( - hasConfigurationFilters ? "en" : "", hasDensities ? "hdpi" : "", behavior) + hasConfigurationFilters ? "en" : "", hasDensities ? "hdpi" : "", filterInAnalysis) .isPrefiltering()); } @Test public void testGetOutputDirectorySuffixEmpty() throws Exception { - assertThat( - makeResourceFilter("", "", FilterBehavior.FILTER_IN_ANALYSIS) - .getOutputDirectorySuffix()) + assertThat(makeResourceFilter("", "", /* filterInAnalysis = */ true).getOutputDirectorySuffix()) .isNull(); } @@ -492,7 +471,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { public void testGetOutputDirectoryOnlyFilterConfigurations() throws Exception { String configurationFilters = "en,es-rUS,fr"; assertThat( - makeResourceFilter(configurationFilters, "", FilterBehavior.FILTER_IN_ANALYSIS) + makeResourceFilter(configurationFilters, "", /* filterInAnalysis = */ true) .getOutputDirectorySuffix()) .isEqualTo(configurationFilters + "_"); } @@ -501,7 +480,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { public void testGetOutputDirectoryOnlyDensities() throws Exception { String densities = "hdpi,ldpi,xhdpi"; assertThat( - makeResourceFilter("", densities, FilterBehavior.FILTER_IN_ANALYSIS) + makeResourceFilter("", densities, /* filterInAnalysis = */ true) .getOutputDirectorySuffix()) .isEqualTo("_" + densities); } @@ -514,9 +493,9 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { public void testGetOutputDirectoryDensitiesAreDifferentFromDensityConfigurationFilters() throws Exception { ResourceFilterFactory configurationFilter = - makeResourceFilter("hdpi", "", FilterBehavior.FILTER_IN_ANALYSIS); + makeResourceFilter("hdpi", "", /* filterInAnalysis = */ true); ResourceFilterFactory densityFilter = - makeResourceFilter("", "hdpi", FilterBehavior.FILTER_IN_ANALYSIS); + makeResourceFilter("", "hdpi", /* filterInAnalysis = */ true); assertThat(configurationFilter.getOutputDirectorySuffix()) .isNotEqualTo(densityFilter.getOutputDirectorySuffix()); @@ -525,7 +504,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { @Test public void testGetOutputDirectory() throws Exception { assertThat( - makeResourceFilter("en,fr-rCA", "hdpi,ldpi", FilterBehavior.FILTER_IN_ANALYSIS) + makeResourceFilter("en,fr-rCA", "hdpi,ldpi", /* filterInAnalysis = */ true) .getOutputDirectorySuffix()) .isEqualTo("en,fr-rCA_hdpi,ldpi"); } @@ -537,38 +516,38 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { @Test public void testGetOutputDirectoryDifferentlyOrdered() throws Exception { ResourceFilterFactory first = - makeResourceFilter("en,fr", "hdpi,ldpi", FilterBehavior.FILTER_IN_ANALYSIS); + makeResourceFilter("en,fr", "hdpi,ldpi", /* filterInAnalysis = */ true); ResourceFilterFactory second = - makeResourceFilter("fr,en", "ldpi,hdpi", FilterBehavior.FILTER_IN_ANALYSIS); + makeResourceFilter("fr,en", "ldpi,hdpi", /* filterInAnalysis = */ true); assertThat(first.getOutputDirectorySuffix()).isEqualTo(second.getOutputDirectorySuffix()); } @Test public void testGetOutputDirectoryDuplicated() throws Exception { ResourceFilterFactory duplicated = - makeResourceFilter("en,en", "hdpi,hdpi", FilterBehavior.FILTER_IN_ANALYSIS); - ResourceFilterFactory normal = - makeResourceFilter("en", "hdpi", FilterBehavior.FILTER_IN_ANALYSIS); + makeResourceFilter("en,en", "hdpi,hdpi", /* filterInAnalysis = */ true); + ResourceFilterFactory normal = makeResourceFilter("en", "hdpi", /* filterInAnalysis = */ true); assertThat(duplicated.getOutputDirectorySuffix()).isEqualTo(normal.getOutputDirectorySuffix()); } private ResourceFilterFactory makeResourceFilter( - String resourceConfigurationFilters, String densities, FilterBehavior behavior) - throws Exception { + String resourceConfigurationFilters, String densities, boolean filterInAnalysis) { return makeResourceFilter( - ImmutableList.of(resourceConfigurationFilters), ImmutableList.of(densities), behavior); + resourceConfigurationFilters.isEmpty() + ? ImmutableList.of() + : ImmutableList.of(resourceConfigurationFilters), + densities.isEmpty() ? ImmutableList.of() : ImmutableList.of(densities), + filterInAnalysis); } private ResourceFilterFactory makeResourceFilter( ImmutableList resourceConfigurationFilters, ImmutableList densities, - FilterBehavior behavior) - throws Exception { + boolean filterInAnalysis) { - return ResourceFilterFactory.forBaseAndAttrs( - ResourceFilterFactory.empty(behavior), - getAttributeMap(resourceConfigurationFilters, densities)); + return ResourceFilterFactory.from( + filterInAnalysis, getAttributeMap(resourceConfigurationFilters, densities)); } private AttributeMap getAttributeMap( @@ -600,150 +579,8 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { @Test public void testWithAttrsFromAttrsNotSpecified() throws Exception { assertThat( - ResourceFilterFactory.forBaseAndAttrs( - ResourceFilterFactory.empty(FilterBehavior.FILTER_IN_ANALYSIS), - FakeAttributeMapper.empty()) + ResourceFilterFactory.from(/* filterInAnalysis = */ true, FakeAttributeMapper.empty()) .hasFilters()) .isFalse(); } - - @Test - public void testGetTopLevelTransitionFilterInExecution() throws Exception { - assertThat( - getTopLevelTransition( - ImmutableList.of("en"), - ImmutableList.of("hdpi"), - FilterBehavior.FILTER_IN_EXECUTION, - true)) - .isNull(); - } - - @Test - public void testGetTopLevelTransitionFilterInAnalysis() throws Exception { - assertThat( - getTopLevelTransition( - ImmutableList.of("en"), - ImmutableList.of("hdpi"), - FilterBehavior.FILTER_IN_ANALYSIS, - true)) - .isNull(); - } - - @Test - public void testGetTopLevelTransitionNotBinary() throws Exception { - assertThat( - getTopLevelTransition( - ImmutableList.of("en"), - ImmutableList.of("hdpi"), - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION, - false)) - .isSameAs( - ResourceFilterFactory.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION); - } - - @Test - public void testGetTopLevelTransitionNoFilters() throws Exception { - assertThat( - getTopLevelTransition( - ImmutableList.of(), - ImmutableList.of(), - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION, - true)) - .isSameAs( - ResourceFilterFactory.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION); - } - - @Test - public void testGetTopLevelTransition() throws Exception { - ImmutableList resourceConfigurationFilters = ImmutableList.of("en"); - ImmutableList densities = ImmutableList.of("hdpi"); - PatchTransition transition = - getTopLevelTransition( - resourceConfigurationFilters, - densities, - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION, - true); - - assertThat(transition).isInstanceOf(AddDynamicallyConfiguredResourceFilteringTransition.class); - - AddDynamicallyConfiguredResourceFilteringTransition addTransition = - (AddDynamicallyConfiguredResourceFilteringTransition) transition; - ResourceFilterFactory foundFilter = - ResourceFilterFactory.forBaseAndAttrs( - ResourceFilterFactory.empty( - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION), - addTransition.getAttrs()); - - ResourceFilterFactory expectedFilter = - makeResourceFilter( - resourceConfigurationFilters, - densities, - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION); - assertThat(foundFilter).isEqualTo(expectedFilter); - } - - private PatchTransition getTopLevelTransition( - ImmutableList resourceConfigurationFilters, - ImmutableList densities, - FilterBehavior behavior, - boolean isBinary) - throws Exception { - AttributeMap attrs = getAttributeMap(resourceConfigurationFilters, densities); - return makeResourceFilter("", "", behavior) - .getTopLevelPatchTransition(isBinary ? "android_binary" : "android_library", attrs); - } - - @Test - public void testRemoveDynamicConfigurationTransition() throws Exception { - assertPatchTransition( - makeResourceFilter( - "en", "ldpi", FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION), - ResourceFilterFactory.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION, - ResourceFilterFactory.empty(FilterBehavior.FILTER_IN_ANALYSIS)); - } - - @Test - public void testAddDynamicConfigurationTransitionDynamicConfiguration() throws Exception { - ImmutableList resourceConfigurationFilters = ImmutableList.of("en", "es-rUS", "fr"); - ImmutableList densities = ImmutableList.of("ldpi", "hdpi"); - - AttributeMap attrs = getAttributeMap(resourceConfigurationFilters, densities); - - assertPatchTransition( - ResourceFilterFactory.empty(FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION), - new ResourceFilterFactory.AddDynamicallyConfiguredResourceFilteringTransition(attrs), - makeResourceFilter( - resourceConfigurationFilters, - densities, - FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION)); - } - - private void assertPatchTransition( - ResourceFilterFactory oldResourceFilterFactory, - PatchTransition transition, - ResourceFilterFactory expectedNewResourceFilterFactory) { - AndroidConfiguration.Options oldAndroidOptions = getAndroidOptions(oldResourceFilterFactory); - - BuildOptions oldOptions = BuildOptions.builder().add(oldAndroidOptions).build(); - BuildOptions newOptions = transition.apply(oldOptions); - - // The old options should not have been changed - assertThat(oldAndroidOptions.resourceFilterFactory).isSameAs(oldResourceFilterFactory); - assertThat(oldAndroidOptions).isEqualTo(getAndroidOptions(oldResourceFilterFactory)); - - // Besides the ResourceFilterFactory, the new options should be the same as the old ones - assertThat(newOptions.getOptions()).hasSize(1); - AndroidConfiguration.Options newAndroidOptions = - newOptions.get(AndroidConfiguration.Options.class); - assertThat(newAndroidOptions).isEqualTo(getAndroidOptions(expectedNewResourceFilterFactory)); - } - - private AndroidConfiguration.Options getAndroidOptions( - ResourceFilterFactory resourceFilterFactory) { - AndroidConfiguration.Options androidOptions = - (AndroidConfiguration.Options) new AndroidConfiguration.Options().getDefault(); - androidOptions.resourceFilterFactory = resourceFilterFactory; - - return androidOptions; - } } -- cgit v1.2.3