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 05419552e2f504..1d905664d08748 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("deps"); + return ImmutableList.of("application_resources", "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 c81703124a6c81..0dcea32526fce6 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,6 +40,8 @@ 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, @@ -50,7 +52,9 @@ public class AndroidApplicationResourceInfo extends NativeInfo Artifact mainDexProguardConfig, Artifact rTxt, Artifact resourcesZip, - Artifact databindingLayoutInfoZip) { + Artifact databindingLayoutInfoZip, + Artifact buildStampJar, + boolean shouldCompileJavaSrcs) { this.resourceApk = resourceApk; this.resourceJavaSrcJar = resourceJavaSrcJar; this.resourceJavaClassJar = resourceJavaClassJar; @@ -60,6 +64,8 @@ public class AndroidApplicationResourceInfo extends NativeInfo this.rTxt = rTxt; this.resourcesZip = resourcesZip; this.databindingLayoutInfoZip = databindingLayoutInfoZip; + this.buildStampJar = buildStampJar; + this.shouldCompileJavaSrcs = shouldCompileJavaSrcs; } @Override @@ -112,6 +118,22 @@ 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 @@ -131,7 +153,9 @@ public AndroidApplicationResourceInfoApi createInfo( Object mainDexProguardConfig, Object rTxt, Object resourcesZip, - Object databindingLayoutInfoZip) + Object databindingLayoutInfoZip, + Object buildStampJar, + boolean shouldCompileJavaSrcs) throws EvalException { return new AndroidApplicationResourceInfo( @@ -143,7 +167,9 @@ public AndroidApplicationResourceInfoApi createInfo( fromNoneable(mainDexProguardConfig, Artifact.class), fromNoneable(rTxt, Artifact.class), fromNoneable(resourcesZip, Artifact.class), - fromNoneable(databindingLayoutInfoZip, Artifact.class)); + fromNoneable(databindingLayoutInfoZip, Artifact.class), + fromNoneable(buildStampJar, Artifact.class), + shouldCompileJavaSrcs); } } } 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 b7f2392097e577..e693bda8556ed7 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("deps"), + ImmutableList.of("application_resources", "deps"), androidSemantics.getNativeDepsFileName(), cppSemantics); @@ -268,6 +268,7 @@ private static RuleConfiguredTargetBuilder init( AndroidApplicationResourceInfo.PROVIDER); final ResourceApk resourceApk; + boolean shouldCompileJavaSrcs = true; if (androidApplicationResourceInfo == null) { resourceApk = new RClassGeneratorActionBuilder() @@ -280,6 +281,7 @@ private static RuleConfiguredTargetBuilder init( resourceApk = ResourceApk.fromAndroidApplicationResourceInfo( ruleContext, dataContext.getAndroidConfig(), androidApplicationResourceInfo); + shouldCompileJavaSrcs = androidApplicationResourceInfo.shouldCompileJavaSrcs(); } if (dataContext.useResourcePathShortening()) { @@ -292,7 +294,11 @@ private static RuleConfiguredTargetBuilder init( JavaCommon javaCommon = AndroidCommon.createJavaCommonWithAndroidDataBinding( - ruleContext, javaSemantics, resourceApk.asDataBindingContext(), /* isLibrary */ false); + ruleContext, + javaSemantics, + resourceApk.asDataBindingContext(), + /* isLibrary */ false, + shouldCompileJavaSrcs); javaSemantics.checkRule(ruleContext, javaCommon); javaSemantics.checkForProtoLibraryAndJavaProtoLibraryOnSameProto(ruleContext, javaCommon); @@ -408,6 +414,9 @@ 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 a8db78abe3a3b0..594885545555a8 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,10 +80,16 @@ public class AndroidCommon { public static final InstrumentationSpec ANDROID_COLLECTION_SPEC = JavaCommon.JAVA_COLLECTION_SPEC.withDependencyAttributes( - "deps", "data", "exports", "instruments", "runtime_deps", "binary_under_test"); + "application_resources", + "deps", + "data", + "exports", + "instruments", + "runtime_deps", + "binary_under_test"); private static final ImmutableSet TRANSITIVE_ATTRIBUTES = - ImmutableSet.of("deps", "exports"); + ImmutableSet.of("application_resources", "deps", "exports"); private static final int DEX_THREADS = 5; private static final ResourceSet DEX_RESOURCE_SET = @@ -846,7 +852,24 @@ static JavaCommon createJavaCommonWithAndroidDataBinding( RuleContext ruleContext, JavaSemantics semantics, DataBindingContext dataBindingContext, - boolean isLibrary) { + 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 */ + } ImmutableList ruleSources = ruleContext.getPrerequisiteArtifacts("srcs").list(); @@ -869,7 +892,11 @@ static JavaCommon createJavaCommonWithAndroidDataBinding( bothDeps = JavaCommon.defaultDeps(ruleContext, semantics, ClasspathType.BOTH); } else { // Binary: - compileDeps = ImmutableList.copyOf(ruleContext.getPrerequisites("deps")); + compileDeps = + ImmutableList.builder() + .addAll(ruleContext.getPrerequisites("application_resources")) + .addAll(ruleContext.getPrerequisites("deps")) + .build(); 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 39b570eb4da785..033b6ce74042cf 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,7 +186,11 @@ public ConfiguredTarget create(RuleContext ruleContext) // rules respectively. JavaCommon javaCommon = AndroidCommon.createJavaCommonWithAndroidDataBinding( - ruleContext, javaSemantics, resourceApk.asDataBindingContext(), /* isLibrary */ true); + ruleContext, + javaSemantics, + resourceApk.asDataBindingContext(), + /* isLibrary */ true, + /* shouldCompileJavaSrcs */ 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 38d589694ce4b4..297002a02df231 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,7 +108,11 @@ public ConfiguredTarget create(RuleContext ruleContext) JavaCommon javaCommon = AndroidCommon.createJavaCommonWithAndroidDataBinding( - ruleContext, javaSemantics, resourceApk.asDataBindingContext(), /* isLibrary= */ false); + ruleContext, + javaSemantics, + resourceApk.asDataBindingContext(), + /* isLibrary */ false, + /* shouldCompileJavaSrcs */ true); 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 e6c8c638f5f1af..6c9d1c28da7e7e 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,7 +894,9 @@ 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.")) + + "Android resource processing to Starlark only.") + .aspect(androidNeverlinkAspect) + .aspect(dexArchiveAspect, DexArchiveAspect.PARAM_EXTRACTOR)) // 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 0592c81ee5cde4..0c137636c04967 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,6 +273,11 @@ 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); @@ -321,6 +326,11 @@ private static Iterable getProducedRuntimeJars( jars.add(rJar); } + Artifact buildStampJar = getAndroidBuildStampJar(base); + if (buildStampJar != null) { + jars.add(buildStampJar); + } + return jars.build(); } return null; @@ -349,6 +359,15 @@ 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 2dfaccc1cb2069..12b1541dfe0ec4 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,6 +125,25 @@ 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", @@ -207,6 +226,16 @@ 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 @@ -219,7 +248,9 @@ AndroidApplicationResourceInfoApi createInfo( Object mainDexProguardConfig, Object rTxt, Object resourcesZip, - Object databindingLayoutInfoZip) + Object databindingLayoutInfoZip, + Object buildStampJar, + boolean shouldCompileJava) 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 06cf8e79bf8fd7..b495ff4992816f 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,7 +54,9 @@ public AndroidApplicationResourceInfoApi createInfo( Object mainDexProguardConfig, Object rTxt, Object resourcesZip, - Object databindingLayoutInfoZip) + Object databindingLayoutInfoZip, + Object buildStampJar, + boolean shouldCompileJavaSrcs) 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 e01e40afd12f4d..48f0bc7e7430f2 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,7 +17,6 @@ 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; @@ -4614,85 +4613,6 @@ 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( @@ -4831,8 +4751,4 @@ 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. }