From ea08613dd35bf8ae9f41e61e6c80a8fe255ce155 Mon Sep 17 00:00:00 2001 From: Ivan Lozano Date: Tue, 8 Dec 2020 14:43:00 -0500 Subject: [PATCH] Move prefer_rlib from binary to base compiler. Moves the prefer_rlib property out from being exclusively a binary property to one thats part of the base compiler properties. This provides a little more control over the libstd linkage in our libraries. Specifically, this enables a usecase where rust_ffi_shared needs to link against libstd statically rather than dynamically. Bug: 175121262 Test: New Soong tests pass. Change-Id: If68014c684a75ba70e9d7dacbb01c7d360dc25a1 --- rust/binary.go | 10 ++-------- rust/compiler.go | 19 ++++++++++++++++++- rust/library.go | 21 ++++++++++++--------- rust/library_test.go | 14 ++++++++++++++ 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/rust/binary.go b/rust/binary.go index af39d383d..c2d97f3ab 100644 --- a/rust/binary.go +++ b/rust/binary.go @@ -24,11 +24,6 @@ func init() { } type BinaryCompilerProperties struct { - // Change the rustlibs linkage to select rlib linkage by default for device targets. - // Also link libstd as an rlib as well on device targets. - // Note: This is the default behavior for host targets. - Prefer_rlib *bool `android:"arch_variant"` - // Builds this binary as a static binary. Implies prefer_rlib true. // // Static executables currently only support for bionic targets. Non-bionic targets will not produce a fully static @@ -115,7 +110,7 @@ func (binary *binaryDecorator) nativeCoverage() bool { } func (binary *binaryDecorator) preferRlib() bool { - return Bool(binary.Properties.Prefer_rlib) || Bool(binary.Properties.Static_executable) + return Bool(binary.baseCompiler.Properties.Prefer_rlib) || Bool(binary.Properties.Static_executable) } func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Path { @@ -156,8 +151,7 @@ func (binary *binaryDecorator) autoDep(ctx BaseModuleContext) autoDep { // Binaries default to dylib dependencies for device, rlib for host. if binary.preferRlib() { return rlibAutoDep - } - if ctx.Device() { + } else if ctx.Device() { return dylibAutoDep } else { return rlibAutoDep diff --git a/rust/compiler.go b/rust/compiler.go index 8d2f09c2b..4312452d2 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -122,6 +122,17 @@ type BaseCompilerProperties struct { // whether to suppress inclusion of standard crates - defaults to false No_stdlibs *bool + + // Change the rustlibs linkage to select rlib linkage by default for device targets. + // Also link libstd as an rlib as well on device targets. + // Note: This is the default behavior for host targets. + // + // This is primarily meant for rust_binary and rust_ffi modules where the default + // linkage of libstd might need to be overridden in some use cases. This should + // generally be avoided with other module types since it may cause collisions at + // linkage if all dependencies of the root binary module do not link against libstd\ + // the same way. + Prefer_rlib *bool `android:"arch_variant"` } type baseCompiler struct { @@ -154,9 +165,15 @@ func (compiler *baseCompiler) coverageOutputZipPath() android.OptionalPath { panic("baseCompiler does not implement coverageOutputZipPath()") } +func (compiler *baseCompiler) preferRlib() bool { + return Bool(compiler.Properties.Prefer_rlib) +} + func (compiler *baseCompiler) stdLinkage(ctx *depsContext) RustLinkage { // For devices, we always link stdlibs in as dylibs by default. - if ctx.Device() { + if compiler.preferRlib() { + return RlibLinkage + } else if ctx.Device() { return DylibLinkage } else { return RlibLinkage diff --git a/rust/library.go b/rust/library.go index 971588d8a..9d731e68f 100644 --- a/rust/library.go +++ b/rust/library.go @@ -159,14 +159,6 @@ func (library *libraryDecorator) static() bool { return library.MutatedProperties.VariantIsStatic } -func (library *libraryDecorator) stdLinkage(ctx *depsContext) RustLinkage { - // libraries should only request the RlibLinkage when building a static FFI or when variant is StaticStd - if library.static() || library.MutatedProperties.VariantIsStaticStd { - return RlibLinkage - } - return DefaultLinkage -} - func (library *libraryDecorator) source() bool { return library.MutatedProperties.VariantIsSource } @@ -228,7 +220,9 @@ func (library *libraryDecorator) setSource() { } func (library *libraryDecorator) autoDep(ctx BaseModuleContext) autoDep { - if library.rlib() || library.static() { + if library.preferRlib() { + return rlibAutoDep + } else if library.rlib() || library.static() { return rlibAutoDep } else if library.dylib() || library.shared() { return dylibAutoDep @@ -237,6 +231,15 @@ func (library *libraryDecorator) autoDep(ctx BaseModuleContext) autoDep { } } +func (library *libraryDecorator) stdLinkage(ctx *depsContext) RustLinkage { + if library.static() || library.MutatedProperties.VariantIsStaticStd { + return RlibLinkage + } else if library.baseCompiler.preferRlib() { + return RlibLinkage + } + return DefaultLinkage +} + var _ compiler = (*libraryDecorator)(nil) var _ libraryInterface = (*libraryDecorator)(nil) var _ exportedFlagsProducer = (*libraryDecorator)(nil) diff --git a/rust/library_test.go b/rust/library_test.go index fec3992aa..54cd2a5b3 100644 --- a/rust/library_test.go +++ b/rust/library_test.go @@ -251,6 +251,13 @@ func TestLibstdLinkage(t *testing.T) { srcs: ["foo.rs"], crate_name: "bar", rustlibs: ["libfoo"], + } + rust_ffi { + name: "libbar.prefer_rlib", + srcs: ["foo.rs"], + crate_name: "bar", + rustlibs: ["libfoo"], + prefer_rlib: true, }`) libfooDylib := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib").Module().(*Module) @@ -260,6 +267,9 @@ func TestLibstdLinkage(t *testing.T) { libbarShared := ctx.ModuleForTests("libbar", "android_arm64_armv8-a_shared").Module().(*Module) libbarStatic := ctx.ModuleForTests("libbar", "android_arm64_armv8-a_static").Module().(*Module) + // prefer_rlib works the same for both rust_library and rust_ffi, so a single check is sufficient here. + libbarRlibStd := ctx.ModuleForTests("libbar.prefer_rlib", "android_arm64_armv8-a_shared").Module().(*Module) + if !android.InList("libstd", libfooRlibStatic.Properties.AndroidMkRlibs) { t.Errorf("rlib-std variant for device rust_library_rlib does not link libstd as an rlib") } @@ -279,4 +289,8 @@ func TestLibstdLinkage(t *testing.T) { if !android.InList("libfoo.rlib-std", libbarStatic.Properties.AndroidMkRlibs) { t.Errorf("Device rust_ffi_static does not link dependent rustlib rlib-std variant") } + if !android.InList("libstd", libbarRlibStd.Properties.AndroidMkRlibs) { + t.Errorf("rust_ffi with prefer_rlib does not link libstd as an rlib") + } + }