From 5e9013be2202effb500a0aa14d95f5fef70cc75e Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Tue, 10 Mar 2020 06:23:13 +0900 Subject: [PATCH] Fix apex_available Checking apex_available was missing some corner cases. For example, the deps of share deps of cc_library modules are missed while those from cc_library_shared are correctly tracked. This was due to.. * calling DepIsInSameApex in WalkDeps: both work fine separately, but when they are used together, it fails to work. It's due to how WalkDeps works. (We might fix this bug too risky since it is used very widely) * incorrect receiver for DepIsInSameApex in apex_deps mutator: receiver is supposed to be parent, but child was used before. Interestingly lots of deps are within the same group of module types(cc to cc, java to java), it has worked. (note that receiver's DepIsInSameApex implementation can be different). This change fixes them by.. * walkPayloadDeps is now relying on ApexVariation, which is calculated correctly by TopDown apex_deps mutator. * use correct receiver for DepIsInSameApex in apex_deps mutator, which requires for java.SdkLibrary to override the method and for java.Library/Import to use passed dep instead of receiver to check its membership of sdk. Bug: 151071238 Test: build/boot Change-Id: I0569ef4bb8e79635e4d97a89f421a8d8b7d26456 --- apex/apex.go | 47 +++++++++++++++++++++++++++++++-------------- apex/apex_test.go | 32 +++++++++++++++--------------- java/java.go | 18 +++++++++++++---- java/sdk_library.go | 14 ++++++++++++-- sdk/sdk_test.go | 3 ++- 5 files changed, 76 insertions(+), 38 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 5a5c02f21..af9fd82b9 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -221,8 +221,11 @@ func makeApexAvailableWhitelist() map[string][]string { "bluetooth-protos-lite", "bluetooth.mapsapi", "com.android.vcard", + "dnsresolver_aidl_interface-V2-java", "fmtlib", "guava", + "ipmemorystore-aidl-interfaces-V5-java", + "ipmemorystore-aidl-interfaces-java", "internal_include_headers", "lib-bt-packets", "lib-bt-packets-avrcp", @@ -288,6 +291,12 @@ func makeApexAvailableWhitelist() map[string][]string { "libutils_headers", "libz", "media_plugin_headers", + "net-utils-services-common", + "netd_aidl_interface-unstable-java", + "netd_event_listener_interface-java", + "netlink-client", + "networkstack-aidl-interfaces-unstable-java", + "networkstack-client", "sap-api-java-static", "services.net", } @@ -305,6 +314,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libcrypto", "libnativehelper_header_only", "libssl", + "unsupportedappusage", } // // Module separator @@ -328,6 +338,7 @@ func makeApexAvailableWhitelist() map[string][]string { "cronet_impl_platform_java", "libcronet.80.0.3986.0", "org.chromium.net.cronet", + "org.chromium.net.cronet.xml", "prebuilt_libcronet.80.0.3986.0", } // @@ -566,6 +577,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libFLAC-config", "libFLAC-headers", "libFraunhoferAAC", + "libLibGuiProperties", "libarect", "libasync_safe", "libaudio_system_headers", @@ -581,6 +593,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libbase", "libbase_headers", "libbinder_headers", + "libbinderthreadstateutils", "libbluetooth-types-header", "libbufferhub_headers", "libc++", @@ -783,6 +796,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libdexfile_external_headers", "libdexfile_support", "libdexfile_support_static", + "libdl_static", "libgtest_prod", "libjemalloc5", "liblinker_main", @@ -874,6 +888,7 @@ func makeApexAvailableWhitelist() map[string][]string { m["com.android.wifi"] = []string{ "PlatformProperties", "android.hardware.wifi-V1.0-java", + "android.hardware.wifi-V1.0-java-constants", "android.hardware.wifi-V1.1-java", "android.hardware.wifi-V1.2-java", "android.hardware.wifi-V1.3-java", @@ -894,6 +909,8 @@ func makeApexAvailableWhitelist() map[string][]string { "bouncycastle-unbundled", "dnsresolver_aidl_interface-V2-java", "error_prone_annotations", + "framework-wifi-pre-jarjar", + "framework-wifi-util-lib", "ipmemorystore-aidl-interfaces-V3-java", "ipmemorystore-aidl-interfaces-java", "ksoap2", @@ -1030,14 +1047,11 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) { var directDep bool if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex { minSdkVersion := a.minSdkVersion(mctx) - - apexBundles = []android.ApexInfo{ - android.ApexInfo{ - ApexName: mctx.ModuleName(), - LegacyAndroid10Support: proptools.Bool(a.properties.Legacy_android10_support), - MinSdkVersion: minSdkVersion, - }, - } + apexBundles = []android.ApexInfo{android.ApexInfo{ + ApexName: mctx.ModuleName(), + LegacyAndroid10Support: proptools.Bool(a.properties.Legacy_android10_support), + MinSdkVersion: minSdkVersion, + }} directDep = true } else if am, ok := mctx.Module().(android.ApexModule); ok { apexBundles = am.ApexVariations() @@ -1048,10 +1062,14 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) { return } + cur := mctx.Module().(interface { + DepIsInSameApex(android.BaseModuleContext, android.Module) bool + }) + mctx.VisitDirectDeps(func(child android.Module) { depName := mctx.OtherModuleName(child) if am, ok := child.(android.ApexModule); ok && am.CanHaveApexVariants() && - (directDep || am.DepIsInSameApex(mctx, child)) { + cur.DepIsInSameApex(mctx, child) { android.UpdateApexDependency(apexBundles, depName, directDep) am.BuildForApexes(apexBundles) } @@ -1964,7 +1982,7 @@ func (a *apexBundle) walkPayloadDeps(ctx android.ModuleContext, } // Check for the indirect dependencies if it is considered as part of the APEX - if am.DepIsInSameApex(ctx, am) { + if am.ApexName() != "" { do(ctx, parent, am, false /* externalDep */) return true } @@ -1997,10 +2015,12 @@ func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) { a.walkPayloadDeps(ctx, func(ctx android.ModuleContext, from blueprint.Module, to android.ApexModule, externalDep bool) { apexName := ctx.ModuleName() - if externalDep || to.AvailableFor(apexName) || whitelistedApexAvailable(apexName, to) { + fromName := ctx.OtherModuleName(from) + toName := ctx.OtherModuleName(to) + if externalDep || to.AvailableFor(apexName) || whitelistedApexAvailable(apexName, toName) { return } - ctx.ModuleErrorf("%q requires %q that is not available for the APEX.", from.Name(), to.Name()) + ctx.ModuleErrorf("%q requires %q that is not available for the APEX.", fromName, toName) }) } @@ -2377,13 +2397,12 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { a.buildApexDependencyInfo(ctx) } -func whitelistedApexAvailable(apex string, module android.Module) bool { +func whitelistedApexAvailable(apex, moduleName string) bool { key := apex key = strings.Replace(key, "test_", "", 1) key = strings.Replace(key, "com.android.art.debug", "com.android.art", 1) key = strings.Replace(key, "com.android.art.release", "com.android.art", 1) - moduleName := module.Name() // Prebuilt modules (e.g. java_import, etc.) have "prebuilt_" prefix added by the build // system. Trim the prefix for the check since they are confusing moduleName = strings.TrimPrefix(moduleName, "prebuilt_") diff --git a/apex/apex_test.go b/apex/apex_test.go index 7b842da77..c11da81fe 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -3164,6 +3164,7 @@ func TestErrorsIfDepsAreNotEnabled(t *testing.T) { stl: "none", system_shared_libs: [], enabled: false, + apex_available: ["myapex"], } `) testApexError(t, `module "myapex" .* depends on disabled module "myjar"`, ` @@ -3185,6 +3186,7 @@ func TestErrorsIfDepsAreNotEnabled(t *testing.T) { sdk_version: "none", system_modules: "none", enabled: false, + apex_available: ["myapex"], } `) } @@ -3345,7 +3347,7 @@ func TestApexWithTestHelperApp(t *testing.T) { func TestApexPropertiesShouldBeDefaultable(t *testing.T) { // libfoo's apex_available comes from cc_defaults - testApexError(t, `"myapex" .*: "myapex" requires "libfoo" that is not available for the APEX`, ` + testApexError(t, `requires "libfoo" that is not available for the APEX`, ` apex { name: "myapex", key: "myapex.key", @@ -3411,8 +3413,8 @@ func TestApexAvailable(t *testing.T) { apex_available: ["otherapex"], }`) - // libbar is an indirect dep - testApexError(t, "requires \"libbar\" that is not available for the APEX", ` + // libbbaz is an indirect dep + testApexError(t, "requires \"libbaz\" that is not available for the APEX", ` apex { name: "myapex", key: "myapex.key", @@ -3425,31 +3427,26 @@ func TestApexAvailable(t *testing.T) { private_key: "testkey.pem", } - apex { - name: "otherapex", - key: "otherapex.key", - native_shared_libs: ["libfoo"], - } - - apex_key { - name: "otherapex.key", - public_key: "testkey.avbpubkey", - private_key: "testkey.pem", - } - cc_library { name: "libfoo", stl: "none", shared_libs: ["libbar"], system_shared_libs: [], - apex_available: ["myapex", "otherapex"], + apex_available: ["myapex"], } cc_library { name: "libbar", stl: "none", + shared_libs: ["libbaz"], + system_shared_libs: [], + apex_available: ["myapex"], + } + + cc_library { + name: "libbaz", + stl: "none", system_shared_libs: [], - apex_available: ["otherapex"], }`) testApexError(t, "\"otherapex\" is not a valid module name", ` @@ -3796,6 +3793,7 @@ func TestRejectNonInstallableJavaLibrary(t *testing.T) { sdk_version: "none", system_modules: "none", compile_dex: false, + apex_available: ["myapex"], } `) } diff --git a/java/java.go b/java/java.go index d0b5adfa9..8c779f5c4 100644 --- a/java/java.go +++ b/java/java.go @@ -1796,11 +1796,16 @@ func (j *Module) hasCode(ctx android.ModuleContext) bool { } func (j *Module) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool { - depTag := ctx.OtherModuleDependencyTag(dep) // Dependencies other than the static linkage are all considered crossing APEX boundary + if staticLibTag == ctx.OtherModuleDependencyTag(dep) { + return true + } // Also, a dependency to an sdk member is also considered as such. This is required because // sdk members should be mutated into APEXes. Refer to sdk.sdkDepsReplaceMutator. - return depTag == staticLibTag || j.IsInAnySdk() + if sa, ok := dep.(android.SdkAware); ok && sa.IsInAnySdk() { + return true + } + return false } func (j *Module) Stem() string { @@ -2504,11 +2509,16 @@ func (j *Import) SrcJarArgs() ([]string, android.Paths) { } func (j *Import) DepIsInSameApex(ctx android.BaseModuleContext, dep android.Module) bool { - depTag := ctx.OtherModuleDependencyTag(dep) // dependencies other than the static linkage are all considered crossing APEX boundary + if staticLibTag == ctx.OtherModuleDependencyTag(dep) { + return true + } // Also, a dependency to an sdk member is also considered as such. This is required because // sdk members should be mutated into APEXes. Refer to sdk.sdkDepsReplaceMutator. - return depTag == staticLibTag || j.IsInAnySdk() + if sa, ok := dep.(android.SdkAware); ok && sa.IsInAnySdk() { + return true + } + return false } // Add compile time check for interface implementation diff --git a/java/sdk_library.go b/java/sdk_library.go index a8edf1d55..6921114bf 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -581,6 +581,14 @@ func (module *SdkLibrary) createStubsSources(mctx android.LoadHookContext, apiSc mctx.CreateModule(DroidstubsFactory, &props) } +func (module *SdkLibrary) DepIsInSameApex(mctx android.BaseModuleContext, dep android.Module) bool { + depTag := mctx.OtherModuleDependencyTag(dep) + if depTag == xmlPermissionsFileTag { + return true + } + return module.Library.DepIsInSameApex(mctx, dep) +} + // Creates the xml file that publicizes the runtime library func (module *SdkLibrary) createXmlFile(mctx android.LoadHookContext) { props := struct { @@ -590,9 +598,11 @@ func (module *SdkLibrary) createXmlFile(mctx android.LoadHookContext) { Device_specific *bool Product_specific *bool System_ext_specific *bool + Apex_available []string }{ - Name: proptools.StringPtr(module.xmlFileName()), - Lib_name: proptools.StringPtr(module.BaseModuleName()), + Name: proptools.StringPtr(module.xmlFileName()), + Lib_name: proptools.StringPtr(module.BaseModuleName()), + Apex_available: module.ApexProperties.Apex_available, } if module.SocSpecific() { diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index 243b9768d..96837e3dd 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -58,6 +58,7 @@ func TestDepNotInRequiredSdks(t *testing.T) { sdk_version: "none", compile_dex: true, host_supported: true, + apex_available: ["myapex"], } // this lib is no in mysdk @@ -113,7 +114,7 @@ func TestSnapshotVisibility(t *testing.T) { java_defaults { name: "java-defaults", - visibility: ["//other/bar"], + visibility: ["//other/bar"], } java_library {