From c9fe10f5b868bead40be5908840d751d840cae8a Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 19 Nov 2020 18:06:03 -0800 Subject: [PATCH] Remove restriction on exported plugins that generate APIs hilt_android requires seven separate annotation processors, which is only feasible to support using exported_plugins to avoid having to list all seven in every module that uses it. Unfortunately they all set generates_api: true. Turbine is already disabled for modules that directly use a plugin that sets generates_api: true, because turbine doesn't run annotation processors. Also add support for disabling turbine if a module transitively uses a plugin that generates APIs via exported_plugins. Bug: 173397767 Test: TestExportedPlugins Change-Id: If70354a3dd67efb4ce88bc9c934d41ccb6241b28 --- java/aar.go | 4 ++-- java/device_host_converter.go | 4 ++-- java/java.go | 40 +++++++++++++++++++++++++---------- java/java_test.go | 27 +++++++++++++++++++++-- 4 files changed, 58 insertions(+), 17 deletions(-) diff --git a/java/aar.go b/java/aar.go index 051715dd9..799e763ac 100644 --- a/java/aar.go +++ b/java/aar.go @@ -836,8 +836,8 @@ func (a *AARImport) ClassLoaderContexts() dexpreopt.ClassLoaderContextMap { return nil } -func (d *AARImport) ExportedPlugins() (android.Paths, []string) { - return nil, nil +func (d *AARImport) ExportedPlugins() (android.Paths, []string, bool) { + return nil, nil, false } func (a *AARImport) SrcJarArgs() ([]string, android.Paths) { diff --git a/java/device_host_converter.go b/java/device_host_converter.go index cd395b1b0..4914d74f6 100644 --- a/java/device_host_converter.go +++ b/java/device_host_converter.go @@ -167,8 +167,8 @@ func (d *DeviceHostConverter) ClassLoaderContexts() dexpreopt.ClassLoaderContext return nil } -func (d *DeviceHostConverter) ExportedPlugins() (android.Paths, []string) { - return nil, nil +func (d *DeviceHostConverter) ExportedPlugins() (android.Paths, []string, bool) { + return nil, nil, false } func (d *DeviceHostConverter) SrcJarArgs() ([]string, android.Paths) { diff --git a/java/java.go b/java/java.go index 1298b9e47..c9a38194c 100644 --- a/java/java.go +++ b/java/java.go @@ -201,7 +201,10 @@ type CompilerProperties struct { // List of modules to use as annotation processors Plugins []string - // List of modules to export to libraries that directly depend on this library as annotation processors + // List of modules to export to libraries that directly depend on this library as annotation + // processors. Note that if the plugins set generates_api: true this will disable the turbine + // optimization on modules that depend on this module, which will reduce parallelism and cause + // more recompilation. Exported_plugins []string // The number of Java source entries each Javac instance can process @@ -428,6 +431,9 @@ type Module struct { // list of plugins that this java module is exporting exportedPluginClasses []string + // if true, the exported plugins generate API and require disabling turbine. + exportedDisableTurbine bool + // list of source files, collected from srcFiles with unique java and all kt files, // will be used by android.IDEInfo struct expandIDEInfoCompiledSrcs []string @@ -513,7 +519,7 @@ type Dependency interface { ResourceJars() android.Paths AidlIncludeDirs() android.Paths ClassLoaderContexts() dexpreopt.ClassLoaderContextMap - ExportedPlugins() (android.Paths, []string) + ExportedPlugins() (android.Paths, []string, bool) SrcJarArgs() ([]string, android.Paths) BaseModuleName() string JacocoReportClassesFile() android.Path @@ -1049,8 +1055,9 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { // sdk lib names from dependencies are re-exported j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName) deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...) - pluginJars, pluginClasses := dep.ExportedPlugins() + pluginJars, pluginClasses, disableTurbine := dep.ExportedPlugins() addPlugins(&deps, pluginJars, pluginClasses...) + deps.disableTurbine = deps.disableTurbine || disableTurbine case java9LibTag: deps.java9Classpath = append(deps.java9Classpath, dep.HeaderJars()...) case staticLibTag: @@ -1061,8 +1068,12 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { // sdk lib names from dependencies are re-exported j.classLoaderContexts.AddContextMap(dep.ClassLoaderContexts(), otherName) deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...) - pluginJars, pluginClasses := dep.ExportedPlugins() + pluginJars, pluginClasses, disableTurbine := dep.ExportedPlugins() addPlugins(&deps, pluginJars, pluginClasses...) + // Turbine doesn't run annotation processors, so any module that uses an + // annotation processor that generates API is incompatible with the turbine + // optimization. + deps.disableTurbine = deps.disableTurbine || disableTurbine case pluginTag: if plugin, ok := dep.(*Plugin); ok { if plugin.pluginProperties.Processor_class != nil { @@ -1070,6 +1081,9 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } else { addPlugins(&deps, plugin.ImplementationAndResourcesJars()) } + // Turbine doesn't run annotation processors, so any module that uses an + // annotation processor that generates API is incompatible with the turbine + // optimization. deps.disableTurbine = deps.disableTurbine || Bool(plugin.pluginProperties.Generates_api) } else { ctx.PropertyErrorf("plugins", "%q is not a java_plugin module", otherName) @@ -1082,13 +1096,14 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } case exportedPluginTag: if plugin, ok := dep.(*Plugin); ok { - if plugin.pluginProperties.Generates_api != nil && *plugin.pluginProperties.Generates_api { - ctx.PropertyErrorf("exported_plugins", "Cannot export plugins with generates_api = true, found %v", otherName) - } j.exportedPluginJars = append(j.exportedPluginJars, plugin.ImplementationAndResourcesJars()...) if plugin.pluginProperties.Processor_class != nil { j.exportedPluginClasses = append(j.exportedPluginClasses, *plugin.pluginProperties.Processor_class) } + // Turbine doesn't run annotation processors, so any module that uses an + // annotation processor that generates API is incompatible with the turbine + // optimization. + j.exportedDisableTurbine = Bool(plugin.pluginProperties.Generates_api) } else { ctx.PropertyErrorf("exported_plugins", "%q is not a java_plugin module", otherName) } @@ -1922,8 +1937,11 @@ func (j *Module) ClassLoaderContexts() dexpreopt.ClassLoaderContextMap { return j.classLoaderContexts } -func (j *Module) ExportedPlugins() (android.Paths, []string) { - return j.exportedPluginJars, j.exportedPluginClasses +// ExportedPlugins returns the list of jars needed to run the exported plugins, the list of +// classes for the plugins, and a boolean for whether turbine needs to be disabled due to plugins +// that generate APIs. +func (j *Module) ExportedPlugins() (android.Paths, []string, bool) { + return j.exportedPluginJars, j.exportedPluginClasses, j.exportedDisableTurbine } func (j *Module) SrcJarArgs() ([]string, android.Paths) { @@ -2865,8 +2883,8 @@ func (j *Import) ClassLoaderContexts() dexpreopt.ClassLoaderContextMap { return j.classLoaderContexts } -func (j *Import) ExportedPlugins() (android.Paths, []string) { - return nil, nil +func (j *Import) ExportedPlugins() (android.Paths, []string, bool) { + return nil, nil, false } func (j *Import) SrcJarArgs() ([]string, android.Paths) { diff --git a/java/java_test.go b/java/java_test.go index cf56e66da..3ab228bd2 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -315,8 +315,9 @@ func TestSimple(t *testing.T) { func TestExportedPlugins(t *testing.T) { type Result struct { - library string - processors string + library string + processors string + disableTurbine bool } var tests = []struct { name string @@ -375,6 +376,18 @@ func TestExportedPlugins(t *testing.T) { {library: "foo", processors: "-processor com.android.TestPlugin,com.android.TestPlugin2"}, }, }, + { + name: "Exports plugin to with generates_api to dependee", + extra: ` + java_library{name: "exports", exported_plugins: ["plugin_generates_api"]} + java_library{name: "foo", srcs: ["a.java"], libs: ["exports"]} + java_library{name: "bar", srcs: ["a.java"], static_libs: ["exports"]} + `, + results: []Result{ + {library: "foo", processors: "-processor com.android.TestPlugin", disableTurbine: true}, + {library: "bar", processors: "-processor com.android.TestPlugin", disableTurbine: true}, + }, + }, } for _, test := range tests { @@ -384,6 +397,11 @@ func TestExportedPlugins(t *testing.T) { name: "plugin", processor_class: "com.android.TestPlugin", } + java_plugin { + name: "plugin_generates_api", + generates_api: true, + processor_class: "com.android.TestPlugin", + } `+test.extra) for _, want := range test.results { @@ -391,6 +409,11 @@ func TestExportedPlugins(t *testing.T) { if javac.Args["processor"] != want.processors { t.Errorf("For library %v, expected %v, found %v", want.library, want.processors, javac.Args["processor"]) } + turbine := ctx.ModuleForTests(want.library, "android_common").MaybeRule("turbine") + disableTurbine := turbine.BuildParams.Rule == nil + if disableTurbine != want.disableTurbine { + t.Errorf("For library %v, expected disableTurbine %v, found %v", want.library, want.disableTurbine, disableTurbine) + } } }) }