Automated rollback of commit 9d5c323a6e66842cfeb98462cf949dee58710fb8. *** Reason for rollback *** Crashes lots of targets in nightly blaze-2018.05.24-1: PiperOrigin-RevId: 198049395
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 1e9f5c5..ed24747 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
@@ -100,7 +100,8 @@ final AndroidDataContext dataContext = androidSemantics.makeContextForNative(ruleContext); final ResourceApk resourceApk; if (AndroidResources.decoupleDataProcessing(dataContext)) { - StampedAndroidManifest manifest = AndroidManifest.forAarImport(androidManifestArtifact); + StampedAndroidManifest manifest = + AndroidManifest.forAarImport(androidManifestArtifact); boolean neverlink = JavaCommon.isNeverLink(ruleContext); ValidatedAndroidResources validatedResources = @@ -121,7 +122,6 @@ resourceApk = androidManifest.packAarWithDataAndResources( ruleContext, - dataContext, AndroidAssets.forAarImport(assets), AndroidResources.forAarImport(resources), ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)), @@ -168,6 +168,8 @@ .addCompileTimeJarAsFullJar(mergedJar) .build()); + + JavaConfiguration javaConfig = ruleContext.getFragment(JavaConfiguration.class); // TODO(cnsun): need to pass the transitive classpath too to emit add dep command. NestedSet<Artifact> directDeps = getCompileTimeJarsFromCollection(targets, /*isStrict=*/ true);
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 96d4fb3..0bc2467 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
@@ -261,7 +261,6 @@ resourceApk = applicationManifest.packBinaryWithDataAndResources( ruleContext, - dataContext, ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK), resourceDeps, ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java index b344411..0c8d4cf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinaryMobileInstall.java
@@ -101,7 +101,6 @@ .addMobileInstallStubApplication(ruleContext) .packIncrementalBinaryWithDataAndResources( ruleContext, - dataContext, ruleContext.getImplicitOutputArtifact( AndroidRuleClasses.ANDROID_INCREMENTAL_RESOURCES_APK), resourceDeps, @@ -115,7 +114,6 @@ .createSplitManifest(ruleContext, "android_resources", false) .packIncrementalBinaryWithDataAndResources( ruleContext, - dataContext, getMobileInstallArtifact(ruleContext, "android_resources.ap_"), resourceDeps, ruleContext.getExpander().withDataLocations().tokenized("nocompress_extensions"),
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 f4ed5d5..67dbd5a 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
@@ -169,7 +169,6 @@ resourceApk = applicationManifest.packLibraryWithDataAndResources( ruleContext, - dataContext, resourceDeps, ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT), ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS),
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 2dccf13..e32d12c 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
@@ -117,7 +117,6 @@ resourceApk = applicationManifest.packBinaryWithDataAndResources( ruleContext, - dataContext, ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK), resourceDependencies, ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifestMergeHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifestMergeHelper.java index 4d99f6d..be6c9ba 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifestMergeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifestMergeHelper.java
@@ -71,4 +71,4 @@ .addCommandLine(commandLine.build()) .build(context)); } -} +} \ No newline at end of file
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java index 9d945e2..1d52563 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
@@ -14,10 +14,21 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; +import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType; +import com.google.devtools.build.lib.util.OS; +import java.util.ArrayList; +import java.util.List; import javax.annotation.Nullable; /** @@ -41,6 +52,9 @@ .withArtifact(CompiledMergableAndroidData::getCompiledSymbols) .build(); + private final RuleContext ruleContext; + private final AndroidSdkProvider sdk; + // Inputs private CompiledMergableAndroidData primary; private ResourceDependencies dependencies; @@ -56,6 +70,12 @@ private boolean throwOnResourceConflict; private boolean useCompiledMerge; + /** @param ruleContext The RuleContext that was used to create the SpawnAction.Builder. */ + public AndroidResourceMergingActionBuilder(RuleContext ruleContext) { + this.ruleContext = ruleContext; + this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); + } + /** * The primary resource for merging. This resource will overwrite any resource or data value in * the transitive closure. @@ -112,102 +132,172 @@ return this; } - private BusyBoxActionBuilder createInputsForBuilder(BusyBoxActionBuilder builder) { - return builder - .addAndroidJar() - .addInput("--primaryManifest", primary.getManifest()) - .maybeAddFlag("--packageForR", customJavaPackage) - .maybeAddFlag("--throwOnResourceConflict", throwOnResourceConflict); - } + private NestedSetBuilder<Artifact> createInputsForBuilder(CustomCommandLine.Builder builder) { + // Use a FluentIterable to avoid flattening the NestedSets + NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); - private void buildCompiledResourceMergingAction(BusyBoxActionBuilder builder) { - Preconditions.checkNotNull(primary); + builder.addExecPath("--androidJar", sdk.getAndroidJar()); + inputs.add(sdk.getAndroidJar()); - createInputsForBuilder(builder) - .addInput( - "--primaryData", - RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED.map(primary), - Iterables.concat( - primary.getArtifacts(), ImmutableList.of(primary.getCompiledSymbols()))); + Preconditions.checkNotNull(primary.getManifest()); + builder.addExecPath("--primaryManifest", primary.getManifest()); + inputs.add(primary.getManifest()); - if (dependencies != null) { - builder - .addTransitiveFlag( - "--data", - dependencies.getTransitiveResourceContainers(), - RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED) - .addTransitiveFlag( - "--directData", - dependencies.getDirectResourceContainers(), - RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED) - .addTransitiveInputValues(dependencies.getTransitiveResources()) - .addTransitiveInputValues(dependencies.getTransitiveAssets()) - .addTransitiveInputValues(dependencies.getTransitiveCompiledSymbols()); + if (!Strings.isNullOrEmpty(customJavaPackage)) { + // Sets an alternative java package for the generated R.java + // this allows android rules to generate resources outside of the java{,tests} tree. + builder.add("--packageForR", customJavaPackage); } - builder.buildAndRegister("Merging compiled Android resources", "AndroidCompiledResourceMerger"); - } - - private void buildParsedResourceMergingAction(BusyBoxActionBuilder builder) { - Preconditions.checkNotNull(primary); - - createInputsForBuilder(builder) - .addInput( - "--primaryData", - RESOURCE_CONTAINER_TO_ARG.map(primary), - Iterables.concat(primary.getArtifacts(), ImmutableList.of(primary.getSymbols()))); - - if (dependencies != null) { - builder - .addTransitiveFlag( - "--data", dependencies.getTransitiveResourceContainers(), RESOURCE_CONTAINER_TO_ARG) - .addTransitiveFlag( - "--directData", dependencies.getDirectResourceContainers(), RESOURCE_CONTAINER_TO_ARG) - .addTransitiveInputValues(dependencies.getTransitiveResources()) - .addTransitiveInputValues(dependencies.getTransitiveAssets()) - .addTransitiveInputValues(dependencies.getTransitiveSymbolsBin()); + if (throwOnResourceConflict) { + builder.add("--throwOnResourceConflict"); } - builder.buildAndRegister("Merging Android resources", "AndroidResourceMerger"); + return inputs; } - private void build(AndroidDataContext dataContext) { - BusyBoxActionBuilder parsedMergeBuilder = BusyBoxActionBuilder.create(dataContext, "MERGE"); - BusyBoxActionBuilder compiledMergeBuilder = - BusyBoxActionBuilder.create(dataContext, "MERGE_COMPILED"); + private void buildCompiledResourceMergingAction( + CustomCommandLine.Builder builder, + List<Artifact> outputs, + ActionConstructionContext context) { + NestedSetBuilder<Artifact> inputs = createInputsForBuilder(builder); - parsedMergeBuilder.addOutput("--resourcesOutput", mergedResourcesOut); + Preconditions.checkNotNull(primary); + builder.add("--primaryData", RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED.map(primary)); + inputs.addAll(primary.getArtifacts()); + inputs.add(primary.getCompiledSymbols()); + + if (dependencies != null) { + RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED.addDepsToCommandLine( + builder, + dependencies.getDirectResourceContainers(), + dependencies.getTransitiveResourceContainers()); + inputs.addTransitive(dependencies.getTransitiveResources()); + inputs.addTransitive(dependencies.getTransitiveAssets()); + inputs.addTransitive(dependencies.getTransitiveCompiledSymbols()); + } + + SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + ParamFileInfo.Builder compiledParamFileInfo = ParamFileInfo.builder(ParameterFileType.UNQUOTED); + compiledParamFileInfo.setUseAlways(OS.getCurrent() == OS.WINDOWS); + // Create the spawn action. + ruleContext.registerAction( + spawnActionBuilder + .useDefaultShellEnvironment() + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.copyOf(outputs)) + .addCommandLine(builder.build(), compiledParamFileInfo.build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Merging compiled Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AndroidCompiledResourceMerger") + .build(context)); + } + + private void buildParsedResourceMergingAction( + CustomCommandLine.Builder builder, + List<Artifact> outputs, + ActionConstructionContext context) { + NestedSetBuilder<Artifact> inputs = createInputsForBuilder(builder); + + Preconditions.checkNotNull(primary); + builder.add("--primaryData", RESOURCE_CONTAINER_TO_ARG.map(primary)); + inputs.addAll(primary.getArtifacts()); + inputs.add(primary.getSymbols()); + + if (dependencies != null) { + RESOURCE_CONTAINER_TO_ARG.addDepsToCommandLine( + builder, + dependencies.getDirectResourceContainers(), + dependencies.getTransitiveResourceContainers()); + inputs.addTransitive(dependencies.getTransitiveResources()); + inputs.addTransitive(dependencies.getTransitiveAssets()); + inputs.addTransitive(dependencies.getTransitiveSymbolsBin()); + } + + SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); + // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special + // characters (colon, semicolon, hashmark, ampersand) that don't work on Windows, and quoting + // semantics are very complicated (more so than in Bash), so let's just always use a parameter + // file. + // TODO(laszlocsomor), TODO(corysmith): restructure the Android BusyBux's flags by deprecating + // list-type and list-of-list-type flags that use such problematic separators in favor of + // multi-value flags (to remove one level of listing) and by changing all list separators to a + // platform-safe character (= comma). + paramFileInfo.setUseAlways(OS.getCurrent() == OS.WINDOWS); + + // Create the spawn action. + ruleContext.registerAction( + spawnActionBuilder + .useDefaultShellEnvironment() + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.copyOf(outputs)) + .addCommandLine(builder.build(), paramFileInfo.build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Merging Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AndroidResourceMerger") + .build(context)); + } + + private void build(RuleContext context) { + CustomCommandLine.Builder parsedMergeBuilder = + new CustomCommandLine.Builder().add("--tool").add("MERGE").add("--"); + CustomCommandLine.Builder compiledMergeBuilder = + new CustomCommandLine.Builder().add("--tool").add("MERGE_COMPILED").add("--"); + List<Artifact> parsedMergeOutputs = new ArrayList<>(); + List<Artifact> compiledMergeOutputs = new ArrayList<>(); + + if (mergedResourcesOut != null) { + parsedMergeBuilder.addExecPath("--resourcesOutput", mergedResourcesOut); + parsedMergeOutputs.add(mergedResourcesOut); + } // TODO(corysmith): Move the data binding parsing out of the merging pass to enable faster // aapt2 builds. - parsedMergeBuilder.maybeAddOutput("--dataBindingInfoOut", dataBindingInfoZip); - - (useCompiledMerge ? compiledMergeBuilder : parsedMergeBuilder) - .addOutput("--classJarOutput", classJarOut) - .addLabelFlag("--targetLabel") - - // For now, do manifest processing to remove placeholders that aren't handled by the legacy - // manifest merger. Remove this once enough users migrate over to the new manifest merger. - .maybeAddOutput("--manifestOutput", manifestOut); - - if (useCompiledMerge) { - buildCompiledResourceMergingAction(compiledMergeBuilder); + if (dataBindingInfoZip != null) { + parsedMergeBuilder.addExecPath("--dataBindingInfoOut", dataBindingInfoZip); + parsedMergeOutputs.add(dataBindingInfoZip); } - // Always make an action for merging parsed resources - the merged resources are still created - // this way. - buildParsedResourceMergingAction(parsedMergeBuilder); + CustomCommandLine.Builder jarAndManifestBuilder = + useCompiledMerge ? compiledMergeBuilder : parsedMergeBuilder; + List<Artifact> jarAndManifestOutputs = + useCompiledMerge ? compiledMergeOutputs : parsedMergeOutputs; + + if (classJarOut != null) { + jarAndManifestBuilder.addExecPath("--classJarOutput", classJarOut); + jarAndManifestBuilder.addLabel("--targetLabel", ruleContext.getLabel()); + jarAndManifestOutputs.add(classJarOut); + } + + // For now, do manifest processing to remove placeholders that aren't handled by the legacy + // manifest merger. Remove this once enough users migrate over to the new manifest merger. + if (manifestOut != null) { + jarAndManifestBuilder.addExecPath("--manifestOutput", manifestOut); + jarAndManifestOutputs.add(manifestOut); + } + + if (!compiledMergeOutputs.isEmpty()) { + buildCompiledResourceMergingAction(compiledMergeBuilder, compiledMergeOutputs, context); + } + + if (!parsedMergeOutputs.isEmpty()) { + buildParsedResourceMergingAction(parsedMergeBuilder, parsedMergeOutputs, context); + } } - public ResourceContainer build( - AndroidDataContext dataContext, ResourceContainer resourceContainer) { - withPrimary(resourceContainer).build(dataContext); + public ResourceContainer build(RuleContext ruleContext, ResourceContainer resourceContainer) { + withPrimary(resourceContainer).build(ruleContext); // Return the full set of processed transitive dependencies. ResourceContainer.Builder result = resourceContainer.toBuilder(); - - result.setJavaClassJar(classJarOut); - + if (classJarOut != null) { + // ensure the classJar is propagated if it exists. Otherwise, AndroidCommon tries to make it. + // TODO(corysmith): Centralize the class jar generation. + result.setJavaClassJar(classJarOut); + } if (manifestOut != null) { result.setManifest(manifestOut); } @@ -217,9 +307,8 @@ return result.build(); } - public MergedAndroidResources build( - AndroidDataContext dataContext, ParsedAndroidResources parsed) { - withPrimary(parsed).build(dataContext); + public MergedAndroidResources build(RuleContext ruleContext, ParsedAndroidResources parsed) { + withPrimary(parsed).build(ruleContext); return MergedAndroidResources.of( parsed,
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 1397655..a3bd477 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
@@ -15,16 +15,31 @@ import static java.util.stream.Collectors.joining; -import com.google.common.collect.Iterables; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Streams; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; +import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; +import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.ArrayList; +import java.util.List; import javax.annotation.Nullable; /** Builder for creating $android_resource_parser action. */ public class AndroidResourceParsingActionBuilder { + private final RuleContext ruleContext; + private final AndroidSdkProvider sdk; + // These are only needed when parsing resources with data binding @Nullable private Artifact manifest; @Nullable private String javaPackage; @@ -39,6 +54,12 @@ @Nullable private Artifact compiledSymbols; @Nullable private Artifact dataBindingInfoZip; + /** @param ruleContext The RuleContext that was used to create the SpawnAction.Builder. */ + public AndroidResourceParsingActionBuilder(RuleContext ruleContext) { + this.ruleContext = ruleContext; + this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); + } + /** Set the artifact location for the output protobuf. */ public AndroidResourceParsingActionBuilder setOutput(Artifact output) { this.output = output; @@ -82,32 +103,86 @@ return Streams.stream(roots).map(Object::toString).collect(joining("#")); } - private void build(AndroidDataContext dataContext) { + private void build(ActionConstructionContext context) { + CustomCommandLine.Builder builder = new CustomCommandLine.Builder(); + + // Set the busybox tool. + builder.add("--tool").add("PARSE").add("--"); + + NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); + String resourceDirectories = convertRoots(resources.getResourceRoots()) + ":" + convertRoots(assets.getAssetRoots()); - Iterable<Artifact> resourceArtifacts = - Iterables.concat(assets.getAssets(), resources.getResources()); + builder.add("--primaryData", resourceDirectories); + inputs.addTransitive( + NestedSetBuilder.<Artifact>naiveLinkOrder() + .addAll(assets.getAssets()) + .addAll(resources.getResources()) + .build()); - BusyBoxActionBuilder.create(dataContext, "PARSE") - .addInput("--primaryData", resourceDirectories, resourceArtifacts) - .addOutput("--output", output) - .buildAndRegister("Parsing Android resources", "AndroidResourceParser"); + Preconditions.checkNotNull(output); + builder.addExecPath("--output", output); + + SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); + // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special + // characters (colon, semicolon, hashmark, ampersand) that don't work on Windows, and quoting + // semantics are very complicated (more so than in Bash), so let's just always use a parameter + // file. + // TODO(laszlocsomor), TODO(corysmith): restructure the Android BusyBux's flags by deprecating + // list-type and list-of-list-type flags that use such problematic separators in favor of + // multi-value flags (to remove one level of listing) and by changing all list separators to a + // platform-safe character (= comma). + paramFileInfo.setUseAlways(OS.getCurrent() == OS.WINDOWS); + + // Create the spawn action. + ruleContext.registerAction( + spawnActionBuilder + .useDefaultShellEnvironment() + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.of(output)) + .addCommandLine(builder.build(), paramFileInfo.build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Parsing Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AndroidResourceParser") + .build(context)); if (compiledSymbols != null) { - BusyBoxActionBuilder compiledBuilder = - BusyBoxActionBuilder.create(dataContext, "COMPILE_LIBRARY_RESOURCES") - .addAapt(AndroidAaptVersion.AAPT2) - .addInput("--resources", resourceDirectories, resourceArtifacts) - .addOutput("--output", compiledSymbols); + List<Artifact> outs = new ArrayList<>(); + CustomCommandLine.Builder flatFileBuilder = new CustomCommandLine.Builder(); + flatFileBuilder + .add("--tool") + .add("COMPILE_LIBRARY_RESOURCES") + .add("--") + .addExecPath("--aapt2", sdk.getAapt2().getExecutable()) + .add("--resources", resourceDirectories) + .addExecPath("--output", compiledSymbols); + inputs.add(sdk.getAapt2().getExecutable()); + outs.add(compiledSymbols); + // The databinding needs to be processed before compilation, so the stripping happens here. if (dataBindingInfoZip != null) { - compiledBuilder - .addInput("--manifest", manifest) - .maybeAddFlag("--packagePath", javaPackage) - .addOutput("--dataBindingInfoOut", dataBindingInfoZip); + flatFileBuilder.addExecPath("--manifest", manifest); + inputs.add(manifest); + if (!Strings.isNullOrEmpty(javaPackage)) { + flatFileBuilder.add("--packagePath", javaPackage); + } + flatFileBuilder.addExecPath("--dataBindingInfoOut", dataBindingInfoZip); + outs.add(dataBindingInfoZip); } - - compiledBuilder.buildAndRegister("Compiling Android resources", "AndroidResourceCompiler"); + // Create the spawn action. + ruleContext.registerAction( + new SpawnAction.Builder() + .useDefaultShellEnvironment() + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.copyOf(outs)) + .addCommandLine(flatFileBuilder.build(), paramFileInfo.build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Compiling Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AndroidResourceCompiler") + .build(context)); } } @@ -116,9 +191,7 @@ * parsed and compiled information. */ public ParsedAndroidResources build( - AndroidDataContext dataContext, - AndroidResources androidResources, - StampedAndroidManifest manifest) { + AndroidResources androidResources, StampedAndroidManifest manifest) { if (dataBindingInfoZip != null) { // Manifest information is needed for data binding setManifest(manifest.getManifest()); @@ -126,17 +199,17 @@ } setResources(androidResources); - build(dataContext); + build(ruleContext); return ParsedAndroidResources.of( - androidResources, output, compiledSymbols, dataContext.getLabel(), manifest); + androidResources, output, compiledSymbols, ruleContext.getLabel(), manifest); } - public ParsedAndroidAssets build(AndroidDataContext dataContext, AndroidAssets assets) { + public ParsedAndroidAssets build(AndroidAssets assets) { setAssets(assets); - build(dataContext); + build(ruleContext); - return ParsedAndroidAssets.of(assets, output, dataContext.getLabel()); + return ParsedAndroidAssets.of(assets, output, ruleContext.getLabel()); } /** @@ -144,8 +217,8 @@ * symbols. */ public ResourceContainer buildAndUpdate( - AndroidDataContext dataContext, ResourceContainer resourceContainer) { - build(dataContext); + RuleContext ruleContext, ResourceContainer resourceContainer) { + build(ruleContext); ResourceContainer.Builder builder = resourceContainer
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java index 13a7c04..41abe2f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceValidatorActionBuilder.java
@@ -14,8 +14,19 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; +import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; +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.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import java.util.ArrayList; +import java.util.List; /** * Builder for creating $android_resource_validator action. This action validates merged resources @@ -26,6 +37,9 @@ */ public class AndroidResourceValidatorActionBuilder { + private final RuleContext ruleContext; + private final AndroidSdkProvider sdk; + // Inputs private CompiledMergableAndroidData primary; private Artifact mergedResources; @@ -44,6 +58,12 @@ private Artifact compiledSymbols; private Artifact apkOut; + /** @param ruleContext The RuleContext that was used to create the SpawnAction.Builder. */ + public AndroidResourceValidatorActionBuilder(RuleContext ruleContext) { + this.ruleContext = ruleContext; + this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); + } + public AndroidResourceValidatorActionBuilder setStaticLibraryOut(Artifact staticLibraryOut) { this.staticLibraryOut = staticLibraryOut; return this; @@ -96,19 +116,19 @@ return this; } - private void build(AndroidDataContext dataContext) { + private void build(ActionConstructionContext context) { if (rTxtOut != null) { - createValidateAction(dataContext); + createValidateAction(context); } if (compiledSymbols != null) { - createLinkStaticLibraryAction(dataContext); + createLinkStaticLibraryAction(context); } } public ResourceContainer build( - AndroidDataContext dataContext, ResourceContainer resourceContainer) { - withPrimary(resourceContainer).build(dataContext); + ActionConstructionContext context, ResourceContainer resourceContainer) { + withPrimary(resourceContainer).build(context); ResourceContainer.Builder builder = resourceContainer.toBuilder(); if (rTxtOut != null) { @@ -126,11 +146,17 @@ } public ValidatedAndroidResources build( - AndroidDataContext dataContext, MergedAndroidResources merged) { - withPrimary(merged).build(dataContext); + ActionConstructionContext context, MergedAndroidResources merged) { + withPrimary(merged).build(context); return ValidatedAndroidResources.of( - merged, rTxtOut, sourceJarOut, apkOut, aapt2RTxtOut, aapt2SourceJarOut, staticLibraryOut); + merged, + rTxtOut, + sourceJarOut, + apkOut, + aapt2RTxtOut, + aapt2SourceJarOut, + staticLibraryOut); } public AndroidResourceValidatorActionBuilder setCompiledSymbols(Artifact compiledSymbols) { @@ -149,47 +175,130 @@ * <p>This allows the link action to replace the validate action for builds that use aapt2, as * opposed to executing both actions. */ - private void createLinkStaticLibraryAction(AndroidDataContext dataContext) { + private void createLinkStaticLibraryAction(ActionConstructionContext context) { + Preconditions.checkNotNull(staticLibraryOut); + Preconditions.checkNotNull(aapt2SourceJarOut); + Preconditions.checkNotNull(aapt2RTxtOut); Preconditions.checkNotNull(resourceDeps); - BusyBoxActionBuilder builder = - BusyBoxActionBuilder.create(dataContext, "LINK_STATIC_LIBRARY") - .addAapt(AndroidAaptVersion.AAPT2) - .addInput("--libraries", dataContext.getSdk().getAndroidJar()) - .addInput("--compiled", compiledSymbols) - .addInput("--manifest", primary.getManifest()) + CustomCommandLine.Builder builder = new CustomCommandLine.Builder(); + ImmutableList.Builder<Artifact> inputs = ImmutableList.builder(); + ImmutableList.Builder<Artifact> outs = ImmutableList.builder(); - // Sets an alternative java package for the generated R.java - // this allows android rules to generate resources outside of the java{,tests} tree. - .maybeAddFlag("--packageForR", customJavaPackage); + // Set the busybox tool. + builder.add("--tool").add("LINK_STATIC_LIBRARY").add("--"); - if (!resourceDeps.getTransitiveCompiledSymbols().isEmpty()) { - builder.addTransitiveVectoredInput( - "--compiledDep", resourceDeps.getTransitiveCompiledSymbols()); + builder.addExecPath("--aapt2", sdk.getAapt2().getExecutable()); + + builder.add("--libraries").addExecPath(sdk.getAndroidJar()); + inputs.add(sdk.getAndroidJar()); + + builder.addExecPath("--compiled", compiledSymbols); + inputs.add(compiledSymbols); + + builder.addExecPath("--manifest", primary.getManifest()); + inputs.add(primary.getManifest()); + + if (!Strings.isNullOrEmpty(customJavaPackage)) { + // Sets an alternative java package for the generated R.java + // this allows android rules to generate resources outside of the java{,tests} tree. + builder.add("--packageForR", customJavaPackage); } - builder - .addOutput("--sourceJarOut", aapt2SourceJarOut) - .addOutput("--rTxtOut", aapt2RTxtOut) - .addOutput("--staticLibraryOut", staticLibraryOut) - .buildAndRegister("Linking static android resource library", "AndroidResourceLink"); + if (!resourceDeps.getTransitiveCompiledSymbols().isEmpty()) { + builder.addExecPaths( + "--compiledDep", + VectorArg.join(context.getConfiguration().getHostPathSeparator()) + .each(resourceDeps.getTransitiveCompiledSymbols())); + inputs.addAll(resourceDeps.getTransitiveCompiledSymbols()); + } + + builder.addExecPath("--sourceJarOut", aapt2SourceJarOut); + outs.add(aapt2SourceJarOut); + + builder.addExecPath("--rTxtOut", aapt2RTxtOut); + outs.add(aapt2RTxtOut); + + builder.addExecPath("--staticLibraryOut", staticLibraryOut); + outs.add(staticLibraryOut); + + ruleContext.registerAction( + new SpawnAction.Builder() + .useDefaultShellEnvironment() + .addTool(sdk.getAapt2()) + .addInputs(inputs.build()) + .addOutputs(outs.build()) + .addCommandLine( + builder.build(), ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage( + "Linking static android resource library for %s", ruleContext.getLabel()) + .setMnemonic("AndroidResourceLink") + .build(context)); } - private void createValidateAction(AndroidDataContext dataContext) { - BusyBoxActionBuilder.create(dataContext, "VALIDATE") - .maybeAddFlag("--buildToolsVersion", dataContext.getSdk().getBuildToolsVersion()) - .addAapt(AndroidAaptVersion.AAPT) - .addAndroidJar() - .addInput("--mergedResources", mergedResources) - .addInput("--manifest", primary.getManifest()) - .maybeAddFlag("--debug", debug) + private void createValidateAction(ActionConstructionContext context) { + CustomCommandLine.Builder builder = new CustomCommandLine.Builder(); - // Sets an alternative java package for the generated R.java - // this allows android rules to generate resources outside of the java{,tests} tree. - .maybeAddFlag("--packageForR", customJavaPackage) - .addOutput("--rOutput", rTxtOut) - .addOutput("--srcJarOutput", sourceJarOut) - .maybeAddOutput("--packagePath", apkOut) - .buildAndRegister("Validating Android resources", "AndroidResourceValidator"); + // Set the busybox tool. + builder.add("--tool").add("VALIDATE").add("--"); + + if (!Strings.isNullOrEmpty(sdk.getBuildToolsVersion())) { + builder.add("--buildToolsVersion", sdk.getBuildToolsVersion()); + } + + builder.addExecPath("--aapt", sdk.getAapt().getExecutable()); + + ImmutableList.Builder<Artifact> inputs = ImmutableList.builder(); + + builder.addExecPath("--androidJar", sdk.getAndroidJar()); + inputs.add(sdk.getAndroidJar()); + + Preconditions.checkNotNull(mergedResources); + builder.addExecPath("--mergedResources", mergedResources); + inputs.add(mergedResources); + + builder.addExecPath("--manifest", primary.getManifest()); + inputs.add(primary.getManifest()); + + if (debug) { + builder.add("--debug"); + } + + if (!Strings.isNullOrEmpty(customJavaPackage)) { + // Sets an alternative java package for the generated R.java + // this allows android rules to generate resources outside of the java{,tests} tree. + builder.add("--packageForR", customJavaPackage); + } + List<Artifact> outs = new ArrayList<>(); + Preconditions.checkNotNull(rTxtOut); + builder.addExecPath("--rOutput", rTxtOut); + outs.add(rTxtOut); + + Preconditions.checkNotNull(sourceJarOut); + builder.addExecPath("--srcJarOutput", sourceJarOut); + outs.add(sourceJarOut); + + if (apkOut != null) { + builder.addExecPath("--packagePath", apkOut); + outs.add(apkOut); + } + + SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + // Create the spawn action. + ruleContext.registerAction( + spawnActionBuilder + .useDefaultShellEnvironment() + .addTool(sdk.getAapt()) + .addInputs(inputs.build()) + .addOutputs(ImmutableList.copyOf(outs)) + .addCommandLine( + builder.build(), ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Validating Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AndroidResourceValidator") + .build(context)); } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java index a566f04..3417f53 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
@@ -13,45 +13,55 @@ // limitations under the License. package com.google.devtools.build.lib.rules.android; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.analysis.RuleContext; +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.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; -import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType; +import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg; +import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes; +import com.google.devtools.build.lib.util.OS; +import java.util.ArrayList; import java.util.Collections; import java.util.List; /** Builder for creating resource processing action. */ public class AndroidResourcesProcessorBuilder { - private static final AndroidDataConverter<ValidatedAndroidData> AAPT2_RESOURCE_DEP_TO_ARG = - AndroidDataConverter.<ValidatedAndroidData>builder(JoinerType.COLON_COMMA) - .withRoots(ValidatedAndroidData::getResourceRoots) - .withRoots(ValidatedAndroidData::getAssetRoots) - .withArtifact(ValidatedAndroidData::getManifest) - .withArtifact(ValidatedAndroidData::getAapt2RTxt) - .withArtifact(ValidatedAndroidData::getCompiledSymbols) - .withArtifact(ValidatedAndroidData::getSymbols) - .build(); + private static final ResourceContainerConverter.ToArg AAPT2_RESOURCE_DEP_TO_ARG = + ResourceContainerConverter.builder() + .include(Includes.ResourceRoots) + .include(Includes.Manifest) + .include(Includes.Aapt2RTxt) + .include(Includes.SymbolsBin) + .include(Includes.CompiledSymbols) + .withSeparator(ToArg.SeparatorType.COLON_COMMA) + .toArgConverter(); - private static final AndroidDataConverter<ValidatedAndroidData> - AAPT2_RESOURCE_DEP_TO_ARG_NO_PARSE = - AndroidDataConverter.<ValidatedAndroidData>builder(JoinerType.COLON_COMMA) - .withRoots(ValidatedAndroidData::getResourceRoots) - .withRoots(ValidatedAndroidData::getAssetRoots) - .withArtifact(ValidatedAndroidData::getManifest) - .withArtifact(ValidatedAndroidData::getAapt2RTxt) - .withArtifact(ValidatedAndroidData::getCompiledSymbols) - .build(); + private static final ResourceContainerConverter.ToArg AAPT2_RESOURCE_DEP_TO_ARG_NO_PARSE = + ResourceContainerConverter.builder() + .include(Includes.ResourceRoots) + .include(Includes.Manifest) + .include(Includes.Aapt2RTxt) + .include(Includes.CompiledSymbols) + .withSeparator(ToArg.SeparatorType.COLON_COMMA) + .toArgConverter(); - private static final AndroidDataConverter<ValidatedAndroidData> RESOURCE_DEP_TO_ARG = - AndroidDataConverter.<ValidatedAndroidData>builder(JoinerType.COLON_COMMA) - .withRoots(ValidatedAndroidData::getResourceRoots) - .withRoots(ValidatedAndroidData::getAssetRoots) - .withArtifact(ValidatedAndroidData::getManifest) - .withArtifact(ValidatedAndroidData::getRTxt) - .withArtifact(ValidatedAndroidData::getSymbols) - .build(); + private static final ResourceContainerConverter.ToArg RESOURCE_DEP_TO_ARG = + ResourceContainerConverter.builder() + .include(Includes.ResourceRoots) + .include(Includes.Manifest) + .include(Includes.RTxt) + .include(Includes.SymbolsBin) + .withSeparator(ToArg.SeparatorType.COLON_COMMA) + .toArgConverter(); private ResourceDependencies resourceDependencies = ResourceDependencies.empty(); private AssetDependencies assetDependencies = AssetDependencies.empty(); @@ -65,7 +75,10 @@ private ResourceFilterFactory resourceFilterFactory = ResourceFilterFactory.empty(); private List<String> uncompressedExtensions = Collections.emptyList(); private Artifact apkOut; + private final AndroidSdkProvider sdk; + private SpawnAction.Builder spawnActionBuilder; private String customJavaPackage; + private final RuleContext ruleContext; private String versionCode; private String applicationId; private String versionName; @@ -84,6 +97,13 @@ private boolean useCompiledResourcesForMerge; private boolean isTestWithResources = false; + /** @param ruleContext The RuleContext that was used to create the SpawnAction.Builder. */ + public AndroidResourcesProcessorBuilder(RuleContext ruleContext) { + this.sdk = AndroidSdkProvider.fromRuleContext(ruleContext); + this.ruleContext = ruleContext; + this.spawnActionBuilder = new SpawnAction.Builder(); + } + /** * The output zip for resource-processed data binding expressions (i.e. a zip of .xml files). * @@ -207,10 +227,9 @@ * @return a {@link ResourceApk} containing the processed resource, asset, and manifest * information. */ - public ResourceApk buildWithoutLocalResources( - AndroidDataContext dataContext, StampedAndroidManifest manifest) { + public ResourceApk buildWithoutLocalResources(StampedAndroidManifest manifest) { - build(dataContext, AndroidResources.empty(), AndroidAssets.empty(), manifest); + build(AndroidResources.empty(), AndroidAssets.empty(), manifest); return ResourceApk.fromTransitiveResources( resourceDependencies, @@ -219,9 +238,8 @@ rTxtOut); } - public ResourceContainer build(AndroidDataContext dataContext, ResourceContainer primary) { + public ResourceContainer build(ResourceContainer primary) { build( - dataContext, primary.getAndroidResources(), primary.getAndroidAssets(), ProcessedAndroidManifest.from(primary)); @@ -247,15 +265,14 @@ } public ProcessedAndroidData build( - AndroidDataContext dataContext, AndroidResources primaryResources, AndroidAssets primaryAssets, StampedAndroidManifest primaryManifest) { if (aaptVersion == AndroidAaptVersion.AAPT2) { - createAapt2ApkAction(dataContext, primaryResources, primaryAssets, primaryManifest); + createAapt2ApkAction(primaryResources, primaryAssets, primaryManifest); } else { - createAaptAction(dataContext, primaryResources, primaryAssets, primaryManifest); + createAaptAction(primaryResources, primaryAssets, primaryManifest); } // Wrap the new manifest, if any @@ -271,12 +288,12 @@ primaryResources, symbols, /* compiledSymbols = */ null, - dataContext.getLabel(), + ruleContext.getLabel(), processedManifest); // Wrap the parsed and merged assets ParsedAndroidAssets parsedAssets = - ParsedAndroidAssets.of(primaryAssets, symbols, dataContext.getLabel()); + ParsedAndroidAssets.of(primaryAssets, symbols, ruleContext.getLabel()); MergedAndroidAssets mergedAssets = MergedAndroidAssets.of(parsedAssets, mergedResourcesOut, assetDependencies); @@ -332,145 +349,289 @@ } private void createAapt2ApkAction( - AndroidDataContext dataContext, AndroidResources primaryResources, AndroidAssets primaryAssets, StampedAndroidManifest primaryManifest) { - BusyBoxActionBuilder builder = - BusyBoxActionBuilder.create(dataContext, "AAPT2_PACKAGE").addAapt(AndroidAaptVersion.AAPT2); + List<Artifact> outs = new ArrayList<>(); + // TODO(corysmith): Convert to an immutable list builder, as there is no benefit to a NestedSet + // here, as it will already have been flattened. + NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); + CustomCommandLine.Builder builder = new CustomCommandLine.Builder(); + // Set the busybox tool. + builder.add("--tool").add("AAPT2_PACKAGE").add("--"); + + builder.addExecPath("--aapt2", sdk.getAapt2().getExecutable()); if (resourceDependencies != null) { - builder - .addTransitiveFlag( - "--data", - resourceDependencies.getTransitiveResourceContainers(), - useCompiledResourcesForMerge - ? AAPT2_RESOURCE_DEP_TO_ARG_NO_PARSE - : AAPT2_RESOURCE_DEP_TO_ARG) - .addTransitiveFlag( - "--directData", - resourceDependencies.getDirectResourceContainers(), - useCompiledResourcesForMerge - ? AAPT2_RESOURCE_DEP_TO_ARG_NO_PARSE - : AAPT2_RESOURCE_DEP_TO_ARG) - .addTransitiveInputValues(resourceDependencies.getTransitiveResources()) - .addTransitiveInputValues(resourceDependencies.getTransitiveAssets()) - .addTransitiveInputValues(resourceDependencies.getTransitiveManifests()) - .addTransitiveInputValues(resourceDependencies.getTransitiveAapt2RTxt()) - .addTransitiveInputValues(resourceDependencies.getTransitiveCompiledSymbols()); + ResourceContainerConverter.addToCommandLine( + resourceDependencies, + builder, + useCompiledResourcesForMerge + ? AAPT2_RESOURCE_DEP_TO_ARG_NO_PARSE + : AAPT2_RESOURCE_DEP_TO_ARG); + inputs + .addTransitive(resourceDependencies.getTransitiveResources()) + .addTransitive(resourceDependencies.getTransitiveAssets()) + .addTransitive(resourceDependencies.getTransitiveManifests()) + .addTransitive(resourceDependencies.getTransitiveAapt2RTxt()) + .addTransitive(resourceDependencies.getTransitiveCompiledSymbols()); if (!useCompiledResourcesForMerge) { - builder.addTransitiveInputValues(resourceDependencies.getTransitiveSymbolsBin()); + inputs.addTransitive(resourceDependencies.getTransitiveSymbolsBin()); } } - addAssetDeps(builder) - .maybeAddFlag("--useCompiledResourcesForMerge", useCompiledResourcesForMerge) - .maybeAddFlag("--conditionalKeepRules", conditionalKeepRules); + addAssetDeps(builder, inputs); - configureCommonFlags(dataContext, primaryResources, primaryAssets, primaryManifest, builder) - .buildAndRegister("Processing Android resources", "AndroidAapt2"); + if (useCompiledResourcesForMerge) { + builder.add("--useCompiledResourcesForMerge"); + } + + if (conditionalKeepRules) { + builder.add("--conditionalKeepRules"); + } + + configureCommonFlags(primaryResources, primaryAssets, primaryManifest, outs, inputs, builder); + + ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); + // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special + // characters (colon, semicolon, hashmark, ampersand) that don't work on Windows, and quoting + // semantics are very complicated (more so than in Bash), so let's just always use a parameter + // file. + // TODO(laszlocsomor), TODO(corysmith): restructure the Android BusyBux's flags by deprecating + // list-type and list-of-list-type flags that use such problematic separators in favor of + // multi-value flags (to remove one level of listing) and by changing all list separators to a + // platform-safe character (= comma). + paramFileInfo.setUseAlways(OS.getCurrent() == OS.WINDOWS); + + // Create the spawn action. + ruleContext.registerAction( + this.spawnActionBuilder + .useDefaultShellEnvironment() + .addTool(sdk.getAapt2()) + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.<Artifact>copyOf(outs)) + .addCommandLine(builder.build(), paramFileInfo.build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Processing Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AndroidAapt2") + .build(ruleContext)); } private void createAaptAction( - AndroidDataContext dataContext, AndroidResources primaryResources, AndroidAssets primaryAssets, StampedAndroidManifest primaryManifest) { - BusyBoxActionBuilder builder = BusyBoxActionBuilder.create(dataContext, "PACKAGE"); + List<Artifact> outs = new ArrayList<>(); + // TODO(corysmith): Convert to an immutable list builder, as there is no benefit to a NestedSet + // here, as it will already have been flattened. + NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); + CustomCommandLine.Builder builder = new CustomCommandLine.Builder(); + + // Set the busybox tool. + builder.add("--tool").add("PACKAGE").add("--"); if (resourceDependencies != null) { - builder - .addTransitiveFlag( - "--data", resourceDependencies.getTransitiveResourceContainers(), RESOURCE_DEP_TO_ARG) - .addTransitiveFlag( - "--directData", - resourceDependencies.getDirectResourceContainers(), - RESOURCE_DEP_TO_ARG) - .addTransitiveInputValues(resourceDependencies.getTransitiveResources()) - .addTransitiveInputValues(resourceDependencies.getTransitiveAssets()) - .addTransitiveInputValues(resourceDependencies.getTransitiveManifests()) - .addTransitiveInputValues(resourceDependencies.getTransitiveRTxt()) - .addTransitiveInputValues(resourceDependencies.getTransitiveSymbolsBin()); + ResourceContainerConverter.addToCommandLine( + resourceDependencies, builder, RESOURCE_DEP_TO_ARG); + inputs + .addTransitive(resourceDependencies.getTransitiveResources()) + .addTransitive(resourceDependencies.getTransitiveAssets()) + .addTransitive(resourceDependencies.getTransitiveManifests()) + .addTransitive(resourceDependencies.getTransitiveRTxt()) + .addTransitive(resourceDependencies.getTransitiveSymbolsBin()); } - addAssetDeps(builder).addAapt(AndroidAaptVersion.AAPT); + addAssetDeps(builder, inputs); - configureCommonFlags(dataContext, primaryResources, primaryAssets, primaryManifest, builder) - .maybeAddVectoredFlag( - "--prefilteredResources", resourceFilterFactory.getResourcesToIgnoreInExecution()) - .buildAndRegister("Processing Android resources", "AaptPackage"); + builder.addExecPath("--aapt", sdk.getAapt().getExecutable()); + configureCommonFlags(primaryResources, primaryAssets, primaryManifest, outs, inputs, builder); + + ImmutableList<String> filteredResources = + resourceFilterFactory.getResourcesToIgnoreInExecution(); + if (!filteredResources.isEmpty()) { + builder.addAll("--prefilteredResources", VectorArg.join(",").each(filteredResources)); + } + + ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED); + // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special + // characters (colon, semicolon, hashmark, ampersand) that don't work on Windows, and quoting + // semantics are very complicated (more so than in Bash), so let's just always use a parameter + // file. + // TODO(laszlocsomor), TODO(corysmith): restructure the Android BusyBux's flags by deprecating + // list-type and list-of-list-type flags that use such problematic separators in favor of + // multi-value flags (to remove one level of listing) and by changing all list separators to a + // platform-safe character (= comma). + paramFileInfo.setUseAlways(OS.getCurrent() == OS.WINDOWS); + + // Create the spawn action. + ruleContext.registerAction( + this.spawnActionBuilder + .useDefaultShellEnvironment() + .addTool(sdk.getAapt()) + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.copyOf(outs)) + .addCommandLine(builder.build(), paramFileInfo.build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Processing Android resources for %s", ruleContext.getLabel()) + .setMnemonic("AaptPackage") + .build(ruleContext)); } - private BusyBoxActionBuilder addAssetDeps(BusyBoxActionBuilder builder) { + private void addAssetDeps(CustomCommandLine.Builder builder, NestedSetBuilder<Artifact> inputs) { if (assetDependencies == null || assetDependencies.getTransitiveAssets().isEmpty()) { - return builder; + return; } - return builder - .addTransitiveFlag( + builder + .addAll( "--directAssets", - assetDependencies.getDirectParsedAssets(), - AndroidDataConverter.MERGABLE_DATA_CONVERTER) - .addTransitiveFlag( + AndroidDataConverter.MERGABLE_DATA_CONVERTER.getVectorArg( + assetDependencies.getDirectParsedAssets())) + .addAll( "--assets", - assetDependencies.getTransitiveParsedAssets(), - AndroidDataConverter.MERGABLE_DATA_CONVERTER) - .addTransitiveInputValues(assetDependencies.getTransitiveAssets()) - .addTransitiveInputValues(assetDependencies.getTransitiveSymbols()); + AndroidDataConverter.MERGABLE_DATA_CONVERTER.getVectorArg( + assetDependencies.getTransitiveParsedAssets())); + + inputs.addTransitive(assetDependencies.getTransitiveAssets()); + inputs.addTransitive(assetDependencies.getTransitiveSymbols()); } - private BusyBoxActionBuilder configureCommonFlags( - AndroidDataContext dataContext, + private void configureCommonFlags( AndroidResources primaryResources, AndroidAssets primaryAssets, StampedAndroidManifest primaryManifest, - BusyBoxActionBuilder builder) { + List<Artifact> outs, + NestedSetBuilder<Artifact> inputs, + CustomCommandLine.Builder builder) { - return builder - .addInput( - "--primaryData", - String.format( - "%s:%s:%s", - AndroidDataConverter.rootsToString(primaryResources.getResourceRoots()), - AndroidDataConverter.rootsToString(primaryAssets.getAssetRoots()), - primaryManifest.getManifest().getExecPathString()), - Iterables.concat( - primaryResources.getResources(), - primaryAssets.getAssets(), - ImmutableList.of(primaryManifest.getManifest()))) - .maybeAddFlag("--buildToolsVersion", dataContext.getSdk().getBuildToolsVersion()) - .addAndroidJar() - .maybeAddFlag("--packageType", isLibrary) - .maybeAddFlag("LIBRARY", isLibrary) - .maybeAddOutput("--rOutput", rTxtOut) - .maybeAddOutput("--symbolsOut", symbols) - .maybeAddOutput("--srcJarOutput", sourceJarOut) - .maybeAddOutput("--proguardOutput", proguardOut) - .maybeAddOutput("--mainDexProguardOutput", mainDexProguardOut) - .maybeAddOutput("--manifestOutput", manifestOut) - .maybeAddOutput("--resourcesOutput", mergedResourcesOut) - .maybeAddOutput("--packagePath", apkOut) + // Add data + builder.add( + "--primaryData", + String.format( + "%s:%s:%s", + AndroidDataConverter.rootsToString(primaryResources.getResourceRoots()), + AndroidDataConverter.rootsToString(primaryAssets.getAssetRoots()), + primaryManifest.getManifest().getExecPathString())); + inputs.addAll(primaryResources.getResources()); + inputs.addAll(primaryAssets.getAssets()); + inputs.add(primaryManifest.getManifest()); - // Always pass density and resource configuration filter strings to execution, even when - // filtering in analysis. Filtering in analysis cannot remove resources from Filesets, and, - // in addition, aapt needs access to resource filters to generate pseudolocalized resources - // and because its resource filtering is somewhat stricter for locales, and resource - // processing needs access to densities to add them to the manifest. - .maybeAddFlag("--resourceConfigs", resourceFilterFactory.getConfigurationFilterString()) - .maybeAddFlag("--densities", resourceFilterFactory.getDensityString()) - .maybeAddVectoredFlag("--uncompressedExtensions", uncompressedExtensions) - .maybeAddFlag("--useAaptCruncher=no", !crunchPng) - .maybeAddFlag("--debug", debug) - .maybeAddFlag("--versionCode", versionCode) - .maybeAddFlag("--versionName", versionName) - .maybeAddFlag("--applicationId", applicationId) - .maybeAddOutput("--dataBindingInfoOut", dataBindingInfoZip) - .maybeAddFlag("--packageForR", customJavaPackage) - .maybeAddInput("--featureOf", featureOf) - .maybeAddInput("--featureAfter", featureAfter) - .maybeAddFlag("--throwOnResourceConflict", throwOnResourceConflict) - .maybeAddFlag("--packageUnderTest", packageUnderTest) - .maybeAddFlag("--isTestWithResources", isTestWithResources); + if (!Strings.isNullOrEmpty(sdk.getBuildToolsVersion())) { + builder.add("--buildToolsVersion", sdk.getBuildToolsVersion()); + } + + builder.addExecPath("--androidJar", sdk.getAndroidJar()); + inputs.add(sdk.getAndroidJar()); + + if (isLibrary) { + builder.add("--packageType").add("LIBRARY"); + } + + if (rTxtOut != null) { + builder.addExecPath("--rOutput", rTxtOut); + outs.add(rTxtOut); + } + + if (symbols != null) { + builder.addExecPath("--symbolsOut", symbols); + outs.add(symbols); + } + if (sourceJarOut != null) { + builder.addExecPath("--srcJarOutput", sourceJarOut); + outs.add(sourceJarOut); + } + if (proguardOut != null) { + builder.addExecPath("--proguardOutput", proguardOut); + outs.add(proguardOut); + } + + if (mainDexProguardOut != null) { + builder.addExecPath("--mainDexProguardOutput", mainDexProguardOut); + outs.add(mainDexProguardOut); + } + + if (manifestOut != null) { + builder.addExecPath("--manifestOutput", manifestOut); + outs.add(manifestOut); + } + + if (mergedResourcesOut != null) { + builder.addExecPath("--resourcesOutput", mergedResourcesOut); + outs.add(mergedResourcesOut); + } + + if (apkOut != null) { + builder.addExecPath("--packagePath", apkOut); + outs.add(apkOut); + } + + // Always pass density and resource configuration filter strings to execution, even when + // filtering in analysis. Filtering in analysis cannot remove resources from Filesets, and, in + // addition, aapt needs access to resource filters to generate pseudolocalized resources and + // because its resource filtering is somewhat stricter for locales, and + // resource processing needs access to densities to add them to the manifest. + if (resourceFilterFactory.hasConfigurationFilters()) { + builder.add("--resourceConfigs", resourceFilterFactory.getConfigurationFilterString()); + } + if (resourceFilterFactory.hasDensities()) { + builder.add("--densities", resourceFilterFactory.getDensityString()); + } + if (!uncompressedExtensions.isEmpty()) { + builder.addAll("--uncompressedExtensions", VectorArg.join(",").each(uncompressedExtensions)); + } + if (!crunchPng) { + builder.add("--useAaptCruncher=no"); + } + if (debug) { + builder.add("--debug"); + } + + if (versionCode != null) { + builder.add("--versionCode", versionCode); + } + + if (versionName != null) { + builder.add("--versionName", versionName); + } + + if (applicationId != null) { + builder.add("--applicationId", applicationId); + } + + if (dataBindingInfoZip != null) { + builder.addExecPath("--dataBindingInfoOut", dataBindingInfoZip); + outs.add(dataBindingInfoZip); + } + + if (!Strings.isNullOrEmpty(customJavaPackage)) { + // Sets an alternative java package for the generated R.java + // this allows android rules to generate resources outside of the java{,tests} tree. + builder.add("--packageForR", customJavaPackage); + } + + if (featureOf != null) { + builder.addExecPath("--featureOf", featureOf); + inputs.add(featureOf); + } + + if (featureAfter != null) { + builder.addExecPath("--featureAfter", featureAfter); + inputs.add(featureAfter); + } + + if (throwOnResourceConflict) { + builder.add("--throwOnResourceConflict"); + } + + if (packageUnderTest != null) { + builder.add("--packageUnderTest", packageUnderTest); + } + + if (isTestWithResources) { + builder.add("--isTestWithResources"); + } } }
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 d93b8a2..900f105 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
@@ -370,7 +370,6 @@ public ResourceApk packTestWithDataAndResources( RuleContext ruleContext, - AndroidDataContext dataContext, Artifact resourceApk, ResourceDependencies resourceDeps, @Nullable Artifact rTxt, @@ -391,7 +390,7 @@ .build(); AndroidResourcesProcessorBuilder builder = - new AndroidResourcesProcessorBuilder() + new AndroidResourcesProcessorBuilder(ruleContext) .setLibrary(false) .setApkOut(resourceContainer.getApk()) .setUncompressedExtensions(ImmutableList.of()) @@ -418,16 +417,16 @@ .setSymbols(resourceContainer.getSymbols()) .setSourceJarOut(resourceContainer.getJavaSourceJar()); } - ResourceContainer processed = builder.build(dataContext, resourceContainer); + ResourceContainer processed = builder.build(resourceContainer); ResourceContainer finalContainer = - new RClassGeneratorActionBuilder() + new RClassGeneratorActionBuilder(ruleContext) .targetAaptVersion(AndroidAaptVersion.chooseTargetAaptVersion(ruleContext)) .withDependencies(resourceDeps) .setClassJarOut( ruleContext.getImplicitOutputArtifact( AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR)) - .build(dataContext, processed); + .build(processed); return ResourceApk.of(finalContainer, resourceDeps, proguardCfg, mainDexProguardCfg); } @@ -435,7 +434,6 @@ /** Packages up the manifest with resource and assets from the LocalResourceContainer. */ public ResourceApk packAarWithDataAndResources( RuleContext ruleContext, - AndroidDataContext dataContext, AndroidAssets assets, AndroidResources resources, ResourceDependencies resourceDeps, @@ -457,14 +455,14 @@ ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR); resourceContainer = - new AndroidResourceParsingActionBuilder() + new AndroidResourceParsingActionBuilder(ruleContext) .setAssets(assets) .setResources(resources) .setOutput(symbols) - .buildAndUpdate(dataContext, resourceContainer); + .buildAndUpdate(ruleContext, resourceContainer); ResourceContainer merged = - new AndroidResourceMergingActionBuilder() + new AndroidResourceMergingActionBuilder(ruleContext) .setJavaPackage(resourceContainer.getJavaPackage()) .withDependencies(resourceDeps) .setMergedResourcesOut(mergedResources) @@ -475,10 +473,10 @@ .getConfiguration() .getFragment(AndroidConfiguration.class) .throwOnResourceConflict()) - .build(dataContext, resourceContainer); + .build(ruleContext, resourceContainer); ResourceContainer processed = - new AndroidResourceValidatorActionBuilder() + new AndroidResourceValidatorActionBuilder(ruleContext) .setJavaPackage(merged.getJavaPackage()) .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT) .setMergedResources(mergedResources) @@ -491,7 +489,7 @@ .setAapt2RTxtOut(merged.getAapt2RTxt()) .setAapt2SourceJarOut(merged.getAapt2JavaSourceJar()) .setStaticLibraryOut(merged.getStaticLibrary()) - .build(dataContext, merged); + .build(ruleContext, merged); return ResourceApk.of(processed, resourceDeps); } @@ -499,7 +497,6 @@ /* Creates an incremental apk from assets and data. */ public ResourceApk packIncrementalBinaryWithDataAndResources( RuleContext ruleContext, - AndroidDataContext dataContext, Artifact resourceApk, ResourceDependencies resourceDeps, List<String> uncompressedExtensions, @@ -528,7 +525,7 @@ .build(); ResourceContainer processed = - new AndroidResourcesProcessorBuilder() + new AndroidResourcesProcessorBuilder(ruleContext) .setLibrary(false) .setApkOut(resourceContainer.getApk()) .setResourceFilterFactory(resourceFilterFactory) @@ -547,7 +544,7 @@ .getFragment(AndroidConfiguration.class) .throwOnResourceConflict()) .setPackageUnderTest(null) - .build(dataContext, resourceContainer); + .build(resourceContainer); // Intentionally skip building an R class JAR - incremental binaries handle this separately. @@ -558,7 +555,6 @@ // TODO(bazel-team): this method calls for some refactoring, 15+ params including some nullables. public ResourceApk packBinaryWithDataAndResources( RuleContext ruleContext, - AndroidDataContext dataContext, Artifact resourceApk, ResourceDependencies resourceDeps, @Nullable Artifact rTxt, @@ -604,7 +600,7 @@ } ResourceContainer processed = - new AndroidResourcesProcessorBuilder() + new AndroidResourcesProcessorBuilder(ruleContext) .setLibrary(false) .setApkOut(resourceContainer.getApk()) .setResourceFilterFactory(resourceFilterFactory) @@ -630,23 +626,22 @@ .setRTxtOut(resourceContainer.getRTxt()) .setSymbols(resourceContainer.getSymbols()) .setSourceJarOut(resourceContainer.getJavaSourceJar()) - .build(dataContext, resourceContainer); + .build(resourceContainer); ResourceContainer finalContainer = - new RClassGeneratorActionBuilder() + new RClassGeneratorActionBuilder(ruleContext) .targetAaptVersion(AndroidAaptVersion.chooseTargetAaptVersion(ruleContext)) .withDependencies(resourceDeps) .setClassJarOut( ruleContext.getImplicitOutputArtifact( AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR)) - .build(dataContext, processed); + .build(processed); return ResourceApk.of(finalContainer, resourceDeps, proguardCfg, mainDexProguardCfg); } public ResourceApk packLibraryWithDataAndResources( RuleContext ruleContext, - AndroidDataContext dataContext, ResourceDependencies resourceDeps, Artifact rTxt, Artifact symbols, @@ -694,7 +689,7 @@ targetAaptVersion == AndroidAaptVersion.AAPT2 && androidConfiguration.skipParsingAction(); AndroidResourceParsingActionBuilder parsingBuilder = - new AndroidResourceParsingActionBuilder() + new AndroidResourceParsingActionBuilder(ruleContext) .setAssets(assets) .setResources(resources) .setOutput(resourceContainer.getSymbols()) @@ -713,10 +708,10 @@ .setManifest(resourceContainer.getManifest()) .setJavaPackage(resourceContainer.getJavaPackage()); } - resourceContainer = parsingBuilder.buildAndUpdate(dataContext, resourceContainer); + resourceContainer = parsingBuilder.buildAndUpdate(ruleContext, resourceContainer); ResourceContainer merged = - new AndroidResourceMergingActionBuilder() + new AndroidResourceMergingActionBuilder(ruleContext) .setJavaPackage(resourceContainer.getJavaPackage()) .withDependencies(resourceDeps) .setThrowOnResourceConflict(androidConfiguration.throwOnResourceConflict()) @@ -726,10 +721,10 @@ .setManifestOut(manifestOut) .setClassJarOut(rJavaClassJar) .setDataBindingInfoZip(dataBindingInfoZip) - .build(dataContext, resourceContainer); + .build(ruleContext, resourceContainer); ResourceContainer processed = - new AndroidResourceValidatorActionBuilder() + new AndroidResourceValidatorActionBuilder(ruleContext) .setJavaPackage(merged.getJavaPackage()) .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT) .setMergedResources(mergedResources) @@ -742,7 +737,7 @@ .setAapt2RTxtOut(merged.getAapt2RTxt()) .setAapt2SourceJarOut(merged.getAapt2JavaSourceJar()) .setStaticLibraryOut(merged.getStaticLibrary()) - .build(dataContext, merged); + .build(ruleContext, merged); return ResourceApk.of(processed, resourceDeps); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java index 16e55f8..07552fe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; -import com.google.devtools.build.lib.analysis.FilesToRunProvider; 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.analysis.actions.SpawnAction; @@ -50,7 +49,7 @@ private final AndroidDataContext dataContext; private final NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); private final ImmutableList.Builder<Artifact> outputs = ImmutableList.builder(); - private final SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + private final ImmutableList.Builder<Artifact> tools = ImmutableList.builder(); private final CustomCommandLine.Builder commandLine = CustomCommandLine.builder(); public static BusyBoxActionBuilder create( @@ -288,16 +287,16 @@ /** Adds aapt to the command line and inputs. */ public BusyBoxActionBuilder addAapt(AndroidAaptVersion aaptVersion) { - FilesToRunProvider aapt; + Artifact aapt; if (aaptVersion == AndroidAaptVersion.AAPT2) { - aapt = dataContext.getSdk().getAapt2(); - commandLine.addExecPath("--aapt2", aapt.getExecutable()); + aapt = dataContext.getSdk().getAapt2().getExecutable(); + commandLine.addExecPath("--aapt2", aapt); } else { - aapt = dataContext.getSdk().getAapt(); - commandLine.addExecPath("--aapt", aapt.getExecutable()); + aapt = dataContext.getSdk().getAapt().getExecutable(); + commandLine.addExecPath("--aapt", aapt); } - spawnActionBuilder.addTool(aapt); + tools.add(aapt); return this; } @@ -316,10 +315,11 @@ */ public void buildAndRegister(String message, String mnemonic) { dataContext.registerAction( - spawnActionBuilder + new SpawnAction.Builder() .useDefaultShellEnvironment() .addTransitiveInputs(inputs.build()) .addOutputs(outputs.build()) + .addTools(tools.build()) .addCommandLine(commandLine.build(), FORCED_PARAM_FILE_INFO) .setExecutable(dataContext.getBusybox()) .setProgressMessage("%s for %s", message, dataContext.getLabel())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java index 29da586..da7ceda 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java
@@ -54,7 +54,7 @@ "Should not use compiled merge if no compiled symbols are available!"); AndroidResourceMergingActionBuilder builder = - new AndroidResourceMergingActionBuilder() + new AndroidResourceMergingActionBuilder(dataContext.getRuleContext()) .setJavaPackage(parsed.getJavaPackage()) .withDependencies(resourceDeps) .setThrowOnResourceConflict(androidConfiguration.throwOnResourceConflict()) @@ -72,7 +72,7 @@ dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP)) .setClassJarOut( dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR)) - .build(dataContext, parsed); + .build(dataContext.getRuleContext(), parsed); } public static MergedAndroidResources of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java index b1b2971..265f22f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java
@@ -24,9 +24,9 @@ public static ParsedAndroidAssets parseFrom(AndroidDataContext dataContext, AndroidAssets assets) throws InterruptedException { - return new AndroidResourceParsingActionBuilder() + return new AndroidResourceParsingActionBuilder(dataContext.getRuleContext()) .setOutput(dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_ASSET_SYMBOLS)) - .build(dataContext, assets); + .build(assets); } public static ParsedAndroidAssets of(AndroidAssets assets, Artifact symbols, Label label) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java index 83b49b6..4ff3787 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java
@@ -40,14 +40,15 @@ boolean isAapt2 = aaptVersion == AndroidAaptVersion.AAPT2; - AndroidResourceParsingActionBuilder builder = new AndroidResourceParsingActionBuilder(); + AndroidResourceParsingActionBuilder builder = + new AndroidResourceParsingActionBuilder(dataContext.getRuleContext()); if (enableDataBinding && isAapt2) { // TODO(corysmith): Centralize the data binding processing and zipping into a single // action. Data binding processing needs to be triggered here as well as the merger to // avoid aapt2 from throwing an error during compilation. builder.setDataBindingInfoZip( - DataBinding.getSuffixedInfoFile(dataContext.getActionConstructionContext(), "_unused")); + DataBinding.getSuffixedInfoFile(dataContext.getRuleContext(), "_unused")); } return builder @@ -56,7 +57,7 @@ isAapt2 ? dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS) : null) - .build(dataContext, resources, manifest); + .build(resources, manifest); } public static ParsedAndroidResources of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java index a9d4267..cbb1034 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java
@@ -95,7 +95,6 @@ .setFeatureOf(featureOf) .setFeatureAfter(featureAfter); return buildActionForBinary( - dataContext, errorConsumer, builder, manifest, @@ -122,7 +121,6 @@ .setApkOut(apkOut); return buildActionForBinary( - dataContext, ruleContext, builder, manifest, @@ -136,7 +134,6 @@ } private static ProcessedAndroidData buildActionForBinary( - AndroidDataContext dataContext, RuleErrorConsumer errorConsumer, AndroidResourcesProcessorBuilder builder, StampedAndroidManifest manifest, @@ -162,7 +159,7 @@ .setCrunchPng(crunchPng) .withResourceDependencies(resourceDeps) .withAssetDependencies(assetDeps) - .build(dataContext, resources, assets, manifest); + .build(resources, assets, manifest); } /** Processes Android data (assets, resources, and manifest) for android_local_test targets. */ @@ -189,7 +186,7 @@ .setCrunchPng(false) .withResourceDependencies(resourceDeps) .withAssetDependencies(assetDeps) - .build(dataContext, resources, assets, manifest); + .build(resources, assets, manifest); } /** Processes Android data (assets, resources, and manifest) for android_test targets. */ @@ -216,7 +213,7 @@ .withResourceDependencies(resourceDeps) .withAssetDependencies(assetDeps); - return builder.build(dataContext, resources, assets, manifest); + return builder.build(resources, assets, manifest); } /** @@ -252,7 +249,7 @@ StampedAndroidManifest manifest, String proguardPrefix, Map<String, String> manifestValues) { - return new AndroidResourcesProcessorBuilder() + return new AndroidResourcesProcessorBuilder(dataContext.getRuleContext()) // Settings .setDebug(dataContext.useDebug()) .setJavaPackage(manifest.getPackage()) @@ -324,12 +321,12 @@ */ public ResourceApk generateRClass(AndroidDataContext dataContext, AndroidAaptVersion aaptVersion) throws InterruptedException { - return new RClassGeneratorActionBuilder() + return new RClassGeneratorActionBuilder(dataContext.getRuleContext()) .targetAaptVersion(aaptVersion) .withDependencies(resourceDeps) .setClassJarOut( dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR)) - .build(dataContext, this); + .build(this); } /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java index c4e33f1..8498812 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -13,9 +13,21 @@ // limitations under the License. package com.google.devtools.build.lib.rules.android; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.Builder; +import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType; +import java.util.ArrayList; +import java.util.List; import java.util.function.Function; /** Builds up the spawn action for $android_rclass_generator. */ @@ -30,12 +42,18 @@ .with(chooseDepsToArg(AndroidAaptVersion.AAPT2)) .build(); + private final RuleContext ruleContext; private ResourceDependencies dependencies; private Artifact classJarOut; private AndroidAaptVersion version; + /** @param ruleContext The RuleContext that is used to create a SpawnAction.Builder. */ + public RClassGeneratorActionBuilder(RuleContext ruleContext) { + this.ruleContext = ruleContext; + } + public RClassGeneratorActionBuilder withDependencies(ResourceDependencies resourceDeps) { this.dependencies = resourceDeps; return this; @@ -51,43 +69,72 @@ return this; } - public ResourceContainer build(AndroidDataContext dataContext, ResourceContainer primary) { - build(dataContext, primary.getRTxt(), ProcessedAndroidManifest.from(primary)); + public ResourceContainer build(ResourceContainer primary) { + build(primary.getRTxt(), ProcessedAndroidManifest.from(primary)); return primary.toBuilder().setJavaClassJar(classJarOut).build(); } - public ResourceApk build(AndroidDataContext dataContext, ProcessedAndroidData data) { - build(dataContext, data.getRTxt(), data.getManifest()); + public ResourceApk build(ProcessedAndroidData data) { + build(data.getRTxt(), data.getManifest()); return data.withValidatedResources(classJarOut); } - private void build( - AndroidDataContext dataContext, Artifact rTxt, ProcessedAndroidManifest manifest) { - BusyBoxActionBuilder builder = - BusyBoxActionBuilder.create(dataContext, "GENERATE_BINARY_R") - .addInput("--primaryRTxt", rTxt) - .addInput("--primaryManifest", manifest.getManifest()) - .maybeAddFlag("--packageForR", manifest.getPackage()); + private void build(Artifact rTxt, ProcessedAndroidManifest manifest) { + Builder builder = new Builder(); - if (dependencies != null && !dependencies.getResourceContainers().isEmpty()) { - builder - .addTransitiveFlagForEach( - "--library", - dependencies.getResourceContainers(), - version == AndroidAaptVersion.AAPT2 ? AAPT2_CONVERTER : AAPT_CONVERTER) - .addTransitiveInputValues( - version == AndroidAaptVersion.AAPT2 - ? dependencies.getTransitiveAapt2RTxt() - : dependencies.getTransitiveRTxt()) - .addTransitiveInputValues(dependencies.getTransitiveManifests()); + // Set the busybox tool. + builder.add("--tool").add("GENERATE_BINARY_R").add("--"); + + NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); + inputs.addAll( + ruleContext + .getExecutablePrerequisite("$android_resources_busybox", Mode.HOST) + .getRunfilesSupport() + .getRunfilesArtifacts()); + + List<Artifact> outs = new ArrayList<>(); + builder.addExecPath("--primaryRTxt", rTxt); + inputs.add(rTxt); + builder.addExecPath("--primaryManifest", manifest.getManifest()); + inputs.add(manifest.getManifest()); + if (!Strings.isNullOrEmpty(manifest.getPackage())) { + builder.add("--packageForR", manifest.getPackage()); } + if (dependencies != null) { + if (!dependencies.getResourceContainers().isEmpty()) { + builder.addAll( + VectorArg.addBefore("--library") + .each(dependencies.getResourceContainers()) + .mapped(version == AndroidAaptVersion.AAPT2 ? AAPT2_CONVERTER : AAPT_CONVERTER)); + if (version == AndroidAaptVersion.AAPT2) { + inputs.addTransitive(dependencies.getTransitiveAapt2RTxt()); + } else { + inputs.addTransitive(dependencies.getTransitiveRTxt()); + } + inputs.addTransitive(dependencies.getTransitiveManifests()); + } + } + builder.addExecPath("--classJarOutput", classJarOut); + outs.add(classJarOut); + builder.addLabel("--targetLabel", ruleContext.getLabel()); - builder - .addOutput("--classJarOutput", classJarOut) - .addLabelFlag("--targetLabel") - .buildAndRegister("Generating R Classes", "RClassGenerator"); + // Create the spawn action. + SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + + ruleContext.registerAction( + spawnActionBuilder + .useDefaultShellEnvironment() + .addTransitiveInputs(inputs.build()) + .addOutputs(ImmutableList.<Artifact>copyOf(outs)) + .addCommandLine( + builder.build(), ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED).build()) + .setExecutable( + ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)) + .setProgressMessage("Generating R Classes: %s", ruleContext.getLabel()) + .setMnemonic("RClassGenerator") + .build(ruleContext)); } private static Function<ValidatedAndroidData, String> chooseDepsToArg(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java index 99d545d..46048e9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
@@ -304,7 +304,7 @@ StampedAndroidManifest manifest) throws InterruptedException { - return new AndroidResourcesProcessorBuilder() + return new AndroidResourcesProcessorBuilder(dataContext.getRuleContext()) .setLibrary(true) .setRTxtOut(dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT)) .setManifestOut( @@ -316,6 +316,6 @@ .withAssetDependencies(assetDeps) .setDebug(dataContext.useDebug()) .setThrowOnResourceConflict(dataContext.getAndroidConfig().throwOnResourceConflict()) - .buildWithoutLocalResources(dataContext, manifest); + .buildWithoutLocalResources(manifest); } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java index b84803e..7abd103 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java
@@ -56,7 +56,7 @@ AndroidDataContext dataContext, MergedAndroidResources merged, AndroidAaptVersion aaptVersion) throws InterruptedException { AndroidResourceValidatorActionBuilder builder = - new AndroidResourceValidatorActionBuilder() + new AndroidResourceValidatorActionBuilder(dataContext.getRuleContext()) .setJavaPackage(merged.getJavaPackage()) .setDebug(dataContext.useDebug()) .setMergedResources(merged.getMergedResources()) @@ -82,7 +82,7 @@ AndroidRuleClasses.ANDROID_RESOURCES_AAPT2_LIBRARY_APK)); } - return builder.build(dataContext, merged); + return builder.build(dataContext.getRuleContext(), merged); } static ValidatedAndroidResources of(
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 cdd630c..6ba8585 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
@@ -1059,9 +1059,7 @@ ConfiguredTarget foo = getConfiguredTarget(target); SpawnAction action = (SpawnAction) actionsTestUtil().getActionForArtifactEndingWith( getFilesToBuild(foo), "r.srcjar"); - - assertThat(ImmutableList.copyOf(paramFileArgsOrActionArgs(action)).contains("--debug")) - .isEqualTo(isDebug); + assertThat(action.getArguments().contains("--debug")).isEqualTo(isDebug); } @Test