From c08897c1e442be2e0f6db99ca310c9dc7c2e3194 Mon Sep 17 00:00:00 2001 From: Ivan Lozano Date: Fri, 2 Apr 2021 12:41:32 -0400 Subject: [PATCH] Add more Rust vendor image support. This adds Rust vendor image support for all module types except Rust prebuilts. Bug: 184042776 Test: New Soong tests. Test: Example cc_library vendor module can depend on rust_ffi_shared. Test: Example rust_library vendor-only module compiles. Change-Id: Iaa30ad51fdaedcbf14687da5472581f6af62ff59 --- cc/cc.go | 23 ++++++++++--------- cc/linkable.go | 12 ++++++++++ rust/androidmk.go | 5 +++++ rust/compiler.go | 5 ++++- rust/image.go | 50 ++++++++++++++++++++++++----------------- rust/image_test.go | 56 ++++++++++++---------------------------------- rust/library.go | 12 +++++----- rust/rust.go | 31 +++++++++++++++---------- rust/rust_test.go | 18 +++++++++++++++ rust/testing.go | 1 + 10 files changed, 121 insertions(+), 92 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 4413df051..bef49b855 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1203,6 +1203,10 @@ func (c *Module) IsVndkExt() bool { return false } +func (c *Module) SubName() string { + return c.Properties.SubName +} + func (c *Module) MustUseVendorVariant() bool { return c.isVndkSp() || c.Properties.MustUseVendorVariant } @@ -2306,12 +2310,7 @@ func checkLinkType(ctx android.BaseModuleContext, from LinkableInterface, to Lin if ccFrom.vndkdep != nil { ccFrom.vndkdep.vndkCheckLinkType(ctx, ccTo, tag) } - } else if linkableMod, ok := to.(LinkableInterface); ok { - // Static libraries from other languages can be linked - if !linkableMod.Static() { - ctx.ModuleErrorf("Attempting to link VNDK cc.Module with unsupported module type") - } - } else { + } else if _, ok := to.(LinkableInterface); !ok { ctx.ModuleErrorf("Attempting to link VNDK cc.Module with unsupported module type") } return @@ -2824,7 +2823,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { c.sabi.Properties.ReexportedIncludes, depExporterInfo.IncludeDirs.Strings()...) } - makeLibName := c.makeLibName(ctx, ccDep, depName) + libDepTag.makeSuffix + makeLibName := MakeLibName(ctx, c, ccDep, depName) + libDepTag.makeSuffix switch { case libDepTag.header(): c.Properties.AndroidMkHeaderLibs = append( @@ -2863,7 +2862,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { switch depTag { case runtimeDepTag: c.Properties.AndroidMkRuntimeLibs = append( - c.Properties.AndroidMkRuntimeLibs, c.makeLibName(ctx, ccDep, depName)+libDepTag.makeSuffix) + c.Properties.AndroidMkRuntimeLibs, MakeLibName(ctx, c, ccDep, depName)+libDepTag.makeSuffix) // Record baseLibName for snapshots. c.Properties.SnapshotRuntimeLibs = append(c.Properties.SnapshotRuntimeLibs, baseLibName(depName)) case objDepTag: @@ -2941,7 +2940,8 @@ func baseLibName(depName string) string { return libName } -func (c *Module) makeLibName(ctx android.ModuleContext, ccDep LinkableInterface, depName string) string { +func MakeLibName(ctx android.ModuleContext, c LinkableInterface, ccDep LinkableInterface, depName string) string { + vendorPublicLibraries := vendorPublicLibraries(ctx.Config()) libName := baseLibName(depName) @@ -2951,6 +2951,7 @@ func (c *Module) makeLibName(ctx android.ModuleContext, ccDep LinkableInterface, nonSystemVariantsExist := ccDep.HasNonSystemVariants() || isLLndk if ccDepModule != nil { + // TODO(ivanlozano) Support snapshots for Rust-produced C library variants. // Use base module name for snapshots when exporting to Makefile. if snapshotPrebuilt, ok := ccDepModule.linker.(snapshotInterface); ok { baseName := ccDepModule.BaseModuleName() @@ -2964,10 +2965,10 @@ func (c *Module) makeLibName(ctx android.ModuleContext, ccDep LinkableInterface, // The vendor module is a no-vendor-variant VNDK library. Depend on the // core module instead. return libName - } else if ccDep.UseVndk() && nonSystemVariantsExist && ccDepModule != nil { + } else if ccDep.UseVndk() && nonSystemVariantsExist { // The vendor and product modules in Make will have been renamed to not conflict with the // core module, so update the dependency name here accordingly. - return libName + ccDepModule.Properties.SubName + return libName + ccDep.SubName() } else if (ctx.Platform() || ctx.ProductSpecific()) && isVendorPublicLib { return libName + vendorPublicLibrarySuffix } else if ccDep.InRamdisk() && !ccDep.OnlyInRamdisk() { diff --git a/cc/linkable.go b/cc/linkable.go index 17bafcc41..571a3bb71 100644 --- a/cc/linkable.go +++ b/cc/linkable.go @@ -124,6 +124,9 @@ type LinkableInterface interface { HasNonSystemVariants() bool InProduct() bool + // SubName returns the modules SubName, used for image and NDK/SDK variations. + SubName() string + SdkVersion() string MinSdkVersion() string AlwaysSdk() bool @@ -170,6 +173,15 @@ func GetImageVariantType(c LinkableInterface) ImageVariantType { } } +// DepTagMakeSuffix returns the makeSuffix value of a particular library dependency tag. +// Returns an empty string if not a library dependency tag. +func DepTagMakeSuffix(depTag blueprint.DependencyTag) string { + if libDepTag, ok := depTag.(libraryDependencyTag); ok { + return libDepTag.makeSuffix + } + return "" +} + // SharedDepTag returns the dependency tag for any C++ shared libraries. func SharedDepTag() blueprint.DependencyTag { return libraryDependencyTag{Kind: sharedLibraryDependency} diff --git a/rust/androidmk.go b/rust/androidmk.go index dea32a34d..f8ab2aa7b 100644 --- a/rust/androidmk.go +++ b/rust/androidmk.go @@ -60,6 +60,10 @@ func (mod *Module) AndroidMkEntries() []android.AndroidMkEntries { entries.AddStrings("LOCAL_SHARED_LIBRARIES", mod.Properties.AndroidMkSharedLibs...) entries.AddStrings("LOCAL_STATIC_LIBRARIES", mod.Properties.AndroidMkStaticLibs...) entries.AddStrings("LOCAL_SOONG_LINK_TYPE", mod.makeLinkType) + if mod.UseVndk() { + entries.SetBool("LOCAL_USE_VNDK", true) + } + }, }, } @@ -75,6 +79,7 @@ func (mod *Module) AndroidMkEntries() []android.AndroidMkEntries { mod.SubAndroidMk(&ret, mod.sanitize) } + ret.SubName += mod.Properties.RustSubName ret.SubName += mod.Properties.SubName return []android.AndroidMkEntries{ret} diff --git a/rust/compiler.go b/rust/compiler.go index aaa192402..bc034d7cc 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -303,7 +303,6 @@ func (compiler *baseCompiler) compilerDeps(ctx DepsContext, deps Deps) Deps { if ctx.Target().Os == android.BuildOs { stdlib = stdlib + "_" + ctx.toolchain().RustTriple() } - deps.Stdlibs = append(deps.Stdlibs, stdlib) } } @@ -344,6 +343,10 @@ func (compiler *baseCompiler) installDir(ctx ModuleContext) android.InstallPath if !ctx.Host() && ctx.Config().HasMultilibConflict(ctx.Arch().ArchType) { dir = filepath.Join(dir, ctx.Arch().ArchType.String()) } + + if compiler.location == InstallInData && ctx.RustModule().UseVndk() { + dir = filepath.Join(dir, "vendor") + } return android.PathForModuleInstall(ctx, dir, compiler.subDir, compiler.relativeInstallPath(), compiler.relative) } diff --git a/rust/image.go b/rust/image.go index 1b2469a3e..7eb49d962 100644 --- a/rust/image.go +++ b/rust/image.go @@ -110,6 +110,24 @@ func (mod *Module) IsSnapshotPrebuilt() bool { return false } +func (ctx *moduleContext) SocSpecific() bool { + // 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 + // unless they have "odm_available: true". + return ctx.ModuleContext.SocSpecific() || (ctx.RustModule().InVendor() && !ctx.RustModule().VendorVariantToOdm()) +} + +func (ctx *moduleContext) DeviceSpecific() bool { + // Some vendor variants want to be installed to /odm by setting "odm_available: true". + return ctx.ModuleContext.DeviceSpecific() || (ctx.RustModule().InVendor() && ctx.RustModule().VendorVariantToOdm()) +} + +// Returns true when this module creates a vendor variant and wants to install the vendor variant +// to the odm partition. +func (c *Module) VendorVariantToOdm() bool { + return Bool(c.VendorProperties.Odm_available) +} + func (ctx *moduleContext) ProductSpecific() bool { return false } @@ -155,6 +173,11 @@ func (mod *Module) InProduct() bool { return false } +// Returns true if the module is "vendor" variant. Usually these modules are installed in /vendor +func (mod *Module) InVendor() bool { + return mod.Properties.ImageVariationPrefix == cc.VendorVariationPrefix +} + func (mod *Module) SetImageVariation(ctx android.BaseModuleContext, variant string, module android.Module) { m := module.(*Module) if variant == android.VendorRamdiskVariation { @@ -185,32 +208,19 @@ func (mod *Module) ImageMutatorBegin(mctx android.BaseModuleContext) { mctx.PropertyErrorf("double_loadable", "Rust modules do not yet support double loading") } - - if mod.HasVendorVariant() { - if lib, ok := mod.compiler.(libraryInterface); ok && lib.buildShared() { - // Explicitly disallow rust_ffi variants which produce shared libraries from setting vendor_available. - // Vendor variants do not produce an error for dylibs, rlibs with dylib-std linkage are disabled in the respective library - // mutators until support is added. - // - // We can't check shared() here because image mutator is called before the library mutator, so we need to - // check buildShared() - - mctx.PropertyErrorf("vendor_available", "cannot be set for rust_ffi or rust_ffi_shared modules.") - } - } - if Bool(mod.Properties.Vendor_ramdisk_available) { if lib, ok := mod.compiler.(libraryInterface); !ok || (ok && lib.buildShared()) { mctx.PropertyErrorf("vendor_ramdisk_available", "cannot be set for rust_ffi or rust_ffi_shared modules.") } } - vendorSpecific := mctx.SocSpecific() || mctx.DeviceSpecific() - if vendorSpecific { - if lib, ok := mod.compiler.(libraryInterface); !ok || (ok && (lib.buildShared() || lib.buildDylib() || lib.buildRlib())) { - mctx.ModuleErrorf("Rust vendor specific modules are currently only supported for rust_ffi_static modules.") + cc.MutateImage(mctx, mod) + + if !mod.Properties.CoreVariantNeeded || mod.HasNonSystemVariants() { + + if _, ok := mod.compiler.(*prebuiltLibraryDecorator); ok { + // Rust does not support prebuilt libraries on non-System images. + mctx.ModuleErrorf("Rust prebuilt modules not supported for non-system images.") } } - - cc.MutateImage(mctx, mod) } diff --git a/rust/image_test.go b/rust/image_test.go index 7677cce3b..95e788f88 100644 --- a/rust/image_test.go +++ b/rust/image_test.go @@ -40,8 +40,8 @@ func TestVendorLinkage(t *testing.T) { vendorBinary := ctx.ModuleForTests("fizz_vendor", "android_vendor.29_arm64_armv8-a").Module().(*cc.Module) - if !android.InList("libfoo_vendor", vendorBinary.Properties.AndroidMkStaticLibs) { - t.Errorf("vendorBinary should have a dependency on libfoo_vendor") + if !android.InList("libfoo_vendor.vendor", vendorBinary.Properties.AndroidMkStaticLibs) { + t.Errorf("vendorBinary should have a dependency on libfoo_vendor: %#v", vendorBinary.Properties.AndroidMkStaticLibs) } } @@ -87,47 +87,19 @@ func TestVendorRamdiskLinkage(t *testing.T) { } } -// Test that shared libraries cannot be made vendor available until proper support is added. +// Test that prebuilt libraries cannot be made vendor available. func TestForbiddenVendorLinkage(t *testing.T) { - testRustError(t, "cannot be set for rust_ffi or rust_ffi_shared modules.", ` - rust_ffi_shared { - name: "libfoo_vendor", - crate_name: "foo", - srcs: ["foo.rs"], - vendor_available: true, - } - `) - testRustError(t, "cannot be set for rust_ffi or rust_ffi_shared modules.", ` - rust_ffi_shared { - name: "libfoo_vendor", - crate_name: "foo", - srcs: ["foo.rs"], - vendor_ramdisk_available: true, - } - `) - testRustError(t, "Rust vendor specific modules are currently only supported for rust_ffi_static modules.", ` - rust_ffi { - name: "libfoo_vendor", - crate_name: "foo", - srcs: ["foo.rs"], + testRustVndkError(t, "Rust prebuilt modules not supported for non-system images.", ` + rust_prebuilt_library { + name: "librust_prebuilt", + crate_name: "rust_prebuilt", + rlib: { + srcs: ["libtest.rlib"], + }, + dylib: { + srcs: ["libtest.so"], + }, vendor: true, } - `) - testRustError(t, "Rust vendor specific modules are currently only supported for rust_ffi_static modules.", ` - rust_library { - name: "libfoo_vendor", - crate_name: "foo", - srcs: ["foo.rs"], - vendor: true, - } - `) - testRustError(t, "Rust vendor specific modules are currently only supported for rust_ffi_static modules.", ` - rust_binary { - name: "foo_vendor", - crate_name: "foo", - srcs: ["foo.rs"], - vendor: true, - } - `) - + `) } diff --git a/rust/library.go b/rust/library.go index 71fe1f538..ae130a353 100644 --- a/rust/library.go +++ b/rust/library.go @@ -596,9 +596,9 @@ func LibraryMutator(mctx android.BottomUpMutatorContext) { v.(*Module).compiler.(libraryInterface).setRlib() case dylibVariation: v.(*Module).compiler.(libraryInterface).setDylib() - if v.(*Module).ModuleBase.ImageVariation().Variation != android.CoreVariation { + if v.(*Module).ModuleBase.ImageVariation().Variation == android.VendorRamdiskVariation { // TODO(b/165791368) - // Disable dylib non-core variations until we support these. + // Disable dylib Vendor Ramdisk variations until we support these. v.(*Module).Disable() } case "source": @@ -637,14 +637,14 @@ func LibstdMutator(mctx android.BottomUpMutatorContext) { dylib := modules[1].(*Module) rlib.compiler.(libraryInterface).setRlibStd() dylib.compiler.(libraryInterface).setDylibStd() - if dylib.ModuleBase.ImageVariation().Variation != android.CoreVariation { + if dylib.ModuleBase.ImageVariation().Variation == android.VendorRamdiskVariation { // TODO(b/165791368) - // Disable rlibs that link against dylib-std on non-core variations until non-core dylib + // Disable rlibs that link against dylib-std on vendor ramdisk variations until those dylib // variants are properly supported. dylib.Disable() } - rlib.Properties.SubName += RlibStdlibSuffix - dylib.Properties.SubName += DylibStdlibSuffix + rlib.Properties.RustSubName += RlibStdlibSuffix + dylib.Properties.RustSubName += DylibStdlibSuffix } } } diff --git a/rust/rust.go b/rust/rust.go index f0da2ced0..abe25a4a8 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -73,6 +73,11 @@ type BaseProperties struct { VndkVersion string `blueprint:"mutated"` SubName string `blueprint:"mutated"` + // SubName is used by CC for tracking image variants / SDK versions. RustSubName is used for Rust-specific + // subnaming which shouldn't be visible to CC modules (such as the rlib stdlinkage subname). This should be + // appended before SubName. + RustSubName string `blueprint:"mutated"` + // Set by imageMutator CoreVariantNeeded bool `blueprint:"mutated"` VendorRamdiskVariantNeeded bool `blueprint:"mutated"` @@ -131,11 +136,6 @@ func (mod *Module) SetPreventInstall() { mod.Properties.PreventInstall = true } -// Returns true if the module is "vendor" variant. Usually these modules are installed in /vendor -func (mod *Module) InVendor() bool { - return mod.Properties.ImageVariationPrefix == cc.VendorVariationPrefix -} - func (mod *Module) SetHideFromMake() { mod.Properties.HideFromMake = true } @@ -231,7 +231,11 @@ func (mod *Module) UseVndk() bool { } func (mod *Module) MustUseVendorVariant() bool { - return false + return true +} + +func (mod *Module) SubName() string { + return mod.Properties.SubName } func (mod *Module) IsVndk() bool { @@ -858,8 +862,10 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { ctx.VisitDirectDeps(func(dep android.Module) { depName := ctx.OtherModuleName(dep) depTag := ctx.OtherModuleDependencyTag(dep) + if rustDep, ok := dep.(*Module); ok && !rustDep.CcLibraryInterface() { //Handle Rust Modules + makeLibName := cc.MakeLibName(ctx, mod, rustDep, depName+rustDep.Properties.RustSubName) switch depTag { case dylibDepTag: @@ -869,19 +875,19 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { return } directDylibDeps = append(directDylibDeps, rustDep) - mod.Properties.AndroidMkDylibs = append(mod.Properties.AndroidMkDylibs, depName) + mod.Properties.AndroidMkDylibs = append(mod.Properties.AndroidMkDylibs, makeLibName) case rlibDepTag: rlib, ok := rustDep.compiler.(libraryInterface) if !ok || !rlib.rlib() { - ctx.ModuleErrorf("mod %q not an rlib library", depName+rustDep.Properties.SubName) + ctx.ModuleErrorf("mod %q not an rlib library", makeLibName) return } directRlibDeps = append(directRlibDeps, rustDep) - mod.Properties.AndroidMkRlibs = append(mod.Properties.AndroidMkRlibs, depName+rustDep.Properties.SubName) + mod.Properties.AndroidMkRlibs = append(mod.Properties.AndroidMkRlibs, makeLibName) case procMacroDepTag: directProcMacroDeps = append(directProcMacroDeps, rustDep) - mod.Properties.AndroidMkProcMacroLibs = append(mod.Properties.AndroidMkProcMacroLibs, depName) + mod.Properties.AndroidMkProcMacroLibs = append(mod.Properties.AndroidMkProcMacroLibs, makeLibName) case android.SourceDepTag: // Since these deps are added in path_properties.go via AddDependencies, we need to ensure the correct // OS/Arch variant is used. @@ -925,6 +931,7 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } else if ccDep, ok := dep.(cc.LinkableInterface); ok { //Handle C dependencies + makeLibName := cc.MakeLibName(ctx, mod, ccDep, depName) if _, ok := ccDep.(*Module); !ok { if ccDep.Module().Target().Os != ctx.Os() { ctx.ModuleErrorf("OS mismatch between %q and %q", ctx.ModuleName(), depName) @@ -966,7 +973,7 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { depPaths.depClangFlags = append(depPaths.depClangFlags, exportedInfo.Flags...) depPaths.depGeneratedHeaders = append(depPaths.depGeneratedHeaders, exportedInfo.GeneratedHeaders...) directStaticLibDeps = append(directStaticLibDeps, ccDep) - mod.Properties.AndroidMkStaticLibs = append(mod.Properties.AndroidMkStaticLibs, depName) + mod.Properties.AndroidMkStaticLibs = append(mod.Properties.AndroidMkStaticLibs, makeLibName) case cc.IsSharedDepTag(depTag): depPaths.linkDirs = append(depPaths.linkDirs, linkPath) depPaths.linkObjects = append(depPaths.linkObjects, linkObject.String()) @@ -976,7 +983,7 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { depPaths.depClangFlags = append(depPaths.depClangFlags, exportedInfo.Flags...) depPaths.depGeneratedHeaders = append(depPaths.depGeneratedHeaders, exportedInfo.GeneratedHeaders...) directSharedLibDeps = append(directSharedLibDeps, ccDep) - mod.Properties.AndroidMkSharedLibs = append(mod.Properties.AndroidMkSharedLibs, depName) + mod.Properties.AndroidMkSharedLibs = append(mod.Properties.AndroidMkSharedLibs, makeLibName) exportDep = true case cc.IsHeaderDepTag(depTag): exportedInfo := ctx.OtherModuleProvider(dep, cc.FlagExporterInfoProvider).(cc.FlagExporterInfo) diff --git a/rust/rust_test.go b/rust/rust_test.go index 890fb262b..47c64a9ad 100644 --- a/rust/rust_test.go +++ b/rust/rust_test.go @@ -113,6 +113,24 @@ func testRustError(t *testing.T, pattern string, bp string) { RunTestWithBp(t, bp) } +// testRustVndkError is similar to testRustError, but can be used to test VNDK-related errors. +func testRustVndkError(t *testing.T, pattern string, bp string) { + skipTestIfOsNotSupported(t) + android.GroupFixturePreparers( + prepareForRustTest, + rustMockedFiles.AddToFixture(), + android.FixtureModifyProductVariables( + func(variables android.FixtureProductVariables) { + variables.DeviceVndkVersion = StringPtr("current") + variables.ProductVndkVersion = StringPtr("current") + variables.Platform_vndk_version = StringPtr("VER") + }, + ), + ). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(pattern)). + RunTestWithBp(t, bp) +} + // testRustCtx is used to build a particular test environment. Unless your // tests requires a specific setup, prefer the wrapping functions: testRust, // testRustCov or testRustError. diff --git a/rust/testing.go b/rust/testing.go index 09c627f70..f41d5a142 100644 --- a/rust/testing.go +++ b/rust/testing.go @@ -127,6 +127,7 @@ func GatherRequiredDepsForTest() string { system_shared_libs: [], apex_available: ["//apex_available:platform", "//apex_available:anyapex"], min_sdk_version: "29", + vendor_available: true, } cc_library { name: "libprotobuf-cpp-full",