Merge "Use turbine instead of kapt for kotlin annotation processors" am: edd46702e1 am: 9ad80f6486

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2030724 Change-Id: I4fa56fbcd8d1c01b742f4ab610b52713b9048c44 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2022-03-28 18:47:13 +00:00
parent b420d23c16 9ad80f6486
commit eed308db9e
4 changed files with 124 additions and 48 deletions
--- 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
 	}
 }
--- 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")
--- 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.
--- 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