aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar asteinb <asteinb@google.com>2018-03-29 10:02:27 -0700
committerGravatar Copybara-Service <copybara-piper@google.com>2018-03-29 10:03:42 -0700
commitf2b66aa31b0a3addb2f3712c6232f4c8c65b55db (patch)
tree776071ffff4d398bd7e72dbf9efaa92f6f2207cb
parentdbe1ed769d4c957d7b8aea2c9c92febfc26f022d (diff)
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
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java9
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java49
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java (renamed from src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java)171
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java55
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java91
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java2
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java12
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java4
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java4
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java (renamed from src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java)64
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/BUILD4
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactoryTest.java39
20 files changed, 245 insertions, 295 deletions
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<Artifact> assets;
private final ImmutableList<PathFragment> 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<LocalResourceContainer, String> {
+ private static class ResourceContainerToArg implements Function<ResourceContainer, String> {
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<LocalResourceContainer, NestedSet<Artifact>> {
+ implements Function<ResourceContainer, NestedSet<Artifact>> {
public ResourceContainerToArtifacts() {}
@Override
- public NestedSet<Artifact> apply(LocalResourceContainer container) {
+ public NestedSet<Artifact> apply(ResourceContainer container) {
NestedSetBuilder<Artifact> 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/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java
index d849166e7a..0402d62ba8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java
@@ -20,10 +20,10 @@ 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.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;
@@ -36,17 +36,18 @@ import java.util.Set;
import javax.annotation.Nullable;
/**
- * The collected resources and assets artifacts and roots.
+ * The collected resources artifacts and roots.
*
- * <p>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.
+ * <p>This is used to encapsulate the logic and the data associated with the resources derived from
+ * an appropriate android rule in a reusable instance.
*/
-@Immutable
-public final class LocalResourceContainer {
+public final class AndroidResources {
+ private static final String DEFAULT_RESOURCES_ATTR = "resource_files";
+
public static final String[] RESOURCES_ATTRIBUTES =
new String[] {
"manifest",
- "resource_files",
+ DEFAULT_RESOURCES_ATTR,
"local_resource_files",
"assets",
"assets_dir",
@@ -66,7 +67,13 @@ public final class LocalResourceContainer {
+ "<resource directory>/{%s}/<file>",
Joiner.on(',').join(RESOURCE_DIRECTORY_TYPES));
- /** Determines if the attributes contain resource and asset attributes. */
+ /**
+ * 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)) {
@@ -77,11 +84,14 @@ public final class LocalResourceContainer {
}
/**
- * Checks validity of a RuleContext to produce an AndroidData.
+ * 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);
@@ -110,61 +120,56 @@ public final class LocalResourceContainer {
}
}
- /**
- * 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)
+ public static AndroidResources from(RuleContext ruleContext, String resourcesAttr)
throws RuleErrorException {
-
if (!hasLocalResourcesAttributes(ruleContext)) {
- return new LocalResourceContainer(
- ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of());
+ return empty();
}
- AndroidAssets assets = AndroidAssets.from(ruleContext);
-
ImmutableList<Artifact> resources =
getResources(
ruleContext.getPrerequisites(resourcesAttr, Mode.TARGET, FileProvider.class));
- return new LocalResourceContainer(
- resources,
- getResourceRoots(ruleContext, resources, resourcesAttr),
- assets.getAssets(),
- assets.getAssetRoots());
+ return forResources(ruleContext, resources, resourcesAttr);
+ }
+ /** Returns an {@link AndroidResources} for a list of resource artifacts. */
+ @VisibleForTesting
+ public static AndroidResources forResources(
+ RuleErrorConsumer ruleErrorConsumer, ImmutableList<Artifact> 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 LocalResourceContainer} containing all the resources and assets in directory
- * artifacts.
+ * Creates a {@link AndroidResources} containing all the resources in directory artifacts, for use
+ * with AarImport rules.
*
- * <p>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.
+ * <p>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 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) {
+ static AndroidResources forAarImport(SpecialArtifact resourcesDir) {
Preconditions.checkArgument(resourcesDir.isTreeArtifact());
- Preconditions.checkArgument(assetsDir.isTreeArtifact());
- return new LocalResourceContainer(
+ return new AndroidResources(
ImmutableList.of(resourcesDir),
- ImmutableList.of(resourcesDir.getExecPath().getChild("res")),
- ImmutableList.of(assetsDir),
- ImmutableList.of(assetsDir.getExecPath().getChild("assets")));
+ ImmutableList.of(resourcesDir.getExecPath().getChild("res")));
}
/**
@@ -286,20 +291,13 @@ public final class LocalResourceContainer {
}
private final ImmutableList<Artifact> resources;
- private final ImmutableList<Artifact> assets;
- private final ImmutableList<PathFragment> assetRoots;
private final ImmutableList<PathFragment> resourceRoots;
@VisibleForTesting
- public LocalResourceContainer(
- ImmutableList<Artifact> resources,
- ImmutableList<PathFragment> resourceRoots,
- ImmutableList<Artifact> assets,
- ImmutableList<PathFragment> assetRoots) {
+ public AndroidResources(
+ ImmutableList<Artifact> resources, ImmutableList<PathFragment> resourceRoots) {
this.resources = resources;
this.resourceRoots = resourceRoots;
- this.assets = assets;
- this.assetRoots = assetRoots;
}
private static ImmutableList<Artifact> getResources(Iterable<FileProvider> targets) {
@@ -315,20 +313,12 @@ public final class LocalResourceContainer {
return resources;
}
- public ImmutableList<Artifact> getAssets() {
- return assets;
- }
-
- public ImmutableList<PathFragment> 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}.
+ * single {@link AndroidResources}. If that's the case, it will be reported to the {@link
+ * RuleErrorConsumer}.
*/
@VisibleForTesting
static ImmutableList<PathFragment> getResourceRoots(
@@ -353,43 +343,46 @@ public final class LocalResourceContainer {
}
/**
+ * Filters this object, assuming it contains the resources of the current target.
+ *
+ * <p>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 a new {@link LocalResourceContainer} with resources filtered by the passed {@link
- * ResourceFilter}, or this object if no resources should be filtered.
+ * @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 LocalResourceContainer filter(
- RuleErrorConsumer ruleErrorConsumer, ResourceFilter resourceFilter)
- throws RuleErrorException {
+ public Optional<AndroidResources> maybeFilter(
+ ResourceFilter resourceFilter, boolean isDependency) {
Optional<ImmutableList<Artifact>> filtered =
- resourceFilter.maybeFilter(resources, /* isDependency= */ false);
+ resourceFilter.maybeFilter(resources, /* isDependency= */ isDependency);
if (!filtered.isPresent()) {
// Nothing was filtered out
- return this;
+ return Optional.empty();
}
- return withResources(ruleErrorConsumer, filtered.get(), "resource_files");
- }
-
- @VisibleForTesting
- static LocalResourceContainer forResources(
- RuleErrorConsumer ruleErrorConsumer, ImmutableList<Artifact> resources)
- throws RuleErrorException {
- return new LocalResourceContainer(
- resources,
- getResourceRoots(ruleErrorConsumer, resources, "resource_files"),
- ImmutableList.of(),
- ImmutableList.of());
- }
+ // If the resources were filtered, also filter the resource roots
+ ImmutableList.Builder<PathFragment> filteredResourcesRootsBuilder = ImmutableList.builder();
+ for (PathFragment resourceRoot : resourceRoots) {
+ for (Artifact resource : filtered.get()) {
+ if (resource.getRootRelativePath().startsWith(resourceRoot)) {
+ filteredResourcesRootsBuilder.add(resourceRoot);
+ break;
+ }
+ }
+ }
- private LocalResourceContainer withResources(
- RuleErrorConsumer ruleErrorConsumer, ImmutableList<Artifact> resources, String resourcesAttr)
- throws RuleErrorException {
- return new LocalResourceContainer(
- resources,
- getResourceRoots(ruleErrorConsumer, resources, resourcesAttr),
- assets,
- assetRoots);
+ 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<Artifact> 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.<Artifact>of();
@@ -306,7 +306,7 @@ public final class DataBinding {
*/
static ImmutableList<Artifact> processDeps(RuleContext ruleContext) {
ImmutableList.Builder<Artifact> dataBindingJavaInputs = ImmutableList.<Artifact>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/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,46 +79,26 @@ public abstract class ResourceContainer {
@Nullable
public abstract Artifact getJavaClassJar();
- abstract ImmutableList<Artifact> getAssets();
+ abstract AndroidAssets getAssets();
@VisibleForTesting
- public abstract ImmutableList<Artifact> getResources();
+ public abstract AndroidResources getResources();
+ /** @deprecated We are moving towards decoupling assets and resources */
+ @Deprecated
public ImmutableList<Artifact> 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<Artifact> getArtifacts() {
- return Iterables.concat(getAssets(), getResources());
+ return Iterables.concat(getAssets().getAssets(), getResources().getResources());
}
/**
- * Gets the directories containing the assets.
- *
- * <p>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<PathFragment> getAssetsRoots();
-
- /**
- * Gets the directories containing the resources.
- *
- * <p>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<PathFragment> getResourcesRoots();
-
- /**
* Gets the directories containing the resources of a specific type.
*
* <p>TODO(b/30308041): Stop using these directories, and remove this code.
@@ -130,7 +110,9 @@ public abstract class ResourceContainer {
*/
@Deprecated
public ImmutableList<PathFragment> 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<ImmutableList<Artifact>> filteredResources =
- filter.maybeFilter(getResources(), isDependency);
+ Optional<AndroidResources> 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<PathFragment> 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.<Artifact>of())
- .setResources(ImmutableList.<Artifact>of())
- .setAssetsRoots(ImmutableList.<PathFragment>of())
- .setResourcesRoots(ImmutableList.<PathFragment>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.
- *
- * <p>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<Artifact> assets);
-
- public abstract Builder setResources(ImmutableList<Artifact> resources);
-
- public abstract Builder setAssetsRoots(ImmutableList<PathFragment> assetsRoots);
+ public abstract Builder setAssets(AndroidAssets assets);
- public abstract Builder setResourcesRoots(ImmutableList<PathFragment> 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.<ResourceContainer>naiveLinkOrder().add(newDirectResource).build(),
NestedSetBuilder.<Artifact>naiveLinkOrder()
.addTransitive(transitiveResources)
- .addAll(newDirectResource.getResources())
+ .addAll(newDirectResource.getResources().getResources())
.build(),
NestedSetBuilder.<Artifact>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 <resource directory>/{"
- + 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 <resource directory>/{"
- + 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"
+ " <resource directory>/{"
- + 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 <resource directory>/{"
- + 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/LocalResourceContainerTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java
index 550f86beb6..056970f322 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/LocalResourceContainerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java
@@ -18,17 +18,19 @@ 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 LocalResourceContainer} */
+/** Tests {@link AndroidResourcesTest} */
@RunWith(JUnit4.class)
-public class LocalResourceContainerTest extends ResourceTestBase {
+public class AndroidResourcesTest extends ResourceTestBase {
private static final PathFragment DEFAULT_RESOURCE_ROOT = PathFragment.create(RESOURCE_ROOT);
private static final ImmutableList<PathFragment> RESOURCES_ROOTS =
ImmutableList.of(DEFAULT_RESOURCE_ROOT);
@@ -84,7 +86,7 @@ public class LocalResourceContainerTest extends ResourceTestBase {
private ImmutableList<PathFragment> getResourceRoots(ImmutableList<Artifact> artifacts)
throws Exception {
- return LocalResourceContainer.getResourceRoots(errorConsumer, artifacts, "resource_files");
+ return AndroidResources.getResourceRoots(errorConsumer, artifacts, "resource_files");
}
@Test
@@ -112,36 +114,58 @@ public class LocalResourceContainerTest extends ResourceTestBase {
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<Artifact> unfilteredResources, ImmutableList<Artifact> filteredResources)
throws Exception {
+ assertFilter(unfilteredResources, filteredResources, /* isDependency = */ false);
+ }
+
+ private void assertFilter(
+ ImmutableList<Artifact> unfilteredResources,
+ ImmutableList<Artifact> filteredResources,
+ boolean isDependency)
+ throws Exception {
ImmutableList<PathFragment> unfilteredResourcesRoots = getResourceRoots(unfilteredResources);
- LocalResourceContainer unfiltered =
- new LocalResourceContainer(
- unfilteredResources,
- unfilteredResourcesRoots,
- unfilteredResources,
- unfilteredResourcesRoots);
+ AndroidResources unfiltered =
+ new AndroidResources(unfilteredResources, unfilteredResourcesRoots);
+
+ ImmutableList.Builder<Artifact> filteredDepsBuilder = ImmutableList.builder();
ResourceFilter fakeFilter =
- ResourceFilter.of(ImmutableSet.copyOf(filteredResources), (artifact) -> {});
+ ResourceFilter.of(ImmutableSet.copyOf(filteredResources), filteredDepsBuilder::add);
- LocalResourceContainer filtered = unfiltered.filter(errorConsumer, fakeFilter);
+ Optional<AndroidResources> filtered = unfiltered.maybeFilter(fakeFilter, isDependency);
- if (unfilteredResources.equals(filteredResources)) {
- // The filtering was a no-op; the original object, not a copy, should be returned
- assertThat(filtered).isSameAs(unfiltered);
+ 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.getResources()).containsExactlyElementsIn(filteredResources).inOrder();
- assertThat(filtered.getResourceRoots())
+ assertThat(filtered.get().getResources())
+ .containsExactlyElementsIn(filteredResources)
+ .inOrder();
+ assertThat(filtered.get().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());
+ 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/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<Artifact> doFilter(
ResourceFilterFactory resourceFilterFactory, ImmutableList<Artifact> 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();
}
}