From b8fa86ad6f8186258b89572bef0468fc873eb30d 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. Exempt-From-Owner-Approval: cherry-pick from aosp/master Bug: 151071238 Test: build/boot Merged-In: I0569ef4bb8e79635e4d97a89f421a8d8b7d26456 (cherry picked from commit 5e9013be2202effb500a0aa14d95f5fef70cc75e) Change-Id: I0569ef4bb8e79635e4d97a89f421a8d8b7d26456 --- apex/apex.go | 39 ++++++++++++++++++++++++++++++++------- apex/apex_test.go | 32 +++++++++++++++----------------- java/java.go | 18 ++++++++++++++---- java/sdk_library.go | 14 ++++++++++++-- sdk/sdk_test.go | 3 ++- 5 files changed, 75 insertions(+), 31 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index ee6f93ae3..268088b24 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -212,8 +212,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", @@ -279,6 +282,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", } @@ -296,6 +305,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libcrypto", "libnativehelper_header_only", "libssl", + "unsupportedappusage", } // // Module separator @@ -319,6 +329,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", } // @@ -559,6 +570,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libFLAC-config", "libFLAC-headers", "libFraunhoferAAC", + "libLibGuiProperties", "libarect", "libasync_safe", "libaudio_system_headers", @@ -574,6 +586,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libbase", "libbase_headers", "libbinder_headers", + "libbinderthreadstateutils", "libbluetooth-types-header", "libbufferhub_headers", "libc++", @@ -779,6 +792,7 @@ func makeApexAvailableWhitelist() map[string][]string { "libdexfile_external_headers", "libdexfile_support", "libdexfile_support_static", + "libdl_static", "libgtest_prod", "libjemalloc5", "liblinker_main", @@ -870,6 +884,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", @@ -890,6 +905,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", @@ -1025,7 +1042,10 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) { var apexBundles []android.ApexInfo var directDep bool if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex { - apexBundles = []android.ApexInfo{{mctx.ModuleName(), proptools.Bool(a.properties.Legacy_android10_support)}} + apexBundles = []android.ApexInfo{android.ApexInfo{ + ApexName: mctx.ModuleName(), + LegacyAndroid10Support: proptools.Bool(a.properties.Legacy_android10_support), + }} directDep = true } else if am, ok := mctx.Module().(android.ApexModule); ok { apexBundles = am.ApexVariations() @@ -1036,10 +1056,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) } @@ -1958,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 } @@ -1979,10 +2003,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) }) } @@ -2352,13 +2378,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 6a9533e37..34736e588 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -2832,6 +2832,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"`, ` @@ -2853,6 +2854,7 @@ func TestErrorsIfDepsAreNotEnabled(t *testing.T) { sdk_version: "none", system_modules: "none", enabled: false, + apex_available: ["myapex"], } `) } @@ -3011,7 +3013,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", @@ -3077,8 +3079,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", @@ -3091,31 +3093,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", ` @@ -3452,6 +3449,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 8d58a9023..23dc0b5c5 100644 --- a/java/java.go +++ b/java/java.go @@ -1790,11 +1790,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 { @@ -2495,11 +2500,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 934bdae01..53c29713f 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 {