From 75ce9eccf3651991ccd2897bba03b8a9bba9b733 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 26 Feb 2021 16:20:32 -0800 Subject: [PATCH] Remove global state from sysprop libraries Sysprop libraries use a global list to rewrite dependencies from implementation libraries to public stub libraries when appropriate. Remove the global list, and instead add a dependency from the implementation to the public stub that forwards the JavaInfo. Bug: 181367697 Test: sysprop_test.go Change-Id: Ia7995feb3c079ca9bb6a403daaae3e3329fd7f6a --- java/Android.bp | 1 - java/java.go | 115 +++++++++++++++++++---------------- java/sdk_library_external.go | 9 +++ java/sysprop.go | 64 ------------------- sysprop/sysprop_library.go | 109 +++++++++++++++------------------ sysprop/sysprop_test.go | 21 ++----- 6 files changed, 124 insertions(+), 195 deletions(-) delete mode 100644 java/sysprop.go diff --git a/java/Android.bp b/java/Android.bp index 9bfd009c6..461b16d58 100644 --- a/java/Android.bp +++ b/java/Android.bp @@ -58,7 +58,6 @@ bootstrap_go_package { "sdk_library.go", "sdk_library_external.go", "support_libraries.go", - "sysprop.go", "system_modules.go", "testing.go", "tradefed.go", diff --git a/java/java.go b/java/java.go index 698c5b915..e471f0de6 100644 --- a/java/java.go +++ b/java/java.go @@ -370,6 +370,10 @@ type CompilerDeviceProperties struct { // If true, generate the signature file of APK Signing Scheme V4, along side the signed APK file. // Defaults to false. V4_signature *bool + + // Only for libraries created by a sysprop_library module, SyspropPublicStub is the name of the + // public stubs library. + SyspropPublicStub string `blueprint:"mutated"` } // Functionality common to Module and Import @@ -580,6 +584,16 @@ type JavaInfo struct { var JavaInfoProvider = blueprint.NewProvider(JavaInfo{}) +// SyspropPublicStubInfo contains info about the sysprop public stub library that corresponds to +// the sysprop implementation library. +type SyspropPublicStubInfo struct { + // JavaInfo is the JavaInfoProvider of the sysprop public stub library that corresponds to + // the sysprop implementation library. + JavaInfo JavaInfo +} + +var SyspropPublicStubInfoProvider = blueprint.NewProvider(SyspropPublicStubInfo{}) + // Methods that need to be implemented for a module that is added to apex java_libs property. type ApexDependency interface { HeaderJars() android.Paths @@ -649,29 +663,30 @@ func IsJniDepTag(depTag blueprint.DependencyTag) bool { } var ( - dataNativeBinsTag = dependencyTag{name: "dataNativeBins"} - staticLibTag = dependencyTag{name: "staticlib"} - libTag = dependencyTag{name: "javalib"} - java9LibTag = dependencyTag{name: "java9lib"} - pluginTag = dependencyTag{name: "plugin"} - errorpronePluginTag = dependencyTag{name: "errorprone-plugin"} - exportedPluginTag = dependencyTag{name: "exported-plugin"} - bootClasspathTag = dependencyTag{name: "bootclasspath"} - systemModulesTag = dependencyTag{name: "system modules"} - frameworkResTag = dependencyTag{name: "framework-res"} - kotlinStdlibTag = dependencyTag{name: "kotlin-stdlib"} - kotlinAnnotationsTag = dependencyTag{name: "kotlin-annotations"} - proguardRaiseTag = dependencyTag{name: "proguard-raise"} - certificateTag = dependencyTag{name: "certificate"} - instrumentationForTag = dependencyTag{name: "instrumentation_for"} - extraLintCheckTag = dependencyTag{name: "extra-lint-check"} - jniLibTag = dependencyTag{name: "jnilib"} - jniInstallTag = installDependencyTag{name: "jni install"} - binaryInstallTag = installDependencyTag{name: "binary install"} - usesLibTag = makeUsesLibraryDependencyTag(dexpreopt.AnySdkVersion) - usesLibCompat28Tag = makeUsesLibraryDependencyTag(28) - usesLibCompat29Tag = makeUsesLibraryDependencyTag(29) - usesLibCompat30Tag = makeUsesLibraryDependencyTag(30) + dataNativeBinsTag = dependencyTag{name: "dataNativeBins"} + staticLibTag = dependencyTag{name: "staticlib"} + libTag = dependencyTag{name: "javalib"} + java9LibTag = dependencyTag{name: "java9lib"} + pluginTag = dependencyTag{name: "plugin"} + errorpronePluginTag = dependencyTag{name: "errorprone-plugin"} + exportedPluginTag = dependencyTag{name: "exported-plugin"} + bootClasspathTag = dependencyTag{name: "bootclasspath"} + systemModulesTag = dependencyTag{name: "system modules"} + frameworkResTag = dependencyTag{name: "framework-res"} + kotlinStdlibTag = dependencyTag{name: "kotlin-stdlib"} + kotlinAnnotationsTag = dependencyTag{name: "kotlin-annotations"} + proguardRaiseTag = dependencyTag{name: "proguard-raise"} + certificateTag = dependencyTag{name: "certificate"} + instrumentationForTag = dependencyTag{name: "instrumentation_for"} + extraLintCheckTag = dependencyTag{name: "extra-lint-check"} + jniLibTag = dependencyTag{name: "jnilib"} + syspropPublicStubDepTag = dependencyTag{name: "sysprop public stub"} + jniInstallTag = installDependencyTag{name: "jni install"} + binaryInstallTag = installDependencyTag{name: "binary install"} + usesLibTag = makeUsesLibraryDependencyTag(dexpreopt.AnySdkVersion) + usesLibCompat28Tag = makeUsesLibraryDependencyTag(28) + usesLibCompat29Tag = makeUsesLibraryDependencyTag(29) + usesLibCompat30Tag = makeUsesLibraryDependencyTag(30) ) func IsLibDepTag(depTag blueprint.DependencyTag) bool { @@ -810,35 +825,17 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { j.linter.deps(ctx) sdkDeps(ctx, sdkContext(j), j.dexer) - } - syspropPublicStubs := syspropPublicStubs(ctx.Config()) - - // rewriteSyspropLibs validates if a java module can link against platform's sysprop_library, - // and redirects dependency to public stub depending on the link type. - rewriteSyspropLibs := func(libs []string, prop string) []string { - // make a copy - ret := android.CopyOf(libs) - - for idx, lib := range libs { - stub, ok := syspropPublicStubs[lib] - - if !ok { - continue - } - - linkType, _ := j.getLinkType(ctx.ModuleName()) - // only platform modules can use internal props - if linkType != javaPlatform { - ret[idx] = stub - } + if j.deviceProperties.SyspropPublicStub != "" { + // This is a sysprop implementation library that has a corresponding sysprop public + // stubs library, and a dependency on it so that dependencies on the implementation can + // be forwarded to the public stubs library when necessary. + ctx.AddVariationDependencies(nil, syspropPublicStubDepTag, j.deviceProperties.SyspropPublicStub) } - - return ret } - libDeps := ctx.AddVariationDependencies(nil, libTag, rewriteSyspropLibs(j.properties.Libs, "libs")...) - ctx.AddVariationDependencies(nil, staticLibTag, rewriteSyspropLibs(j.properties.Static_libs, "static_libs")...) + libDeps := ctx.AddVariationDependencies(nil, libTag, j.properties.Libs...) + ctx.AddVariationDependencies(nil, staticLibTag, j.properties.Static_libs...) // Add dependency on libraries that provide additional hidden api annotations. ctx.AddVariationDependencies(nil, hiddenApiAnnotationsTag, j.properties.Hiddenapi_additional_annotations...) @@ -853,15 +850,11 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { // if true, enable enforcement // PRODUCT_INTER_PARTITION_JAVA_LIBRARY_ALLOWLIST // exception list of java_library names to allow inter-partition dependency - for idx, lib := range j.properties.Libs { + for idx := range j.properties.Libs { if libDeps[idx] == nil { continue } - if _, ok := syspropPublicStubs[lib]; ok { - continue - } - if javaDep, ok := libDeps[idx].(javaSdkLibraryEnforceContext); ok { // java_sdk_library is always allowed at inter-partition dependency. // So, skip check. @@ -1131,6 +1124,8 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } } + linkType, _ := j.getLinkType(ctx.ModuleName()) + ctx.VisitDirectDeps(func(module android.Module) { otherName := ctx.OtherModuleName(module) tag := ctx.OtherModuleDependencyTag(module) @@ -1153,6 +1148,14 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { } } else if ctx.OtherModuleHasProvider(module, JavaInfoProvider) { dep := ctx.OtherModuleProvider(module, JavaInfoProvider).(JavaInfo) + if linkType != 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 + // dep with the JavaInfo from the SyspropPublicStubInfoProvider. + syspropDep := ctx.OtherModuleProvider(module, SyspropPublicStubInfoProvider).(SyspropPublicStubInfo) + dep = syspropDep.JavaInfo + } switch tag { case bootClasspathTag: deps.bootClasspath = append(deps.bootClasspath, dep.HeaderJars...) @@ -1211,6 +1214,12 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { deps.kotlinStdlib = append(deps.kotlinStdlib, dep.HeaderJars...) case kotlinAnnotationsTag: deps.kotlinAnnotations = dep.HeaderJars + case syspropPublicStubDepTag: + // This is a sysprop implementation library, forward the JavaInfoProvider from + // the corresponding sysprop public stub library as SyspropPublicStubInfoProvider. + ctx.SetProvider(SyspropPublicStubInfoProvider, SyspropPublicStubInfo{ + JavaInfo: dep, + }) } } else if dep, ok := module.(android.SourceFileProducer); ok { switch tag { diff --git a/java/sdk_library_external.go b/java/sdk_library_external.go index 293493685..0acaa13b2 100644 --- a/java/sdk_library_external.go +++ b/java/sdk_library_external.go @@ -75,10 +75,15 @@ func (j *Module) allowListedInterPartitionJavaLibrary(ctx android.EarlyModuleCon return inList(j.Name(), ctx.Config().InterPartitionJavaLibraryAllowList()) } +func (j *Module) syspropWithPublicStubs() bool { + return j.deviceProperties.SyspropPublicStub != "" +} + type javaSdkLibraryEnforceContext interface { Name() string allowListedInterPartitionJavaLibrary(ctx android.EarlyModuleContext) bool partitionGroup(ctx android.EarlyModuleContext) partitionGroup + syspropWithPublicStubs() bool } var _ javaSdkLibraryEnforceContext = (*Module)(nil) @@ -88,6 +93,10 @@ func (j *Module) checkPartitionsForJavaDependency(ctx android.EarlyModuleContext return } + if dep.syspropWithPublicStubs() { + return + } + // If product interface is not enforced, skip check between system and product partition. // But still need to check between product and vendor partition because product interface flag // just represents enforcement between product and system, and vendor interface enforcement diff --git a/java/sysprop.go b/java/sysprop.go deleted file mode 100644 index e41aef68a..000000000 --- a/java/sysprop.go +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (C) 2019 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package java - -// This file contains a map to redirect dependencies towards sysprop_library. If a sysprop_library -// is owned by Platform, and the client module links against system API, the public stub of the -// sysprop_library should be used. The map will contain public stub names of sysprop_libraries. - -import ( - "sync" - - "android/soong/android" -) - -type syspropLibraryInterface interface { - BaseModuleName() string - Owner() string - HasPublicStub() bool - JavaPublicStubName() string -} - -var ( - syspropPublicStubsKey = android.NewOnceKey("syspropPublicStubsJava") - syspropPublicStubsLock sync.Mutex -) - -func init() { - android.PreDepsMutators(func(ctx android.RegisterMutatorsContext) { - ctx.BottomUp("sysprop_java", SyspropMutator).Parallel() - }) -} - -func syspropPublicStubs(config android.Config) map[string]string { - return config.Once(syspropPublicStubsKey, func() interface{} { - return make(map[string]string) - }).(map[string]string) -} - -// gather list of sysprop libraries owned by platform. -func SyspropMutator(mctx android.BottomUpMutatorContext) { - if m, ok := mctx.Module().(syspropLibraryInterface); ok { - if m.Owner() != "Platform" || !m.HasPublicStub() { - return - } - - syspropPublicStubs := syspropPublicStubs(mctx.Config()) - syspropPublicStubsLock.Lock() - defer syspropPublicStubsLock.Unlock() - - syspropPublicStubs[m.BaseModuleName()] = m.JavaPublicStubName() - } -} diff --git a/sysprop/sysprop_library.go b/sysprop/sysprop_library.go index 2ccce1524..4ad68eadc 100644 --- a/sysprop/sysprop_library.go +++ b/sysprop/sysprop_library.go @@ -37,9 +37,10 @@ type dependencyTag struct { } type syspropGenProperties struct { - Srcs []string `android:"path"` - Scope string - Name *string + Srcs []string `android:"path"` + Scope string + Name *string + Check_api *string } type syspropJavaGenRule struct { @@ -68,10 +69,6 @@ var ( func init() { pctx.HostBinToolVariable("soongZipCmd", "soong_zip") pctx.HostBinToolVariable("syspropJavaCmd", "sysprop_java") - - android.PreArchMutators(func(ctx android.RegisterMutatorsContext) { - ctx.BottomUp("sysprop_deps", syspropDepsMutator).Parallel() - }) } // syspropJavaGenRule module generates srcjar containing generated java APIs. @@ -103,6 +100,12 @@ func (g *syspropJavaGenRule) GenerateAndroidBuildActions(ctx android.ModuleConte } } +func (g *syspropJavaGenRule) DepsMutator(ctx android.BottomUpMutatorContext) { + // Add a dependency from the stubs to sysprop library so that the generator rule can depend on + // the check API rule of the sysprop library. + ctx.AddFarVariationDependencies(nil, nil, proptools.String(g.properties.Check_api)) +} + func (g *syspropJavaGenRule) OutputFiles(tag string) (android.Paths, error) { switch tag { case "": @@ -157,9 +160,6 @@ type syspropLibraryProperties struct { // If set to true, build a variant of the module for the host. Defaults to false. Host_supported *bool - // Whether public stub exists or not. - Public_stub *bool `blueprint:"mutated"` - Cpp struct { // Minimum sdk version that the artifact should support when it runs as part of mainline modules(APEX). // Forwarded to cc_library.min_sdk_version @@ -202,11 +202,8 @@ func (m *syspropLibrary) CcImplementationModuleName() string { return "lib" + m.BaseModuleName() } -func (m *syspropLibrary) JavaPublicStubName() string { - if proptools.Bool(m.properties.Public_stub) { - return m.BaseModuleName() + "_public" - } - return "" +func (m *syspropLibrary) javaPublicStubName() string { + return m.BaseModuleName() + "_public" } func (m *syspropLibrary) javaGenModuleName() string { @@ -221,10 +218,6 @@ func (m *syspropLibrary) BaseModuleName() string { return m.ModuleBase.Name() } -func (m *syspropLibrary) HasPublicStub() bool { - return proptools.Bool(m.properties.Public_stub) -} - func (m *syspropLibrary) CurrentSyspropApiFile() android.OptionalPath { return m.currentApiFile } @@ -399,16 +392,17 @@ type ccLibraryProperties struct { } type javaLibraryProperties struct { - Name *string - Srcs []string - Soc_specific *bool - Device_specific *bool - Product_specific *bool - Required []string - Sdk_version *string - Installable *bool - Libs []string - Stem *string + Name *string + Srcs []string + Soc_specific *bool + Device_specific *bool + Product_specific *bool + Required []string + Sdk_version *string + Installable *bool + Libs []string + Stem *string + SyspropPublicStub string } func syspropLibraryHook(ctx android.LoadHookContext, m *syspropLibrary) { @@ -490,35 +484,42 @@ func syspropLibraryHook(ctx android.LoadHookContext, m *syspropLibrary) { // Contrast to C++, syspropJavaGenRule module will generate srcjar and the srcjar will be fed // to Java implementation library. ctx.CreateModule(syspropJavaGenFactory, &syspropGenProperties{ - Srcs: m.properties.Srcs, - Scope: scope, - Name: proptools.StringPtr(m.javaGenModuleName()), - }) - - ctx.CreateModule(java.LibraryFactory, &javaLibraryProperties{ - Name: proptools.StringPtr(m.BaseModuleName()), - Srcs: []string{":" + m.javaGenModuleName()}, - Soc_specific: proptools.BoolPtr(ctx.SocSpecific()), - Device_specific: proptools.BoolPtr(ctx.DeviceSpecific()), - Product_specific: proptools.BoolPtr(ctx.ProductSpecific()), - Installable: m.properties.Installable, - Sdk_version: proptools.StringPtr("core_current"), - Libs: []string{javaSyspropStub}, + Srcs: m.properties.Srcs, + Scope: scope, + Name: proptools.StringPtr(m.javaGenModuleName()), + Check_api: proptools.StringPtr(ctx.ModuleName()), }) // if platform sysprop_library is installed in /system or /system-ext, we regard it as an API // and allow any modules (even from different partition) to link against the sysprop_library. // To do that, we create a public stub and expose it to modules with sdk_version: system_*. + var publicStub string if isOwnerPlatform && installedInSystem { - m.properties.Public_stub = proptools.BoolPtr(true) + publicStub = m.javaPublicStubName() + } + + ctx.CreateModule(java.LibraryFactory, &javaLibraryProperties{ + Name: proptools.StringPtr(m.BaseModuleName()), + Srcs: []string{":" + m.javaGenModuleName()}, + Soc_specific: proptools.BoolPtr(ctx.SocSpecific()), + Device_specific: proptools.BoolPtr(ctx.DeviceSpecific()), + Product_specific: proptools.BoolPtr(ctx.ProductSpecific()), + Installable: m.properties.Installable, + Sdk_version: proptools.StringPtr("core_current"), + Libs: []string{javaSyspropStub}, + SyspropPublicStub: publicStub, + }) + + if publicStub != "" { ctx.CreateModule(syspropJavaGenFactory, &syspropGenProperties{ - Srcs: m.properties.Srcs, - Scope: "public", - Name: proptools.StringPtr(m.javaGenPublicStubName()), + Srcs: m.properties.Srcs, + Scope: "public", + Name: proptools.StringPtr(m.javaGenPublicStubName()), + Check_api: proptools.StringPtr(ctx.ModuleName()), }) ctx.CreateModule(java.LibraryFactory, &javaLibraryProperties{ - Name: proptools.StringPtr(m.JavaPublicStubName()), + Name: proptools.StringPtr(publicStub), Srcs: []string{":" + m.javaGenPublicStubName()}, Installable: proptools.BoolPtr(false), Sdk_version: proptools.StringPtr("core_current"), @@ -537,15 +538,3 @@ func syspropLibraryHook(ctx android.LoadHookContext, m *syspropLibrary) { *libraries = append(*libraries, "//"+ctx.ModuleDir()+":"+ctx.ModuleName()) } } - -// syspropDepsMutator adds dependencies from java implementation library to sysprop library. -// java implementation library then depends on check API rule of sysprop library. -func syspropDepsMutator(ctx android.BottomUpMutatorContext) { - if m, ok := ctx.Module().(*syspropLibrary); ok { - ctx.AddReverseDependency(m, nil, m.javaGenModuleName()) - - if proptools.Bool(m.properties.Public_stub) { - ctx.AddReverseDependency(m, nil, m.javaGenPublicStubName()) - } - } -} diff --git a/sysprop/sysprop_test.go b/sysprop/sysprop_test.go index 5cb9e64ee..9d914e317 100644 --- a/sysprop/sysprop_test.go +++ b/sysprop/sysprop_test.go @@ -26,7 +26,6 @@ import ( "strings" "testing" - "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -61,16 +60,10 @@ func testContext(config android.Config) *android.TestContext { java.RegisterRequiredBuildComponentsForTest(ctx) ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators) - ctx.PreArchMutators(func(ctx android.RegisterMutatorsContext) { - ctx.BottomUp("sysprop_deps", syspropDepsMutator).Parallel() - }) android.RegisterPrebuiltMutators(ctx) cc.RegisterRequiredBuildComponentsForTest(ctx) - ctx.PreDepsMutators(func(ctx android.RegisterMutatorsContext) { - ctx.BottomUp("sysprop_java", java.SyspropMutator).Parallel() - }) ctx.RegisterModuleType("sysprop_library", syspropLibraryFactory) @@ -392,15 +385,9 @@ func TestSyspropLibrary(t *testing.T) { } // Java modules linking against system API should use public stub - javaSystemApiClient := ctx.ModuleForTests("java-platform", "android_common") - publicStubFound := false - ctx.VisitDirectDeps(javaSystemApiClient.Module(), func(dep blueprint.Module) { - if dep.Name() == "sysprop-platform_public" { - publicStubFound = true - } - }) - if !publicStubFound { - t.Errorf("system api client should use public stub") + javaSystemApiClient := ctx.ModuleForTests("java-platform", "android_common").Rule("javac") + syspropPlatformPublic := ctx.ModuleForTests("sysprop-platform_public", "android_common").Description("for turbine") + if g, w := javaSystemApiClient.Implicits.Strings(), syspropPlatformPublic.Output.String(); !android.InList(w, g) { + t.Errorf("system api client should use public stub %q, got %q", w, g) } - }