From bc15e3a798d4cb8fcaa77ed245818a91e7a56395 Mon Sep 17 00:00:00 2001 From: Jaewoong Jung Date: Wed, 10 Mar 2021 17:02:43 -0800 Subject: [PATCH] Improve java.go readability. Test: TreeHugger Change-Id: I98bb7bddb740451ff2ccd28dcdaddff27e35a8cd --- java/app.go | 2 +- java/java.go | 215 +++++++++++++++++++++++--------------------- java/proto.go | 2 +- java/sdk_library.go | 2 +- 4 files changed, 118 insertions(+), 103 deletions(-) diff --git a/java/app.go b/java/app.go index eef627c14..59f823e99 100755 --- a/java/app.go +++ b/java/app.go @@ -302,7 +302,7 @@ func (a *AndroidApp) checkAppSdkVersions(ctx android.ModuleContext) { // This check is enforced for "updatable" APKs (including APK-in-APEX). // b/155209650: until min_sdk_version is properly supported, use sdk_version instead. // because, sdk_version is overridden by min_sdk_version (if set as smaller) -// and linkType is checked with dependencies so we can be sure that the whole dependency tree +// and sdkLinkType is checked with dependencies so we can be sure that the whole dependency tree // will meet the requirements. func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVersion sdkVersion) { // It's enough to check direct JNI deps' sdk_version because all transitive deps from JNI deps are checked in cc.checkLinkType() diff --git a/java/java.go b/java/java.go index 036c7afd4..6982f522d 100644 --- a/java/java.go +++ b/java/java.go @@ -37,6 +37,36 @@ import ( func init() { RegisterJavaBuildComponents(android.InitRegistrationContext) + RegisterJavaSdkMemberTypes() +} + +func RegisterJavaBuildComponents(ctx android.RegistrationContext) { + ctx.RegisterModuleType("java_defaults", DefaultsFactory) + + ctx.RegisterModuleType("java_library", LibraryFactory) + ctx.RegisterModuleType("java_library_static", LibraryStaticFactory) + ctx.RegisterModuleType("java_library_host", LibraryHostFactory) + ctx.RegisterModuleType("java_binary", BinaryFactory) + ctx.RegisterModuleType("java_binary_host", BinaryHostFactory) + ctx.RegisterModuleType("java_test", TestFactory) + ctx.RegisterModuleType("java_test_helper_library", TestHelperLibraryFactory) + ctx.RegisterModuleType("java_test_host", TestHostFactory) + ctx.RegisterModuleType("java_test_import", JavaTestImportFactory) + ctx.RegisterModuleType("java_import", ImportFactory) + ctx.RegisterModuleType("java_import_host", ImportFactoryHost) + ctx.RegisterModuleType("java_device_for_host", DeviceForHostFactory) + ctx.RegisterModuleType("java_host_for_device", HostForDeviceFactory) + ctx.RegisterModuleType("dex_import", DexImportFactory) + + ctx.FinalDepsMutators(func(ctx android.RegisterMutatorsContext) { + ctx.BottomUp("dexpreopt_tool_deps", dexpreoptToolDepsMutator).Parallel() + }) + + ctx.RegisterSingletonType("logtags", LogtagsSingleton) + ctx.RegisterSingletonType("kythe_java_extract", kytheExtractJavaFactory) +} + +func RegisterJavaSdkMemberTypes() { // Register sdk member types. android.RegisterSdkMemberType(javaHeaderLibsSdkMemberType) @@ -89,85 +119,7 @@ func init() { PropertyName: "java_tests", }, }) -} -func RegisterJavaBuildComponents(ctx android.RegistrationContext) { - ctx.RegisterModuleType("java_defaults", DefaultsFactory) - - ctx.RegisterModuleType("java_library", LibraryFactory) - ctx.RegisterModuleType("java_library_static", LibraryStaticFactory) - ctx.RegisterModuleType("java_library_host", LibraryHostFactory) - ctx.RegisterModuleType("java_binary", BinaryFactory) - ctx.RegisterModuleType("java_binary_host", BinaryHostFactory) - ctx.RegisterModuleType("java_test", TestFactory) - ctx.RegisterModuleType("java_test_helper_library", TestHelperLibraryFactory) - ctx.RegisterModuleType("java_test_host", TestHostFactory) - ctx.RegisterModuleType("java_test_import", JavaTestImportFactory) - ctx.RegisterModuleType("java_import", ImportFactory) - ctx.RegisterModuleType("java_import_host", ImportFactoryHost) - ctx.RegisterModuleType("java_device_for_host", DeviceForHostFactory) - ctx.RegisterModuleType("java_host_for_device", HostForDeviceFactory) - ctx.RegisterModuleType("dex_import", DexImportFactory) - - ctx.FinalDepsMutators(func(ctx android.RegisterMutatorsContext) { - ctx.BottomUp("dexpreopt_tool_deps", dexpreoptToolDepsMutator).Parallel() - }) - - ctx.RegisterSingletonType("logtags", LogtagsSingleton) - ctx.RegisterSingletonType("kythe_java_extract", kytheExtractJavaFactory) -} - -func (j *Module) CheckStableSdkVersion() error { - sdkVersion := j.sdkVersion() - if sdkVersion.stable() { - return nil - } - if sdkVersion.kind == sdkCorePlatform { - if useLegacyCorePlatformApiByName(j.BaseModuleName()) { - return fmt.Errorf("non stable SDK %v - uses legacy core platform", sdkVersion) - } else { - // Treat stable core platform as stable. - return nil - } - } else { - return fmt.Errorf("non stable SDK %v", sdkVersion) - } -} - -func (j *Module) checkSdkVersions(ctx android.ModuleContext) { - if j.RequiresStableAPIs(ctx) { - if sc, ok := ctx.Module().(sdkContext); ok { - if !sc.sdkVersion().specified() { - ctx.PropertyErrorf("sdk_version", - "sdk_version must have a value when the module is located at vendor or product(only if PRODUCT_ENFORCE_PRODUCT_PARTITION_INTERFACE is set).") - } - } - } - - ctx.VisitDirectDeps(func(module android.Module) { - tag := ctx.OtherModuleDependencyTag(module) - switch module.(type) { - // TODO(satayev): cover other types as well, e.g. imports - case *Library, *AndroidLibrary: - switch tag { - case bootClasspathTag, libTag, staticLibTag, java9LibTag: - checkLinkType(ctx, j, module.(linkTypeContext), tag.(dependencyTag)) - } - } - }) -} - -func (j *Module) checkPlatformAPI(ctx android.ModuleContext) { - if sc, ok := ctx.Module().(sdkContext); ok { - usePlatformAPI := proptools.Bool(j.deviceProperties.Platform_apis) - sdkVersionSpecified := sc.sdkVersion().specified() - if usePlatformAPI && sdkVersionSpecified { - ctx.PropertyErrorf("platform_apis", "platform_apis must be false when sdk_version is not empty.") - } else if !usePlatformAPI && !sdkVersionSpecified { - ctx.PropertyErrorf("platform_apis", "platform_apis must be true when sdk_version is empty.") - } - - } } // TODO: @@ -179,7 +131,8 @@ func (j *Module) checkPlatformAPI(ctx android.ModuleContext) { // DroidDoc // Findbugs -type CompilerProperties struct { +// Properties that are common to most Java modules, i.e. whether it's a host or device module. +type CommonProperties struct { // list of source files used to compile the Java module. May be .java, .kt, .logtags, .proto, // or .aidl files. Srcs []string `android:"path,arch_variant"` @@ -312,7 +265,9 @@ type CompilerProperties struct { Hiddenapi_additional_annotations []string } -type CompilerDeviceProperties struct { +// Properties that are specific to device modules. Host module factories should not add these when +// constructing a new module. +type DeviceProperties struct { // if not blank, set to the version of the sdk to compile against. // Defaults to compiling against the current platform. Sdk_version *string @@ -421,9 +376,9 @@ type Module struct { // Functionality common to Module and Import. embeddableInModuleAndImport - properties CompilerProperties + properties CommonProperties protoProperties android.ProtoProperties - deviceProperties CompilerDeviceProperties + deviceProperties DeviceProperties // jar file containing header classes including static library dependencies, suitable for // inserting into the bootclasspath/classpath of another compile @@ -508,6 +463,62 @@ type Module struct { hideApexVariantFromMake bool } +func (j *Module) CheckStableSdkVersion() error { + sdkVersion := j.sdkVersion() + if sdkVersion.stable() { + return nil + } + if sdkVersion.kind == sdkCorePlatform { + if useLegacyCorePlatformApiByName(j.BaseModuleName()) { + return fmt.Errorf("non stable SDK %v - uses legacy core platform", sdkVersion) + } else { + // Treat stable core platform as stable. + return nil + } + } else { + return fmt.Errorf("non stable SDK %v", sdkVersion) + } +} + +// checkSdkVersions enforces restrictions around SDK dependencies. +func (j *Module) checkSdkVersions(ctx android.ModuleContext) { + if j.RequiresStableAPIs(ctx) { + if sc, ok := ctx.Module().(sdkContext); ok { + if !sc.sdkVersion().specified() { + ctx.PropertyErrorf("sdk_version", + "sdk_version must have a value when the module is located at vendor or product(only if PRODUCT_ENFORCE_PRODUCT_PARTITION_INTERFACE is set).") + } + } + } + + // Make sure this module doesn't statically link to modules with lower-ranked SDK link type. + // See rank() for details. + ctx.VisitDirectDeps(func(module android.Module) { + tag := ctx.OtherModuleDependencyTag(module) + switch module.(type) { + // TODO(satayev): cover other types as well, e.g. imports + case *Library, *AndroidLibrary: + switch tag { + case bootClasspathTag, libTag, staticLibTag, java9LibTag: + j.checkSdkLinkType(ctx, module.(moduleWithSdkDep), tag.(dependencyTag)) + } + } + }) +} + +func (j *Module) checkPlatformAPI(ctx android.ModuleContext) { + if sc, ok := ctx.Module().(sdkContext); ok { + usePlatformAPI := proptools.Bool(j.deviceProperties.Platform_apis) + sdkVersionSpecified := sc.sdkVersion().specified() + if usePlatformAPI && sdkVersionSpecified { + ctx.PropertyErrorf("platform_apis", "platform_apis must be false when sdk_version is not empty.") + } else if !usePlatformAPI && !sdkVersionSpecified { + ctx.PropertyErrorf("platform_apis", "platform_apis must be true when sdk_version is empty.") + } + + } +} + func (j *Module) addHostProperties() { j.AddProperties( &j.properties, @@ -1008,13 +1019,13 @@ func checkProducesJars(ctx android.ModuleContext, dep android.SourceFileProducer } } -type linkType int +type sdkLinkType int const ( // TODO(jiyong) rename these for better readability. Make the allowed // and disallowed link types explicit // order is important here. See rank() - javaCore linkType = iota + javaCore sdkLinkType = iota javaSdk javaSystem javaModule @@ -1022,7 +1033,7 @@ const ( javaPlatform ) -func (lt linkType) String() string { +func (lt sdkLinkType) String() string { switch lt { case javaCore: return "core Java API" @@ -1041,18 +1052,19 @@ func (lt linkType) String() string { } } -// rank determins the total order among linkTypes. A link type of rank A can link to another link -// type of rank B only when B <= A -func (lt linkType) rank() int { +// rank determines the total order among sdkLinkType. An SDK link type of rank A can link to +// another SDK link type of rank B only when B <= A. For example, a module linking to Android SDK +// can't statically depend on modules that use Platform API. +func (lt sdkLinkType) rank() int { return int(lt) } -type linkTypeContext interface { +type moduleWithSdkDep interface { android.Module - getLinkType(name string) (ret linkType, stubs bool) + getSdkLinkType(name string) (ret sdkLinkType, stubs bool) } -func (m *Module) getLinkType(name string) (ret linkType, stubs bool) { +func (m *Module) getSdkLinkType(name string) (ret sdkLinkType, stubs bool) { switch name { case "core.current.stubs", "legacy.core.platform.api.stubs", "stable.core.platform.api.stubs", "stub-annotations", "private-stub-annotations-jar", @@ -1096,23 +1108,26 @@ func (m *Module) getLinkType(name string) (ret linkType, stubs bool) { return javaSdk, false } -func checkLinkType(ctx android.ModuleContext, from *Module, to linkTypeContext, tag dependencyTag) { +// checkSdkLinkType make sures the given dependency doesn't have a lower SDK link type rank than +// this module's. See the comment on rank() for details and an example. +func (j *Module) checkSdkLinkType( + ctx android.ModuleContext, dep moduleWithSdkDep, tag dependencyTag) { if ctx.Host() { return } - myLinkType, stubs := from.getLinkType(ctx.ModuleName()) + myLinkType, stubs := j.getSdkLinkType(ctx.ModuleName()) if stubs { return } - otherLinkType, _ := to.getLinkType(ctx.OtherModuleName(to)) + depLinkType, _ := dep.getSdkLinkType(ctx.OtherModuleName(dep)) - if myLinkType.rank() < otherLinkType.rank() { + if myLinkType.rank() < depLinkType.rank() { ctx.ModuleErrorf("compiles against %v, but dependency %q is compiling against %v. "+ "In order to fix this, consider adjusting sdk_version: OR platform_apis: "+ "property of the source or target module so that target module is built "+ "with the same or smaller API set when compared to the source.", - myLinkType, ctx.OtherModuleName(to), otherLinkType) + myLinkType, ctx.OtherModuleName(dep), depLinkType) } } @@ -1133,7 +1148,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } } - linkType, _ := j.getLinkType(ctx.ModuleName()) + sdkLinkType, _ := j.getSdkLinkType(ctx.ModuleName()) ctx.VisitDirectDeps(func(module android.Module) { otherName := ctx.OtherModuleName(module) @@ -1157,7 +1172,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } } else if ctx.OtherModuleHasProvider(module, JavaInfoProvider) { dep := ctx.OtherModuleProvider(module, JavaInfoProvider).(JavaInfo) - if linkType != javaPlatform && + if sdkLinkType != javaPlatform && ctx.OtherModuleHasProvider(module, SyspropPublicStubInfoProvider) { // dep is a sysprop implementation library, but this module is not linking against // the platform, so it gets the sysprop public stubs library instead. Replace @@ -3332,8 +3347,8 @@ func DefaultsFactory() android.Module { module := &Defaults{} module.AddProperties( - &CompilerProperties{}, - &CompilerDeviceProperties{}, + &CommonProperties{}, + &DeviceProperties{}, &DexProperties{}, &DexpreoptProperties{}, &android.ProtoProperties{}, diff --git a/java/proto.go b/java/proto.go index 652a4daec..8731822c3 100644 --- a/java/proto.go +++ b/java/proto.go @@ -94,7 +94,7 @@ func protoDeps(ctx android.BottomUpMutatorContext, p *android.ProtoProperties) { } } -func protoFlags(ctx android.ModuleContext, j *CompilerProperties, p *android.ProtoProperties, +func protoFlags(ctx android.ModuleContext, j *CommonProperties, p *android.ProtoProperties, flags javaBuilderFlags) javaBuilderFlags { flags.proto = android.GetProtoFlags(ctx, p) diff --git a/java/sdk_library.go b/java/sdk_library.go index b03f90cf4..e1ca77dcf 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -1681,7 +1681,7 @@ func (s *defaultNamingScheme) apiModuleName(scope *apiScope, baseName string) st var _ sdkLibraryComponentNamingScheme = (*defaultNamingScheme)(nil) -func moduleStubLinkType(name string) (stub bool, ret linkType) { +func moduleStubLinkType(name string) (stub bool, ret sdkLinkType) { // This suffix-based approach is fragile and could potentially mis-trigger. // TODO(b/155164730): Clean this up when modules no longer reference sdk_lib stubs directly. if strings.HasSuffix(name, ".stubs.public") || strings.HasSuffix(name, "-stubs-publicapi") {