diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java index 1d905664d08748..05419552e2f504 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java @@ -79,7 +79,7 @@ public void addCoverageSupport( public ImmutableList getAttributesWithJavaRuntimeDeps(RuleContext ruleContext) { switch (ruleContext.getRule().getRuleClass()) { case "android_binary": - return ImmutableList.of("application_resources", "deps"); + return ImmutableList.of("deps"); default: throw new UnsupportedOperationException("Only supported for top-level binaries"); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidApplicationResourceInfo.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidApplicationResourceInfo.java index 0dcea32526fce6..c81703124a6c81 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidApplicationResourceInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidApplicationResourceInfo.java @@ -40,8 +40,6 @@ public class AndroidApplicationResourceInfo extends NativeInfo private final Artifact rTxt; private final Artifact resourcesZip; private final Artifact databindingLayoutInfoZip; - private final Artifact buildStampJar; - private final boolean shouldCompileJavaSrcs; AndroidApplicationResourceInfo( Artifact resourceApk, @@ -52,9 +50,7 @@ public class AndroidApplicationResourceInfo extends NativeInfo Artifact mainDexProguardConfig, Artifact rTxt, Artifact resourcesZip, - Artifact databindingLayoutInfoZip, - Artifact buildStampJar, - boolean shouldCompileJavaSrcs) { + Artifact databindingLayoutInfoZip) { this.resourceApk = resourceApk; this.resourceJavaSrcJar = resourceJavaSrcJar; this.resourceJavaClassJar = resourceJavaClassJar; @@ -64,8 +60,6 @@ public class AndroidApplicationResourceInfo extends NativeInfo this.rTxt = rTxt; this.resourcesZip = resourcesZip; this.databindingLayoutInfoZip = databindingLayoutInfoZip; - this.buildStampJar = buildStampJar; - this.shouldCompileJavaSrcs = shouldCompileJavaSrcs; } @Override @@ -118,22 +112,6 @@ public Artifact getDatabindingLayoutInfoZip() { return databindingLayoutInfoZip; } - @Override - public Artifact getBuildStampJar() { - return buildStampJar; - } - - /** - * A signal that indicates whether the android_binary rule should compile its Java sources in - * android_binary.srcs. When false, android_binary.application_resources will provide a JavaInfo - * that contains the compiled sources of the android_binary target. This step allows - * android_binary Java compilation to be offloaded to a Starlark rule. - */ - @Override - public boolean shouldCompileJavaSrcs() { - return shouldCompileJavaSrcs; - } - /** Provider for {@link AndroidApplicationResourceInfo}. */ public static class AndroidApplicationResourceInfoProvider extends BuiltinProvider @@ -153,9 +131,7 @@ public AndroidApplicationResourceInfoApi createInfo( Object mainDexProguardConfig, Object rTxt, Object resourcesZip, - Object databindingLayoutInfoZip, - Object buildStampJar, - boolean shouldCompileJavaSrcs) + Object databindingLayoutInfoZip) throws EvalException { return new AndroidApplicationResourceInfo( @@ -167,9 +143,7 @@ public AndroidApplicationResourceInfoApi createInfo( fromNoneable(mainDexProguardConfig, Artifact.class), fromNoneable(rTxt, Artifact.class), fromNoneable(resourcesZip, Artifact.class), - fromNoneable(databindingLayoutInfoZip, Artifact.class), - fromNoneable(buildStampJar, Artifact.class), - shouldCompileJavaSrcs); + fromNoneable(databindingLayoutInfoZip, Artifact.class)); } } } 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 c4f1435a4099f9..bc2b31a30043e3 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 @@ -214,7 +214,7 @@ private static RuleConfiguredTargetBuilder init( NativeLibs nativeLibs = NativeLibs.fromLinkedNativeDeps( ruleContext, - ImmutableList.of("application_resources", "deps"), + ImmutableList.of("deps"), androidSemantics.getNativeDepsFileName(), cppSemantics); @@ -268,7 +268,6 @@ private static RuleConfiguredTargetBuilder init( AndroidApplicationResourceInfo.PROVIDER); final ResourceApk resourceApk; - boolean shouldCompileJavaSrcs = true; if (androidApplicationResourceInfo == null) { resourceApk = new RClassGeneratorActionBuilder() @@ -281,7 +280,6 @@ private static RuleConfiguredTargetBuilder init( resourceApk = ResourceApk.fromAndroidApplicationResourceInfo( ruleContext, dataContext.getAndroidConfig(), androidApplicationResourceInfo); - shouldCompileJavaSrcs = androidApplicationResourceInfo.shouldCompileJavaSrcs(); } if (dataContext.useResourcePathShortening()) { @@ -294,11 +292,7 @@ private static RuleConfiguredTargetBuilder init( JavaCommon javaCommon = AndroidCommon.createJavaCommonWithAndroidDataBinding( - ruleContext, - javaSemantics, - resourceApk.asDataBindingContext(), - /* isLibrary */ false, - shouldCompileJavaSrcs); + ruleContext, javaSemantics, resourceApk.asDataBindingContext(), /* isLibrary */ false); javaSemantics.checkRule(ruleContext, javaCommon); javaSemantics.checkForProtoLibraryAndJavaProtoLibraryOnSameProto(ruleContext, javaCommon); @@ -414,9 +408,6 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( List proguardDeps = new ArrayList<>(); Iterables.addAll( proguardDeps, ruleContext.getPrerequisites("deps", ProguardSpecProvider.PROVIDER)); - Iterables.addAll( - proguardDeps, - ruleContext.getPrerequisites("application_resources", ProguardSpecProvider.PROVIDER)); if (ruleContext.getConfiguration().isCodeCoverageEnabled() && ruleContext.attributes().has("$jacoco_runtime", BuildType.LABEL)) { proguardDeps.add( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index 594885545555a8..a8db78abe3a3b0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -80,16 +80,10 @@ public class AndroidCommon { public static final InstrumentationSpec ANDROID_COLLECTION_SPEC = JavaCommon.JAVA_COLLECTION_SPEC.withDependencyAttributes( - "application_resources", - "deps", - "data", - "exports", - "instruments", - "runtime_deps", - "binary_under_test"); + "deps", "data", "exports", "instruments", "runtime_deps", "binary_under_test"); private static final ImmutableSet TRANSITIVE_ATTRIBUTES = - ImmutableSet.of("application_resources", "deps", "exports"); + ImmutableSet.of("deps", "exports"); private static final int DEX_THREADS = 5; private static final ResourceSet DEX_RESOURCE_SET = @@ -852,24 +846,7 @@ static JavaCommon createJavaCommonWithAndroidDataBinding( RuleContext ruleContext, JavaSemantics semantics, DataBindingContext dataBindingContext, - boolean isLibrary, - boolean shouldCompileJavaSrcs) { - - /** - * When within the context of an android_binary rule and shouldCompileJavaSrcs is False, the - * Java compilation happens within the Starlark rule. - */ - if (!isLibrary && !shouldCompileJavaSrcs) { - ImmutableList runtimeDeps = - ImmutableList.copyOf(ruleContext.getPrerequisites("application_resources")); - return new JavaCommon( - ruleContext, - semantics, - ImmutableList.of(), - runtimeDeps, /* compileDeps */ - runtimeDeps, - runtimeDeps); /* bothDeps */ - } + boolean isLibrary) { ImmutableList ruleSources = ruleContext.getPrerequisiteArtifacts("srcs").list(); @@ -892,11 +869,7 @@ static JavaCommon createJavaCommonWithAndroidDataBinding( bothDeps = JavaCommon.defaultDeps(ruleContext, semantics, ClasspathType.BOTH); } else { // Binary: - compileDeps = - ImmutableList.builder() - .addAll(ruleContext.getPrerequisites("application_resources")) - .addAll(ruleContext.getPrerequisites("deps")) - .build(); + compileDeps = ImmutableList.copyOf(ruleContext.getPrerequisites("deps")); runtimeDeps = compileDeps; bothDeps = compileDeps; } 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 033b6ce74042cf..39b570eb4da785 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 @@ -186,11 +186,7 @@ public ConfiguredTarget create(RuleContext ruleContext) // rules respectively. JavaCommon javaCommon = AndroidCommon.createJavaCommonWithAndroidDataBinding( - ruleContext, - javaSemantics, - resourceApk.asDataBindingContext(), - /* isLibrary */ true, - /* shouldCompileJavaSrcs */ true); + ruleContext, javaSemantics, resourceApk.asDataBindingContext(), /* isLibrary */ true); javaSemantics.checkRule(ruleContext, javaCommon); AndroidCommon androidCommon = new AndroidCommon(javaCommon); 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 297002a02df231..38d589694ce4b4 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 @@ -108,11 +108,7 @@ public ConfiguredTarget create(RuleContext ruleContext) JavaCommon javaCommon = AndroidCommon.createJavaCommonWithAndroidDataBinding( - ruleContext, - javaSemantics, - resourceApk.asDataBindingContext(), - /* isLibrary */ false, - /* shouldCompileJavaSrcs */ true); + ruleContext, javaSemantics, resourceApk.asDataBindingContext(), /* isLibrary= */ false); javaSemantics.checkRule(ruleContext, javaCommon); // Use the regular Java javacopts, plus any extra needed for databinding. Enforcing diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index 6c9d1c28da7e7e..e6c8c638f5f1af 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -894,9 +894,7 @@ public Object getDefault(AttributeMap rule) { .allowedFileTypes(NO_FILE) .undocumented( "Do not use this attribute. It's for the migration of " - + "Android resource processing to Starlark only.") - .aspect(androidNeverlinkAspect) - .aspect(dexArchiveAspect, DexArchiveAspect.PARAM_EXTRACTOR)) + + "Android resource processing to Starlark only.")) // Nothing in the native rule reads this. This is only for facilitating the Starlark // migration of the android rules. .add(attr("stamp", TRISTATE).value(TriState.AUTO)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java index 0c137636c04967..0592c81ee5cde4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java @@ -273,11 +273,6 @@ private Function desugarJarsIfRequested( jars.add(rJar); } - Artifact buildStampJar = getAndroidBuildStampJar(base); - if (buildStampJar != null) { - jars.add(buildStampJar); - } - // For android_* targets we need to honor their bootclasspath (nicer in general to do so) NestedSet bootclasspath = getBootclasspath(base, ruleContext); @@ -326,11 +321,6 @@ private static Iterable getProducedRuntimeJars( jars.add(rJar); } - Artifact buildStampJar = getAndroidBuildStampJar(base); - if (buildStampJar != null) { - jars.add(buildStampJar); - } - return jars.build(); } return null; @@ -359,15 +349,6 @@ private static Artifact getAndroidLibraryRJar(ConfiguredTarget base) { return null; } - private static Artifact getAndroidBuildStampJar(ConfiguredTarget base) { - AndroidApplicationResourceInfo provider = - (AndroidApplicationResourceInfo) base.get(AndroidApplicationResourceInfo.PROVIDER.getKey()); - if (provider != null && provider.getBuildStampJar() != null) { - return provider.getBuildStampJar(); - } - return null; - } - private static boolean checkBasenameClash(Iterable artifacts) { HashSet seen = new HashSet<>(); for (Artifact artifact : artifacts) { diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidApplicationResourceInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidApplicationResourceInfoApi.java index 12b1541dfe0ec4..2dfaccc1cb2069 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidApplicationResourceInfoApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidApplicationResourceInfoApi.java @@ -125,25 +125,6 @@ public interface AndroidApplicationResourceInfoApi extend @Nullable FileT getDatabindingLayoutInfoZip(); - /** The build stamp jar file */ - @StarlarkMethod( - name = "build_stamp_jar", - doc = "The build stamp jar file.", - documented = false, - allowReturnNones = true, - structField = true) - @Nullable - FileT getBuildStampJar(); - - /** Whether to compile Java Srcs within the android_binary rule */ - @StarlarkMethod( - name = "should_compile_java_srcs", - doc = "Whether to compile Java Srcs within the android_binary rule.", - documented = false, - allowReturnNones = false, - structField = true) - boolean shouldCompileJavaSrcs(); - /** Provider for {@link AndroidApplicationResourceInfoApi}. */ @StarlarkBuiltin( name = "Provider", @@ -226,16 +207,6 @@ interface AndroidApplicationResourceInfoApiProvider exten named = true, doc = "", defaultValue = "None"), - @Param( - name = "build_stamp_jar", - allowedTypes = { - @ParamType(type = FileApi.class), - @ParamType(type = NoneType.class), - }, - named = true, - doc = "", - defaultValue = "None"), - @Param(name = "should_compile_java_srcs", named = true, doc = "", defaultValue = "True"), }, selfCall = true) @StarlarkConstructor @@ -248,9 +219,7 @@ AndroidApplicationResourceInfoApi createInfo( Object mainDexProguardConfig, Object rTxt, Object resourcesZip, - Object databindingLayoutInfoZip, - Object buildStampJar, - boolean shouldCompileJava) + Object databindingLayoutInfoZip) throws EvalException; } } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidApplicationResourceInfo.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidApplicationResourceInfo.java index b495ff4992816f..06cf8e79bf8fd7 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidApplicationResourceInfo.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/android/FakeAndroidApplicationResourceInfo.java @@ -54,9 +54,7 @@ public AndroidApplicationResourceInfoApi createInfo( Object mainDexProguardConfig, Object rTxt, Object resourcesZip, - Object databindingLayoutInfoZip, - Object buildStampJar, - boolean shouldCompileJavaSrcs) + Object databindingLayoutInfoZip) throws EvalException { return null; } 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 4d387149b5355d..7a30f3806859df 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 @@ -17,6 +17,7 @@ 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.getArtifactsEndingWith; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.hasInput; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames; @@ -4663,6 +4664,85 @@ public void androidManifestMergerOrderAlphabeticalByConfiguration_MergeesSortedB .inOrder(); } + @Test + public void androidBinaryResourceInjection() throws Exception { + // ResourceApk.toResourceInfo() is not compatible with the AndroidResourcesInfo provider. + useConfiguration("--experimental_omit_resources_info_provider_from_android_binary"); + + scratch.file( + "java/com/app/android_resource_injection.bzl", + "def _android_application_resources_impl(ctx):", + " resource_proguard_config = ctx.actions.declare_file(ctx.label.name + '/proguard.cfg')", + " ctx.actions.write(resource_proguard_config, '# Empty proguard.cfg')", + "", + " resource_apk = ctx.actions.declare_file(ctx.label.name + '/injected_resource.ap_')", + " ctx.actions.write(resource_apk, 'empty ap_')", + "", + " manifest = ctx.actions.declare_file(ctx.label.name + '/AndroidManifest.xml')", + " ctx.actions.write(manifest, 'empty manifest')", + "", + " resource_java_src_jar = ctx.actions.declare_file(", + " ctx.label.name + '/resources.srcjar')", + " ctx.actions.write(resource_java_src_jar, 'empty manifest')", + "", + " resource_java_class_jar = ctx.actions.declare_file(", + " ctx.label.name + '/resources.jar')", + " ctx.actions.write(resource_java_class_jar, 'empty manifest')", + "", + " r_txt = ctx.actions.declare_file(ctx.label.name + '/r.txt')", + " ctx.actions.write(r_txt, 'empty r_txt')", + "", + " resources_zip = ctx.actions.declare_file(ctx.label.name + '/resource_files.zip')", + " ctx.actions.write(resources_zip, 'empty resources zip')", + "", + " databinding_info = ctx.actions.declare_file(ctx.label.name + '/layout-info.zip')", + " ctx.actions.write(databinding_info, 'empty databinding layout info zip')", + "", + " return [", + " DefaultInfo(files = depset([resource_apk, resource_proguard_config])),", + " AndroidApplicationResourceInfo(", + " resource_apk = resource_apk,", + " resource_java_src_jar = resource_java_src_jar,", + " resource_java_class_jar = resource_java_class_jar,", + " manifest = manifest,", + " resource_proguard_config = resource_proguard_config,", + " main_dex_proguard_config = None,", + " r_txt = r_txt,", + " resources_zip = resources_zip,", + " databinding_info = databinding_info,", + " ),", + " ]", + "", + "android_application_resources = rule(", + " implementation = _android_application_resources_impl,", + " provides = [AndroidApplicationResourceInfo],", + ")", + "", + "def resource_injected_android_binary(**attrs):", + " android_application_resources(name = 'application_resources')", + " attrs['application_resources'] = ':application_resources'", + " native.android_binary(**attrs)"); + + scratch.file( + "java/com/app/BUILD", + "load(':android_resource_injection.bzl', 'resource_injected_android_binary')", + "resource_injected_android_binary(", + " name = 'app',", + " manifest = 'AndroidManifest.xml')"); + + ConfiguredTarget app = getConfiguredTarget("//java/com/app:app"); + assertThat(app).isNotNull(); + + // Assert that the injected resource apk is the only resource apk being merged into the final + // apk. + Action singleJarAction = getGeneratingAction(getFinalUnsignedApk(app)); + List resourceApks = + getArtifactsEndingWith(singleJarAction.getInputs().toList(), ".ap_"); + assertThat(resourceApks).hasSize(1); + assertThat(resourceApks.get(0).getExecPathString()) + .endsWith("java/com/app/application_resources/injected_resource.ap_"); + } + @Test public void testAndroidStarlarkApiNativeLibs() throws Exception { scratch.file( @@ -4801,4 +4881,8 @@ public void testOptimizedJavaResourcesEnabled() throws Exception { assertThat(args).doesNotContain("a_deploy.jar"); assertThat(args).contains("/a_proguard.jar"); } + + // DEPENDENCY order is not tested; the incorrect order of dependencies means the test would + // have to enforce incorrect behavior. + // TODO(b/117338320): Add a test when dependency order is fixed. }