diff options
9 files changed, 213 insertions, 121 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java index 7733909840..ee098453e6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarGeneratorBuilder.java @@ -20,12 +20,14 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; 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.CommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType; +import com.google.devtools.build.lib.util.OS; import java.util.ArrayList; import java.util.List; @@ -89,7 +91,7 @@ public class AarGeneratorBuilder { List<Artifact> outs = new ArrayList<>(); List<Artifact> ins = new ArrayList<>(); List<String> args = new ArrayList<>(); - + // Set the busybox tool args.add("--tool"); args.add("GENERATE_AAR"); @@ -125,6 +127,18 @@ public class AarGeneratorBuilder { args.add("--throwOnResourceConflict"); } + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + builder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } + ruleContext.registerAction( this.builder .addInputs(ImmutableList.<Artifact>copyOf(ins)) 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 d5ba575068..abb7735e7c 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 @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.Builder.SeparatorType; +import com.google.devtools.build.lib.util.OS; import java.util.ArrayList; import java.util.List; @@ -182,10 +183,24 @@ public class AndroidResourceMergingActionBuilder { } SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } else { + spawnActionBuilder.useParameterFile(ParameterFileType.UNQUOTED); + } + // Create the spawn action. ruleContext.registerAction( spawnActionBuilder - .useParameterFile(ParameterFileType.UNQUOTED) .addTransitiveInputs(inputs.build()) .addOutputs(ImmutableList.copyOf(outs)) .setCommandLine(builder.build()) 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 2ff46cd9d7..62ea11d86e 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 @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; 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; @@ -141,10 +142,24 @@ public class AndroidResourceParsingActionBuilder { Preconditions.checkNotNull(output); builder.addExecPath("--output", output); + SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } else { + spawnActionBuilder.useParameterFile(ParameterFileType.UNQUOTED); + } + // Create the spawn action. ruleContext.registerAction( - new SpawnAction.Builder() - .useParameterFile(ParameterFileType.UNQUOTED) + spawnActionBuilder .addTransitiveInputs(inputs.build()) .addOutputs(ImmutableList.of(output)) .setCommandLine(builder.build()) 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 fa12915c6a..563570c384 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 @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction; 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.ResourceContainerConverter.Builder.SeparatorType; +import com.google.devtools.build.lib.util.OS; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -286,10 +287,24 @@ public class AndroidResourcesProcessorBuilder { configureCommonFlags(outs, inputs, builder); + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + this.spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } else { + this.spawnActionBuilder.useParameterFile(ParameterFileType.UNQUOTED); + } + // Create the spawn action. ruleContext.registerAction( this.spawnActionBuilder - .useParameterFile(ParameterFileType.UNQUOTED) + .useDefaultShellEnvironment() .addTool(sdk.getAapt2()) .addTransitiveInputs(inputs.build()) .addOutputs(ImmutableList.<Artifact>copyOf(outs)) @@ -331,10 +346,24 @@ public class AndroidResourcesProcessorBuilder { builder.addExecPath("--aapt", sdk.getAapt().getExecutable()); configureCommonFlags(outs, inputs, builder); + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + this.spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } else { + this.spawnActionBuilder.useParameterFile(ParameterFileType.UNQUOTED); + } + // Create the spawn action. ruleContext.registerAction( this.spawnActionBuilder - .useParameterFile(ParameterFileType.UNQUOTED) + .useDefaultShellEnvironment() .addTool(sdk.getAapt()) .addTransitiveInputs(inputs.build()) .addOutputs(ImmutableList.copyOf(outs)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java index d9846b3c53..7e6aaad3e7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java @@ -18,6 +18,7 @@ import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; @@ -25,6 +26,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.util.OS; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; @@ -88,7 +90,7 @@ public class ManifestMergerActionBuilder { NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder(); ImmutableList.Builder<Artifact> outputs = ImmutableList.builder(); CustomCommandLine.Builder builder = new CustomCommandLine.Builder(); - + // Set the busybox tool. builder.add("--tool").add("MERGE_MANIFEST").add("--"); @@ -130,6 +132,18 @@ public class ManifestMergerActionBuilder { outputs.add(logOut); } + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + this.spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } + ruleContext.registerAction( this.spawnActionBuilder .addTransitiveInputs(inputs.build()) 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 0c40818e16..8bc64cbf01 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 @@ -28,6 +28,7 @@ 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.AndroidConfiguration.AndroidAaptVersion; +import com.google.devtools.build.lib.util.OS; import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; @@ -115,6 +116,21 @@ public class RClassGeneratorActionBuilder { // Create the spawn action. SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } else { + spawnActionBuilder.useParameterFile(ParameterFileType.SHELL_QUOTED); + } + ruleContext.registerAction( spawnActionBuilder .useParameterFile(ParameterFileType.UNQUOTED) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java index 203d61e000..354b3c710e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.Builder.SeparatorType; +import com.google.devtools.build.lib.util.OS; import java.util.ArrayList; import java.util.List; @@ -97,9 +98,23 @@ public class RobolectricResourceSymbolsActionBuilder { builder.addExecPath("--classJarOutput", classJarOut); SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder(); + + if (OS.getCurrent() == OS.WINDOWS) { + // 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). + spawnActionBuilder.alwaysUseParameterFile(ParameterFileType.UNQUOTED); + } else { + spawnActionBuilder.useParameterFile(ParameterFileType.UNQUOTED); + } + ruleContext.registerAction( spawnActionBuilder - .useParameterFile(ParameterFileType.UNQUOTED) .addInputs(inputs) .addOutput(classJarOut) .setCommandLine(builder.build()) diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index f305c3efa1..200798d843 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -115,7 +115,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { Artifact intermediateJar = artifactByPath(ImmutableList.of(getCompressedUnsignedApk(ct)), ".apk", ".dex.zip", ".dex.zip", "main_dex_list.txt", "_intermediate.jar"); - List<String> args = getGeneratingSpawnAction(intermediateJar).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(intermediateJar); MoreAsserts.assertContainsSublist(args, "-include", "java/a/a.spec"); assertThat(Joiner.on(" ").join(args)).doesNotContain("mainDexClasses.rules"); } @@ -718,12 +718,12 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { SpawnAction zipalignAction = (SpawnAction) actionsTestUtil() .getActionForArtifactEndingWith(artifacts, "zipaligned_hello.apk"); assertThat(zipalignAction.getCommandFilename()).endsWith("sdk/zipalign"); - SpawnAction apkAction = (SpawnAction) actionsTestUtil() - .getActionForArtifactEndingWith(artifacts, "hello.apk"); - assertThat(apkAction.getCommandFilename()).endsWith("sdk/apksigner"); + Artifact a = ActionsTestUtil.getFirstArtifactEndingWith(artifacts, "hello.apk"); + assertThat(getGeneratingSpawnAction(a).getCommandFilename()).endsWith("sdk/apksigner"); + List<String> args = getGeneratingSpawnActionArgs(a); - assertThat(flagValue("--v1-signing-enabled", apkAction.getArguments())).isEqualTo(signV1); - assertThat(flagValue("--v2-signing-enabled", apkAction.getArguments())).isEqualTo(signV2); + assertThat(flagValue("--v1-signing-enabled", args)).isEqualTo(signV1); + assertThat(flagValue("--v2-signing-enabled", args)).isEqualTo(signV2); } @Test @@ -746,25 +746,20 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { getFirstArtifactEndingWith(artifacts, "proguard.jar"), getFirstArtifactEndingWith(artifacts, "shrunk.ap_")); - final SpawnAction resourceProcessing = - getGeneratingSpawnAction(getFirstArtifactEndingWith(artifacts, "resource_files.zip")); - List<String> processingArgs = resourceProcessing.getArguments(); + List<String> processingArgs = + getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "resource_files.zip")); assertThat(flagValue("--resourcesOutput", processingArgs)) .endsWith("hello_files/resource_files.zip"); - final SpawnAction proguard = - getGeneratingSpawnAction(getFirstArtifactEndingWith(artifacts, "proguard.jar")); - assertThat(proguard).isNotNull(); - List<String> proguardArgs = proguard.getArguments(); + List<String> proguardArgs = + getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "proguard.jar")); assertThat(flagValue("-outjars", proguardArgs)).endsWith("hello_proguard.jar"); - final SpawnAction resourceShrinking = - getGeneratingSpawnAction(getFirstArtifactEndingWith(artifacts, "shrunk.ap_")); - assertThat(resourceShrinking).isNotNull(); + List<String> shrinkingArgs = + getGeneratingSpawnActionArgs(getFirstArtifactEndingWith(artifacts, "shrunk.ap_")); - List<String> shrinkingArgs = resourceShrinking.getArguments(); assertThat(flagValue("--resources", shrinkingArgs)) .isEqualTo(flagValue("--resourcesOutput", processingArgs)); assertThat(flagValue("--shrunkJar", shrinkingArgs)) @@ -807,16 +802,15 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget output = getConfiguredTarget("//java/com/google/android/hello:b"); // Checks that ProGuard is called with the appropriate options. - SpawnAction action = - (SpawnAction) - actionsTestUtil() - .getActionForArtifactEndingWith(getFilesToBuild(output), "_proguard.jar"); + Artifact a = getFirstArtifactEndingWith(getFilesToBuild(output), "_proguard.jar"); + SpawnAction action = getGeneratingSpawnAction(a); + List<String> args = getGeneratingSpawnActionArgs(a); // Assert that the ProGuard executable set in the android_sdk rule appeared in the command-line // of the SpawnAction that generated the _proguard.jar. assertThat( Iterables.any( - action.getArguments(), + args, new Predicate<String>() { @Override public boolean apply(String s) { @@ -824,7 +818,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { } })) .isTrue(); - assertThat(action.getArguments()) + assertThat(args) .containsAllOf( "-injars", execPathEndingWith(action.getInputs(), "b_deploy.jar"), @@ -967,9 +961,10 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { // Ensure that the args that immediately follow "--dex" match the expectation. ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android:b"); - SpawnAction dexAction = (SpawnAction) actionsTestUtil().getActionForArtifactEndingWith( - actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)), "classes.dex"); - List<String> args = dexAction.getArguments(); + List<String> args = + getGeneratingSpawnActionArgs( + ActionsTestUtil.getFirstArtifactEndingWith( + actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)), "classes.dex")); int start = args.indexOf("--dex") + 1; assertThat(start).isNotEqualTo(0); int end = Math.min(args.size(), start + expectedArgs.size()); @@ -1003,9 +998,11 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { // Ensure that the args that immediately follow the main class in the shell command // match the expectation. ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android:b"); - SpawnAction mainDexListAction = (SpawnAction) actionsTestUtil().getActionForArtifactEndingWith( - actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)), "main_dex_list.txt"); - List<String> args = mainDexListAction.getArguments(); + List<String> args = + getGeneratingSpawnActionArgs( + ActionsTestUtil.getFirstArtifactEndingWith( + actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)), "main_dex_list.txt")); + // args: [ "bash", "-c", "java -cp dx.jar main opts other" ] MoreAsserts.assertContainsSublist(args, expectedArgs); } @@ -1084,8 +1081,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { // Validate that the filters are correctly passed to the resource processing action // This includes trimming whitespace and ignoring empty filters. - assertThat(resourceGeneratingAction(directResources).getArguments()).contains("en,es"); - assertThat(resourceGeneratingAction(directResources).getArguments()).contains("hdpi,xhdpi"); + assertThat(resourceArguments(directResources)).contains("en,es"); + assertThat(resourceArguments(directResources)).contains("hdpi,xhdpi"); } @Test @@ -1472,10 +1469,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { } // Validate resource filters are not passed to execution, since they were applied in analysis - assertThat(resourceGeneratingAction(directResources).getArguments()) + assertThat(resourceArguments(directResources)) .doesNotContain(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME); - assertThat(resourceGeneratingAction(directResources).getArguments()) - .doesNotContain(ResourceFilter.DENSITIES_NAME); + assertThat(resourceArguments(directResources)).doesNotContain(ResourceFilter.DENSITIES_NAME); } @Test @@ -1563,7 +1559,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " ])"); List<String> resourceProcessingArgs = - getGeneratingSpawnAction(getResourceContainer(binary).getRTxt()).getArguments(); + getGeneratingSpawnActionArgs(getResourceContainer(binary).getRTxt()); assertThat( Iterables.filter( @@ -1594,9 +1590,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { useConfiguration("--experimental_android_throw_on_resource_conflict"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List<String> resourceProcessingArgs = - resourceGeneratingAction(getResourceContainer(binary)).getArguments(); - + List<String> resourceProcessingArgs = resourceArguments(getResourceContainer(binary)); assertThat(resourceProcessingArgs).contains("--throwOnResourceConflict"); } @@ -1805,11 +1799,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "<resources><string name = 'lib_string'>Libs!</string></resources>"); scratch.file("java/r/android/res/values/strings.xml", "<resources><string name = 'hello'>Hello Android!</string></resources>"); - ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - SpawnAction compilerAction = ((SpawnAction) getResourceClassJarAction(binary)); - assertThat(compilerAction.getMnemonic()).isEqualTo("RClassGenerator"); - List<String> args = compilerAction.getArguments(); - assertThat(args) + Artifact jar = getResourceClassJar(getConfiguredTarget("//java/r/android:r")); + assertThat(getGeneratingAction(jar).getMnemonic()).isEqualTo("RClassGenerator"); + assertThat(getGeneratingSpawnActionArgs(jar)) .containsAllOf("--primaryRTxt", "--primaryManifest", "--library", "--classJarOutput"); } @@ -1822,10 +1814,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " )"); scratch.file("java/r/android/res/values/strings.xml", "<resources><string name = 'hello'>Hello Android!</string></resources>"); - ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - SpawnAction compilerAction = ((SpawnAction) getResourceClassJarAction(binary)); - assertThat(compilerAction.getMnemonic()).isEqualTo("RClassGenerator"); - List<String> args = compilerAction.getArguments(); + Artifact jar = getResourceClassJar(getConfiguredTarget("//java/r/android:r")); + assertThat(getGeneratingAction(jar).getMnemonic()).isEqualTo("RClassGenerator"); + List<String> args = getGeneratingSpawnActionArgs(jar); assertThat(args).containsAllOf("--primaryRTxt", "--primaryManifest", "--classJarOutput"); assertThat(args).doesNotContain("--libraries"); } @@ -1849,9 +1840,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { scratch.file("java/r/android/res/values/strings.xml", "<resources><string name = 'hello'>Hello Android!</string></resources>"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - SpawnAction compilerAction = ((SpawnAction) getResourceClassJarAction(binary)); - assertThat(compilerAction.getMnemonic()).isEqualTo("RClassGenerator"); - List<String> args = compilerAction.getArguments(); + Artifact jar = getResourceClassJar(binary); + assertThat(getGeneratingAction(jar).getMnemonic()).isEqualTo("RClassGenerator"); + List<String> args = getGeneratingSpawnActionArgs(jar); assertThat(args) .containsAllOf("--primaryRTxt", "--primaryManifest", "--library", "--classJarOutput", "--packageForR", "com.binary.custom"); @@ -1868,7 +1859,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " crunch_png = 0,", " )"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(binary)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(binary)); assertThat(args).contains("--useAaptCruncher=no"); } @@ -1883,7 +1874,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " crunch_png = 1,", " )"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(binary)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(binary)); assertThat(args).doesNotContain("--useAaptCruncher=no"); } @@ -1897,7 +1888,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " 'res/drawable-hdpi-v4/bar.9.png'],", " )"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(binary)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(binary)); assertThat(args).doesNotContain("--useAaptCruncher=no"); } @@ -1917,7 +1908,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " crunch_png = 0,", " )"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(binary)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(binary)); assertThat(args).contains("--useAaptCruncher=no"); } @@ -1938,18 +1929,21 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " crunch_png = 0,", " )"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(binary)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(binary)); assertThat(args).contains("--useAaptCruncher=no"); } @Test public void testZipaligned() throws Exception { ConfiguredTarget binary = getConfiguredTarget("//java/android:app"); - SpawnAction action = (SpawnAction) actionsTestUtil().getActionForArtifactEndingWith( - actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)), "zipaligned_app.apk"); + Artifact a = + ActionsTestUtil.getFirstArtifactEndingWith( + actionsTestUtil().artifactClosureOf(getFilesToBuild(binary)), "zipaligned_app.apk"); + SpawnAction action = getGeneratingSpawnAction(a); + assertThat(action.getMnemonic()).isEqualTo("AndroidZipAlign"); - List<String> arguments = action.getArguments(); + List<String> arguments = getGeneratingSpawnActionArgs(a); assertThat(Iterables.frequency(arguments, "4")).isEqualTo(1); Artifact zipAlignTool = @@ -2270,8 +2264,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget a = getConfiguredTarget("//java/a:a"); Artifact mainDexList = ActionsTestUtil.getFirstArtifactEndingWith( actionsTestUtil().artifactClosureOf(getFilesToBuild(a)), "main_dex_list.txt"); - SpawnAction spawnAction = getGeneratingSpawnAction(mainDexList); - assertThat(spawnAction.getArguments()).containsAllOf("--hello", "--world"); + List<String> args = getGeneratingSpawnActionArgs(mainDexList); + assertThat(args).containsAllOf("--hello", "--world"); } @Test @@ -2306,7 +2300,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget a = getConfiguredTarget("//java/a:a"); Artifact intermediateJar = artifactByPath(ImmutableList.of(getCompressedUnsignedApk(a)), ".apk", ".dex.zip", ".dex.zip", "main_dex_list.txt", "_intermediate.jar"); - List<String> args = getGeneratingSpawnAction(intermediateJar).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(intermediateJar); assertContainsSublist( args, ImmutableList.of( @@ -2327,7 +2321,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget a = getConfiguredTarget("//java/foo:abin"); Artifact intermediateJar = artifactByPath(ImmutableList.of(getCompressedUnsignedApk(a)), ".apk", ".dex.zip", ".dex.zip", "main_dex_list.txt", "_intermediate.jar"); - List<String> args = getGeneratingSpawnAction(intermediateJar).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(intermediateJar); MoreAsserts.assertDoesNotContainSublist( args, "-previousobfuscationmap"); @@ -2384,7 +2378,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "<resources><string name = 'hello'>Hello Android!</string></resources>"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(resource)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(resource)); assertPrimaryResourceDirs(ImmutableList.of("java/android/resources/res"), args); } @@ -2400,7 +2394,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "exports_files(['res/values/strings.xml'])"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(resource)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(resource)); assertPrimaryResourceDirs(ImmutableList.of("java/resources/other/res"), args); assertNoEvents(); } @@ -2418,7 +2412,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ")"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(resource)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(resource)); assertPrimaryResourceDirs(ImmutableList.of("java/other/resources/res"), args); assertNoEvents(); } @@ -2437,7 +2431,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "exports_files(['res/values/strings.xml'])"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(resource)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(resource)); assertPrimaryResourceDirs(ImmutableList.of("java/other/resources/res"), args); assertNoEvents(); } @@ -2464,7 +2458,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " ])"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(resource)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(resource)); assertPrimaryResourceDirs(ImmutableList.of("java/android/resources/bin-res"), args); assertThat(getDirectDependentResourceDirs(args)) .containsAllOf("java/android/resources/d1/d1-res", "java/android/resources/d2/d2-res"); @@ -2487,7 +2481,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ")"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources:r"); - List<String> args = getGeneratingSpawnAction(getResourceApk(resource)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(resource)); assertPrimaryResourceDirs(ImmutableList.of("java/other/resources/res"), args); assertNoEvents(); } @@ -2600,7 +2594,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " )"); ConfiguredTarget r = getConfiguredTarget("//a/r:r"); assertNoEvents(); - List<String> args = getGeneratingSpawnAction(getResourceApk(r)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(r)); assertContainsSublist(args, ImmutableList.of("--packageForR", "com.google.android.bar")); } @@ -2645,7 +2639,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " )"); ConfiguredTarget r = getConfiguredTarget("//java/a/r:r"); assertNoEvents(); - List<String> args = getGeneratingSpawnAction(getResourceApk(r)).getArguments(); + List<String> args = getGeneratingSpawnActionArgs(getResourceApk(r)); Truth.assertThat(args).doesNotContain("--applicationId"); } @@ -2675,11 +2669,10 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ")"); ConfiguredTarget ct = getConfiguredTarget("//java/com/google/android:foo"); - SpawnAction proguardAction = - getGeneratingSpawnAction(artifactByPath(getFilesToBuild(ct), "_proguard.jar")); MoreAsserts.assertContainsSublist( - proguardAction.getArguments(), - "-applymapping", "java/com/google/android/proguard.map"); + getGeneratingSpawnActionArgs(artifactByPath(getFilesToBuild(ct), "_proguard.jar")), + "-applymapping", + "java/com/google/android/proguard.map"); } @Test @@ -2978,10 +2971,10 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ")"); useConfiguration("--experimental_android_use_nocompress_extensions_on_apk"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - assertThat(getCompressedUnsignedApkAction(binary).getArguments()) + assertThat(getGeneratingSpawnActionArgs(getCompressedUnsignedApk(binary))) .containsAllOf("--nocompress_suffixes", ".apk", ".so") .inOrder(); - assertThat(getFinalUnsignedApkAction(binary).getArguments()) + assertThat(getGeneratingSpawnActionArgs(getFinalUnsignedApk(binary))) .containsAllOf("--nocompress_suffixes", ".apk", ".so") .inOrder(); } @@ -3181,8 +3174,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget a = getConfiguredTarget("//java/a:a"); Artifact apk = getImplicitOutputArtifact(a, AndroidRuleClasses.ANDROID_RESOURCES_APK); - SpawnAction apkAction = getGeneratingSpawnAction(apk); - assertThat(apkAction.getArguments()) + assertThat(getGeneratingSpawnActionArgs(apk)) .containsAllOf("--aapt2", "sdk/aapt2", "--tool", "AAPT2_PACKAGE"); } @@ -3238,7 +3230,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { Artifact apk = getImplicitOutputArtifact(a, AndroidRuleClasses.ANDROID_RESOURCES_APK); SpawnAction apkAction = getGeneratingSpawnAction(apk); - assertThat(apkAction.getArguments()) + assertThat(getGeneratingSpawnActionArgs(apk)) .containsAllOf("--aapt2", "sdk/aapt2", "--tool", "AAPT2_PACKAGE"); assertThat(apkAction.getInputs()) diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java index ebe3aee434..978fb9301a 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java @@ -24,7 +24,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -61,17 +60,9 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { } protected Iterable<Artifact> getNativeLibrariesInApk(ConfiguredTarget target) { - SpawnAction compressedUnsignedApkaction = getCompressedUnsignedApkAction(target); - ImmutableList.Builder<Artifact> result = ImmutableList.builder(); - for (Artifact output : compressedUnsignedApkaction.getInputs()) { - if (!output.getExecPathString().endsWith(".so")) { - continue; - } - - result.add(output); - } - - return result.build(); + return Iterables.filter( + getGeneratingAction(getCompressedUnsignedApk(target)).getInputs(), + a -> a.getFilename().endsWith(".so")); } protected Label getGeneratingLabelForArtifact(Artifact artifact) { @@ -110,19 +101,11 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { "_unsigned.apk"); } - protected SpawnAction getCompressedUnsignedApkAction(ConfiguredTarget target) { - return getGeneratingSpawnAction(getCompressedUnsignedApk(target)); - } - protected Artifact getFinalUnsignedApk(ConfiguredTarget target) { return getFirstArtifactEndingWith( target.getProvider(FileProvider.class).getFilesToBuild(), "_unsigned.apk"); } - protected SpawnAction getFinalUnsignedApkAction(ConfiguredTarget target) { - return getGeneratingSpawnAction(getFinalUnsignedApk(target)); - } - protected Artifact getResourceApk(ConfiguredTarget target) { Artifact resourceApk = getFirstArtifactEndingWith( @@ -140,7 +123,7 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { } protected List<String> resourceArguments(ResourceContainer resource) { - return resourceGeneratingAction(resource).getArguments(); + return getGeneratingSpawnActionArgs(resource.getApk()); } protected SpawnAction resourceGeneratingAction(ResourceContainer resource) { @@ -163,21 +146,20 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { : provider.getDirectAndroidResources()); } - protected ActionAnalysisMetadata getResourceClassJarAction(final ConfiguredTarget target) { + protected Artifact getResourceClassJar(final ConfiguredTarget target) { JavaRuleOutputJarsProvider jarProvider = target.getProvider(JavaRuleOutputJarsProvider.class); assertThat(jarProvider).isNotNull(); - return getGeneratingAction( - Iterables.find( - jarProvider.getOutputJars(), - outputJar -> { - assertThat(outputJar).isNotNull(); - assertThat(outputJar.getClassJar()).isNotNull(); - return outputJar - .getClassJar() - .getFilename() - .equals(target.getTarget().getName() + "_resources.jar"); - }) - .getClassJar()); + return Iterables.find( + jarProvider.getOutputJars(), + outputJar -> { + assertThat(outputJar).isNotNull(); + assertThat(outputJar.getClassJar()).isNotNull(); + return outputJar + .getClassJar() + .getFilename() + .equals(target.getTarget().getName() + "_resources.jar"); + }) + .getClassJar(); } // android resources related tests |