diff options
4 files changed, 187 insertions, 127 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java index 8f4143b0d4..6a3fd50045 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImportBaseRule.java @@ -73,12 +73,13 @@ public class AarImportBaseRule implements RuleDefinition { .cfg(HostTransition.INSTANCE) .exec() .value(env.getToolsLabel("//tools/android:aar_import_deps_checker"))) - .add(attr(ZIPPER, LABEL) - .cfg(HostTransition.INSTANCE) - .exec() - .value(env.getToolsLabel("//tools/zip:zipper"))) + .add( + attr(ZIPPER, LABEL) + .cfg(HostTransition.INSTANCE) + .exec() + .value(env.getToolsLabel("//tools/zip:zipper"))) .advertiseSkylarkProvider(SkylarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey())) - .requiresConfigurationFragments(JavaConfiguration.class) + .requiresConfigurationFragments(AndroidConfiguration.class, JavaConfiguration.class) .build(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java index b94c2e3025..be9889c848 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.truth.Truth; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLineExpansionException; @@ -48,9 +49,27 @@ import org.junit.runners.JUnit4; /** Tests for {@link com.google.devtools.build.lib.rules.android.AarImport}. */ @RunWith(JUnit4.class) public class AarImportTest extends BuildViewTestCase { + @Before public void setup() throws Exception { scratch.file( + "aapt2/sdk/BUILD", + "android_sdk(", + " name = 'sdk',", + " aapt = 'aapt',", + " aapt2 = 'aapt2',", + " adb = 'adb',", + " aidl = 'aidl',", + " android_jar = 'android.jar',", + " apksigner = 'apksigner',", + " dx = 'dx',", + " framework_aidl = 'framework_aidl',", + " main_dex_classes = 'main_dex_classes',", + " main_dex_list_creator = 'main_dex_list_creator',", + " proguard = 'proguard',", + " shrinked_android_jar = 'shrinked_android_jar',", + " zipalign = 'zipalign')"); + scratch.file( "a/BUILD", "aar_import(", " name = 'foo',", @@ -75,8 +94,17 @@ public class AarImportTest extends BuildViewTestCase { " name = 'last',", " aar = 'last.aar',", " deps = [':intermediate'],", + ")", + "android_library(", + " name = 'library',", + " manifest = 'AndroidManifest.xml',", + " custom_package = 'com.google.arrimport',", + " resource_files = ['res/values/values.xml'],", + " srcs = ['App.java'],", + " deps = [':foo'],", ")"); - scratch.file("java/BUILD", + scratch.file( + "java/BUILD", "android_binary(", " name = 'app',", " manifest = 'AndroidManifest.xml',", @@ -107,18 +135,48 @@ public class AarImportTest extends BuildViewTestCase { ValidatedAndroidData resourceContainer = directResources.iterator().next(); assertThat(resourceContainer.getManifest()).isNotNull(); - Artifact resourceTreeArtifact = - Iterables.getOnlyElement(resourceContainer.getResources()); + Artifact resourceTreeArtifact = Iterables.getOnlyElement(resourceContainer.getResources()); assertThat(resourceTreeArtifact.isTreeArtifact()).isTrue(); assertThat(resourceTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/resources/foo"); - Artifact assetsTreeArtifact = - Iterables.getOnlyElement(resourceContainer.getAssets()); + Artifact assetsTreeArtifact = Iterables.getOnlyElement(resourceContainer.getAssets()); assertThat(assetsTreeArtifact.isTreeArtifact()).isTrue(); assertThat(assetsTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/assets/foo"); } @Test + public void aapt2RTxtProvided() throws Exception { + useConfiguration( + "--android_decouple_data_processing", + "--android_sdk=//aapt2/sdk:sdk", + "--experimental_skip_parsing_action", + "--android_aapt=aapt2"); + + ConfiguredTarget libTarget = getConfiguredTarget("//a:library"); + + NestedSet<Artifact> transitiveCompiledSymbols = + libTarget.get(AndroidResourcesInfo.PROVIDER).getTransitiveCompiledSymbols(); + + assertThat(transitiveCompiledSymbols).hasSize(2); + + assertThat( + transitiveCompiledSymbols + .toSet() + .stream() + .map(Artifact::getRootRelativePathString) + .collect(Collectors.toSet())) + .containsExactly("a/foo_symbols/symbols.zip", "a/library_symbols/symbols.zip"); + + NestedSet<ValidatedAndroidData> directResources = + libTarget.get(AndroidResourcesInfo.PROVIDER).getDirectAndroidResources(); + + assertThat(directResources).hasSize(1); + + ValidatedAndroidData resourceContainer = directResources.iterator().next(); + Truth.assertThat(resourceContainer.getAapt2RTxt()).isNotNull(); + } + + @Test public void testResourcesProvided() throws Exception { useConfiguration("--android_decouple_data_processing"); @@ -240,9 +298,9 @@ public class AarImportTest extends BuildViewTestCase { ensureArgumentsHaveClassEntryOptionWithSuffix( arguments, "/intermediate/classes_and_libs_merged.jar"); assertThat(arguments.stream().filter(arg -> "--classpath_entry".equals(arg)).count()) - .isEqualTo(5); // transitive classpath + .isEqualTo(5); // transitive classpath assertThat(arguments.stream().filter(arg -> "--directdep".equals(arg)).count()) - .isEqualTo(1); // 1 declared dep + .isEqualTo(1); // 1 declared dep } @Test @@ -282,9 +340,9 @@ public class AarImportTest extends BuildViewTestCase { ensureArgumentsHaveClassEntryOptionWithSuffix( arguments, "/intermediate/classes_and_libs_merged.jar"); assertThat(arguments.stream().filter(arg -> "--classpath_entry".equals(arg)).count()) - .isEqualTo(5); // transitive classpath + .isEqualTo(5); // transitive classpath assertThat(arguments.stream().filter(arg -> "--directdep".equals(arg)).count()) - .isEqualTo(1); // 1 declared dep + .isEqualTo(1); // 1 declared dep } @Test @@ -390,15 +448,17 @@ public class AarImportTest extends BuildViewTestCase { NestedSet<Artifact> nativeLibs = androidLibraryTarget.get(AndroidNativeLibsInfo.PROVIDER).getNativeLibs(); - assertThat(nativeLibs).containsExactly( - ActionsTestUtil.getFirstArtifactEndingWith(nativeLibs, "foo/native_libs.zip"), - ActionsTestUtil.getFirstArtifactEndingWith(nativeLibs, "bar/native_libs.zip"), - ActionsTestUtil.getFirstArtifactEndingWith(nativeLibs, "baz/native_libs.zip")); + assertThat(nativeLibs) + .containsExactly( + ActionsTestUtil.getFirstArtifactEndingWith(nativeLibs, "foo/native_libs.zip"), + ActionsTestUtil.getFirstArtifactEndingWith(nativeLibs, "bar/native_libs.zip"), + ActionsTestUtil.getFirstArtifactEndingWith(nativeLibs, "baz/native_libs.zip")); } @Test public void testNativeLibsMakesItIntoApk() throws Exception { - scratch.file("java/com/google/android/hello/BUILD", + scratch.file( + "java/com/google/android/hello/BUILD", "aar_import(", " name = 'my_aar',", " aar = 'my_aar.aar',", @@ -410,8 +470,10 @@ public class AarImportTest extends BuildViewTestCase { " manifest = 'AndroidManifest.xml',", ")"); ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android/hello:my_app"); - SpawnAction apkBuilderAction = (SpawnAction) actionsTestUtil() - .getActionForArtifactEndingWith(getFilesToBuild(binary), "my_app_unsigned.apk"); + SpawnAction apkBuilderAction = + (SpawnAction) + actionsTestUtil() + .getActionForArtifactEndingWith(getFilesToBuild(binary), "my_app_unsigned.apk"); assertThat( Iterables.find( apkBuilderAction.getArguments(), @@ -419,7 +481,6 @@ public class AarImportTest extends BuildViewTestCase { .isNotEmpty(); } - @Test public void testClassesJarProvided() throws Exception { ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:foo"); @@ -454,8 +515,8 @@ public class AarImportTest extends BuildViewTestCase { Action appCompileAction = getGeneratingAction( ActionsTestUtil.getFirstArtifactEndingWith( - actionsTestUtil().artifactClosureOf( - getFileConfiguredTarget("//java:app.apk").getArtifact()), + actionsTestUtil() + .artifactClosureOf(getFileConfiguredTarget("//java:app.apk").getArtifact()), "libapp.jar")); assertThat(appCompileAction).isNotNull(); assertThat(ActionsTestUtil.prettyArtifactNames(appCompileAction.getInputs())) @@ -485,8 +546,7 @@ public class AarImportTest extends BuildViewTestCase { assertThat(fooClassesJar.getArtifactOwner().getLabel()).isEqualTo(foo.getLabel()); ConfiguredTarget baz = getConfiguredTarget("//java:baz.jar"); - Artifact bazJar = - ActionsTestUtil.getFirstArtifactEndingWith(artifacts, "baz.jar"); + Artifact bazJar = ActionsTestUtil.getFirstArtifactEndingWith(artifacts, "baz.jar"); // Verify that baz.jar was in the artifact closure assertThat(bazJar).isNotNull(); assertThat(bazJar.getArtifactOwner().getLabel()).isEqualTo(baz.getLabel()); @@ -514,8 +574,8 @@ public class AarImportTest extends BuildViewTestCase { public void testJavaCompilationArgsProvider() throws Exception { ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar"); - JavaCompilationArgsProvider provider = JavaInfo - .getProvider(JavaCompilationArgsProvider.class, aarImportTarget); + JavaCompilationArgsProvider provider = + JavaInfo.getProvider(JavaCompilationArgsProvider.class, aarImportTarget); assertThat(provider).isNotNull(); assertThat(artifactsToStrings(provider.getRuntimeJars())) .containsExactly( @@ -542,13 +602,11 @@ public class AarImportTest extends BuildViewTestCase { @Test public void testFailsWithoutAndroidSdk() throws Exception { - scratch.file("sdk/BUILD", - "alias(", - " name = 'sdk',", - " actual = 'doesnotexist',", - ")"); + scratch.file("sdk/BUILD", "alias(", " name = 'sdk',", " actual = 'doesnotexist',", ")"); useConfiguration("--android_sdk=//sdk"); - checkError("aar", "aar", + checkError( + "aar", + "aar", "No Android SDK found. Use the --android_sdk command line option to specify one.", "aar_import(", " name = 'aar',", @@ -563,9 +621,9 @@ public class AarImportTest extends BuildViewTestCase { // Compare root relative path strings instead of artifacts due to difference in configuration // caused by the Android split transition. assertThat( - Iterables.transform( - getGeneratingAction(binaryMergedManifest).getInputs(), - Artifact::getRootRelativePathString)) + Iterables.transform( + getGeneratingAction(binaryMergedManifest).getInputs(), + Artifact::getRootRelativePathString)) .containsAllOf(getAndroidManifest("//a:foo"), getAndroidManifest("//a:bar")); } diff --git a/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java b/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java index cd8befc7b8..18b5cf359c 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/ValidateAndLinkResourcesAction.java @@ -14,7 +14,9 @@ // Copyright 2017 The Bazel Authors. All rights reserved. package com.google.devtools.build.android; +import com.android.builder.core.VariantConfiguration; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.devtools.build.android.aapt2.Aapt2ConfigOptions; import com.google.devtools.build.android.aapt2.CompiledResources; import com.google.devtools.build.android.aapt2.ResourceLinker; @@ -37,121 +39,111 @@ public class ValidateAndLinkResourcesAction { /** Action configuration options. */ public static class Options extends OptionsBase { @Option( - name = "compiled", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.ExistingPathConverter.class, - category = "input", - help = "Compiled resources to link.", - deprecationWarning = "Use --resources." - ) + name = "compiled", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.ExistingPathConverter.class, + category = "input", + help = "Compiled resources to link.", + deprecationWarning = "Use --resources.") // TODO(b/64570523): Still used by blaze. Will be removed as part of the command line cleanup. @Deprecated public Path compiled; @Option( - name = "compiledDep", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "", - converter = Converters.PathListConverter.class, - category = "input", - allowMultiple = true, - help = "Compiled resource dependencies to link." - ) + name = "compiledDep", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "", + converter = Converters.PathListConverter.class, + category = "input", + allowMultiple = true, + help = "Compiled resource dependencies to link.") public List<Path> compiledDeps; @Option( - name = "manifest", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.ExistingPathConverter.class, - category = "input", - help = "Manifest for the library.", - deprecationWarning = "Use --resources." - ) + name = "manifest", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.ExistingPathConverter.class, + category = "input", + help = "Manifest for the library.", + deprecationWarning = "Use --resources.") // TODO(b/64570523): Still used by blaze. Will be removed as part of the command line cleanup. @Deprecated public Path manifest; @Option( - name = "resources", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.CompiledResourcesConverter.class, - category = "input", - help = "Compiled resources to link." - ) + name = "resources", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.CompiledResourcesConverter.class, + category = "input", + help = "Compiled resources to link.") public CompiledResources resources; // TODO(b/64570523): remove this flag when it is no longer used. @Option( - name = "libraries", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.StaticLibraryListConverter.class, - category = "input", - help = "Static libraries to link against. Deprecated, use --library" - ) + name = "libraries", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.StaticLibraryListConverter.class, + category = "input", + help = "Static libraries to link against. Deprecated, use --library") public List<StaticLibrary> deprecatedLibraries; @Option( - name = "library", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.StaticLibraryConverter.class, - category = "input", - allowMultiple = true, - help = "Static libraries to link against." - ) + name = "library", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.StaticLibraryConverter.class, + category = "input", + allowMultiple = true, + help = "Static libraries to link against.") public List<StaticLibrary> libraries; @Option( - name = "packageForR", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - category = "input", - help = "Package for the resources." - ) + name = "packageForR", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + category = "input", + help = "Package for the resources.") public String packageForR; @Option( - name = "staticLibraryOut", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.PathConverter.class, - category = "output", - help = "Static library produced." - ) + name = "staticLibraryOut", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.PathConverter.class, + category = "output", + help = "Static library produced.") public Path staticLibraryOut; @Option( - name = "rTxtOut", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - defaultValue = "null", - converter = Converters.PathConverter.class, - category = "output", - help = "R.txt out." - ) + name = "rTxtOut", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "null", + converter = Converters.PathConverter.class, + category = "output", + help = "R.txt out.") public Path rTxtOut; @Option( - name = "sourceJarOut", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - converter = Converters.PathConverter.class, - defaultValue = "null", - category = "output", - help = "Generated java classes from the resources." - ) + name = "sourceJarOut", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + converter = Converters.PathConverter.class, + defaultValue = "null", + category = "output", + help = "Generated java classes from the resources.") public Path sourceJarOut; } @@ -180,10 +172,15 @@ public class ValidateAndLinkResourcesAction { // We need to make the manifest aapt safe (w.r.t., placeholders). For now, just stub // it out. .processManifest( - manifest -> - AndroidManifestProcessor.writeDummyManifestForAapt( - scopedTmp.getPath().resolve("manifest-aapt-dummy/AndroidManifest.xml"), - options.packageForR)); + manifest -> { + final String packageForR = + Strings.isNullOrEmpty(options.packageForR) + ? VariantConfiguration.getManifestPackage(manifest.toFile()) + : options.packageForR; + return AndroidManifestProcessor.writeDummyManifestForAapt( + scopedTmp.getPath().resolve("manifest-aapt-dummy/AndroidManifest.xml"), + packageForR); + }); profiler.recordEndOf("manifest").startTask("link"); ResourceLinker.create(aapt2Options.aapt2, executorService, scopedTmp.getPath()) .profileUsing(profiler) @@ -195,6 +192,7 @@ public class ValidateAndLinkResourcesAction { .map(CompiledResources::from) .collect(Collectors.toList())) .buildVersion(aapt2Options.buildToolsVersion) + .outputAsProto(aapt2Options.resourceTableAsProto) .linkStatically(resources) .copyLibraryTo(options.staticLibraryOut) .copySourceJarTo(options.sourceJarOut) diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java index c758d2dbf9..3839a5d94b 100644 --- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java +++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java @@ -203,14 +203,17 @@ public class ResourceLinker { .forBuildToolsVersion(buildToolsVersion) .forVariantType(VariantType.LIBRARY) .add("link") + .when( + outputAsProto) // Used for testing: aapt2 does not output static libraries in + // proto format. + .thenAdd("--proto-format") + .when(!outputAsProto) + .thenAdd("--static-lib") .add("--manifest", compiled.getManifest()) - .add("--static-lib") .add("--no-static-lib-packages") .add("--custom-package", customPackage) .whenVersionIsAtLeast(new Revision(23)) .thenAdd("--no-version-vectors") - .when(outputAsProto) - .thenAdd("--proto-format") .addParameterableRepeated( "-R", compiledResourcesToPaths(compiled, IS_FLAT_FILE), workingDirectory) .addRepeated("-I", pathsToLinkAgainst) |