From fd9e804470b0a073423251f8d04dc561534c080d Mon Sep 17 00:00:00 2001 From: Justin Yun Date: Wed, 23 Dec 2020 18:23:14 +0900 Subject: [PATCH] Define vndk.private property for VNDK-private libraries To define VNDK-private libraries, we used `vendor_available: false`. Because of it, `vendor_available == nil` had different meaning from `vendor_available: false` for the VNDK libraries. To clarify this, we change the logic for defining VNDK-private libraries which was: cc_library { name: "vndk_private", vendor_available: false, product_available: false, vndk: { enabled: true, }, } It must be replaced with cc_library { name: "vndk_private", vendor_available: true, product_available: true, vndk: { enabled: true, private: true, }, } Bug: 175768895 Test: m nothing Change-Id: I81769f57c2231e54b682a28e4b82631ab9f3d390 --- cc/cc.go | 24 +++++++++++++--- cc/cc_test.go | 76 +++++++++++++++++++++++++-------------------------- cc/image.go | 10 ++----- cc/vndk.go | 51 ++++++++++------------------------ 4 files changed, 76 insertions(+), 85 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 26250a7ee..ac6a2582c 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1076,11 +1076,27 @@ func (c *Module) isImplementationForLLNDKPublic() bool { c.BaseModuleName() != "libft2") } +// Returns true for LLNDK-private, VNDK-SP-private, and VNDK-core-private. func (c *Module) IsVndkPrivate() bool { - // Returns true for LLNDK-private, VNDK-SP-private, and VNDK-core-private. - library, _ := c.library.(*libraryDecorator) - return library != nil && !Bool(library.Properties.Llndk.Vendor_available) && - !Bool(c.VendorProperties.Vendor_available) && !c.IsVndkExt() + // Check if VNDK-core-private or VNDK-SP-private + if c.IsVndk() { + if Bool(c.vndkdep.Properties.Vndk.Private) { + return true + } + // TODO(b/175768895) remove this when we clean up "vendor_available: false" use cases. + if c.VendorProperties.Vendor_available != nil && !Bool(c.VendorProperties.Vendor_available) { + return true + } + return false + } + + // Check if LLNDK-private + if library, ok := c.library.(*libraryDecorator); ok && c.IsLlndk() { + // TODO(b/175768895) replace this with 'private' property. + return !Bool(library.Properties.Llndk.Vendor_available) + } + + return false } func (c *Module) IsVndk() bool { diff --git a/cc/cc_test.go b/cc/cc_test.go index 071e813c5..fb8533641 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -334,9 +334,10 @@ func TestVndk(t *testing.T) { cc_library { name: "libvndk_private", - vendor_available: false, + vendor_available: true, vndk: { enabled: true, + private: true, }, nocrt: true, stem: "libvndk-private", @@ -373,10 +374,11 @@ func TestVndk(t *testing.T) { cc_library { name: "libvndk_sp_private", - vendor_available: false, + vendor_available: true, vndk: { enabled: true, support_system_process: true, + private: true, }, nocrt: true, target: { @@ -388,11 +390,12 @@ func TestVndk(t *testing.T) { cc_library { name: "libvndk_sp_product_private", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, support_system_process: true, + private: true, }, nocrt: true, target: { @@ -574,10 +577,11 @@ func TestVndkUsingCoreVariant(t *testing.T) { cc_library { name: "libvndk2", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, nocrt: true, } @@ -743,18 +747,6 @@ func TestVndkModuleError(t *testing.T) { } `) - testCcErrorProductVndk(t, "product_available: may not have different value than `vendor_available` for a VNDK", ` - cc_library { - name: "libvndk", - vendor_available: true, - product_available: false, - vndk: { - enabled: true, - }, - nocrt: true, - } - `) - testCcErrorProductVndk(t, "product properties must have the same values with the vendor properties for VNDK modules", ` cc_library { name: "libvndkprop", @@ -903,10 +895,11 @@ func TestVndkDepError(t *testing.T) { testCcError(t, "module \".*\" variant \".*\": \\(.*\\) should not link to \".*\"", ` cc_library { name: "libvndkprivate", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, shared_libs: ["libnonvndk"], nocrt: true, @@ -944,11 +937,12 @@ func TestVndkDepError(t *testing.T) { testCcError(t, "module \".*\" variant \".*\": \\(.*\\) should not link to \".*\"", ` cc_library { name: "libvndkspprivate", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, support_system_process: true, + private: true, }, shared_libs: ["libnonvndk"], nocrt: true, @@ -1039,10 +1033,11 @@ func TestDoubleLoadbleDep(t *testing.T) { cc_library { name: "libnondoubleloadable", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, double_loadable: true, } @@ -1908,10 +1903,11 @@ func TestDoubleLoadableDepError(t *testing.T) { cc_library { name: "libnondoubleloadable", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, } `) @@ -2262,14 +2258,15 @@ func TestVndkExtInconsistentSupportSystemProcessError(t *testing.T) { func TestVndkExtVendorAvailableFalseError(t *testing.T) { // This test ensures an error is emitted when a VNDK-Ext library extends a VNDK library - // with `vendor_available: false`. - testCcError(t, "`extends` refers module \".*\" which does not have `vendor_available: true`", ` + // with `private: true`. + testCcError(t, "`extends` refers module \".*\" which has `private: true`", ` cc_library { name: "libvndk", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, nocrt: true, } @@ -2285,13 +2282,14 @@ func TestVndkExtVendorAvailableFalseError(t *testing.T) { } `) - testCcErrorProductVndk(t, "`extends` refers module \".*\" which does not have `product_available: true`", ` + testCcErrorProductVndk(t, "`extends` refers module \".*\" which has `private: true`", ` cc_library { name: "libvndk", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, nocrt: true, } @@ -2873,7 +2871,7 @@ func TestEnforceProductVndkVersionErrors(t *testing.T) { nocrt: true, } `) - testCcErrorProductVndk(t, "product module that is not VNDK should not link to \".*\" which is marked as `product_available: false`", ` + testCcErrorProductVndk(t, "non-VNDK module should not link to \".*\" which has `private: true`", ` cc_library { name: "libprod", product_specific: true, @@ -2884,10 +2882,11 @@ func TestEnforceProductVndkVersionErrors(t *testing.T) { } cc_library { name: "libvndk_private", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, nocrt: true, } @@ -2945,10 +2944,11 @@ func TestMakeLinkType(t *testing.T) { } cc_library { name: "libvndkprivate", - vendor_available: false, - product_available: false, + vendor_available: true, + product_available: true, vndk: { enabled: true, + private: true, }, } cc_library { diff --git a/cc/image.go b/cc/image.go index b55e1f417..13d77cc2d 100644 --- a/cc/image.go +++ b/cc/image.go @@ -245,13 +245,9 @@ func (m *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { "vendor_available must be set to either true or false when `vndk: {enabled: true}`") } if m.VendorProperties.Product_available != nil { - // If product_available is defined for a VNDK, make sure vendor_available and - // product_available has the same value since `false` for these properties - // means the module is VNDK-private. - if Bool(m.VendorProperties.Vendor_available) != Bool(m.VendorProperties.Product_available) { - mctx.PropertyErrorf("product_available", "may not have different value than `vendor_available` for a VNDK") - } - // Also, both variants must have the same properties since they share a single VNDK library on runtime. + // If a VNDK module creates both product and vendor variants, they + // must have the same properties since they share a single VNDK + // library on runtime. if !m.compareVendorAndProductProps() { mctx.ModuleErrorf("product properties must have the same values with the vendor properties for VNDK modules") } diff --git a/cc/vndk.go b/cc/vndk.go index 1708841c6..6bc713135 100644 --- a/cc/vndk.go +++ b/cc/vndk.go @@ -78,6 +78,13 @@ type VndkProperties struct { // VNDK-SP or LL-NDK modules only. Support_system_process *bool + // declared as a VNDK-private module. + // This module still creates the vendor and product variants refering + // to the `vendor_available: true` and `product_available: true` + // properties. However, it is only available to the other VNDK modules + // but not to the non-VNDK vendor or product modules. + Private *bool + // Extending another module Extends *string } @@ -135,31 +142,11 @@ func (vndk *vndkdep) vndkCheckLinkType(ctx android.BaseModuleContext, to *Module return } if !vndk.isVndk() { - // Non-VNDK modules those installed to /vendor or /system/vendor - // can't depend on modules marked with vendor_available: false; - // or those installed to /product or /system/product can't depend - // on modules marked with product_available: false. - violation := false - variant := "vendor" - if lib, ok := to.linker.(*llndkStubDecorator); ok && !Bool(lib.Properties.Vendor_available) { - violation = true - if to.InProduct() { - variant = "product" - } - } else if _, ok := to.linker.(libraryInterface); ok { - if to.inVendor() && to.VendorProperties.Vendor_available != nil && !Bool(to.VendorProperties.Vendor_available) { - // A vendor module with Vendor_available == nil should be okay since it means a - // vendor-only library which is a valid dependency for non-VNDK vendor modules. - violation = true - } else if to.InProduct() && to.VendorProperties.Product_available != nil && !Bool(to.VendorProperties.Product_available) { - // A product module with Product_available == nil should be okay since it means a - // product-only library which is a valid dependency for non-VNDK product modules. - violation = true - variant = "product" - } - } - if violation { - ctx.ModuleErrorf("%s module that is not VNDK should not link to %q which is marked as `%s_available: false`", variant, to.Name(), variant) + // Non-VNDK modules those installed to /vendor, /system/vendor, + // /product or /system/product cannot depend on VNDK-private modules + // that include VNDK-core-private, VNDK-SP-private and LLNDK-private. + if to.IsVndkPrivate() { + ctx.ModuleErrorf("non-VNDK module should not link to %q which has `private: true`", to.Name()) } } if lib, ok := to.linker.(*libraryDecorator); !ok || !lib.shared() { @@ -191,15 +178,9 @@ func (vndk *vndkdep) vndkCheckLinkType(ctx android.BaseModuleContext, to *Module to.Name()) return } - if to.inVendor() && !Bool(to.VendorProperties.Vendor_available) { + if to.IsVndkPrivate() { ctx.ModuleErrorf( - "`extends` refers module %q which does not have `vendor_available: true`", - to.Name()) - return - } - if to.InProduct() && !Bool(to.VendorProperties.Product_available) { - ctx.ModuleErrorf( - "`extends` refers module %q which does not have `product_available: true`", + "`extends` refers module %q which has `private: true`", to.Name()) return } @@ -355,9 +336,7 @@ func processVndkLibrary(mctx android.BottomUpMutatorContext, m *Module) { } else { vndkCoreLibraries(mctx.Config())[name] = filename } - // As `vendor_available` and `product_available` has the same value for VNDK modules, - // we don't need to check both values. - if !Bool(m.VendorProperties.Vendor_available) { + if m.IsVndkPrivate() { vndkPrivateLibraries(mctx.Config())[name] = filename } }