From f2b66aa31b0a3addb2f3712c6232f4c8c65b55db Mon Sep 17 00:00:00 2001 From: asteinb Date: Thu, 29 Mar 2018 10:02:27 -0700 Subject: Rename LocalResourceContainer to AndroidResources and remove asset code from it As part of decoupling Android resources and assets, rename LocalResourceContainer to AndroidResources and remove asset code from it. Some general asset and manfiest code still remains and will be dealt with in future changes. Remove LocalResourceContainer from the ParsingActionBuilder, since it's always used to build the ResourceContainer that is subsequently passed in. RELNOTES: none PiperOrigin-RevId: 190945260 --- .../build/lib/rules/android/AarImport.java | 10 +- .../build/lib/rules/android/AndroidAssets.java | 9 +- .../build/lib/rules/android/AndroidBinary.java | 2 +- .../build/lib/rules/android/AndroidCommon.java | 4 +- .../build/lib/rules/android/AndroidLibrary.java | 4 +- .../lib/rules/android/AndroidLocalTestBase.java | 4 +- .../AndroidResourceParsingActionBuilder.java | 49 +-- .../build/lib/rules/android/AndroidResources.java | 388 ++++++++++++++++++++ .../lib/rules/android/AndroidRuleClasses.java | 2 +- .../lib/rules/android/ApplicationManifest.java | 55 ++- .../build/lib/rules/android/DataBinding.java | 6 +- .../lib/rules/android/LocalResourceContainer.java | 395 --------------------- .../build/lib/rules/android/ResourceContainer.java | 91 +---- .../lib/rules/android/ResourceDependencies.java | 4 +- .../lib/rules/android/ResourceFilterFactory.java | 2 +- .../build/lib/rules/android/AarImportTest.java | 12 +- .../build/lib/rules/android/AndroidBinaryTest.java | 4 +- .../lib/rules/android/AndroidLibraryTest.java | 4 +- .../lib/rules/android/AndroidResourcesTest.java | 171 +++++++++ .../google/devtools/build/lib/rules/android/BUILD | 4 +- .../rules/android/LocalResourceContainerTest.java | 147 -------- .../rules/android/ResourceFilterFactoryTest.java | 39 +- 22 files changed, 678 insertions(+), 728 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java create mode 100644 src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java delete mode 100644 src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java index d75adb4d95..74264b7f88 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; @@ -83,8 +84,8 @@ public class AarImport implements RuleConfiguredTargetFactory { // AndroidManifest.xml is required in every AAR. Artifact androidManifestArtifact = createAarArtifact(ruleContext, ANDROID_MANIFEST); - Artifact resources = createAarTreeArtifact(ruleContext, "resources"); - Artifact assets = createAarTreeArtifact(ruleContext, "assets"); + SpecialArtifact resources = createAarTreeArtifact(ruleContext, "resources"); + SpecialArtifact assets = createAarTreeArtifact(ruleContext, "assets"); ruleContext.registerAction( createAarResourcesExtractorActions(ruleContext, aar, resources, assets)); @@ -97,7 +98,8 @@ public class AarImport implements RuleConfiguredTargetFactory { ResourceApk resourceApk = androidManifest.packAarWithDataAndResources( ruleContext, - LocalResourceContainer.forAssetsAndResourcesDirectories(assets, resources), + AndroidAssets.forAarImport(assets), + AndroidResources.forAarImport(resources), ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)), ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT), ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LOCAL_SYMBOLS), @@ -342,7 +344,7 @@ public class AarImport implements RuleConfiguredTargetFactory { "_aar", name, ruleContext.getBinOrGenfilesDirectory()); } - private static Artifact createAarTreeArtifact(RuleContext ruleContext, String name) { + private static SpecialArtifact createAarTreeArtifact(RuleContext ruleContext, String name) { PathFragment rootRelativePath = ruleContext.getUniqueDirectory("_aar/unzipped/" + name); return ruleContext.getTreeArtifact(rootRelativePath, ruleContext.getBinOrGenfilesDirectory()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java index 10c82988a6..ffed91b2ae 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java @@ -77,10 +77,7 @@ public final class AndroidAssets { return new AndroidAssets(assets.build(), assetRoots.build()); } - static PathFragment getAssetDir(RuleContext ruleContext) { - if (!ruleContext.attributes().has(ASSETS_DIR_ATTR)) { - return PathFragment.EMPTY_FRAGMENT; - } + private static PathFragment getAssetDir(RuleContext ruleContext) { return PathFragment.create(ruleContext.attributes().get(ASSETS_DIR_ATTR, Type.STRING)); } @@ -99,6 +96,10 @@ public final class AndroidAssets { ImmutableList.of(assetsDir), ImmutableList.of(assetsDir.getExecPath().getChild("assets"))); } + static AndroidAssets empty() { + return new AndroidAssets(ImmutableList.of(), ImmutableList.of()); + } + private final ImmutableList assets; private final ImmutableList assetRoots; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 2d1c5e9e5e..4c173780f5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -184,7 +184,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { boolean shrinkResources = shouldShrinkResources(ruleContext); // Retrieve and compile the resources defined on the android_binary rule. - LocalResourceContainer.validateRuleContext(ruleContext); + AndroidResources.validateRuleContext(ruleContext); ApplicationManifest applicationManifest = androidSemantics.getManifestForRule(ruleContext).mergeWith(ruleContext, resourceDeps); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index 5834c50b0f..3bb8df4839 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -266,7 +266,7 @@ public class AndroidCommon { } // If the rule defines resources, put those in the IDE info. - if (LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) { + if (AndroidResources.definesAndroidResources(ruleContext.attributes())) { ideInfoProviderBuilder .setDefinesAndroidResources(true) // Sets the possibly merged manifest and the raw manifest. @@ -299,7 +299,7 @@ public class AndroidCommon { } static PathFragment getSourceDirectoryRelativePathFromResource(Artifact resource) { - PathFragment resourceDir = LocalResourceContainer.findResourceDir(resource); + PathFragment resourceDir = AndroidResources.findResourceDir(resource); if (resourceDir == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java index b5ce0c123a..6aea0db21b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java @@ -131,9 +131,9 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { AndroidCommon androidCommon = new AndroidCommon(javaCommon); boolean definesLocalResources = - LocalResourceContainer.definesAndroidResources(ruleContext.attributes()); + AndroidResources.definesAndroidResources(ruleContext.attributes()); if (definesLocalResources) { - LocalResourceContainer.validateRuleContext(ruleContext); + AndroidResources.validateRuleContext(ruleContext); } // TODO(b/69668042): Always correctly apply neverlinking for resources diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java index 21cee094e2..dca5a7f142 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java @@ -395,8 +395,8 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor throws InterruptedException, RuleErrorException { ApplicationManifest applicationManifest; - if (LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) { - LocalResourceContainer.validateRuleContext(ruleContext); + if (AndroidResources.definesAndroidResources(ruleContext.attributes())) { + AndroidResources.validateRuleContext(ruleContext); ApplicationManifest ruleManifest = androidSemantics.getManifestForRule(ruleContext); applicationManifest = ruleManifest.mergeWith(ruleContext, resourceDependencies, false); } else { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java index a3f3834017..67076096b7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java @@ -47,10 +47,9 @@ public class AndroidResourceParsingActionBuilder { private final RuleContext ruleContext; private final AndroidSdkProvider sdk; - private LocalResourceContainer primary; + private ResourceContainer primary; private Artifact output; - private ResourceContainer resourceContainer; private Artifact compiledSymbols; private Artifact dataBindingInfoZip; @@ -60,12 +59,6 @@ public class AndroidResourceParsingActionBuilder { this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); } - /** Set the resource container to parse. */ - public AndroidResourceParsingActionBuilder setParse(LocalResourceContainer primary) { - this.primary = primary; - return this; - } - /** Set the artifact location for the output protobuf. */ public AndroidResourceParsingActionBuilder setOutput(Artifact output) { this.output = output; @@ -73,8 +66,8 @@ public class AndroidResourceParsingActionBuilder { } /** Set the primary resources. */ - public AndroidResourceParsingActionBuilder withPrimary(ResourceContainer resourceContainer) { - this.resourceContainer = resourceContainer; + public AndroidResourceParsingActionBuilder setPrimary(ResourceContainer primary) { + this.primary = primary; return this; } @@ -89,30 +82,28 @@ public class AndroidResourceParsingActionBuilder { return this; } - private static class ResourceContainerToArg implements Function { + private static class ResourceContainerToArg implements Function { public ResourceContainerToArg() {} @Override - public String apply(LocalResourceContainer container) { - return new StringBuilder() - .append(convertRoots(container.getResourceRoots())) - .append(":") - .append(convertRoots(container.getAssetRoots())) - .toString(); + public String apply(ResourceContainer container) { + return convertRoots(container.getResources().getResourceRoots()) + + ":" + + convertRoots(container.getAssets().getAssetRoots()); } } private static class ResourceContainerToArtifacts - implements Function> { + implements Function> { public ResourceContainerToArtifacts() {} @Override - public NestedSet apply(LocalResourceContainer container) { + public NestedSet apply(ResourceContainer container) { NestedSetBuilder artifacts = NestedSetBuilder.naiveLinkOrder(); - artifacts.addAll(container.getAssets()); - artifacts.addAll(container.getResources()); + artifacts.addAll(container.getAssets().getAssets()); + artifacts.addAll(container.getResources().getResources()); return artifacts.build(); } } @@ -177,10 +168,10 @@ public class AndroidResourceParsingActionBuilder { // The databinding needs to be processed before compilation, so the stripping happens here. if (dataBindingInfoZip != null) { - flatFileBuilder.addExecPath("--manifest", resourceContainer.getManifest()); - inputs.add(resourceContainer.getManifest()); - if (!Strings.isNullOrEmpty(resourceContainer.getJavaPackage())) { - flatFileBuilder.add("--packagePath", resourceContainer.getJavaPackage()); + flatFileBuilder.addExecPath("--manifest", primary.getManifest()); + inputs.add(primary.getManifest()); + if (!Strings.isNullOrEmpty(primary.getJavaPackage())) { + flatFileBuilder.add("--packagePath", primary.getJavaPackage()); } flatFileBuilder.addExecPath("--dataBindingInfoOut", dataBindingInfoZip); outs.add(dataBindingInfoZip); @@ -197,13 +188,9 @@ public class AndroidResourceParsingActionBuilder { .setProgressMessage("Compiling Android resources for %s", ruleContext.getLabel()) .setMnemonic("AndroidResourceCompiler") .build(context)); - return resourceContainer - .toBuilder() - .setCompiledSymbols(compiledSymbols) - .setSymbols(output) - .build(); + return primary.toBuilder().setCompiledSymbols(compiledSymbols).setSymbols(output).build(); } else { - return resourceContainer.toBuilder().setSymbols(output).build(); + return primary.toBuilder().setSymbols(output).build(); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java new file mode 100644 index 0000000000..0402d62ba8 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java @@ -0,0 +1,388 @@ +// Copyright 2015 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.android; + +import com.android.resources.ResourceFolderType; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +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.vfs.PathFragment; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import javax.annotation.Nullable; + +/** + * The collected resources artifacts and roots. + * + *

