Skip to content

Commit

Permalink
Do not flatten resource artifacts in Android rules.
Browse files Browse the repository at this point in the history 
This involves rather tediously rolling up each artifact type in its own individual nested set.

PiperOrigin-RevId: 168729120
  • Loading branch information
@tomlu @philwo
tomlu authored and philwo committed Sep 15, 2017
1 parent d9b141d commit f38b433
Show file tree
Hide file tree
Showing 6 changed files with 308 additions and 154 deletions.
15 changes: 7 additions & 8 deletions ...java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@
*/
public class AndroidResourceMergingActionBuilder {

private static final ResourceContainerConverter.ToArtifacts RESOURCE_CONTAINER_TO_ARTIFACTS =
ResourceContainerConverter.builder()
.includeResourceRoots()
.includeSymbolsBin()
.toArtifactConverter();
private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
ResourceContainerConverter.builder()
.includeResourceRoots()
Expand Down Expand Up @@ -138,14 +133,18 @@ public ResourceContainer build(ActionConstructionContext context) {

Preconditions.checkNotNull(primary);
builder.add("--primaryData", RESOURCE_CONTAINER_TO_ARG.apply(primary));
inputs.addTransitive(RESOURCE_CONTAINER_TO_ARTIFACTS.apply(primary));
inputs.addAll(primary.getArtifacts());
inputs.add(primary.getSymbols());

Preconditions.checkNotNull(primary.getManifest());
builder.addExecPath("--primaryManifest", primary.getManifest());
inputs.add(primary.getManifest());

ResourceContainerConverter.convertDependencies(
dependencies, builder, inputs, RESOURCE_CONTAINER_TO_ARG, RESOURCE_CONTAINER_TO_ARTIFACTS);
if (dependencies != null) {
ResourceContainerConverter.addToCommandLine(dependencies, builder, RESOURCE_CONTAINER_TO_ARG);
inputs.addTransitive(dependencies.getTransitiveResourceRoots());
inputs.addTransitive(dependencies.getTransitiveSymbolsBin());
}

List<Artifact> outs = new ArrayList<>();
if (classJarOut != null) {
Expand Down
47 changes: 19 additions & 28 deletions ...in/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,6 @@
*/
public class AndroidResourcesProcessorBuilder {

private static final ResourceContainerConverter.ToArtifacts AAPT2_RESOURCE_DEP_TO_ARTIFACTS =
ResourceContainerConverter.builder()
.includeResourceRoots()
.includeManifest()
.includeAapt2RTxt()
.includeSymbolsBin()
.includeStaticLibrary()
.toArtifactConverter();

private static final ResourceContainerConverter.ToArg AAPT2_RESOURCE_DEP_TO_ARG =
ResourceContainerConverter.builder()
.includeResourceRoots()
Expand All @@ -56,20 +47,6 @@ public class AndroidResourcesProcessorBuilder {
.withSeparator(SeparatorType.COLON_COMMA)
.toArgConverter();

private static final ResourceContainerConverter.ToArtifacts RESOURCE_CONTAINER_TO_ARTIFACTS =
ResourceContainerConverter.builder()
.includeResourceRoots()
.includeManifest()
.toArtifactConverter();

private static final ResourceContainerConverter.ToArtifacts RESOURCE_DEP_TO_ARTIFACTS =
ResourceContainerConverter.builder()
.includeResourceRoots()
.includeManifest()
.includeRTxt()
.includeSymbolsBin()
.toArtifactConverter();

private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
ResourceContainerConverter.builder()
.includeResourceRoots()
Expand Down Expand Up @@ -289,8 +266,15 @@ private ResourceContainer createAapt2ApkAction(ActionConstructionContext context
builder.add("--tool").add("AAPT2_PACKAGE").add("--");

builder.addExecPath("--aapt2", sdk.getAapt2().getExecutable());
ResourceContainerConverter.convertDependencies(
dependencies, builder, inputs, AAPT2_RESOURCE_DEP_TO_ARG, AAPT2_RESOURCE_DEP_TO_ARTIFACTS);
if (dependencies != null) {
ResourceContainerConverter.addToCommandLine(dependencies, builder, AAPT2_RESOURCE_DEP_TO_ARG);
inputs
.addTransitive(dependencies.getTransitiveResourceRoots())
.addTransitive(dependencies.getTransitiveManifests())
.addTransitive(dependencies.getTransitiveAapt2RTxt())
.addTransitive(dependencies.getTransitiveSymbolsBin())
.addTransitive(dependencies.getTransitiveStaticLib());
}

configureCommonFlags(outs, inputs, builder);

Expand Down Expand Up @@ -349,8 +333,14 @@ private ResourceContainer createAaptAction(ActionConstructionContext context) {
// Set the busybox tool.
builder.add("--tool").add("PACKAGE").add("--");

ResourceContainerConverter.convertDependencies(
dependencies, builder, inputs, RESOURCE_DEP_TO_ARG, RESOURCE_DEP_TO_ARTIFACTS);
if (dependencies != null) {
ResourceContainerConverter.addToCommandLine(dependencies, builder, RESOURCE_DEP_TO_ARG);
inputs
.addTransitive(dependencies.getTransitiveResourceRoots())
.addTransitive(dependencies.getTransitiveManifests())
.addTransitive(dependencies.getTransitiveRTxt())
.addTransitive(dependencies.getTransitiveSymbolsBin());
}
builder.addExecPath("--aapt", sdk.getAapt().getExecutable());
configureCommonFlags(outs, inputs, builder);

Expand Down Expand Up @@ -404,7 +394,8 @@ private void configureCommonFlags(

// Add data
builder.add("--primaryData", RESOURCE_CONTAINER_TO_ARG.apply(primary));
inputs.addTransitive(RESOURCE_CONTAINER_TO_ARTIFACTS.apply(primary));
inputs.addAll(primary.getArtifacts());
inputs.add(primary.getManifest());

if (!Strings.isNullOrEmpty(sdk.getBuildToolsVersion())) {
builder.add("--buildToolsVersion", sdk.getBuildToolsVersion());
Expand Down
30 changes: 29 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.android;

import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
Expand All @@ -28,9 +29,24 @@ public static AndroidResourcesProvider create(
Label label,
NestedSet<ResourceContainer> transitiveAndroidResources,
NestedSet<ResourceContainer> directAndroidResources,
NestedSet<Artifact> transitiveResourceRoots,
NestedSet<Artifact> transitiveManifests,
NestedSet<Artifact> transitiveAapt2RTxt,
NestedSet<Artifact> transitiveSymbolsBin,
NestedSet<Artifact> transitiveStaticLib,
NestedSet<Artifact> transitiveRTxt,
boolean isResourcesOnly) {
return new AutoValue_AndroidResourcesProvider(
label, transitiveAndroidResources, directAndroidResources, isResourcesOnly);
label,
transitiveAndroidResources,
directAndroidResources,
transitiveResourceRoots,
transitiveManifests,
transitiveAapt2RTxt,
transitiveSymbolsBin,
transitiveStaticLib,
transitiveRTxt,
isResourcesOnly);
}

/** Returns the label that is associated with this piece of information. */
Expand All @@ -42,6 +58,18 @@ public static AndroidResourcesProvider create(
/** Returns the immediate ResourceContainers for the label. */
public abstract NestedSet<ResourceContainer> getDirectAndroidResources();

public abstract NestedSet<Artifact> getTransitiveResourceRoots();

public abstract NestedSet<Artifact> getTransitiveManifests();

public abstract NestedSet<Artifact> getTransitiveAapt2RTxt();

public abstract NestedSet<Artifact> getTransitiveSymbolsBin();

public abstract NestedSet<Artifact> getTransitiveStaticLib();

public abstract NestedSet<Artifact> getTransitiveRTxt();

/**
* Returns whether the targets contained within this provider only represent android resources or
* also contain other information.
Expand Down
88 changes: 12 additions & 76 deletions src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,11 @@
import com.google.common.base.Functions;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterators;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType;
import javax.annotation.Nullable;

/**
* Factory for functions to convert a {@link ResourceContainer} to a commandline argument, or a
Expand All @@ -47,10 +41,6 @@ interface ToArg extends Function<ResourceContainer, String> {
String listSeparator();
}

interface ToArtifacts extends Function<ResourceContainer, NestedSet<Artifact>> {

}

static class Builder {

private boolean includeResourceRoots;
Expand Down Expand Up @@ -183,41 +173,6 @@ public String listSeparator() {
}
};
}

ToArtifacts toArtifactConverter() {
return new ToArtifacts() {
@Override
public NestedSet<Artifact> apply(ResourceContainer container) {
NestedSetBuilder<Artifact> artifacts = NestedSetBuilder.naiveLinkOrder();
if (includeResourceRoots) {
artifacts.addAll(container.getArtifacts());
}
if (includeManifest) {
addIfNotNull(container.getManifest(), artifacts);
}
if (includeRTxt) {
addIfNotNull(container.getRTxt(), artifacts);
}
if (includeSymbolsBin) {
addIfNotNull(container.getSymbols(), artifacts);
}
if (includeAapt2RTxt) {
addIfNotNull(container.getAapt2RTxt(), artifacts);
}
if (includeStaticLibrary) {
addIfNotNull(container.getStaticLibrary(), artifacts);
}
return artifacts.build();
}
};
}
}

private static void addIfNotNull(
@Nullable Artifact artifact, NestedSetBuilder<Artifact> artifacts) {
if (artifact != null) {
artifacts.add(artifact);
}
}

@VisibleForTesting
Expand All @@ -232,36 +187,17 @@ public static String convertRoots(ResourceContainer container, ResourceType reso
* Convert ResourceDependencies to commandline args and artifacts, assuming the commandline
* arguments should be split into direct deps and transitive deps.
*/
static void convertDependencies(
ResourceDependencies dependencies,
CustomCommandLine.Builder cmdBuilder,
NestedSetBuilder<Artifact> inputs,
ToArg toArg,
ToArtifacts toArtifacts) {

if (dependencies != null) {
if (!dependencies.getTransitiveResources().isEmpty()) {
cmdBuilder.addAll(
"--data",
VectorArg.join(toArg.listSeparator())
.each(dependencies.getTransitiveResources())
.mapped(toArg));
}
if (!dependencies.getDirectResources().isEmpty()) {
cmdBuilder.addAll(
"--directData",
VectorArg.join(toArg.listSeparator())
.each(dependencies.getDirectResources())
.mapped(toArg));
}
// This flattens the nested set. Since each ResourceContainer needs to be transformed into
// Artifacts, and the NestedSetBuilder.wrap doesn't support lazy Iterator evaluation
// and SpawnActionBuilder.addInputs evaluates Iterables, it becomes necessary to make the
// best effort and let it get flattened.
inputs.addTransitive(
NestedSetBuilder.wrap(
Order.NAIVE_LINK_ORDER,
FluentIterable.from(dependencies.getResources()).transformAndConcat(toArtifacts)));
}
static void addToCommandLine(
ResourceDependencies dependencies, CustomCommandLine.Builder cmdBuilder, ToArg toArg) {
cmdBuilder.addAll(
"--data",
VectorArg.join(toArg.listSeparator())
.each(dependencies.getTransitiveResources())
.mapped(toArg));
cmdBuilder.addAll(
"--directData",
VectorArg.join(toArg.listSeparator())
.each(dependencies.getDirectResources())
.mapped(toArg));
}
}
Loading

0 comments on commit f38b433

Please sign in to comment.