From 6a9ddc37a93c875788ff59931135b17f4bdc4414 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. Exempt-From-Owner-Approval: cherry-pick from AOSP 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 Merged-In: Ia304cc5f41f173229e8a154e90cea4dce46dcebe (cherry picked from commit 89e850ab840e6efb10787b620b97ab009b4c2d86) 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 c296a5bf9..6ba68af06 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 9bf6fc717..1a43ce4c4 100644 --- a/android/apex.go +++ b/android/apex.go @@ -100,6 +100,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 @@ -117,6 +127,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 @@ -201,6 +213,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 89313d441..16027e1da 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -839,6 +839,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 @@ -876,6 +877,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 eed0d72f9..44cc5274b 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -3668,22 +3668,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 { @@ -3696,16 +3686,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", @@ -3728,17 +3754,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) {