diff options
author | ajmichael <ajmichael@google.com> | 2017-07-25 16:06:11 +0200 |
---|---|---|
committer | Jakob Buchgraber <buchgr@google.com> | 2017-07-26 10:34:52 +0200 |
commit | e24c97e4fd999ebf566fe30f614c569856a999b2 (patch) | |
tree | 3b1d6524c27aa5e4499994e38177e5938fe45e80 /src | |
parent | 3edde6fe2ecc1471b1611fbe54fe446443a30855 (diff) |
Make --experimental_use_parallel_android_resource_processing a no-op.
Also, a few small cleanups of some duplicate test methods.
RELNOTES: None
PiperOrigin-RevId: 163066349
Diffstat (limited to 'src')
6 files changed, 68 insertions, 161 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index 4e0e9b67f0..e34d0d3d03 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -570,6 +570,8 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { @Option( name = "experimental_use_parallel_android_resource_processing", defaultValue = "true", + deprecationWarning = + "This flag is deprecated and is a no-op. It will be removed in a future release.", documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, effectTags = {OptionEffectTag.UNKNOWN}, help = @@ -748,7 +750,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean useRexToCompressDexFiles; private final boolean allowAndroidLibraryDepsWithoutSrcs; private final boolean useAndroidResourceShrinking; - private final boolean useParallelResourceProcessing; private final AndroidManifestMerger manifestMerger; private final ApkSigningMethod apkSigningMethod; private final boolean useSingleJarApkBuilder; @@ -784,7 +785,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.allowAndroidLibraryDepsWithoutSrcs = options.allowAndroidLibraryDepsWithoutSrcs; this.useAndroidResourceShrinking = options.useAndroidResourceShrinking || options.useExperimentalAndroidResourceShrinking; - this.useParallelResourceProcessing = options.useParallelResourceProcessing; this.manifestMerger = options.manifestMerger; this.apkSigningMethod = options.apkSigningMethod; this.useSingleJarApkBuilder = options.useSingleJarApkBuilder; @@ -878,10 +878,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { return useAndroidResourceShrinking; } - public boolean useParallelResourceProcessing() { - return useParallelResourceProcessing; - } - public AndroidAaptVersion getAndroidAaptVersion() { return androidAaptVersion; } 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 5e5454200b..e4d492b473 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 @@ -94,7 +94,6 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { resourceApk = applicationManifest.packLibraryWithDataAndResources( ruleContext, - null /* resourceApk, optional */, ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)), 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/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java index 0fea1e2567..ecc18479d7 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 @@ -307,7 +307,6 @@ public final class ApplicationManifest { Artifact resourceApk, RuleContext ruleContext, ResourceDependencies resourceDeps, - Artifact rTxt, boolean incremental, Artifact proguardCfg) throws InterruptedException, RuleErrorException { LocalResourceContainer data = new LocalResourceContainer.Builder(ruleContext) @@ -323,12 +322,10 @@ public final class ApplicationManifest { ruleContext, false, /* isLibrary */ resourceDeps, - ImmutableList.<String>of(), /* uncompressedExtensions */ + ImmutableList.of(), /* uncompressedExtensions */ true, /* crunchPng */ incremental, - ResourceContainer.builderFromRule(ruleContext) - .setRTxt(rTxt) - .setApk(resourceApk), + ResourceContainer.builderFromRule(ruleContext).setApk(resourceApk), data, proguardCfg, null, /* Artifact mainDexProguardCfg */ @@ -481,7 +478,6 @@ public final class ApplicationManifest { public ResourceApk packLibraryWithDataAndResources( RuleContext ruleContext, - @Nullable Artifact resourceApk, ResourceDependencies resourceDeps, Artifact rTxt, Artifact symbols, @@ -528,7 +524,7 @@ public final class ApplicationManifest { ruleContext, true /* isLibrary */, resourceDeps, - ImmutableList.<String>of() /* uncompressedExtensions */, + ImmutableList.of() /* uncompressedExtensions */, false /* crunchPng */, false /* incremental */, builder, @@ -583,7 +579,7 @@ public final class ApplicationManifest { } ResourceContainer processed; - if (isLibrary && AndroidCommon.getAndroidConfig(ruleContext).useParallelResourceProcessing()) { + if (isLibrary) { // android_library should only build the APK one way (!incremental). Preconditions.checkArgument(!incremental); Artifact rJavaClassJar = ruleContext.getImplicitOutputArtifact( 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 3821b2d37b..ebe3aee434 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 @@ -17,13 +17,12 @@ import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith; +import static org.junit.Assert.fail; -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; -import com.google.common.truth.Truth; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.Artifact; @@ -37,7 +36,6 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.rules.android.deployinfo.AndroidDeployInfoOuterClass.AndroidDeployInfo; import com.google.devtools.build.lib.rules.java.JavaCompileAction; import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider; -import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.OutputJar; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.util.Preconditions; import java.io.IOException; @@ -169,28 +167,33 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { JavaRuleOutputJarsProvider jarProvider = target.getProvider(JavaRuleOutputJarsProvider.class); assertThat(jarProvider).isNotNull(); return getGeneratingAction( - Iterables.find(jarProvider.getOutputJars(), new Predicate<OutputJar>() { - @Override - public boolean apply(@Nullable OutputJar outputJar) { - assertThat(outputJar).isNotNull(); - assertThat(outputJar.getClassJar()).isNotNull(); - return outputJar.getClassJar().getFilename().equals( - target.getTarget().getName() + "_resources.jar"); - } - } - ).getClassJar() - ); + 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 protected void assertPrimaryResourceDirs(List<String> expectedPaths, List<String> actualArgs) { assertThat(actualArgs).contains("--primaryData"); - String actualFlagValue = actualArgs.get(actualArgs.indexOf("--primaryData") + 1); - assertThat(actualFlagValue).matches("[^:]*:[^:]*:[^:]*"); - ImmutableList.Builder<String> actualPaths = ImmutableList.builder(); - actualPaths.add(actualFlagValue.split(":")[0].split("#")); - assertThat(actualPaths.build()).containsAllIn(expectedPaths); + List<String> actualPaths = null; + if (actualFlagValue.matches("[^;]*;[^;]*;.*")) { + actualPaths = Arrays.asList(actualFlagValue.split(";")[0].split("#")); + + } else if (actualFlagValue.matches("[^:]*:[^:]*:.*")) { + actualPaths = Arrays.asList(actualFlagValue.split(":")[0].split("#")); + } else { + fail(String.format("Failed to parse --primaryData: %s", actualFlagValue)); + } + assertThat(actualPaths).containsAllIn(expectedPaths); } protected List<String> getDirectDependentResourceDirs(List<String> actualArgs) { @@ -199,11 +202,24 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { return getDependentResourceDirs(actualFlagValue); } - protected List<String> getDependentResourceDirs(String actualFlagValue) { + protected List<String> getTransitiveDependentResourceDirs(List<String> actualArgs) { + assertThat(actualArgs).contains("--data"); + String actualFlagValue = actualArgs.get(actualArgs.indexOf("--data") + 1); + return getDependentResourceDirs(actualFlagValue); + } + + private static List<String> getDependentResourceDirs(String actualFlagValue) { + String separator = null; + if (actualFlagValue.matches("[^;]*;[^;]*;[^;]*;.*")) { + separator = ";"; + } else if (actualFlagValue.matches("[^:]*:[^:]*:[^:]*:.*")) { + separator = ":"; + } else { + fail(String.format("Failed to parse flag: %s", actualFlagValue)); + } ImmutableList.Builder<String> actualPaths = ImmutableList.builder(); for (String resourceDependency : actualFlagValue.split(",")) { - assertThat(actualFlagValue).matches("[^:]*:[^:]*:[^:]*:.*"); - actualPaths.add(resourceDependency.split(":")[0].split("#")); + actualPaths.add(resourceDependency.split(separator)[0].split("#")); } return actualPaths.build(); } @@ -232,37 +248,9 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { // Returns an artifact that will be generated when a rule has resources. protected static Artifact getResourceArtifact(ConfiguredTarget target) { // the last provider is the provider from the target. - if (target - .getConfiguration() - .getFragment(AndroidConfiguration.class) - .useParallelResourceProcessing()) { - return Iterables.getLast(target.getProvider(AndroidResourcesProvider.class) - .getDirectAndroidResources()).getJavaClassJar(); - } else { - return Iterables.getLast(target.getProvider(AndroidResourcesProvider.class) - .getDirectAndroidResources()).getJavaSourceJar(); - } - } - - protected static void assertPrimaryResourceDirs( - ConfiguredTarget target, List<String> expectedPaths, List<String> actualArgs) { - assertThat(actualArgs).contains("--primaryData"); - - String actualFlagValue = actualArgs.get(actualArgs.indexOf("--primaryData") + 1); - if (target - .getConfiguration() - .getFragment(AndroidConfiguration.class) - .useParallelResourceProcessing()) { - assertThat(actualFlagValue).matches("[^;]*;[^;]*;.*"); - ImmutableList.Builder<String> actualPaths = ImmutableList.builder(); - actualPaths.add(actualFlagValue.split(";")[0].split("#")); - Truth.assertThat(actualPaths.build()).containsAllIn(expectedPaths); - } else { - assertThat(actualFlagValue).matches("[^:]*:[^:]*:.*"); - ImmutableList.Builder<String> actualPaths = ImmutableList.builder(); - actualPaths.add(actualFlagValue.split(":")[0].split("#")); - Truth.assertThat(actualPaths.build()).containsAllIn(expectedPaths); - } + return Iterables.getLast( + target.getProvider(AndroidResourcesProvider.class).getDirectAndroidResources()) + .getJavaClassJar(); } protected static Set<Artifact> getNonToolInputs(Action action) { diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java index 38ad88e592..cac7c2a54b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidDataBindingTest.java @@ -100,18 +100,7 @@ public class AndroidDataBindingTest extends AndroidBuildViewTestCase { } @Test - public void basicDataBindingIntegrationParallelResourceProcessing() throws Exception { - useConfiguration("--experimental_use_parallel_android_resource_processing"); - basicDataBindingIntegration(); - } - - @Test - public void basicDataBindingIntegrationLegacyResourceProcessing() throws Exception { - useConfiguration("--noexperimental_use_parallel_android_resource_processing"); - basicDataBindingIntegration(); - } - - private void basicDataBindingIntegration() throws Exception { + public void basicDataBindingIntegration() throws Exception { writeDataBindingFiles(); ConfiguredTarget ctapp = getConfiguredTarget("//java/android/binary:app"); Set<Artifact> allArtifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(ctapp)); @@ -153,20 +142,7 @@ public class AndroidDataBindingTest extends AndroidBuildViewTestCase { } @Test - public void dataBindingCompilationUsesMetadataFromDepsParallelResourceProcessing() - throws Exception { - useConfiguration("--experimental_use_parallel_android_resource_processing"); - dataBindingCompilationUsesMetadataFromDeps(); - } - - @Test - public void dataBindingCompilationUsesMetadataFromDepsLegacyResourceProcessing() - throws Exception { - useConfiguration("--noexperimental_use_parallel_android_resource_processing"); - dataBindingCompilationUsesMetadataFromDeps(); - } - - private void dataBindingCompilationUsesMetadataFromDeps() throws Exception { + public void dataBindingCompilationUsesMetadataFromDeps() throws Exception { writeDataBindingFiles(); ConfiguredTarget ctapp = getConfiguredTarget("//java/android/binary:app"); Set<Artifact> allArtifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(ctapp)); 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 b283d934e8..cdecfb9dcf 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 @@ -624,54 +624,13 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { .isNull(); } - private List<String> getTransitiveDependentResourceDirs( - ConfiguredTarget target, List<String> actualArgs) { - assertThat(actualArgs).contains("--data"); - String actualFlagValue = actualArgs.get(actualArgs.indexOf("--data") + 1); - return getDependentResourceDirs(target, actualFlagValue); - } - - private List<String> getDirectDependentResourceDirs( - ConfiguredTarget target, List<String> actualArgs) { - assertThat(actualArgs).contains("--directData"); - String actualFlagValue = actualArgs.get(actualArgs.indexOf("--directData") + 1); - return getDependentResourceDirs(target, actualFlagValue); - } - - private List<String> getDependentResourceDirs( - ConfiguredTarget target, String actualFlagValue) { - ImmutableList.Builder<String> actualPaths = ImmutableList.builder(); - for (String resourceDependency : actualFlagValue.split(",")) { - if (target - .getConfiguration() - .getFragment(AndroidConfiguration.class) - .useParallelResourceProcessing()) { - assertThat(actualFlagValue).matches("[^;]*;[^;]*;[^;]*;.*"); - actualPaths.add(resourceDependency.split(";")[0].split("#")); - } else { - assertThat(actualFlagValue).matches("[^:]*:[^:]*:[^:]*:.*"); - actualPaths.add(resourceDependency.split(":")[0].split("#")); - } - } - return actualPaths.build(); - } - - private List<String> getDependentAssetDirs( - ConfiguredTarget target, String flag, List<String> actualArgs) { + private List<String> getDependentAssetDirs(String flag, List<String> actualArgs) { assertThat(actualArgs).contains(flag); String actualFlagValue = actualArgs.get(actualArgs.indexOf(flag) + 1); ImmutableList.Builder<String> actualPaths = ImmutableList.builder(); for (String resourceDependency : actualFlagValue.split(",")) { - if (target - .getConfiguration() - .getFragment(AndroidConfiguration.class) - .useParallelResourceProcessing()) { - assertThat(actualFlagValue).matches("[^;]*;[^;]*;[^;]*;.*"); - actualPaths.add(resourceDependency.split(";")[1].split("#")); - } else { - assertThat(actualFlagValue).matches("[^:]*:[^:]*:[^:]*:.*"); - actualPaths.add(resourceDependency.split(":")[1].split("#")); - } + assertThat(actualFlagValue).matches("[^;]*;[^;]*;[^;]*;.*"); + actualPaths.add(resourceDependency.split(";")[1].split("#")); } return actualPaths.build(); } @@ -691,7 +650,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//c/b/m/a:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("c/b/m/a/b_/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("c/b/m/a/b_/res"), args); } @Test @@ -706,7 +665,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/android/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("java/android/res"), args); } @Test @@ -723,7 +682,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/android/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("java/android/res"), args); } @Test @@ -738,7 +697,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/other/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("java/other/res"), args); assertNoEvents(); } @@ -756,7 +715,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/other/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("java/other/res"), args); assertNoEvents(); } @@ -775,7 +734,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/other/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("java/other/res"), args); assertNoEvents(); } @@ -796,7 +755,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android:r"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/other/res"), args); + assertPrimaryResourceDirs(ImmutableList.of("java/other/res"), args); assertNoEvents(); } @@ -949,12 +908,7 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { " deps = [':bar'])", "android_library(name = 'bar',", " manifest = 'AndroidManifest.xml')"); - Function<ResourceContainer, Label> getLabel = new Function<ResourceContainer, Label>() { - @Override - public Label apply(ResourceContainer container) { - return container.getLabel(); - } - }; + Function<ResourceContainer, Label> getLabel = ResourceContainer::getLabel; ConfiguredTarget foo = getConfiguredTarget("//java/apps/android:foo"); assertThat(Iterables.transform( foo.getProvider(AndroidResourcesProvider.class).getTransitiveAndroidResources(), getLabel)) @@ -1309,10 +1263,9 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { " )"); ConfiguredTarget resource = getConfiguredTarget("//java/android/resources/d1:d1"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs(resource, ImmutableList.of("java/android/resources/d1/d1-res"), args); - Truth.assertThat(getDirectDependentResourceDirs(resource, args)) - .contains("java/android/resources/d2/d2-res"); - Truth.assertThat(getDependentAssetDirs(resource, "--directData", args)) + assertPrimaryResourceDirs(ImmutableList.of("java/android/resources/d1/d1-res"), args); + assertThat(getDirectDependentResourceDirs(args)).contains("java/android/resources/d2/d2-res"); + assertThat(getDependentAssetDirs("--directData", args)) .contains("java/android/resources/d2/assets-d2"); assertNoEvents(); } @@ -1346,15 +1299,14 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { ConfiguredTarget resource = getConfiguredTarget("//java/android/resources/d1:d1"); List<String> args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); - assertPrimaryResourceDirs( - resource, ImmutableList.of("java/android/resources/d1/d1-res"), args); - Truth.assertThat(getDirectDependentResourceDirs(resource, args)) + assertPrimaryResourceDirs(ImmutableList.of("java/android/resources/d1/d1-res"), args); + Truth.assertThat(getDirectDependentResourceDirs(args)) .contains("java/android/resources/d2/d2-res"); - Truth.assertThat(getDependentAssetDirs(resource, "--directData", args)) + Truth.assertThat(getDependentAssetDirs("--directData", args)) .contains("java/android/resources/d2/assets-d2"); - Truth.assertThat(getTransitiveDependentResourceDirs(resource, args)) + Truth.assertThat(getTransitiveDependentResourceDirs(args)) .contains("java/android/resources/d3/d3-res"); - Truth.assertThat(getDependentAssetDirs(resource, "--data", args)) + Truth.assertThat(getDependentAssetDirs("--data", args)) .contains("java/android/resources/d3/assets-d3"); assertNoEvents(); } |