From 89e850ab840e6efb10787b620b97ab009b4c2d86 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Tue, 7 Apr 2020 16:37:39 +0900 Subject: [PATCH] mark platform un-availability A module is marked unavailable for platform when 1) it does not have "//apex_available:platform" in its apex_available property, or 2) it depends on another module that is unavailable for platform. In that case, LOCAL_NOT_AVAILABLE_FOR_PLATFORM is set to true for the module in the Make world. Later, that flag is used to ensure that there is no module with the flag is installed to the device. The reason why this isn't entirely done in Soong is because Soong doesn't know if a module will be installed to the device or not. To explain this, let's have an example. cc_test { name: "mytest", static_libs: ["libfoo"]} cc_library_static { name: "libfoo", static_libs: ["libbar"]} cc_library { name: "libbar", apex_available: ["com.android.xxx"]} Here, libbar is not available for platform, but is used by libfoo which is available for platform (apex_available defaults to "//apex_available:platform"). libfoo is again depended on by mytest which again is available for platform. The use of libbar should be allowed in the context of test; we don't want to make libbar available to platform just for the dependency from test because it will allow non-test uses of the library as well. Soong by itself can't tell whether libfoo and libbar are used only in the context of a test. There could be another module depending them, e.g., cc_library_shared { name: "mylib", static_libs: ["libfoo"] } can exist and it might be installed to the device, in which case we really should trigger an error. Since Make has the knowledge of what's installed and what's not, the check should be done there. Bug: 153073816 Test: m Test: remove "//apex_available:platform" from libmdnssd (it is currently installed to /system/lib), and check that `m system_image` fails Change-Id: Ia304cc5f41f173229e8a154e90cea4dce46dcebe --- android/androidmk.go | 4 +++ android/apex.go | 20 ++++++++++++ apex/apex.go | 55 +++++++++++++++++++++++++++++++ apex/apex_test.go | 77 ++++++++++++++++++++++++++++---------------- 4 files changed, 129 insertions(+), 27 deletions(-) diff --git a/android/androidmk.go b/android/androidmk.go index b5f4b2bd5..d579e30c3 100644 --- a/android/androidmk.go +++ b/android/androidmk.go @@ -198,6 +198,10 @@ func (a *AndroidMkEntries) fillInEntries(config Config, bpPath string, mod bluep a.AddStrings("LOCAL_HOST_REQUIRED_MODULES", a.Host_required...) a.AddStrings("LOCAL_TARGET_REQUIRED_MODULES", a.Target_required...) + if am, ok := mod.(ApexModule); ok { + a.SetBoolIfTrue("LOCAL_NOT_AVAILABLE_FOR_PLATFORM", am.NotAvailableForPlatform()) + } + archStr := amod.Arch().ArchType.String() host := false switch amod.Os().Class { diff --git a/android/apex.go b/android/apex.go index 839a03af0..d1449904f 100644 --- a/android/apex.go +++ b/android/apex.go @@ -101,6 +101,16 @@ type ApexModule interface { // Tests if this module is available for the specified APEX or ":platform" AvailableFor(what string) bool + // Return true if this module is not available to platform (i.e. apex_available + // property doesn't have "//apex_available:platform"), or shouldn't be available + // to platform, which is the case when this module depends on other module that + // isn't available to platform. + NotAvailableForPlatform() bool + + // Mark that this module is not available to platform. Set by the + // check-platform-availability mutator in the apex package. + SetNotAvailableForPlatform() + // Returns the highest version which is <= maxSdkVersion. // For example, with maxSdkVersion is 10 and versionList is [9,11] // it returns 9 as string @@ -121,6 +131,8 @@ type ApexProperties struct { Apex_available []string Info ApexInfo `blueprint:"mutated"` + + NotAvailableForPlatform bool `blueprint:"mutated"` } // Marker interface that identifies dependencies that are excluded from APEX @@ -205,6 +217,14 @@ func (m *ApexModuleBase) AvailableFor(what string) bool { return CheckAvailableForApex(what, m.ApexProperties.Apex_available) } +func (m *ApexModuleBase) NotAvailableForPlatform() bool { + return m.ApexProperties.NotAvailableForPlatform +} + +func (m *ApexModuleBase) SetNotAvailableForPlatform() { + m.ApexProperties.NotAvailableForPlatform = true +} + func (m *ApexModuleBase) DepIsInSameApex(ctx BaseModuleContext, dep Module) bool { // By default, if there is a dependency from A to B, we try to include both in the same APEX, // unless B is explicitly from outside of the APEX (i.e. a stubs lib). Thus, returning true. diff --git a/apex/apex.go b/apex/apex.go index 3b1788699..8bcd7ef34 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -830,6 +830,7 @@ func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) { ctx.BottomUp("apex", apexMutator).Parallel() ctx.BottomUp("apex_flattened", apexFlattenedMutator).Parallel() ctx.BottomUp("apex_uses", apexUsesMutator).Parallel() + ctx.BottomUp("mark_platform_availability", markPlatformAvailability).Parallel() } // Mark the direct and transitive dependencies of apex bundles so that they @@ -868,6 +869,60 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) { }) } +// mark if a module cannot be available to platform. A module cannot be available +// to platform if 1) it is explicitly marked as not available (i.e. "//apex_available:platform" +// is absent) or 2) it depends on another module that isn't (or can't be) available to platform +func markPlatformAvailability(mctx android.BottomUpMutatorContext) { + // Host and recovery are not considered as platform + if mctx.Host() || mctx.Module().InstallInRecovery() { + return + } + + if am, ok := mctx.Module().(android.ApexModule); ok { + availableToPlatform := am.AvailableFor(android.AvailableToPlatform) + + // In a rare case when a lib is marked as available only to an apex + // but the apex doesn't exist. This can happen in a partial manifest branch + // like master-art. Currently, libstatssocket in the stats APEX is causing + // this problem. + // Include the lib in platform because the module SDK that ought to provide + // it doesn't exist, so it would otherwise be left out completely. + // TODO(b/154888298) remove this by adding those libraries in module SDKS and skipping + // this check for libraries provided by SDKs. + if !availableToPlatform && !android.InAnyApex(am.Name()) { + availableToPlatform = true + } + + // If any of the dep is not available to platform, this module is also considered + // as being not available to platform even if it has "//apex_available:platform" + mctx.VisitDirectDeps(func(child android.Module) { + if !am.DepIsInSameApex(mctx, child) { + // if the dependency crosses apex boundary, don't consider it + return + } + if dep, ok := child.(android.ApexModule); ok && dep.NotAvailableForPlatform() { + availableToPlatform = false + // TODO(b/154889534) trigger an error when 'am' has "//apex_available:platform" + } + }) + + // Exception 1: stub libraries and native bridge libraries are always available to platform + if cc, ok := mctx.Module().(*cc.Module); ok && + (cc.IsStubs() || cc.Target().NativeBridge == android.NativeBridgeEnabled) { + availableToPlatform = true + } + + // Exception 2: bootstrap bionic libraries are also always available to platform + if cc.InstallToBootstrap(mctx.ModuleName(), mctx.Config()) { + availableToPlatform = true + } + + if !availableToPlatform { + am.SetNotAvailableForPlatform() + } + } +} + // If a module in an APEX depends on a module from an SDK then it needs an APEX // specific variant created for it. Refer to sdk.sdkDepsReplaceMutator. func inAnySdk(module android.Module) bool { diff --git a/apex/apex_test.go b/apex/apex_test.go index eabbdace4..bb59a540e 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -3696,22 +3696,12 @@ func TestApexAvailable_InvalidApexName(t *testing.T) { }`) } -func TestApexAvailable_CreatedForPlatform(t *testing.T) { - // check that libfoo and libbar are created only for myapex, but not for the platform - // TODO(jiyong) the checks for the platform variant are removed because we now create - // the platform variant regardless of the apex_availability. Instead, we will make sure that - // the platform variants are not used from other platform modules. When that is done, - // these checks will be replaced by expecting a specific error message that will be - // emitted when the platform variant is used. - // ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex") - // ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared") - // ensureListContains(t, ctx.ModuleVariantsForTests("libbar"), "android_arm64_armv8-a_shared_myapex") - // ensureListNotContains(t, ctx.ModuleVariantsForTests("libbar"), "android_arm64_armv8-a_shared") - +func TestApexAvailable_CheckForPlatform(t *testing.T) { ctx, _ := testApex(t, ` apex { name: "myapex", key: "myapex.key", + native_shared_libs: ["libbar", "libbaz"], } apex_key { @@ -3724,16 +3714,52 @@ func TestApexAvailable_CreatedForPlatform(t *testing.T) { name: "libfoo", stl: "none", system_shared_libs: [], + shared_libs: ["libbar"], apex_available: ["//apex_available:platform"], + } + + cc_library { + name: "libfoo2", + stl: "none", + system_shared_libs: [], + shared_libs: ["libbaz"], + apex_available: ["//apex_available:platform"], + } + + cc_library { + name: "libbar", + stl: "none", + system_shared_libs: [], + apex_available: ["myapex"], + } + + cc_library { + name: "libbaz", + stl: "none", + system_shared_libs: [], + apex_available: ["myapex"], + stubs: { + versions: ["1"], + }, }`) - // check that libfoo is created only for the platform - ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex") - ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared") + // libfoo shouldn't be available to platform even though it has "//apex_available:platform", + // because it depends on libbar which isn't available to platform + libfoo := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_shared").Module().(*cc.Module) + if libfoo.NotAvailableForPlatform() != true { + t.Errorf("%q shouldn't be available to platform", libfoo.String()) + } + + // libfoo2 however can be available to platform because it depends on libbaz which provides + // stubs + libfoo2 := ctx.ModuleForTests("libfoo2", "android_arm64_armv8-a_shared").Module().(*cc.Module) + if libfoo2.NotAvailableForPlatform() == true { + t.Errorf("%q should be available to platform", libfoo2.String()) + } } func TestApexAvailable_CreatedForApex(t *testing.T) { - testApex(t, ` + ctx, _ := testApex(t, ` apex { name: "myapex", key: "myapex.key", @@ -3756,17 +3782,14 @@ func TestApexAvailable_CreatedForApex(t *testing.T) { }, }`) - // shared variant of libfoo is only available to myapex - // TODO(jiyong) the checks for the platform variant are removed because we now create - // the platform variant regardless of the apex_availability. Instead, we will make sure that - // the platform variants are not used from other platform modules. When that is done, - // these checks will be replaced by expecting a specific error message that will be - // emitted when the platform variant is used. - // ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared_myapex") - // ensureListNotContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_shared") - // // but the static variant is available to both myapex and the platform - // ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_static_myapex") - // ensureListContains(t, ctx.ModuleVariantsForTests("libfoo"), "android_arm64_armv8-a_static") + libfooShared := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_shared").Module().(*cc.Module) + if libfooShared.NotAvailableForPlatform() != true { + t.Errorf("%q shouldn't be available to platform", libfooShared.String()) + } + libfooStatic := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_static").Module().(*cc.Module) + if libfooStatic.NotAvailableForPlatform() != false { + t.Errorf("%q should be available to platform", libfooStatic.String()) + } } func TestOverrideApex(t *testing.T) {