This is used to encapsulate the logic and the data associated with the resources derived from + * an appropriate android rule in a reusable instance. + */ +public final class AndroidResources { + private static final String DEFAULT_RESOURCES_ATTR = "resource_files"; + + public static final String[] RESOURCES_ATTRIBUTES = + new String[] { + "manifest", + DEFAULT_RESOURCES_ATTR, + "local_resource_files", + "assets", + "assets_dir", + "inline_constants", + "exports_manifest" + }; + + /** Set of allowable android directories prefixes. */ + public static final ImmutableSet RESOURCE_DIRECTORY_TYPES = + Arrays.stream(ResourceFolderType.values()) + .map(ResourceFolderType::getName) + .collect(ImmutableSet.toImmutableSet()); + + public static final String INCORRECT_RESOURCE_LAYOUT_MESSAGE = + String.format( + "'%%s' is not in the expected resource directory structure of " + + "/{%s}/", + Joiner.on(',').join(RESOURCE_DIRECTORY_TYPES)); + + /** + * Determines if the attributes contain resource and asset attributes. + * + * @deprecated We are moving towards processing Android assets, resources, and manifests + * separately. Use a separate method that just checks the attributes you need. + */ + @Deprecated + public static boolean definesAndroidResources(AttributeMap attributes) { + for (String attribute : RESOURCES_ATTRIBUTES) { + if (attributes.isAttributeValueExplicitlySpecified(attribute)) { + return true; + } + } + return false; + } + + /** + * Checks validity of a RuleContext to produce Android resources, assets, and manifests. + * + * @throws RuleErrorException if the RuleContext is invalid. Accumulated errors will be available + * via {@code ruleContext} + * @deprecated We are moving towards processing Android assets, resources, and manifests + * separately. Use a separate method that just checks the values you need. + */ + @Deprecated + public static void validateRuleContext(RuleContext ruleContext) throws RuleErrorException { + AndroidAssets.validateAssetsAndAssetsDir(ruleContext); + validateNoAndroidResourcesInSources(ruleContext); + validateManifest(ruleContext); + } + + /** + * Validates that there are no targets with resources in the srcs, as they + * should not be used with the Android data logic. + */ + private static void validateNoAndroidResourcesInSources(RuleContext ruleContext) + throws RuleErrorException { + Iterable resources = + ruleContext.getPrerequisites("srcs", Mode.TARGET, AndroidResourcesInfo.PROVIDER); + for (AndroidResourcesInfo info : resources) { + ruleContext.throwWithAttributeError( + "srcs", + String.format("srcs should not contain label with resources %s", info.getLabel())); + } + } + + private static void validateManifest(RuleContext ruleContext) throws RuleErrorException { + if (ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET) == null) { + ruleContext.throwWithAttributeError( + "manifest", "manifest is required when resource_files or assets are defined."); + } + } + + public static AndroidResources from(RuleContext ruleContext, String resourcesAttr) + throws RuleErrorException { + if (!hasLocalResourcesAttributes(ruleContext)) { + return empty(); + } + + ImmutableList resources = + getResources( + ruleContext.getPrerequisites(resourcesAttr, Mode.TARGET, FileProvider.class)); + + return forResources(ruleContext, resources, resourcesAttr); + } + + /** Returns an {@link AndroidResources} for a list of resource artifacts. */ + @VisibleForTesting + public static AndroidResources forResources( + RuleErrorConsumer ruleErrorConsumer, ImmutableList resources, String resourcesAttr) + throws RuleErrorException { + return new AndroidResources( + resources, getResourceRoots(ruleErrorConsumer, resources, resourcesAttr)); + } + + /** + * TODO(b/76218640): Whether local resources are built into a target should depend on that + * target's resource attribute ("resource_files" in general, but local_resource_files for + * android_test), not any other attributes. + */ + private static boolean hasLocalResourcesAttributes(RuleContext ruleContext) { + return ruleContext.attributes().has("assets") || ruleContext.attributes().has("resource_files"); + } + + static AndroidResources empty() { + return new AndroidResources(ImmutableList.of(), ImmutableList.of()); + } + + /** + * Creates a {@link AndroidResources} containing all the resources in directory artifacts, for use + * with AarImport rules. + * + *

In general, {@link #from(RuleContext, String)} should be used instead, but it can't be for + * AarImport since we don't know about its individual assets at analysis time. No transitive + * resources will be included in the container produced by this method. + * + * @param resourcesDir the tree artifact containing a {@code res/} directory + */ + static AndroidResources forAarImport(SpecialArtifact resourcesDir) { + Preconditions.checkArgument(resourcesDir.isTreeArtifact()); + return new AndroidResources( + ImmutableList.of(resourcesDir), + ImmutableList.of(resourcesDir.getExecPath().getChild("res"))); + } + + /** + * Inner method for adding resource roots to a collection. May fail and report to the {@link + * RuleErrorConsumer} if the input is invalid. + * + * @param file the file to add the resource directory for + * @param lastFile the last file this method was called on. May be null if this is the first call + * for this set of resources. + * @param lastResourceDir the resource directory of the last file, as returned by the most recent + * call to this method, or null if this is the first call. + * @param resourceRoots the collection to add resources to + * @param resourcesAttr the attribute used to refer to resources. While we're moving towards + * "resource_files" everywhere, there are still uses of other attributes for different kinds + * of rules. + * @param ruleErrorConsumer for reporting errors + * @return the resource root of {@code file}. + * @throws RuleErrorException if the current resource has no resource directory or if it is + * incompatible with {@code lastResourceDir}. An error will also be reported to the {@link + * RuleErrorConsumer} in this case. + */ + private static PathFragment addResourceDir( + Artifact file, + @Nullable Artifact lastFile, + @Nullable PathFragment lastResourceDir, + Set resourceRoots, + String resourcesAttr, + RuleErrorConsumer ruleErrorConsumer) + throws RuleErrorException { + PathFragment resourceDir = findResourceDir(file); + if (resourceDir == null) { + ruleErrorConsumer.attributeError( + resourcesAttr, + String.format(INCORRECT_RESOURCE_LAYOUT_MESSAGE, file.getRootRelativePath())); + throw new RuleErrorException(); + } + + if (lastResourceDir != null && !resourceDir.equals(lastResourceDir)) { + ruleErrorConsumer.attributeError( + resourcesAttr, + String.format( + "'%s' (generated by '%s') is not in the same directory '%s' (derived from %s)." + + " All resources must share a common directory.", + file.getRootRelativePath(), + file.getArtifactOwner().getLabel(), + lastResourceDir, + lastFile.getRootRelativePath())); + throw new RuleErrorException(); + } + + PathFragment packageFragment = + file.getArtifactOwner().getLabel().getPackageIdentifier().getSourceRoot(); + PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment); + try { + PathFragment path = file.getExecPath(); + resourceRoots.add( + path.subFragment( + 0, + path.segmentCount() - segmentCountAfterAncestor(resourceDir, packageRelativePath))); + } catch (IllegalArgumentException e) { + ruleErrorConsumer.attributeError( + resourcesAttr, + String.format( + "'%s' (generated by '%s') is not under the directory '%s' (derived from %s).", + file.getRootRelativePath(), + file.getArtifactOwner().getLabel(), + packageRelativePath, + file.getRootRelativePath())); + throw new RuleErrorException(); + } + return resourceDir; + } + + /** + * Finds and validates the resource directory PathFragment from the artifact Path. + * + *

If the artifact is not a Fileset, the resource directory is presumed to be the second + * directory from the end. Filesets are expect to have the last directory as the resource + * directory. + */ + public static PathFragment findResourceDir(Artifact artifact) { + PathFragment fragment = artifact.getPath().asFragment(); + int segmentCount = fragment.segmentCount(); + if (segmentCount < 3) { + return null; + } + // TODO(bazel-team): Expand Fileset to verify, or remove Fileset as an option for resources. + if (artifact.isFileset() || artifact.isTreeArtifact()) { + return fragment.subFragment(segmentCount - 1); + } + + // Check the resource folder type layout. + // get the prefix of the parent folder of the fragment. + String parentDirectory = fragment.getSegment(segmentCount - 2); + int dashIndex = parentDirectory.indexOf('-'); + String androidFolder = + dashIndex == -1 ? parentDirectory : parentDirectory.substring(0, dashIndex); + if (!RESOURCE_DIRECTORY_TYPES.contains(androidFolder)) { + return null; + } + + return fragment.subFragment(segmentCount - 3, segmentCount - 2); + } + + private static int segmentCountAfterAncestor(PathFragment ancestor, PathFragment path) { + String cutAtSegment = ancestor.getSegment(ancestor.segmentCount() - 1); + int index = -1; + List segments = path.getSegments(); + for (int i = segments.size() - 1; i >= 0; i--) { + if (segments.get(i).equals(cutAtSegment)) { + index = i; + break; + } + } + if (index == -1) { + throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor); + } + return segments.size() - index - 1; + } + + private final ImmutableList resources; + private final ImmutableList resourceRoots; + + @VisibleForTesting + public AndroidResources( + ImmutableList resources, ImmutableList resourceRoots) { + this.resources = resources; + this.resourceRoots = resourceRoots; + } + + private static ImmutableList getResources(Iterable targets) { + ImmutableList.Builder builder = ImmutableList.builder(); + for (FileProvider target : targets) { + builder.addAll(target.getFilesToBuild()); + } + + return builder.build(); + } + + public ImmutableList getResources() { + return resources; + } + + /** + * Gets the roots of some resources. + * + * @return a list of roots, or an empty list of the passed resources cannot all be contained in a + * single {@link AndroidResources}. If that's the case, it will be reported to the {@link + * RuleErrorConsumer}. + */ + @VisibleForTesting + static ImmutableList getResourceRoots( + RuleErrorConsumer ruleErrorConsumer, Iterable files, String resourcesAttr) + throws RuleErrorException { + Artifact lastFile = null; + PathFragment lastResourceDir = null; + Set resourceRoots = new LinkedHashSet<>(); + for (Artifact file : files) { + PathFragment resourceDir = + addResourceDir( + file, lastFile, lastResourceDir, resourceRoots, resourcesAttr, ruleErrorConsumer); + lastFile = file; + lastResourceDir = resourceDir; + } + + return ImmutableList.copyOf(resourceRoots); + } + + public ImmutableList getResourceRoots() { + return resourceRoots; + } + + /** + * Filters this object, assuming it contains the resources of the current target. + * + *

If this object contains the resources from a dependency of this target, use {@link + * #maybeFilter(ResourceFilter, boolean)} instead. + * + * @return a filtered {@link AndroidResources} object. If no filtering was done, this object will + * be returned. + */ + public AndroidResources filterLocalResources(ResourceFilter resourceFilter) { + return maybeFilter(resourceFilter, /* isDependency = */ false).orElse(this); + } + + /** + * Filters this object. + * + * @return an optional wrapping a = new {@link AndroidResources} with resources filtered by the + * passed {@link ResourceFilter}, or {@link Optional#empty()} if no resources should be + * filtered. + */ + public Optional maybeFilter( + ResourceFilter resourceFilter, boolean isDependency) { + Optional> filtered = + resourceFilter.maybeFilter(resources, /* isDependency= */ isDependency); + + if (!filtered.isPresent()) { + // Nothing was filtered out + return Optional.empty(); + } + + // If the resources were filtered, also filter the resource roots + ImmutableList.Builder filteredResourcesRootsBuilder = ImmutableList.builder(); + for (PathFragment resourceRoot : resourceRoots) { + for (Artifact resource : filtered.get()) { + if (resource.getRootRelativePath().startsWith(resourceRoot)) { + filteredResourcesRootsBuilder.add(resourceRoot); + break; + } + } + } + + return Optional.of(new AndroidResources(filtered.get(), filteredResourcesRootsBuilder.build())); + } +} 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 6b0156d546..2309d13c05 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 @@ -350,7 +350,7 @@ public final class AndroidRuleClasses { AndroidRuleClasses.ANDROID_LIBRARY_SOURCE_JAR, AndroidRuleClasses.ANDROID_LIBRARY_AAR); - if (LocalResourceContainer.definesAndroidResources(attributes)) { + if (AndroidResources.definesAndroidResources(attributes)) { implicitOutputs.add( AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR, AndroidRuleClasses.ANDROID_R_TXT, diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java index ac2a51d376..0b22c36e5d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java @@ -294,13 +294,11 @@ public final class ApplicationManifest { @Nullable String packageUnderTest, boolean hasLocalResourceFiles) throws InterruptedException, RuleErrorException { - LocalResourceContainer data = - LocalResourceContainer.forAssetsAndResources( - ruleContext, "assets", AndroidAssets.getAssetDir(ruleContext), "local_resource_files"); ResourceContainer resourceContainer = ResourceContainer.builderFromRule(ruleContext) - .setAssetsAndResourcesFrom(data) + .setAssets(AndroidAssets.from(ruleContext)) + .setResources(AndroidResources.from(ruleContext, "local_resource_files")) .setManifest(getManifest()) .setApk(resourceApk) .setRTxt(rTxt) @@ -351,7 +349,8 @@ public final class ApplicationManifest { /** Packages up the manifest with resource and assets from the LocalResourceContainer. */ public ResourceApk packAarWithDataAndResources( RuleContext ruleContext, - LocalResourceContainer data, + AndroidAssets assets, + AndroidResources resources, ResourceDependencies resourceDeps, Artifact rTxt, Artifact symbols, @@ -362,8 +361,8 @@ public final class ApplicationManifest { // resources during execution. ResourceFilter resourceFilter = ResourceFilterFactory.fromRuleContext(ruleContext) - .getResourceFilter(ruleContext, resourceDeps, data); - data = data.filter(ruleContext, resourceFilter); + .getResourceFilter(ruleContext, resourceDeps, resources); + resources = resources.filterLocalResources(resourceFilter); resourceDeps = resourceDeps.filter(resourceFilter); // Now that the LocalResourceContainer has been filtered, we can build a filtered resource @@ -375,7 +374,8 @@ public final class ApplicationManifest { .setJavaPackageFrom(JavaPackageSource.MANIFEST) .setManifestExported(true) .setManifest(getManifest()) - .setAssetsAndResourcesFrom(data) + .setAssets(assets) + .setResources(resources) .build(); // android_library should only build the APK one way (!incremental). @@ -385,8 +385,7 @@ public final class ApplicationManifest { if (resourceContainer.getSymbols() != null) { AndroidResourceParsingActionBuilder parsingBuilder = new AndroidResourceParsingActionBuilder(ruleContext) - .withPrimary(resourceContainer) - .setParse(data) + .setPrimary(resourceContainer) .setOutput(resourceContainer.getSymbols()) .setCompiledSymbolsOutput(resourceContainer.getCompiledSymbols()); @@ -445,17 +444,15 @@ public final class ApplicationManifest { boolean crunchPng, Artifact proguardCfg) throws InterruptedException, RuleErrorException { - LocalResourceContainer data = - LocalResourceContainer.forAssetsAndResources( - ruleContext, "assets", AndroidAssets.getAssetDir(ruleContext), "resource_files"); + AndroidResources resources = AndroidResources.from(ruleContext, "resource_files"); // Filter the resources during analysis to prevent processing of dependencies on unwanted // resources during execution. ResourceFilterFactory resourceFilterFactory = ResourceFilterFactory.fromRuleContext(ruleContext); ResourceFilter resourceFilter = - resourceFilterFactory.getResourceFilter(ruleContext, resourceDeps, data); - data = data.filter(ruleContext, resourceFilter); + resourceFilterFactory.getResourceFilter(ruleContext, resourceDeps, resources); + resources = resources.filterLocalResources(resourceFilter); resourceDeps = resourceDeps.filter(resourceFilter); // Now that the LocalResourceContainer has been filtered, we can build a filtered resource @@ -464,7 +461,8 @@ public final class ApplicationManifest { ResourceContainer.builderFromRule(ruleContext) .setApk(resourceApk) .setManifest(getManifest()) - .setAssetsAndResourcesFrom(data) + .setAssets(AndroidAssets.from(ruleContext)) + .setResources(resources) .build(); ResourceContainer processed = @@ -520,20 +518,19 @@ public final class ApplicationManifest { @Nullable Artifact featureOf, @Nullable Artifact featureAfter) throws InterruptedException, RuleErrorException { - LocalResourceContainer data = - LocalResourceContainer.forAssetsAndResources( - ruleContext, "assets", AndroidAssets.getAssetDir(ruleContext), "resource_files"); + AndroidResources resources = AndroidResources.from(ruleContext, "resource_files"); ResourceFilter resourceFilter = - resourceFilterFactory.getResourceFilter(ruleContext, resourceDeps, data); - data = data.filter(ruleContext, resourceFilter); + resourceFilterFactory.getResourceFilter(ruleContext, resourceDeps, resources); + resources = resources.filterLocalResources(resourceFilter); resourceDeps = resourceDeps.filter(resourceFilter); // Now that the LocalResourceContainer has been filtered, we can build a filtered resource // container from it. ResourceContainer resourceContainer = ResourceContainer.builderFromRule(ruleContext) - .setAssetsAndResourcesFrom(data) + .setAssets(AndroidAssets.from(ruleContext)) + .setResources(resources) .setManifest(getManifest()) .setRTxt(rTxt) .setApk(resourceApk) @@ -602,18 +599,17 @@ public final class ApplicationManifest { throws InterruptedException, RuleErrorException { // Filter the resources during analysis to prevent processing of dependencies on unwanted // resources during execution. - LocalResourceContainer data = - LocalResourceContainer.forAssetsAndResources( - ruleContext, "assets", AndroidAssets.getAssetDir(ruleContext), "resource_files"); + AndroidResources resources = AndroidResources.from(ruleContext, "resource_files"); ResourceFilter resourceFilter = ResourceFilterFactory.fromRuleContext(ruleContext) - .getResourceFilter(ruleContext, resourceDeps, data); - data = data.filter(ruleContext, resourceFilter); + .getResourceFilter(ruleContext, resourceDeps, resources); + resources = resources.filterLocalResources(resourceFilter); resourceDeps = resourceDeps.filter(resourceFilter); ResourceContainer.Builder builder = ResourceContainer.builderFromRule(ruleContext) - .setAssetsAndResourcesFrom(data) + .setAssets(AndroidAssets.from(ruleContext)) + .setResources(resources) .setManifest(getManifest()) .setSymbols(symbols) .setRTxt(rTxt) @@ -651,8 +647,7 @@ public final class ApplicationManifest { if (resourceContainer.getSymbols() != null) { AndroidResourceParsingActionBuilder parsingBuilder = new AndroidResourceParsingActionBuilder(ruleContext) - .withPrimary(resourceContainer) - .setParse(data) + .setPrimary(resourceContainer) .setOutput(resourceContainer.getSymbols()) .setCompiledSymbolsOutput(resourceContainer.getCompiledSymbols()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java b/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java index 65027087e2..b8925ea8f2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java @@ -242,7 +242,7 @@ public final class DataBinding { dataBindingMetadataOutputs.addAll(getMetadataOutputs(ruleContext)); } dataBindingMetadataOutputs.addAll(getTransitiveMetadata(ruleContext, "exports")); - if (!LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) { + if (!AndroidResources.definesAndroidResources(ruleContext.attributes())) { // If this rule doesn't declare direct resources, no resource processing is run so no data // binding outputs are produced. In that case, we need to explicitly propagate data binding // outputs from the deps to make sure they continue up the build graph. @@ -279,7 +279,7 @@ public final class DataBinding { * binary's compilation, enough information is available to only use the first version. */ private static List getMetadataOutputs(RuleContext ruleContext) { - if (!LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) { + if (!AndroidResources.definesAndroidResources(ruleContext.attributes())) { // If this rule doesn't define local resources, no resource processing was done, so it // doesn't produce data binding output. return ImmutableList.of(); @@ -306,7 +306,7 @@ public final class DataBinding { */ static ImmutableList processDeps(RuleContext ruleContext) { ImmutableList.Builder dataBindingJavaInputs = ImmutableList.builder(); - if (LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) { + if (AndroidResources.definesAndroidResources(ruleContext.attributes())) { dataBindingJavaInputs.add(DataBinding.getLayoutInfoFile(ruleContext)); } for (Artifact dataBindingDepMetadata : getTransitiveMetadata(ruleContext, "deps")) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java deleted file mode 100644 index d849166e7a..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java +++ /dev/null @@ -1,395 +0,0 @@ -// Copyright 2015 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.rules.android; - -import com.android.resources.ResourceFolderType; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.FileProvider; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; -import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -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.vfs.PathFragment; -import java.util.Arrays; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import javax.annotation.Nullable; - -/** - * The collected resources and assets artifacts and roots. - * - *

This is used to encapsulate the logic and the data associated with the resources and assets - * derived from an appropriate android rule in a reusable instance. - */ -@Immutable -public final class LocalResourceContainer { - public static final String[] RESOURCES_ATTRIBUTES = - new String[] { - "manifest", - "resource_files", - "local_resource_files", - "assets", - "assets_dir", - "inline_constants", - "exports_manifest" - }; - - /** Set of allowable android directories prefixes. */ - public static final ImmutableSet RESOURCE_DIRECTORY_TYPES = - Arrays.stream(ResourceFolderType.values()) - .map(ResourceFolderType::getName) - .collect(ImmutableSet.toImmutableSet()); - - public static final String INCORRECT_RESOURCE_LAYOUT_MESSAGE = - String.format( - "'%%s' is not in the expected resource directory structure of " - + "/{%s}/", - Joiner.on(',').join(RESOURCE_DIRECTORY_TYPES)); - - /** Determines if the attributes contain resource and asset attributes. */ - public static boolean definesAndroidResources(AttributeMap attributes) { - for (String attribute : RESOURCES_ATTRIBUTES) { - if (attributes.isAttributeValueExplicitlySpecified(attribute)) { - return true; - } - } - return false; - } - - /** - * Checks validity of a RuleContext to produce an AndroidData. - * - * @throws RuleErrorException if the RuleContext is invalid. Accumulated errors will be available - * via {@code ruleContext} - */ - public static void validateRuleContext(RuleContext ruleContext) throws RuleErrorException { - AndroidAssets.validateAssetsAndAssetsDir(ruleContext); - validateNoAndroidResourcesInSources(ruleContext); - validateManifest(ruleContext); - } - - /** - * Validates that there are no targets with resources in the srcs, as they - * should not be used with the Android data logic. - */ - private static void validateNoAndroidResourcesInSources(RuleContext ruleContext) - throws RuleErrorException { - Iterable resources = - ruleContext.getPrerequisites("srcs", Mode.TARGET, AndroidResourcesInfo.PROVIDER); - for (AndroidResourcesInfo info : resources) { - ruleContext.throwWithAttributeError( - "srcs", - String.format("srcs should not contain label with resources %s", info.getLabel())); - } - } - - private static void validateManifest(RuleContext ruleContext) throws RuleErrorException { - if (ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET) == null) { - ruleContext.throwWithAttributeError( - "manifest", "manifest is required when resource_files or assets are defined."); - } - } - - /** - * Creates a {@link LocalResourceContainer} containing this target's assets and resources. - * - * @param ruleContext the current context - * @param assetsAttr the attribute used to refer to assets in this target - * @param assetsDir the path to the assets directory - * @param resourcesAttr the attribute used to refer to resource files in this target - */ - public static LocalResourceContainer forAssetsAndResources( - RuleContext ruleContext, String assetsAttr, PathFragment assetsDir, String resourcesAttr) - throws RuleErrorException { - - if (!hasLocalResourcesAttributes(ruleContext)) { - return new LocalResourceContainer( - ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of()); - } - - AndroidAssets assets = AndroidAssets.from(ruleContext); - - ImmutableList resources = - getResources( - ruleContext.getPrerequisites(resourcesAttr, Mode.TARGET, FileProvider.class)); - - return new LocalResourceContainer( - resources, - getResourceRoots(ruleContext, resources, resourcesAttr), - assets.getAssets(), - assets.getAssetRoots()); - - } - - private static boolean hasLocalResourcesAttributes(RuleContext ruleContext) { - return ruleContext.attributes().has("assets") || ruleContext.attributes().has("resource_files"); - } - - /** - * Creates a {@link LocalResourceContainer} containing all the resources and assets in directory - * artifacts. - * - *

In general, {@link #forAssetsAndResources(RuleContext, String, PathFragment, String)} should - * be used instead. No assets or transitive resources will be included in the container produced - * by this method. - * - * @param assetsDir the tree artifact containing a {@code assets/} directory - * @param resourcesDir the tree artifact containing a {@code res/} directory - */ - static LocalResourceContainer forAssetsAndResourcesDirectories( - Artifact assetsDir, Artifact resourcesDir) { - Preconditions.checkArgument(resourcesDir.isTreeArtifact()); - Preconditions.checkArgument(assetsDir.isTreeArtifact()); - return new LocalResourceContainer( - ImmutableList.of(resourcesDir), - ImmutableList.of(resourcesDir.getExecPath().getChild("res")), - ImmutableList.of(assetsDir), - ImmutableList.of(assetsDir.getExecPath().getChild("assets"))); - } - - /** - * Inner method for adding resource roots to a collection. May fail and report to the {@link - * RuleErrorConsumer} if the input is invalid. - * - * @param file the file to add the resource directory for - * @param lastFile the last file this method was called on. May be null if this is the first call - * for this set of resources. - * @param lastResourceDir the resource directory of the last file, as returned by the most recent - * call to this method, or null if this is the first call. - * @param resourceRoots the collection to add resources to - * @param resourcesAttr the attribute used to refer to resources. While we're moving towards - * "resource_files" everywhere, there are still uses of other attributes for different kinds - * of rules. - * @param ruleErrorConsumer for reporting errors - * @return the resource root of {@code file}. - * @throws RuleErrorException if the current resource has no resource directory or if it is - * incompatible with {@code lastResourceDir}. An error will also be reported to the {@link - * RuleErrorConsumer} in this case. - */ - private static PathFragment addResourceDir( - Artifact file, - @Nullable Artifact lastFile, - @Nullable PathFragment lastResourceDir, - Set resourceRoots, - String resourcesAttr, - RuleErrorConsumer ruleErrorConsumer) - throws RuleErrorException { - PathFragment resourceDir = findResourceDir(file); - if (resourceDir == null) { - ruleErrorConsumer.attributeError( - resourcesAttr, - String.format(INCORRECT_RESOURCE_LAYOUT_MESSAGE, file.getRootRelativePath())); - throw new RuleErrorException(); - } - - if (lastResourceDir != null && !resourceDir.equals(lastResourceDir)) { - ruleErrorConsumer.attributeError( - resourcesAttr, - String.format( - "'%s' (generated by '%s') is not in the same directory '%s' (derived from %s)." - + " All resources must share a common directory.", - file.getRootRelativePath(), - file.getArtifactOwner().getLabel(), - lastResourceDir, - lastFile.getRootRelativePath())); - throw new RuleErrorException(); - } - - PathFragment packageFragment = - file.getArtifactOwner().getLabel().getPackageIdentifier().getSourceRoot(); - PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment); - try { - PathFragment path = file.getExecPath(); - resourceRoots.add( - path.subFragment( - 0, - path.segmentCount() - segmentCountAfterAncestor(resourceDir, packageRelativePath))); - } catch (IllegalArgumentException e) { - ruleErrorConsumer.attributeError( - resourcesAttr, - String.format( - "'%s' (generated by '%s') is not under the directory '%s' (derived from %s).", - file.getRootRelativePath(), - file.getArtifactOwner().getLabel(), - packageRelativePath, - file.getRootRelativePath())); - throw new RuleErrorException(); - } - return resourceDir; - } - - /** - * Finds and validates the resource directory PathFragment from the artifact Path. - * - *

If the artifact is not a Fileset, the resource directory is presumed to be the second - * directory from the end. Filesets are expect to have the last directory as the resource - * directory. - */ - public static PathFragment findResourceDir(Artifact artifact) { - PathFragment fragment = artifact.getPath().asFragment(); - int segmentCount = fragment.segmentCount(); - if (segmentCount < 3) { - return null; - } - // TODO(bazel-team): Expand Fileset to verify, or remove Fileset as an option for resources. - if (artifact.isFileset() || artifact.isTreeArtifact()) { - return fragment.subFragment(segmentCount - 1); - } - - // Check the resource folder type layout. - // get the prefix of the parent folder of the fragment. - String parentDirectory = fragment.getSegment(segmentCount - 2); - int dashIndex = parentDirectory.indexOf('-'); - String androidFolder = - dashIndex == -1 ? parentDirectory : parentDirectory.substring(0, dashIndex); - if (!RESOURCE_DIRECTORY_TYPES.contains(androidFolder)) { - return null; - } - - return fragment.subFragment(segmentCount - 3, segmentCount - 2); - } - - private static int segmentCountAfterAncestor(PathFragment ancestor, PathFragment path) { - String cutAtSegment = ancestor.getSegment(ancestor.segmentCount() - 1); - int index = -1; - List segments = path.getSegments(); - for (int i = segments.size() - 1; i >= 0; i--) { - if (segments.get(i).equals(cutAtSegment)) { - index = i; - break; - } - } - if (index == -1) { - throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor); - } - return segments.size() - index - 1; - } - - private final ImmutableList resources; - private final ImmutableList assets; - private final ImmutableList assetRoots; - private final ImmutableList resourceRoots; - - @VisibleForTesting - public LocalResourceContainer( - ImmutableList resources, - ImmutableList resourceRoots, - ImmutableList assets, - ImmutableList assetRoots) { - this.resources = resources; - this.resourceRoots = resourceRoots; - this.assets = assets; - this.assetRoots = assetRoots; - } - - private static ImmutableList getResources(Iterable targets) { - ImmutableList.Builder builder = ImmutableList.builder(); - for (FileProvider target : targets) { - builder.addAll(target.getFilesToBuild()); - } - - return builder.build(); - } - - public ImmutableList getResources() { - return resources; - } - - public ImmutableList getAssets() { - return assets; - } - - public ImmutableList getAssetRoots() { - return assetRoots; - } - - /** - * Gets the roots of some resources. - * - * @return a list of roots, or an empty list of the passed resources cannot all be contained in a - * single {@link LocalResourceContainer}. If that's the case, it will be reported to the - * {@link RuleErrorConsumer}. - */ - @VisibleForTesting - static ImmutableList getResourceRoots( - RuleErrorConsumer ruleErrorConsumer, Iterable files, String resourcesAttr) - throws RuleErrorException { - Artifact lastFile = null; - PathFragment lastResourceDir = null; - Set resourceRoots = new LinkedHashSet<>(); - for (Artifact file : files) { - PathFragment resourceDir = - addResourceDir( - file, lastFile, lastResourceDir, resourceRoots, resourcesAttr, ruleErrorConsumer); - lastFile = file; - lastResourceDir = resourceDir; - } - - return ImmutableList.copyOf(resourceRoots); - } - - public ImmutableList getResourceRoots() { - return resourceRoots; - } - - /** - * Filters this object. - * - * @return a new {@link LocalResourceContainer} with resources filtered by the passed {@link - * ResourceFilter}, or this object if no resources should be filtered. - */ - public LocalResourceContainer filter( - RuleErrorConsumer ruleErrorConsumer, ResourceFilter resourceFilter) - throws RuleErrorException { - Optional> filtered = - resourceFilter.maybeFilter(resources, /* isDependency= */ false); - - if (!filtered.isPresent()) { - // Nothing was filtered out - return this; - } - - return withResources(ruleErrorConsumer, filtered.get(), "resource_files"); - } - - @VisibleForTesting - static LocalResourceContainer forResources( - RuleErrorConsumer ruleErrorConsumer, ImmutableList resources) - throws RuleErrorException { - return new LocalResourceContainer( - resources, - getResourceRoots(ruleErrorConsumer, resources, "resource_files"), - ImmutableList.of(), - ImmutableList.of()); - } - - private LocalResourceContainer withResources( - RuleErrorConsumer ruleErrorConsumer, ImmutableList resources, String resourcesAttr) - throws RuleErrorException { - return new LocalResourceContainer( - resources, - getResourceRoots(ruleErrorConsumer, resources, resourcesAttr), - assets, - assetRoots); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java index 7457da4eb0..564597a5bb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java @@ -79,45 +79,25 @@ public abstract class ResourceContainer { @Nullable public abstract Artifact getJavaClassJar(); - abstract ImmutableList getAssets(); + abstract AndroidAssets getAssets(); @VisibleForTesting - public abstract ImmutableList getResources(); + public abstract AndroidResources getResources(); + /** @deprecated We are moving towards decoupling assets and resources */ + @Deprecated public ImmutableList getArtifacts(ResourceType resourceType) { - return resourceType == ResourceType.ASSETS ? getAssets() : getResources(); + return resourceType == ResourceType.ASSETS + ? getAssets().getAssets() + : getResources().getResources(); } + /** @deprecated We are moving towards decoupling assets and resources */ + @Deprecated public Iterable getArtifacts() { - return Iterables.concat(getAssets(), getResources()); + return Iterables.concat(getAssets().getAssets(), getResources().getResources()); } - /** - * Gets the directories containing the assets. - * - *

TODO(b/30308041): Stop using these directories, and remove this code. - * - * @deprecated We are moving towards passing around the actual artifacts, rather than the - * directories that contain them. If the resources were provided with a glob() that excludes - * some files, the resource directory will still contain those files, resulting in unwanted - * inputs. - */ - @Deprecated - abstract ImmutableList getAssetsRoots(); - - /** - * Gets the directories containing the resources. - * - *

TODO(b/30308041): Stop using these directories, and remove this code. - * - * @deprecated We are moving towards passing around the actual artifacts, rather than the - * directories that contain them. If the resources were provided with a glob() that excludes - * some files, the resource directory will still contain those files, resulting in unwanted - * inputs. - */ - @Deprecated - abstract ImmutableList getResourcesRoots(); - /** * Gets the directories containing the resources of a specific type. * @@ -130,7 +110,9 @@ public abstract class ResourceContainer { */ @Deprecated public ImmutableList getRoots(ResourceType resourceType) { - return resourceType == ResourceType.ASSETS ? getAssetsRoots() : getResourcesRoots(); + return resourceType == ResourceType.ASSETS + ? getAssets().getAssetRoots() + : getResources().getResourceRoots(); } public abstract boolean isManifestExported(); @@ -204,39 +186,21 @@ public abstract class ResourceContainer { * should be filtered. The original container is unchanged. */ public ResourceContainer filter(ResourceFilter filter, boolean isDependency) { - Optional> filteredResources = - filter.maybeFilter(getResources(), isDependency); + Optional filteredResources = getResources().maybeFilter(filter, isDependency); if (!filteredResources.isPresent()) { // No filtering was done; return this container return this; } - - // If the resources were filtered, also filter the resource roots - ImmutableList.Builder filteredResourcesRootsBuilder = ImmutableList.builder(); - for (PathFragment resourceRoot : getResourcesRoots()) { - for (Artifact resource : filteredResources.get()) { - if (resource.getRootRelativePath().startsWith(resourceRoot)) { - filteredResourcesRootsBuilder.add(resourceRoot); - break; - } - } - } - - return toBuilder() - .setResources(filteredResources.get()) - .setResourcesRoots(filteredResourcesRootsBuilder.build()) - .build(); + return toBuilder().setResources(filteredResources.get()).build(); } /** Creates a new builder with default values. */ public static Builder builder() { return new AutoValue_ResourceContainer.Builder() .setJavaPackageFrom(Builder.JavaPackageSource.MANIFEST) - .setAssets(ImmutableList.of()) - .setResources(ImmutableList.of()) - .setAssetsRoots(ImmutableList.of()) - .setResourcesRoots(ImmutableList.of()); + .setAssets(AndroidAssets.empty()) + .setResources(AndroidResources.empty()); } /** @@ -310,19 +274,6 @@ public abstract class ResourceContainer { return this.setJavaPackage(javaPackageOverride); } - /** - * Sets the assets, resources, asset roots, and resource roots from the given local resource - * container. - * - *

This will override any of these values which were previously set directly. - */ - public Builder setAssetsAndResourcesFrom(LocalResourceContainer data) { - return this.setAssets(data.getAssets()) - .setResources(data.getResources()) - .setAssetsRoots(data.getAssetRoots()) - .setResourcesRoots(data.getResourceRoots()); - } - public abstract Builder setLabel(Label label); abstract Builder setJavaPackage(@Nullable String javaPackage); @@ -338,13 +289,9 @@ public abstract class ResourceContainer { public abstract Builder setJavaClassJar(@Nullable Artifact javaClassJar); - public abstract Builder setAssets(ImmutableList assets); - - public abstract Builder setResources(ImmutableList resources); - - public abstract Builder setAssetsRoots(ImmutableList assetsRoots); + public abstract Builder setAssets(AndroidAssets assets); - public abstract Builder setResourcesRoots(ImmutableList resourcesRoots); + public abstract Builder setResources(AndroidResources resources); public abstract Builder setManifestExported(boolean manifestExported); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java index 9226d3376d..a9ee3b34fa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java @@ -257,11 +257,11 @@ public final class ResourceDependencies { NestedSetBuilder.naiveLinkOrder().add(newDirectResource).build(), NestedSetBuilder.naiveLinkOrder() .addTransitive(transitiveResources) - .addAll(newDirectResource.getResources()) + .addAll(newDirectResource.getResources().getResources()) .build(), NestedSetBuilder.naiveLinkOrder() .addTransitive(transitiveAssets) - .addAll(newDirectResource.getAssets()) + .addAll(newDirectResource.getAssets().getAssets()) .build(), withDirectAndTransitive(newDirectResource.getManifest(), transitiveManifests), withDirectAndTransitive(newDirectResource.getAapt2RTxt(), transitiveAapt2RTxt), 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 18f337253b..277b599ab4 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 @@ -316,7 +316,7 @@ public class ResourceFilterFactory { ResourceFilter getResourceFilter( RuleErrorConsumer ruleErrorConsumer, ResourceDependencies resourceDeps, - LocalResourceContainer localResources) { + AndroidResources localResources) { if (!isPrefiltering()) { return ResourceFilter.empty(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java index 7a17eeaf52..a861b18517 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java @@ -89,11 +89,13 @@ public class AarImportTest extends BuildViewTestCase { ResourceContainer resourceContainer = directResources.iterator().next(); assertThat(resourceContainer.getManifest()).isNotNull(); - Artifact resourceTreeArtifact = Iterables.getOnlyElement(resourceContainer.getResources()); + Artifact resourceTreeArtifact = + Iterables.getOnlyElement(resourceContainer.getResources().getResources()); assertThat(resourceTreeArtifact.isTreeArtifact()).isTrue(); assertThat(resourceTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/resources/foo"); - Artifact assetsTreeArtifact = Iterables.getOnlyElement(resourceContainer.getAssets()); + Artifact assetsTreeArtifact = + Iterables.getOnlyElement(resourceContainer.getAssets().getAssets()); assertThat(assetsTreeArtifact.isTreeArtifact()).isTrue(); assertThat(assetsTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/assets/foo"); } @@ -107,8 +109,8 @@ public class AarImportTest extends BuildViewTestCase { .toList() .get(0); - Artifact resourceTreeArtifact = resourceContainer.getResources().get(0); - Artifact assetsTreeArtifact = resourceContainer.getAssets().get(0); + Artifact resourceTreeArtifact = resourceContainer.getResources().getResources().get(0); + Artifact assetsTreeArtifact = resourceContainer.getAssets().getAssets().get(0); Artifact aarResourcesExtractor = getHostConfiguredTarget( ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor") @@ -326,7 +328,7 @@ public class AarImportTest extends BuildViewTestCase { .getManifest() .getRootRelativePathString(); } - + @Test public void testTransitiveExports() throws Exception { assertThat(getConfiguredTarget("//a:bar").get(JavaInfo.PROVIDER).getTransitiveExports()) 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 9b81ffcd66..499da948b7 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 @@ -2635,7 +2635,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { checkError("java/android/resources", "r", "'java/android/resources/res/somefile.xml' is not in the expected resource directory " + "structure of /{" - + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}", + + Joiner.on(',').join(AndroidResources.RESOURCE_DIRECTORY_TYPES) + "}", "android_binary(name = 'r',", " manifest = 'AndroidManifest.xml',", " resource_files = ['res/somefile.xml', 'r/t/f/m/raw/fold']", @@ -2649,7 +2649,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "r", "'java/android/resources/res/other/somefile.xml' is not in the expected resource directory " + "structure of /{" - + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}", + + Joiner.on(',').join(AndroidResources.RESOURCE_DIRECTORY_TYPES) + "}", "android_binary(name = 'r',", " manifest = 'AndroidManifest.xml',", " resource_files = ['res/other/somefile.xml', 'r/t/f/m/raw/fold']", diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java index e9bbb6e7de..72c27bcd5a 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java @@ -806,7 +806,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { checkError("java/android", "r", "'java/android/res/somefile.xml' is not in the expected resource directory structure of" + " /{" - + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}", + + Joiner.on(',').join(AndroidResources.RESOURCE_DIRECTORY_TYPES) + "}", "android_library(name = 'r',", " manifest = 'AndroidManifest.xml',", " resource_files = ['res/somefile.xml', 'r/t/f/m/raw/fold']", @@ -818,7 +818,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { checkError("java/android", "r", "'java/android/res/other/somefile.xml' is not in the expected resource directory structure" + " of /{" - + Joiner.on(',').join(LocalResourceContainer.RESOURCE_DIRECTORY_TYPES) + "}", + + Joiner.on(',').join(AndroidResources.RESOURCE_DIRECTORY_TYPES) + "}", "android_library(name = 'r',", " manifest = 'AndroidManifest.xml',", " resource_files = ['res/other/somefile.xml', 'r/t/f/m/raw/fold']", diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java new file mode 100644 index 0000000000..056970f322 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java @@ -0,0 +1,171 @@ +// 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.rules.android; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Optional; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests {@link AndroidResourcesTest} */ +@RunWith(JUnit4.class) +public class AndroidResourcesTest extends ResourceTestBase { + private static final PathFragment DEFAULT_RESOURCE_ROOT = PathFragment.create(RESOURCE_ROOT); + private static final ImmutableList RESOURCES_ROOTS = + ImmutableList.of(DEFAULT_RESOURCE_ROOT); + + @Before + @Test + public void testGetResourceRootsNoResources() throws Exception { + assertThat(getResourceRoots()).isEmpty(); + } + + @Test + public void testGetResourceRootsInvalidResourceDirectory() throws Exception { + try { + getResourceRoots("is-this-drawable-or-values/foo.xml"); + assertWithMessage("Expected exception not thrown!").fail(); + } catch (RuleErrorException e) { + // expected + } + + errorConsumer.assertAttributeError( + "resource_files", "is not in the expected resource directory structure"); + } + + @Test + public void testGetResourceRootsMultipleRoots() throws Exception { + try { + getResourceRoots("subdir/values/foo.xml", "otherdir/values/bar.xml"); + assertWithMessage("Expected exception not thrown!").fail(); + } catch (RuleErrorException e) { + // expected + } + + errorConsumer.assertAttributeError( + "resource_files", "All resources must share a common directory"); + } + + @Test + public void testGetResourceRoots() throws Exception { + assertThat(getResourceRoots("values-hdpi/foo.xml", "values-mdpi/bar.xml")) + .isEqualTo(RESOURCES_ROOTS); + } + + @Test + public void testGetResourceRootsCommonSubdirectory() throws Exception { + assertThat(getResourceRoots("subdir/values-hdpi/foo.xml", "subdir/values-mdpi/bar.xml")) + .containsExactly(DEFAULT_RESOURCE_ROOT.getRelative("subdir")); + } + + private ImmutableList getResourceRoots(String... pathResourceStrings) + throws Exception { + return getResourceRoots(getResources(pathResourceStrings)); + } + + private ImmutableList getResourceRoots(ImmutableList artifacts) + throws Exception { + return AndroidResources.getResourceRoots(errorConsumer, artifacts, "resource_files"); + } + + @Test + public void testFilterEmpty() throws Exception { + assertFilter(ImmutableList.of(), ImmutableList.of()); + } + + @Test + public void testFilterNoop() throws Exception { + ImmutableList resources = getResources("values-en/foo.xml", "values-es/bar.xml"); + assertFilter(resources, resources); + } + + @Test + public void testFilterToEmpty() throws Exception { + assertFilter( + getResources("values-en/foo.xml", "values-es/bar.xml"), ImmutableList.of()); + } + + @Test + public void testPartiallyFilter() throws Exception { + Artifact keptResource = getResource("values-en/foo.xml"); + assertFilter( + ImmutableList.of(keptResource, getResource("values-es/bar.xml")), + ImmutableList.of(keptResource)); + } + + @Test + public void testFilterIsDependency() throws Exception { + Artifact keptResource = getResource("values-en/foo.xml"); + assertFilter( + ImmutableList.of(keptResource, getResource("drawable/bar.png")), + ImmutableList.of(keptResource), + /* isDependency = */ true); + } + + private void assertFilter( + ImmutableList unfilteredResources, ImmutableList filteredResources) + throws Exception { + assertFilter(unfilteredResources, filteredResources, /* isDependency = */ false); + } + + private void assertFilter( + ImmutableList unfilteredResources, + ImmutableList filteredResources, + boolean isDependency) + throws Exception { + ImmutableList unfilteredResourcesRoots = getResourceRoots(unfilteredResources); + AndroidResources unfiltered = + new AndroidResources(unfilteredResources, unfilteredResourcesRoots); + + ImmutableList.Builder filteredDepsBuilder = ImmutableList.builder(); + + ResourceFilter fakeFilter = + ResourceFilter.of(ImmutableSet.copyOf(filteredResources), filteredDepsBuilder::add); + + Optional filtered = unfiltered.maybeFilter(fakeFilter, isDependency); + + if (filteredResources.equals(unfilteredResources)) { + // We expect filtering to have been a no-op + assertThat(filtered.isPresent()).isFalse(); + } else { + // The resources and their roots should be filtered + assertThat(filtered.get().getResources()) + .containsExactlyElementsIn(filteredResources) + .inOrder(); + assertThat(filtered.get().getResourceRoots()) + .containsExactlyElementsIn(getResourceRoots(filteredResources)) + .inOrder(); + } + + if (!isDependency) { + // The filter should not record any filtered dependencies + assertThat(filteredDepsBuilder.build()).isEmpty(); + } else { + // The filtered dependencies should be exactly the list of filtered resources + assertThat(unfilteredResources) + .containsExactlyElementsIn( + Iterables.concat(filteredDepsBuilder.build(), filteredResources)); + } + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD index b2ad32d9ff..6bc33e4aaa 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD @@ -104,8 +104,8 @@ java_test( ) java_test( - name = "LocalResourceContainerTest", - srcs = ["LocalResourceContainerTest.java"], + name = "AndroidResourcesTest", + srcs = ["AndroidResourcesTest.java"], deps = [ ":ResourceTestBase", "//src/main/java/com/google/devtools/build/lib:android-rules", diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java deleted file mode 100644 index 550f86beb6..0000000000 --- a/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java +++ /dev/null @@ -1,147 +0,0 @@ -// 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.rules.android; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.vfs.PathFragment; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests {@link LocalResourceContainer} */ -@RunWith(JUnit4.class) -public class LocalResourceContainerTest extends ResourceTestBase { - private static final PathFragment DEFAULT_RESOURCE_ROOT = PathFragment.create(RESOURCE_ROOT); - private static final ImmutableList RESOURCES_ROOTS = - ImmutableList.of(DEFAULT_RESOURCE_ROOT); - - @Before - @Test - public void testGetResourceRootsNoResources() throws Exception { - assertThat(getResourceRoots()).isEmpty(); - } - - @Test - public void testGetResourceRootsInvalidResourceDirectory() throws Exception { - try { - getResourceRoots("is-this-drawable-or-values/foo.xml"); - assertWithMessage("Expected exception not thrown!").fail(); - } catch (RuleErrorException e) { - // expected - } - - errorConsumer.assertAttributeError( - "resource_files", "is not in the expected resource directory structure"); - } - - @Test - public void testGetResourceRootsMultipleRoots() throws Exception { - try { - getResourceRoots("subdir/values/foo.xml", "otherdir/values/bar.xml"); - assertWithMessage("Expected exception not thrown!").fail(); - } catch (RuleErrorException e) { - // expected - } - - errorConsumer.assertAttributeError( - "resource_files", "All resources must share a common directory"); - } - - @Test - public void testGetResourceRoots() throws Exception { - assertThat(getResourceRoots("values-hdpi/foo.xml", "values-mdpi/bar.xml")) - .isEqualTo(RESOURCES_ROOTS); - } - - @Test - public void testGetResourceRootsCommonSubdirectory() throws Exception { - assertThat(getResourceRoots("subdir/values-hdpi/foo.xml", "subdir/values-mdpi/bar.xml")) - .containsExactly(DEFAULT_RESOURCE_ROOT.getRelative("subdir")); - } - - private ImmutableList getResourceRoots(String... pathResourceStrings) - throws Exception { - return getResourceRoots(getResources(pathResourceStrings)); - } - - private ImmutableList getResourceRoots(ImmutableList artifacts) - throws Exception { - return LocalResourceContainer.getResourceRoots(errorConsumer, artifacts, "resource_files"); - } - - @Test - public void testFilterEmpty() throws Exception { - assertFilter(ImmutableList.of(), ImmutableList.of()); - } - - @Test - public void testFilterNoop() throws Exception { - ImmutableList resources = getResources("values-en/foo.xml", "values-es/bar.xml"); - assertFilter(resources, resources); - } - - @Test - public void testFilterToEmpty() throws Exception { - assertFilter( - getResources("values-en/foo.xml", "values-es/bar.xml"), ImmutableList.of()); - } - - @Test - public void testPartiallyFilter() throws Exception { - Artifact keptResource = getResource("values-en/foo.xml"); - assertFilter( - ImmutableList.of(keptResource, getResource("values-es/bar.xml")), - ImmutableList.of(keptResource)); - } - - private void assertFilter( - ImmutableList unfilteredResources, ImmutableList filteredResources) - throws Exception { - ImmutableList unfilteredResourcesRoots = getResourceRoots(unfilteredResources); - LocalResourceContainer unfiltered = - new LocalResourceContainer( - unfilteredResources, - unfilteredResourcesRoots, - unfilteredResources, - unfilteredResourcesRoots); - - ResourceFilter fakeFilter = - ResourceFilter.of(ImmutableSet.copyOf(filteredResources), (artifact) -> {}); - - LocalResourceContainer filtered = unfiltered.filter(errorConsumer, fakeFilter); - - if (unfilteredResources.equals(filteredResources)) { - // The filtering was a no-op; the original object, not a copy, should be returned - assertThat(filtered).isSameAs(unfiltered); - } else { - // The resources and their roots should be filtered - assertThat(filtered.getResources()).containsExactlyElementsIn(filteredResources).inOrder(); - assertThat(filtered.getResourceRoots()) - .containsExactlyElementsIn(getResourceRoots(filteredResources)) - .inOrder(); - - // The assets and their roots should not be filtered; the original objects, not a copy, should - // be returned. - assertThat(filtered.getAssets()).isSameAs(unfiltered.getAssets()); - assertThat(filtered.getAssetRoots()).isSameAs(unfiltered.getAssetRoots()); - } - } -} 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 3aa3be15e5..0ddd905060 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 @@ -60,9 +60,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { manifest.getOwnerLabel().getPackageName(), "resourceContainer_" + resources.hashCode()); return ResourceContainer.builder() - .setResources(resources) - .setResourcesRoots( - LocalResourceContainer.getResourceRoots(errorConsumer, resources, "resource_files")) + .setResources(AndroidResources.forResources(errorConsumer, resources, "resource_files")) .setLabel(label) .setManifestExported(false) .setManifest(manifest) @@ -354,9 +352,11 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { Artifact transitiveResourceToDiscard = getResource("transitive/drawable-en-ldpi/transitive.png"); - LocalResourceContainer localResources = - LocalResourceContainer.forResources( - errorConsumer, ImmutableList.of(localResourceToKeep, localResourceToDiscard)); + AndroidResources localResources = + AndroidResources.forResources( + errorConsumer, + ImmutableList.of(localResourceToKeep, localResourceToDiscard), + "resource_files"); ResourceDependencies resourceDependencies = ResourceDependencies.empty() @@ -383,7 +383,7 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { resourceFilterFactory.getResourceFilter( errorConsumer, resourceDependencies, localResources); - assertThat(localResources.filter(errorConsumer, filter).getResources()) + assertThat(localResources.filterLocalResources(filter).getResources()) .containsExactly(localResourceToKeep); ResourceDependencies filteredResourceDeps = resourceDependencies.filter(filter); @@ -401,13 +401,13 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { filteredResourceDeps.getDirectResourceContainers().toList(); assertThat(directContainers).hasSize(2); - ResourceContainer directToDiscard = directContainers.get(0); + AndroidResources directToDiscard = directContainers.get(0).getResources(); assertThat(directToDiscard.getResources()).isEmpty(); - assertThat(directToDiscard.getResourcesRoots()).isEmpty(); + assertThat(directToDiscard.getResourceRoots()).isEmpty(); - ResourceContainer directToKeep = directContainers.get(1); + AndroidResources directToKeep = directContainers.get(1).getResources(); assertThat(directToKeep.getResources()).containsExactly(directResourceToKeep); - assertThat(directToKeep.getResourcesRoots()) + assertThat(directToKeep.getResourceRoots()) .containsExactly( directResourceToKeep.getExecPath().getParentDirectory().getParentDirectory()); @@ -415,13 +415,13 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { filteredResourceDeps.getTransitiveResourceContainers().toList(); assertThat(transitiveContainers).hasSize(2); - ResourceContainer transitiveToDiscard = transitiveContainers.get(0); + AndroidResources transitiveToDiscard = transitiveContainers.get(0).getResources(); assertThat(transitiveToDiscard.getResources()).isEmpty(); - assertThat(transitiveToDiscard.getResourcesRoots()).isEmpty(); + assertThat(transitiveToDiscard.getResourceRoots()).isEmpty(); - ResourceContainer transitiveToKeep = transitiveContainers.get(1); + AndroidResources transitiveToKeep = transitiveContainers.get(1).getResources(); assertThat(transitiveToKeep.getResources()).containsExactly(transitiveResourceToKeep); - assertThat(transitiveToKeep.getResourcesRoots()) + assertThat(transitiveToKeep.getResourceRoots()) .containsExactly( transitiveResourceToKeep.getExecPath().getParentDirectory().getParentDirectory()); @@ -492,17 +492,16 @@ public class ResourceFilterFactoryTest extends ResourceTestBase { private ImmutableList doFilter( ResourceFilterFactory resourceFilterFactory, ImmutableList artifacts) throws RuleErrorException { - LocalResourceContainer localResourceContainer = - LocalResourceContainer.forResources(errorConsumer, artifacts); + AndroidResources localResources = + AndroidResources.forResources(errorConsumer, artifacts, "resource_files"); ResourceDependencies resourceDeps = ResourceDependencies.empty(); ResourceFilter filter = - resourceFilterFactory.getResourceFilter( - errorConsumer, resourceDeps, localResourceContainer); + resourceFilterFactory.getResourceFilter(errorConsumer, resourceDeps, localResources); assertThat(resourceDeps.filter(filter)).isSameAs(resourceDeps); - return localResourceContainer.filter(errorConsumer, filter).getResources(); + return localResources.filterLocalResources(filter).getResources(); } } -- cgit v1.2.3