From c0d8c49224cff8d7a5bb160841ea3d8a7e70e376 Mon Sep 17 00:00:00 2001 From: Justin Yun Date: Thu, 7 Jan 2021 17:45:31 +0900 Subject: [PATCH] Do not read 'vendor_available: false' In case of VNDK, 'vendor_available: false' had a special meaning that defines VNDK-private libraries. It is not trivial because not defining a boolean property means 'false' normally. To avoid the confusion replace it with the 'vndk.private: true' for VNDK-private libraries and 'private: true' for LLNDK-private libraries. All VNDK libraries must define 'vendor_available: true' and may have 'vndk.private: true' if they are VNDK-private. With this change '(vendor|product)_available: false' is the same as not defining the property. LLNDK-private must define 'private: true' instead of 'vendor_available: false'. Bug: 175768895 Test: build Change-Id: I57fbca351be317257d95027f3cdcdbbe537eab23 --- cc/cc.go | 20 ++++++-------------- cc/cc_test.go | 14 ++++++-------- cc/image.go | 36 ++++++++++++++++-------------------- cc/llndk_library.go | 8 -------- cc/testing.go | 2 +- cc/vndk.go | 4 ++-- rust/image.go | 6 +++--- 7 files changed, 34 insertions(+), 56 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 0d0aec75e..f45b654da 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -419,7 +419,7 @@ type VendorProperties struct { IsLLNDK bool `blueprint:"mutated"` // IsLLNDKPrivate is set to true for the vendor variant of a cc_library module that has LLNDK - // stubs and also sets llndk.vendor_available: false. + // stubs and also sets llndk.private: true. IsLLNDKPrivate bool `blueprint:"mutated"` } @@ -1079,11 +1079,11 @@ func (c *Module) IsLlndkPublic() bool { func (c *Module) isImplementationForLLNDKPublic() bool { library, _ := c.library.(*libraryDecorator) return library != nil && library.hasLLNDKStubs() && - (Bool(library.Properties.Llndk.Vendor_available) || + (!Bool(library.Properties.Llndk.Private) || // TODO(b/170784825): until the LLNDK properties are moved into the cc_library, // the non-Vendor variants of the cc_library don't know if the corresponding - // llndk_library set vendor_available: false. Since libft2 is the only - // private LLNDK library, hardcode it during the transition. + // llndk_library set private: true. Since libft2 is the only private LLNDK + // library, hardcode it during the transition. c.BaseModuleName() != "libft2") } @@ -1091,20 +1091,12 @@ func (c *Module) isImplementationForLLNDKPublic() bool { func (c *Module) IsVndkPrivate() bool { // 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 + return Bool(c.vndkdep.Properties.Vndk.Private) } // 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 Bool(library.Properties.Llndk.Private) } return false diff --git a/cc/cc_test.go b/cc/cc_test.go index 2d02a6a6d..09b728e9b 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -234,9 +234,6 @@ func checkVndkModule(t *testing.T, ctx *android.TestContext, name, subDir string t.Helper() mod := ctx.ModuleForTests(name, variant).Module().(*Module) - if !mod.HasVendorVariant() { - t.Errorf("%q must have variant %q", name, variant) - } // Check library properties. lib, ok := mod.compiler.(*libraryDecorator) @@ -730,10 +727,11 @@ func TestVndkWhenVndkVersionIsNotSet(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, } @@ -757,7 +755,7 @@ func TestVndkWhenVndkVersionIsNotSet(t *testing.T) { func TestVndkModuleError(t *testing.T) { // Check the error message for vendor_available and product_available properties. - testCcErrorProductVndk(t, "vndk: vendor_available must be set to either true or false when `vndk: {enabled: true}`", ` + testCcErrorProductVndk(t, "vndk: vendor_available must be set to true when `vndk: {enabled: true}`", ` cc_library { name: "libvndk", vndk: { @@ -767,7 +765,7 @@ func TestVndkModuleError(t *testing.T) { } `) - testCcErrorProductVndk(t, "vndk: vendor_available must be set to either true or false when `vndk: {enabled: true}`", ` + testCcErrorProductVndk(t, "vndk: vendor_available must be set to true when `vndk: {enabled: true}`", ` cc_library { name: "libvndk", product_available: true, @@ -3136,7 +3134,7 @@ func TestMakeLinkType(t *testing.T) { } llndk_library { name: "libllndkprivate.llndk", - vendor_available: false, + private: true, symbol_file: "", }` diff --git a/cc/image.go b/cc/image.go index 11ba55c84..12bd65b67 100644 --- a/cc/image.go +++ b/cc/image.go @@ -67,13 +67,15 @@ const ( ) func (ctx *moduleContext) ProductSpecific() bool { - return ctx.ModuleContext.ProductSpecific() || - (ctx.mod.HasProductVariant() && ctx.mod.InProduct()) + // Additionally check if this module is inProduct() that means it is a "product" variant of a + // module. As well as product specific modules, product variants must be installed to /product. + return ctx.ModuleContext.ProductSpecific() || ctx.mod.InProduct() } func (ctx *moduleContext) SocSpecific() bool { - return ctx.ModuleContext.SocSpecific() || - (ctx.mod.HasVendorVariant() && ctx.mod.inVendor()) + // Additionally check if this module is inVendor() that means it is a "vendor" variant of a + // module. As well as SoC specific modules, vendor variants must be installed to /vendor. + return ctx.ModuleContext.SocSpecific() || ctx.mod.inVendor() } func (ctx *moduleContextImpl) inProduct() bool { @@ -98,18 +100,12 @@ func (ctx *moduleContextImpl) inRecovery() bool { // Returns true when this module is configured to have core and vendor variants. func (c *Module) HasVendorVariant() bool { - // In case of a VNDK, 'vendor_available: false' still creates a vendor variant. - return c.IsVndk() || Bool(c.VendorProperties.Vendor_available) + return Bool(c.VendorProperties.Vendor_available) } // Returns true when this module is configured to have core and product variants. func (c *Module) HasProductVariant() bool { - if c.VendorProperties.Product_available == nil { - // Without 'product_available', product variant will not be created even for VNDKs. - return false - } - // However, 'product_available: false' in a VNDK still creates a product variant. - return c.IsVndk() || Bool(c.VendorProperties.Product_available) + return Bool(c.VendorProperties.Product_available) } // Returns true when this module is configured to have core and either product or vendor variants. @@ -186,7 +182,7 @@ func visitPropsAndCompareVendorAndProductProps(v reflect.Value) bool { // This function is used only for the VNDK modules that is available to both vendor // and product partitions. func (c *Module) compareVendorAndProductProps() bool { - if !c.IsVndk() && c.VendorProperties.Product_available != nil { + if !c.IsVndk() && !Bool(c.VendorProperties.Product_available) { panic(fmt.Errorf("This is only for product available VNDK libs. %q is not a VNDK library or not product available", c.Name())) } for _, properties := range c.GetProperties() { @@ -202,14 +198,14 @@ func (m *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { vendorSpecific := mctx.SocSpecific() || mctx.DeviceSpecific() productSpecific := mctx.ProductSpecific() - if m.VendorProperties.Vendor_available != nil { + if Bool(m.VendorProperties.Vendor_available) { if vendorSpecific { mctx.PropertyErrorf("vendor_available", "doesn't make sense at the same time as `vendor: true`, `proprietary: true`, or `device_specific:true`") } } - if m.VendorProperties.Product_available != nil { + if Bool(m.VendorProperties.Product_available) { if productSpecific { mctx.PropertyErrorf("product_available", "doesn't make sense at the same time as `product_specific: true`") @@ -226,10 +222,10 @@ func (m *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { if !vndkdep.isVndkExt() { mctx.PropertyErrorf("vndk", "must set `extends: \"...\"` to vndk extension") - } else if m.VendorProperties.Vendor_available != nil { + } else if Bool(m.VendorProperties.Vendor_available) { mctx.PropertyErrorf("vendor_available", "must not set at the same time as `vndk: {extends: \"...\"}`") - } else if m.VendorProperties.Product_available != nil { + } else if Bool(m.VendorProperties.Product_available) { mctx.PropertyErrorf("product_available", "must not set at the same time as `vndk: {extends: \"...\"}`") } @@ -239,11 +235,11 @@ func (m *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { "must set `vendor: true` or `product_specific: true` to set `extends: %q`", m.getVndkExtendsModuleName()) } - if m.VendorProperties.Vendor_available == nil { + if !Bool(m.VendorProperties.Vendor_available) { mctx.PropertyErrorf("vndk", - "vendor_available must be set to either true or false when `vndk: {enabled: true}`") + "vendor_available must be set to true when `vndk: {enabled: true}`") } - if m.VendorProperties.Product_available != nil { + if Bool(m.VendorProperties.Product_available) { // 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. diff --git a/cc/llndk_library.go b/cc/llndk_library.go index 0c4bcfdbc..bd48501a3 100644 --- a/cc/llndk_library.go +++ b/cc/llndk_library.go @@ -55,13 +55,6 @@ type llndkLibraryProperties struct { // Whether the system library uses symbol versions. Unversioned *bool - // whether this module can be directly depended upon by libs that are installed - // to /vendor and /product. - // When set to false, this module can only be depended on by VNDK libraries, not - // vendor nor product libraries. This effectively hides this module from - // non-system modules. Default value is true. - Vendor_available *bool - // list of llndk headers to re-export include directories from. Export_llndk_headers []string `android:"arch_variant"` @@ -136,7 +129,6 @@ func NewLLndkStubLibrary() *Module { stub := &llndkStubDecorator{ libraryDecorator: library, } - stub.Properties.Vendor_available = BoolPtr(true) module.compiler = stub module.linker = stub module.installer = nil diff --git a/cc/testing.go b/cc/testing.go index fc5b030c7..a69e03ee9 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -299,7 +299,7 @@ func GatherRequiredDepsForTest(oses ...android.OsType) string { llndk_library { name: "libft2.llndk", symbol_file: "", - vendor_available: false, + private: true, sdk_version: "current", } cc_library { diff --git a/cc/vndk.go b/cc/vndk.go index 2071a0302..cb29d1241 100644 --- a/cc/vndk.go +++ b/cc/vndk.go @@ -302,7 +302,7 @@ func processLlndkLibrary(mctx android.BottomUpMutatorContext, m *Module) { llndkLibraries(mctx.Config())[name] = filename m.VendorProperties.IsLLNDK = true - if !Bool(lib.Properties.Vendor_available) { + if Bool(lib.Properties.Private) { vndkPrivateLibraries(mctx.Config())[name] = filename m.VendorProperties.IsLLNDKPrivate = true } @@ -349,7 +349,7 @@ func processVndkLibrary(mctx android.BottomUpMutatorContext, m *Module) { if m.IsVndkPrivate() { vndkPrivateLibraries(mctx.Config())[name] = filename } - if m.VendorProperties.Product_available != nil { + if Bool(m.VendorProperties.Product_available) { vndkProductLibraries(mctx.Config())[name] = filename } } diff --git a/rust/image.go b/rust/image.go index af8c3b2fd..5e55e22c9 100644 --- a/rust/image.go +++ b/rust/image.go @@ -100,13 +100,13 @@ func (mod *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { platformVndkVersion := mctx.DeviceConfig().PlatformVndkVersion() // Rust does not support installing to the product image yet. - if mod.VendorProperties.Product_available != nil { + if Bool(mod.VendorProperties.Product_available) { mctx.PropertyErrorf("product_available", "Rust modules do not yet support being available to the product image") } else if mctx.ProductSpecific() { mctx.PropertyErrorf("product_specific", "Rust modules do not yet support installing to the product image.") - } else if mod.VendorProperties.Double_loadable != nil { + } else if Bool(mod.VendorProperties.Double_loadable) { mctx.PropertyErrorf("double_loadable", "Rust modules do not yet support double loading") } @@ -114,7 +114,7 @@ func (mod *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { coreVariantNeeded := true var vendorVariants []string - if mod.VendorProperties.Vendor_available != nil { + if Bool(mod.VendorProperties.Vendor_available) { if vendorSpecific { mctx.PropertyErrorf("vendor_available", "doesn't make sense at the same time as `vendor: true`, `proprietary: true`, or `device_specific:true`")