From f61766e98797a00d06071e5120d0b67aa3014ebd Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 16 Mar 2022 18:06:48 -0700 Subject: [PATCH] Use turbine instead of kapt for kotlin annotation processors Follow Bazel by using turbine instead of kapt to run annotation processors. This still requires using kapt to generate java stubs of kotlin soruces, then uses turbine to run annotation processors on the java stubs and any java sources to generate sources and resources, and passes the annotation processor generated sources to kotlinc and javac. Bug: 225013372 Test: m checkbuild Test: TestKapt Change-Id: I9c6fc496a9fba64658bb062538bc5f7b9478b07a --- java/builder.go | 83 ++++++++++++++++++++++++++++++++++++--------- java/java_test.go | 2 +- java/kotlin.go | 25 ++++++++------ java/kotlin_test.go | 62 +++++++++++++++++++++------------ 4 files changed, 124 insertions(+), 48 deletions(-) diff --git a/java/builder.go b/java/builder.go index 1a115a4ad..c0fadd42c 100644 --- a/java/builder.go +++ b/java/builder.go @@ -131,11 +131,11 @@ var ( turbine, turbineRE = pctx.RemoteStaticRules("turbine", blueprint.RuleParams{ - Command: `$reTemplate${config.JavaCmd} ${config.JavaVmFlags} -jar ${config.TurbineJar} --output $out.tmp ` + + Command: `$reTemplate${config.JavaCmd} ${config.JavaVmFlags} -jar ${config.TurbineJar} $outputFlags ` + `--sources @$out.rsp --source_jars $srcJars ` + `--javacopts ${config.CommonJdkFlags} ` + - `$javacFlags -source $javaVersion -target $javaVersion -- $bootClasspath $classpath && ` + - `(if cmp -s $out.tmp $out ; then rm $out.tmp ; else mv $out.tmp $out ; fi )`, + `$javacFlags -source $javaVersion -target $javaVersion -- $turbineFlags && ` + + `(for o in $outputs; do if cmp -s $${o}.tmp $${o} ; then rm $${o}.tmp ; else mv $${o}.tmp $${o} ; fi; done )`, CommandDeps: []string{ "${config.TurbineJar}", "${config.JavaCmd}", @@ -148,10 +148,11 @@ var ( ExecStrategy: "${config.RETurbineExecStrategy}", Inputs: []string{"${config.TurbineJar}", "${out}.rsp", "$implicits"}, RSPFiles: []string{"${out}.rsp"}, - OutputFiles: []string{"$out.tmp"}, + OutputFiles: []string{"$rbeOutputs"}, ToolchainInputs: []string{"${config.JavaCmd}"}, Platform: map[string]string{remoteexec.PoolKey: "${config.REJavaPool}"}, - }, []string{"javacFlags", "bootClasspath", "classpath", "srcJars", "javaVersion"}, []string{"implicits"}) + }, + []string{"javacFlags", "turbineFlags", "outputFlags", "javaVersion", "outputs", "rbeOutputs", "srcJars"}, []string{"implicits"}) jar, jarRE = pctx.RemoteStaticRules("jar", blueprint.RuleParams{ @@ -354,11 +355,8 @@ func emitXrefRule(ctx android.ModuleContext, xrefFile android.WritablePath, idx }) } -func TransformJavaToHeaderClasses(ctx android.ModuleContext, outputFile android.WritablePath, - srcFiles, srcJars android.Paths, flags javaBuilderFlags) { - +func turbineFlags(ctx android.ModuleContext, flags javaBuilderFlags) (string, android.Paths) { var deps android.Paths - deps = append(deps, srcJars...) classpath := flags.classpath @@ -380,19 +378,31 @@ func TransformJavaToHeaderClasses(ctx android.ModuleContext, outputFile android. } deps = append(deps, classpath...) - deps = append(deps, flags.processorPath...) + turbineFlags := bootClasspath + " " + classpath.FormTurbineClassPath("--classpath ") + + return turbineFlags, deps +} + +func TransformJavaToHeaderClasses(ctx android.ModuleContext, outputFile android.WritablePath, + srcFiles, srcJars android.Paths, flags javaBuilderFlags) { + + turbineFlags, deps := turbineFlags(ctx, flags) + + deps = append(deps, srcJars...) rule := turbine args := map[string]string{ - "javacFlags": flags.javacFlags, - "bootClasspath": bootClasspath, - "srcJars": strings.Join(srcJars.Strings(), " "), - "classpath": classpath.FormTurbineClassPath("--classpath "), - "javaVersion": flags.javaVersion.String(), + "javacFlags": flags.javacFlags, + "srcJars": strings.Join(srcJars.Strings(), " "), + "javaVersion": flags.javaVersion.String(), + "turbineFlags": turbineFlags, + "outputFlags": "--output " + outputFile.String() + ".tmp", + "outputs": outputFile.String(), } if ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_TURBINE") { rule = turbineRE args["implicits"] = strings.Join(deps.Strings(), ",") + args["rbeOutputs"] = outputFile.String() + ".tmp" } ctx.Build(pctx, android.BuildParams{ Rule: rule, @@ -404,6 +414,47 @@ func TransformJavaToHeaderClasses(ctx android.ModuleContext, outputFile android. }) } +// TurbineApt produces a rule to run annotation processors using turbine. +func TurbineApt(ctx android.ModuleContext, outputSrcJar, outputResJar android.WritablePath, + srcFiles, srcJars android.Paths, flags javaBuilderFlags) { + + turbineFlags, deps := turbineFlags(ctx, flags) + + deps = append(deps, srcJars...) + + deps = append(deps, flags.processorPath...) + turbineFlags += " " + flags.processorPath.FormTurbineClassPath("--processorpath ") + turbineFlags += " --processors " + strings.Join(flags.processors, " ") + + outputs := android.WritablePaths{outputSrcJar, outputResJar} + outputFlags := "--gensrc_output " + outputSrcJar.String() + ".tmp " + + "--resource_output " + outputResJar.String() + ".tmp" + + rule := turbine + args := map[string]string{ + "javacFlags": flags.javacFlags, + "srcJars": strings.Join(srcJars.Strings(), " "), + "javaVersion": flags.javaVersion.String(), + "turbineFlags": turbineFlags, + "outputFlags": outputFlags, + "outputs": strings.Join(outputs.Strings(), " "), + } + if ctx.Config().UseRBE() && ctx.Config().IsEnvTrue("RBE_TURBINE") { + rule = turbineRE + args["implicits"] = strings.Join(deps.Strings(), ",") + args["rbeOutputs"] = outputSrcJar.String() + ".tmp," + outputResJar.String() + ".tmp" + } + ctx.Build(pctx, android.BuildParams{ + Rule: rule, + Description: "turbine apt", + Output: outputs[0], + ImplicitOutputs: outputs[1:], + Inputs: srcFiles, + Implicits: deps, + Args: args, + }) +} + // transformJavaToClasses takes source files and converts them to a jar containing .class files. // srcFiles is a list of paths to sources, srcJars is a list of paths to jar files that contain // sources. flags contains various command line flags to be passed to the compiler. @@ -665,6 +716,6 @@ func (x *systemModules) FormTurbineSystemModulesPath(forceEmpty bool) (string, a } else if forceEmpty { return `--bootclasspath ""`, nil } else { - return "", nil + return "--system ${config.JavaHome}", nil } } diff --git a/java/java_test.go b/java/java_test.go index f095c5e48..4c9382413 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -973,7 +973,7 @@ func TestTurbine(t *testing.T) { fooHeaderJar := filepath.Join("out", "soong", ".intermediates", "foo", "android_common", "turbine-combined", "foo.jar") barTurbineJar := filepath.Join("out", "soong", ".intermediates", "bar", "android_common", "turbine", "bar.jar") - android.AssertStringDoesContain(t, "bar turbine classpath", barTurbine.Args["classpath"], fooHeaderJar) + android.AssertStringDoesContain(t, "bar turbine classpath", barTurbine.Args["turbineFlags"], fooHeaderJar) android.AssertStringDoesContain(t, "bar javac classpath", barJavac.Args["classpath"], fooHeaderJar) android.AssertPathsRelativeToTopEquals(t, "bar turbine combineJar", []string{barTurbineJar, fooHeaderJar}, barTurbineCombined.Inputs) android.AssertStringDoesContain(t, "baz javac classpath", bazJavac.Args["classpath"], "prebuilts/sdk/14/public/android.jar") diff --git a/java/kotlin.go b/java/kotlin.go index 3e5cec04c..ce79bae16 100644 --- a/java/kotlin.go +++ b/java/kotlin.go @@ -118,7 +118,7 @@ func kotlinCompile(ctx android.ModuleContext, outputFile android.WritablePath, }) } -var kapt = pctx.AndroidRemoteStaticRule("kapt", android.RemoteRuleSupports{Goma: true}, +var kaptStubs = pctx.AndroidRemoteStaticRule("kaptStubs", android.RemoteRuleSupports{Goma: true}, blueprint.RuleParams{ Command: `rm -rf "$srcJarDir" "$kotlinBuildFile" "$kaptDir" && ` + `mkdir -p "$srcJarDir" "$kaptDir/sources" "$kaptDir/classes" && ` + @@ -133,13 +133,12 @@ var kapt = pctx.AndroidRemoteStaticRule("kapt", android.RemoteRuleSupports{Goma: `-P plugin:org.jetbrains.kotlin.kapt3:classes=$kaptDir/classes ` + `-P plugin:org.jetbrains.kotlin.kapt3:stubs=$kaptDir/stubs ` + `-P plugin:org.jetbrains.kotlin.kapt3:correctErrorTypes=true ` + - `-P plugin:org.jetbrains.kotlin.kapt3:aptMode=stubsAndApt ` + + `-P plugin:org.jetbrains.kotlin.kapt3:aptMode=stubs ` + `-P plugin:org.jetbrains.kotlin.kapt3:javacArguments=$encodedJavacFlags ` + `$kaptProcessorPath ` + `$kaptProcessor ` + `-Xbuild-file=$kotlinBuildFile && ` + - `${config.SoongZipCmd} -jar -o $out -C $kaptDir/sources -D $kaptDir/sources && ` + - `${config.SoongZipCmd} -jar -o $classesJarOut -C $kaptDir/classes -D $kaptDir/classes && ` + + `${config.SoongZipCmd} -jar -o $out -C $kaptDir/stubs -D $kaptDir/stubs && ` + `rm -rf "$srcJarDir"`, CommandDeps: []string{ "${config.KotlincCmd}", @@ -197,13 +196,14 @@ func kotlinKapt(ctx android.ModuleContext, srcJarOutputFile, resJarOutputFile an kotlinName := filepath.Join(ctx.ModuleDir(), ctx.ModuleSubDir(), ctx.ModuleName()) kotlinName = strings.ReplaceAll(kotlinName, "/", "__") + // First run kapt to generate .java stubs from .kt files + kaptStubsJar := android.PathForModuleOut(ctx, "kapt", "stubs.jar") ctx.Build(pctx, android.BuildParams{ - Rule: kapt, - Description: "kapt", - Output: srcJarOutputFile, - ImplicitOutput: resJarOutputFile, - Inputs: srcFiles, - Implicits: deps, + Rule: kaptStubs, + Description: "kapt stubs", + Output: kaptStubsJar, + Inputs: srcFiles, + Implicits: deps, Args: map[string]string{ "classpath": flags.kotlincClasspath.FormJavaClassPath(""), "kotlincFlags": flags.kotlincFlags, @@ -219,6 +219,11 @@ func kotlinKapt(ctx android.ModuleContext, srcJarOutputFile, resJarOutputFile an "classesJarOut": resJarOutputFile.String(), }, }) + + // Then run turbine to perform annotation processing on the stubs and any .java srcFiles. + javaSrcFiles := srcFiles.FilterByExt(".java") + turbineSrcJars := append(android.Paths{kaptStubsJar}, srcJars...) + TurbineApt(ctx, srcJarOutputFile, resJarOutputFile, javaSrcFiles, turbineSrcJars, flags) } // kapt converts a list of key, value pairs into a base64 encoded Java serialization, which is what kapt expects. diff --git a/java/kotlin_test.go b/java/kotlin_test.go index cac0af3b9..d51bc041d 100644 --- a/java/kotlin_test.go +++ b/java/kotlin_test.go @@ -117,51 +117,71 @@ func TestKapt(t *testing.T) { buildOS := ctx.Config().BuildOS.String() - kapt := ctx.ModuleForTests("foo", "android_common").Rule("kapt") - kotlinc := ctx.ModuleForTests("foo", "android_common").Rule("kotlinc") - javac := ctx.ModuleForTests("foo", "android_common").Rule("javac") + foo := ctx.ModuleForTests("foo", "android_common") + kaptStubs := foo.Rule("kapt") + turbineApt := foo.Description("turbine apt") + kotlinc := foo.Rule("kotlinc") + javac := foo.Rule("javac") bar := ctx.ModuleForTests("bar", buildOS+"_common").Rule("javac").Output.String() baz := ctx.ModuleForTests("baz", buildOS+"_common").Rule("javac").Output.String() // Test that the kotlin and java sources are passed to kapt and kotlinc - if len(kapt.Inputs) != 2 || kapt.Inputs[0].String() != "a.java" || kapt.Inputs[1].String() != "b.kt" { - t.Errorf(`foo kapt inputs %v != ["a.java", "b.kt"]`, kapt.Inputs) + if len(kaptStubs.Inputs) != 2 || kaptStubs.Inputs[0].String() != "a.java" || kaptStubs.Inputs[1].String() != "b.kt" { + t.Errorf(`foo kapt inputs %v != ["a.java", "b.kt"]`, kaptStubs.Inputs) } if len(kotlinc.Inputs) != 2 || kotlinc.Inputs[0].String() != "a.java" || kotlinc.Inputs[1].String() != "b.kt" { t.Errorf(`foo kotlinc inputs %v != ["a.java", "b.kt"]`, kotlinc.Inputs) } - // Test that only the java sources are passed to javac + // Test that only the java sources are passed to turbine-apt and javac + if len(turbineApt.Inputs) != 1 || turbineApt.Inputs[0].String() != "a.java" { + t.Errorf(`foo turbine apt inputs %v != ["a.java"]`, turbineApt.Inputs) + } if len(javac.Inputs) != 1 || javac.Inputs[0].String() != "a.java" { t.Errorf(`foo inputs %v != ["a.java"]`, javac.Inputs) } - // Test that the kapt srcjar is a dependency of kotlinc and javac rules - if !inList(kapt.Output.String(), kotlinc.Implicits.Strings()) { - t.Errorf("expected %q in kotlinc implicits %v", kapt.Output.String(), kotlinc.Implicits.Strings()) - } - if !inList(kapt.Output.String(), javac.Implicits.Strings()) { - t.Errorf("expected %q in javac implicits %v", kapt.Output.String(), javac.Implicits.Strings()) + // Test that the kapt stubs jar is a dependency of turbine-apt + if !inList(kaptStubs.Output.String(), turbineApt.Implicits.Strings()) { + t.Errorf("expected %q in turbine-apt implicits %v", kaptStubs.Output.String(), kotlinc.Implicits.Strings()) } - // Test that the kapt srcjar is extracted by the kotlinc and javac rules - if kotlinc.Args["srcJars"] != kapt.Output.String() { - t.Errorf("expected %q in kotlinc srcjars %v", kapt.Output.String(), kotlinc.Args["srcJars"]) + // Test that the turbine-apt srcjar is a dependency of kotlinc and javac rules + if !inList(turbineApt.Output.String(), kotlinc.Implicits.Strings()) { + t.Errorf("expected %q in kotlinc implicits %v", turbineApt.Output.String(), kotlinc.Implicits.Strings()) } - if javac.Args["srcJars"] != kapt.Output.String() { - t.Errorf("expected %q in javac srcjars %v", kapt.Output.String(), kotlinc.Args["srcJars"]) + if !inList(turbineApt.Output.String(), javac.Implicits.Strings()) { + t.Errorf("expected %q in javac implicits %v", turbineApt.Output.String(), javac.Implicits.Strings()) + } + + // Test that the turbine-apt srcjar is extracted by the kotlinc and javac rules + if kotlinc.Args["srcJars"] != turbineApt.Output.String() { + t.Errorf("expected %q in kotlinc srcjars %v", turbineApt.Output.String(), kotlinc.Args["srcJars"]) + } + if javac.Args["srcJars"] != turbineApt.Output.String() { + t.Errorf("expected %q in javac srcjars %v", turbineApt.Output.String(), kotlinc.Args["srcJars"]) } // Test that the processors are passed to kapt expectedProcessorPath := "-P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + bar + " -P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + baz - if kapt.Args["kaptProcessorPath"] != expectedProcessorPath { - t.Errorf("expected kaptProcessorPath %q, got %q", expectedProcessorPath, kapt.Args["kaptProcessorPath"]) + if kaptStubs.Args["kaptProcessorPath"] != expectedProcessorPath { + t.Errorf("expected kaptProcessorPath %q, got %q", expectedProcessorPath, kaptStubs.Args["kaptProcessorPath"]) } expectedProcessor := "-P plugin:org.jetbrains.kotlin.kapt3:processors=com.bar -P plugin:org.jetbrains.kotlin.kapt3:processors=com.baz" - if kapt.Args["kaptProcessor"] != expectedProcessor { - t.Errorf("expected kaptProcessor %q, got %q", expectedProcessor, kapt.Args["kaptProcessor"]) + if kaptStubs.Args["kaptProcessor"] != expectedProcessor { + t.Errorf("expected kaptProcessor %q, got %q", expectedProcessor, kaptStubs.Args["kaptProcessor"]) + } + + // Test that the processors are passed to turbine-apt + expectedProcessorPath = "--processorpath " + bar + " " + baz + if !strings.Contains(turbineApt.Args["turbineFlags"], expectedProcessorPath) { + t.Errorf("expected turbine-apt processorpath %q, got %q", expectedProcessorPath, turbineApt.Args["turbineFlags"]) + } + expectedProcessor = "--processors com.bar com.baz" + if !strings.Contains(turbineApt.Args["turbineFlags"], expectedProcessor) { + t.Errorf("expected turbine-apt processor %q, got %q", expectedProcessor, turbineApt.Args["turbineFlags"]) } // Test that the processors are not passed to javac