From 250e6198d45d7d29bea4c121d42afc8cb6a672d9 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 7 Jun 2019 10:44:37 +0100 Subject: [PATCH] Make sdkDep/decodeSdkDep the source of truth about the sdk Previously, different parts of the build used different sources of information about the SDK (i.e. the default libraries) against which a Java module aimed at the device was built. Some used the sdk_version property, others used the no_standard_libs or no_framework_libs, some used a combination of all three. That lead to inconsistent handling in the code, e.g. some parts treated no_standard_libs: true as implying no_framework_libs: true and others did not, and also in the build files, e.g. some modules specified no_framework_libs: true and sdk_version: "system_current" which makes no sense, or no_standard_libs: true and sdk_version: "core_current" which are inconsistent. This is the first step in a refactoring to simplify the sdk selection process by replacing the no_standard_libs/no_framework_libs properties with some extra options for the sdk_version property. This change consists of: 1) Extra functions sdkContext to access the no_standard_libs and no_framework_libs properties. 2) Extra field/functions in sdkDep to store and access the value of no_standard_libs/no_framework_libs. 3) Changes to decodeSdkDep(...) to pass the values of the no_... properties through to the returned sdkDep. 4) Change all code that accesses the no_... properties directly to call decodeSdkDep(...) to get an sdkDep object and then accessing the values of the no_... properties from there. The accessor functions on sdkDep are called has...() rather than no...() as most callers of the methods invert the value anyway and !no...() is harder to reason about than has...(). The hasFrameworkLibs() function returns true if and only if no_standard_libs and no_framework_libs are false. That is consistent with all but one usage of the no_framework_libs property and that is not affected by it. Bug: 134566750 Test: m droid Change-Id: I196e3304e8bd802fb154e897397b0dd337f868e2 Exempt-From-Owner-Approval: Colin has already given +2 modulo some minor nits and this blocking other changes. --- java/aar.go | 16 ++++++++++++---- java/app.go | 16 ++++++++++------ java/droiddoc.go | 14 +++++++++++--- java/java.go | 26 ++++++++++++++++++++++---- java/sdk.go | 20 +++++++++++++++++++- java/sdk_library.go | 14 ++++++++++---- 6 files changed, 84 insertions(+), 22 deletions(-) diff --git a/java/aar.go b/java/aar.go index 1e8e6d8c4..adea87ff6 100644 --- a/java/aar.go +++ b/java/aar.go @@ -188,8 +188,7 @@ func (a *aapt) aapt2Flags(ctx android.ModuleContext, sdkContext sdkContext, mani return linkFlags, linkDeps, resDirs, overlayDirs, rroDirs, resourceZips } -func (a *aapt) deps(ctx android.BottomUpMutatorContext, sdkContext sdkContext) { - sdkDep := decodeSdkDep(ctx, sdkContext) +func (a *aapt) deps(ctx android.BottomUpMutatorContext, sdkDep sdkDep) { if sdkDep.frameworkResModule != "" { ctx.AddVariationDependencies(nil, frameworkResTag, sdkDep.frameworkResModule) } @@ -401,8 +400,9 @@ var _ AndroidLibraryDependency = (*AndroidLibrary)(nil) func (a *AndroidLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { a.Module.deps(ctx) - if !Bool(a.properties.No_framework_libs) && !Bool(a.properties.No_standard_libs) { - a.aapt.deps(ctx, sdkContext(a)) + sdkDep := decodeSdkDep(ctx, sdkContext(a)) + if sdkDep.hasFrameworkLibs() { + a.aapt.deps(ctx, sdkDep) } } @@ -513,6 +513,14 @@ func (a *AARImport) targetSdkVersion() string { return a.sdkVersion() } +func (a *AARImport) noFrameworkLibs() bool { + return false +} + +func (a *AARImport) noStandardLibs() bool { + return false +} + var _ AndroidLibraryDependency = (*AARImport)(nil) func (a *AARImport) ExportPackage() android.Path { diff --git a/java/app.go b/java/app.go index 78e05012c..cab97de45 100644 --- a/java/app.go +++ b/java/app.go @@ -159,8 +159,9 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { ctx.PropertyErrorf("stl", "sdk_version must be set in order to use c++_shared") } - if !Bool(a.properties.No_framework_libs) && !Bool(a.properties.No_standard_libs) { - a.aapt.deps(ctx, sdkContext(a)) + sdkDep := decodeSdkDep(ctx, sdkContext(a)) + if sdkDep.hasFrameworkLibs() { + a.aapt.deps(ctx, sdkDep) } embedJni := a.shouldEmbedJnis(ctx) @@ -180,7 +181,7 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { } } - a.usesLibrary.deps(ctx, Bool(a.properties.No_framework_libs)) + a.usesLibrary.deps(ctx, sdkDep.hasFrameworkLibs()) } func (a *AndroidApp) OverridablePropertiesDepsMutator(ctx android.BottomUpMutatorContext) { @@ -783,7 +784,7 @@ func (a *AndroidAppImport) DepsMutator(ctx android.BottomUpMutatorContext) { ctx.AddDependency(ctx.Module(), certificateTag, cert) } - a.usesLibrary.deps(ctx, false) + a.usesLibrary.deps(ctx, true) } func (a *AndroidAppImport) uncompressEmbeddedJniLibs( @@ -937,11 +938,14 @@ type usesLibrary struct { usesLibraryProperties UsesLibraryProperties } -func (u *usesLibrary) deps(ctx android.BottomUpMutatorContext, noFrameworkLibs bool) { +func (u *usesLibrary) deps(ctx android.BottomUpMutatorContext, hasFrameworkLibs bool) { if !ctx.Config().UnbundledBuild() { ctx.AddVariationDependencies(nil, usesLibTag, u.usesLibraryProperties.Uses_libs...) ctx.AddVariationDependencies(nil, usesLibTag, u.presentOptionalUsesLibs(ctx)...) - if !noFrameworkLibs { + // Only add these extra dependencies if the module depends on framework libs. This avoids + // creating a cyclic dependency: + // e.g. framework-res -> org.apache.http.legacy -> ... -> framework-res. + if hasFrameworkLibs { // dexpreopt/dexpreopt.go needs the paths to the dex jars of these libraries in case construct_context.sh needs // to pass them to dex2oat. Add them as a dependency so we can determine the path to the dex jar of each // library to dexpreopt. diff --git a/java/droiddoc.go b/java/droiddoc.go index 992c8b57c..e1476a27f 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -538,16 +538,24 @@ func (j *Javadoc) targetSdkVersion() string { return j.sdkVersion() } +func (j *Javadoc) noFrameworkLibs() bool { + return Bool(j.properties.No_framework_libs) +} + +func (j *Javadoc) noStandardLibs() bool { + return Bool(j.properties.No_standard_libs) +} + func (j *Javadoc) addDeps(ctx android.BottomUpMutatorContext) { if ctx.Device() { - if !Bool(j.properties.No_standard_libs) { - sdkDep := decodeSdkDep(ctx, sdkContext(j)) + sdkDep := decodeSdkDep(ctx, sdkContext(j)) + if sdkDep.hasStandardLibs() { if sdkDep.useDefaultLibs { ctx.AddVariationDependencies(nil, bootClasspathTag, config.DefaultBootclasspathLibraries...) if ctx.Config().TargetOpenJDK9() { ctx.AddVariationDependencies(nil, systemModulesTag, config.DefaultSystemModules) } - if !Bool(j.properties.No_framework_libs) { + if sdkDep.hasFrameworkLibs() { ctx.AddVariationDependencies(nil, libTag, config.DefaultLibraries...) } } else if sdkDep.useModule { diff --git a/java/java.go b/java/java.go index a1addb38c..82f799e83 100644 --- a/java/java.go +++ b/java/java.go @@ -440,6 +440,16 @@ type sdkDep struct { jars android.Paths aidl android.OptionalPath + + noStandardLibs, noFrameworksLibs bool +} + +func (s sdkDep) hasStandardLibs() bool { + return !s.noStandardLibs +} + +func (s sdkDep) hasFrameworkLibs() bool { + return !s.noStandardLibs && !s.noFrameworksLibs } type jniLib struct { @@ -476,14 +486,22 @@ func (j *Module) targetSdkVersion() string { return j.sdkVersion() } +func (j *Module) noFrameworkLibs() bool { + return Bool(j.properties.No_framework_libs) +} + +func (j *Module) noStandardLibs() bool { + return Bool(j.properties.No_standard_libs) +} + func (j *Module) deps(ctx android.BottomUpMutatorContext) { if ctx.Device() { - if !Bool(j.properties.No_standard_libs) { - sdkDep := decodeSdkDep(ctx, sdkContext(j)) + sdkDep := decodeSdkDep(ctx, sdkContext(j)) + if sdkDep.hasStandardLibs() { if sdkDep.useDefaultLibs { ctx.AddVariationDependencies(nil, bootClasspathTag, config.DefaultBootclasspathLibraries...) ctx.AddVariationDependencies(nil, systemModulesTag, config.DefaultSystemModules) - if !Bool(j.properties.No_framework_libs) { + if sdkDep.hasFrameworkLibs() { ctx.AddVariationDependencies(nil, libTag, config.DefaultLibraries...) } } else if sdkDep.useModule { @@ -913,7 +931,7 @@ func (j *Module) collectBuilderFlags(ctx android.ModuleContext, deps deps) javaB flags.processor = strings.Join(deps.processorClasses, ",") if len(flags.bootClasspath) == 0 && ctx.Host() && flags.javaVersion != "1.9" && - !Bool(j.properties.No_standard_libs) && + decodeSdkDep(ctx, sdkContext(j)).hasStandardLibs() && inList(flags.javaVersion, []string{"1.6", "1.7", "1.8"}) { // Give host-side tools a version of OpenJDK's standard libraries // close to what they're targeting. As of Dec 2017, AOSP is only diff --git a/java/sdk.go b/java/sdk.go index 90b8fac28..76d36f6bd 100644 --- a/java/sdk.go +++ b/java/sdk.go @@ -38,12 +38,18 @@ var sdkFrameworkAidlPathKey = android.NewOnceKey("sdkFrameworkAidlPathKey") var apiFingerprintPathKey = android.NewOnceKey("apiFingerprintPathKey") type sdkContext interface { - // sdkVersion eturns the sdk_version property of the current module, or an empty string if it is not set. + // sdkVersion returns the sdk_version property of the current module, or an empty string if it is not set. sdkVersion() string // minSdkVersion returns the min_sdk_version property of the current module, or sdkVersion() if it is not set. minSdkVersion() string // targetSdkVersion returns the target_sdk_version property of the current module, or sdkVersion() if it is not set. targetSdkVersion() string + + // Temporarily provide access to the no_standard_libs property (where present). + noStandardLibs() bool + + // Temporarily provide access to the no_frameworks_libs property (where present). + noFrameworkLibs() bool } func sdkVersionOrDefault(ctx android.BaseModuleContext, v string) string { @@ -138,6 +144,10 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { useFiles: true, jars: android.Paths{jarPath.Path(), lambdaStubsPath}, aidl: android.OptionalPathForPath(aidlPath.Path()), + + // Pass values straight through for now to match previous behavior. + noStandardLibs: sdkContext.noStandardLibs(), + noFrameworksLibs: sdkContext.noFrameworkLibs(), } } @@ -148,6 +158,10 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { systemModules: m + "_system_modules", frameworkResModule: r, aidl: android.OptionalPathForPath(aidl), + + // Pass values straight through for now to match previous behavior. + noStandardLibs: sdkContext.noStandardLibs(), + noFrameworksLibs: sdkContext.noFrameworkLibs(), } if m == "core.current.stubs" { @@ -182,6 +196,10 @@ func decodeSdkDep(ctx android.BaseModuleContext, sdkContext sdkContext) sdkDep { return sdkDep{ useDefaultLibs: true, frameworkResModule: "framework-res", + + // Pass values straight through for now to match previous behavior. + noStandardLibs: sdkContext.noStandardLibs(), + noFrameworksLibs: sdkContext.noFrameworkLibs(), } case "current": return toModule("android_stubs_current", "framework-res", sdkFrameworkAidlPath(ctx)) diff --git a/java/sdk_library.go b/java/sdk_library.go index e38353340..05c7e814d 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -159,7 +159,8 @@ func (module *SdkLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { ctx.AddVariationDependencies(nil, publicApiStubsTag, module.stubsName(apiScopePublic)) ctx.AddVariationDependencies(nil, publicApiFileTag, module.docsName(apiScopePublic)) - if !Bool(module.properties.No_standard_libs) { + sdkDep := decodeSdkDep(ctx, sdkContext(&module.Library)) + if sdkDep.hasStandardLibs() { ctx.AddVariationDependencies(nil, systemApiStubsTag, module.stubsName(apiScopeSystem)) ctx.AddVariationDependencies(nil, systemApiFileTag, module.docsName(apiScopeSystem)) ctx.AddVariationDependencies(nil, testApiFileTag, module.docsName(apiScopeTest)) @@ -401,6 +402,8 @@ func (module *SdkLibrary) createStubsLibrary(mctx android.LoadHookContext, apiSc } }{} + sdkDep := decodeSdkDep(mctx, sdkContext(&module.Library)) + props.Name = proptools.StringPtr(module.stubsName(apiScope)) // sources are generated from the droiddoc props.Srcs = []string{":" + module.docsName(apiScope)} @@ -411,7 +414,7 @@ func (module *SdkLibrary) createStubsLibrary(mctx android.LoadHookContext, apiSc props.Product_variables.Unbundled_build.Enabled = proptools.BoolPtr(false) } props.Product_variables.Pdk.Enabled = proptools.BoolPtr(false) - props.No_standard_libs = module.Library.Module.properties.No_standard_libs + props.No_standard_libs = proptools.BoolPtr(!sdkDep.hasStandardLibs()) props.System_modules = module.Library.Module.deviceProperties.System_modules props.Openjdk9.Srcs = module.Library.Module.properties.Openjdk9.Srcs props.Openjdk9.Javacflags = module.Library.Module.properties.Openjdk9.Javacflags @@ -462,6 +465,8 @@ func (module *SdkLibrary) createDocs(mctx android.LoadHookContext, apiScope apiS } }{} + sdkDep := decodeSdkDep(mctx, sdkContext(&module.Library)) + props.Name = proptools.StringPtr(module.docsName(apiScope)) props.Srcs = append(props.Srcs, module.Library.Module.properties.Srcs...) props.Srcs = append(props.Srcs, module.sdkLibraryProperties.Api_srcs...) @@ -472,7 +477,7 @@ func (module *SdkLibrary) createDocs(mctx android.LoadHookContext, apiScope apiS props.Libs = append(props.Libs, module.Library.Module.properties.Static_libs...) props.Aidl.Include_dirs = module.Library.Module.deviceProperties.Aidl.Include_dirs props.Aidl.Local_include_dirs = module.Library.Module.deviceProperties.Aidl.Local_include_dirs - props.No_standard_libs = module.Library.Module.properties.No_standard_libs + props.No_standard_libs = proptools.BoolPtr(!sdkDep.hasStandardLibs()) props.Java_version = module.Library.Module.properties.Java_version props.Merge_annotations_dirs = module.sdkLibraryProperties.Merge_annotations_dirs @@ -701,7 +706,8 @@ func (module *SdkLibrary) CreateInternalModules(mctx android.LoadHookContext) { module.createStubsLibrary(mctx, apiScopePublic) module.createDocs(mctx, apiScopePublic) - if !Bool(module.properties.No_standard_libs) { + sdkDep := decodeSdkDep(mctx, sdkContext(&module.Library)) + if sdkDep.hasStandardLibs() { // for system API stubs module.createStubsLibrary(mctx, apiScopeSystem) module.createDocs(mctx, apiScopeSystem)