Link device binaries dynamically by default.

Device binaries currently are linked statically by default. Instead we
should be linking these dynamic by default. To avoid conflicts when
manually specifying rlib dependencies on modules, we always link libstd
dynamically for all device modules except static libraries.

This removes the "prefer_dynamic" property entirely to avoid confusion.

Bug: 165161105
Test: m profcollectd is built dynamically.
Test: cd external/rust/; mma
Test: cd external/crosvm/; mma
Change-Id: I25ac897040acbcc2d97c791a33e8e01610632272

rust/binary.go

@@ -24,9 +24,6 @@ func init() {
 }
 
 type BinaryCompilerProperties struct {
-	// passes -C prefer-dynamic to rustc, which tells it to dynamically link the stdlib
-	// (assuming it has no dylib dependencies already)
-	Prefer_dynamic *bool
 }
 
 type binaryDecorator struct {
@@ -60,10 +57,6 @@ func NewRustBinary(hod android.HostOrDeviceSupported) (*Module, *binaryDecorator
 	return module, binary
 }
 
-func (binary *binaryDecorator) preferDynamic() bool {
-	return Bool(binary.Properties.Prefer_dynamic)
-}
-
 func (binary *binaryDecorator) compilerFlags(ctx ModuleContext, flags Flags) Flags {
 	flags = binary.baseCompiler.compilerFlags(ctx, flags)
 
@@ -76,9 +69,6 @@ func (binary *binaryDecorator) compilerFlags(ctx ModuleContext, flags Flags) Fla
 			"-Wl,--no-undefined-version")
 	}
 
-	if binary.preferDynamic() {
-		flags.RustFlags = append(flags.RustFlags, "-C prefer-dynamic")
-	}
 	return flags
 }
 
@@ -132,8 +122,9 @@ func (binary *binaryDecorator) coverageOutputZipPath() android.OptionalPath {
 	return binary.coverageOutputZipFile
 }
 
-func (binary *binaryDecorator) autoDep() autoDep {
-	if binary.preferDynamic() {
+func (binary *binaryDecorator) autoDep(ctx BaseModuleContext) autoDep {
+	// Binaries default to dylib dependencies for device, rlib for host.
+	if ctx.Device() {
 		return dylibAutoDep
 	} else {
 		return rlibAutoDep

rust/binary_test.go

@@ -17,44 +17,64 @@ package rust
 import (
 	"strings"
 	"testing"
+
+	"android/soong/android"
 )
 
-// Test that the prefer_dynamic property is handled correctly.
-func TestPreferDynamicBinary(t *testing.T) {
+// Test that rustlibs default linkage is correct for binaries.
+func TestBinaryLinkage(t *testing.T) {
+	ctx := testRust(t, `
+		rust_binary {
+			name: "fizz-buzz",
+			srcs: ["foo.rs"],
+			rustlibs: ["libfoo"],
+			host_supported: true,
+		}
+		rust_library {
+			name: "libfoo",
+			srcs: ["foo.rs"],
+			crate_name: "foo",
+			host_supported: true,
+		}`)
+
+	fizzBuzzHost := ctx.ModuleForTests("fizz-buzz", "linux_glibc_x86_64").Module().(*Module)
+	fizzBuzzDevice := ctx.ModuleForTests("fizz-buzz", "android_arm64_armv8-a").Module().(*Module)
+
+	if !android.InList("libfoo", fizzBuzzHost.Properties.AndroidMkRlibs) {
+		t.Errorf("rustlibs dependency libfoo should be an rlib dep for host modules")
+	}
+
+	if !android.InList("libfoo", fizzBuzzDevice.Properties.AndroidMkDylibs) {
+		t.Errorf("rustlibs dependency libfoo should be an dylib dep for device modules")
+	}
+}
+
+// Test that the path returned by HostToolPath is correct
+func TestHostToolPath(t *testing.T) {
 	ctx := testRust(t, `
 		rust_binary_host {
-			name: "fizz-buzz-dynamic",
+			name: "fizz-buzz",
 			srcs: ["foo.rs"],
-			prefer_dynamic: true,
-		}
+		}`)
 
+	path := ctx.ModuleForTests("fizz-buzz", "linux_glibc_x86_64").Module().(*Module).HostToolPath()
+	if g, w := path.String(), "/host/linux-x86/bin/fizz-buzz"; !strings.Contains(g, w) {
+		t.Errorf("wrong host tool path, expected %q got %q", w, g)
+	}
+}
+
+// Test that the flags being passed to rust_binary modules are as expected
+func TestBinaryFlags(t *testing.T) {
+	ctx := testRust(t, `
 		rust_binary_host {
 			name: "fizz-buzz",
 			srcs: ["foo.rs"],
 		}`)
 
 	fizzBuzz := ctx.ModuleForTests("fizz-buzz", "linux_glibc_x86_64").Output("fizz-buzz")
-	fizzBuzzDynamic := ctx.ModuleForTests("fizz-buzz-dynamic", "linux_glibc_x86_64").Output("fizz-buzz-dynamic")
 
-	path := ctx.ModuleForTests("fizz-buzz", "linux_glibc_x86_64").Module().(*Module).HostToolPath()
-	if g, w := path.String(), "/host/linux-x86/bin/fizz-buzz"; !strings.Contains(g, w) {
-		t.Errorf("wrong host tool path, expected %q got %q", w, g)
-	}
-
-	// Do not compile binary modules with the --test flag.
-	flags := fizzBuzzDynamic.Args["rustcFlags"]
+	flags := fizzBuzz.Args["rustcFlags"]
 	if strings.Contains(flags, "--test") {
 		t.Errorf("extra --test flag, rustcFlags: %#v", flags)
 	}
-	if !strings.Contains(flags, "prefer-dynamic") {
-		t.Errorf("missing prefer-dynamic flag, rustcFlags: %#v", flags)
-	}
-
-	flags = fizzBuzz.Args["rustcFlags"]
-	if strings.Contains(flags, "--test") {
-		t.Errorf("extra --test flag, rustcFlags: %#v", flags)
-	}
-	if strings.Contains(flags, "prefer-dynamic") {
-		t.Errorf("unexpected prefer-dynamic flag, rustcFlags: %#v", flags)
-	}
 }

rust/compiler.go

@@ -145,6 +145,10 @@ func (compiler *baseCompiler) coverageOutputZipPath() android.OptionalPath {
 	panic("baseCompiler does not implement coverageOutputZipPath()")
 }
 
+func (compiler *baseCompiler) static() bool {
+	return false
+}
+
 var _ compiler = (*baseCompiler)(nil)
 
 func (compiler *baseCompiler) inData() bool {
@@ -216,7 +220,15 @@ func (compiler *baseCompiler) compilerDeps(ctx DepsContext, deps Deps) Deps {
 				stdlib = stdlib + "_" + ctx.toolchain().RustTriple()
 			}
 
-			deps.Rustlibs = append(deps.Rustlibs, stdlib)
+			// For devices, we always link stdlibs in as dylibs except for ffi static libraries.
+			// (rustc does not support linking libstd as a dylib for ffi static libraries)
+			if ctx.Host() {
+				deps.Rustlibs = append(deps.Rustlibs, stdlib)
+			} else if ctx.RustModule().compiler.static() {
+				deps.Rlibs = append(deps.Rlibs, stdlib)
+			} else {
+				deps.Dylibs = append(deps.Dylibs, stdlib)
+			}
 		}
 	}
 	return deps

rust/compiler_test.go

@@ -177,3 +177,30 @@ func TestLints(t *testing.T) {
 		})
 	}
 }
+
+// Test that devices are linking the stdlib dynamically
+func TestStdDeviceLinkage(t *testing.T) {
+	ctx := testRust(t, `
+		rust_binary {
+			name: "fizz",
+			srcs: ["foo.rs"],
+		}
+		rust_library {
+			name: "libfoo",
+			srcs: ["foo.rs"],
+			crate_name: "foo",
+		}`)
+	fizz := ctx.ModuleForTests("fizz", "android_arm64_armv8-a").Module().(*Module)
+	fooRlib := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_rlib").Module().(*Module)
+	fooDylib := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib").Module().(*Module)
+
+	if !android.InList("libstd", fizz.Properties.AndroidMkDylibs) {
+		t.Errorf("libstd is not linked dynamically for device binaries")
+	}
+	if !android.InList("libstd", fooRlib.Properties.AndroidMkDylibs) {
+		t.Errorf("libstd is not linked dynamically for rlibs")
+	}
+	if !android.InList("libstd", fooDylib.Properties.AndroidMkDylibs) {
+		t.Errorf("libstd is not linked dynamically for dylibs")
+	}
+}

rust/library.go

@@ -176,13 +176,13 @@ func (library *libraryDecorator) setStatic() {
 	library.MutatedProperties.VariantIsDylib = false
 }
 
-func (library *libraryDecorator) autoDep() autoDep {
+func (library *libraryDecorator) autoDep(ctx BaseModuleContext) autoDep {
 	if library.rlib() || library.static() {
 		return rlibAutoDep
 	} else if library.dylib() || library.shared() {
 		return dylibAutoDep
 	} else {
-		return rlibAutoDep
+		panic("autoDep called on library" + ctx.ModuleName() + "that has no enabled variants.")
 	}
 }
 

rust/library_test.go

@@ -144,6 +144,22 @@ func TestSharedLibrary(t *testing.T) {
 	}
 }
 
+func TestStaticLibraryLinkage(t *testing.T) {
+	ctx := testRust(t, `
+		rust_ffi_static {
+			name: "libfoo",
+			srcs: ["foo.rs"],
+			crate_name: "foo",
+		}`)
+
+	libfoo := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_static")
+
+	if !android.InList("libstd", libfoo.Module().(*Module).Properties.AndroidMkRlibs) {
+		t.Errorf("Static libstd rlib expected to be a dependency of Rust static libraries. Rlib deps are: %#v",
+			libfoo.Module().(*Module).Properties.AndroidMkDylibs)
+	}
+}
+
 // Test that variants pull in the right type of rustlib autodep
 func TestAutoDeps(t *testing.T) {
 

rust/proc_macro.go

@@ -80,6 +80,6 @@ func (procMacro *procMacroDecorator) getStem(ctx ModuleContext) string {
 	return stem + String(procMacro.baseCompiler.Properties.Suffix)
 }
 
-func (procMacro *procMacroDecorator) autoDep() autoDep {
+func (procMacro *procMacroDecorator) autoDep(ctx BaseModuleContext) autoDep {
 	return rlibAutoDep
 }

rust/rust.go

@@ -289,6 +289,8 @@ type compiler interface {
 
 	Disabled() bool
 	SetDisabled()
+
+	static() bool
 }
 
 type exportedFlagsProducer interface {
@@ -740,7 +742,7 @@ var (
 )
 
 type autoDeppable interface {
-	autoDep() autoDep
+	autoDep(ctx BaseModuleContext) autoDep
 }
 
 func (mod *Module) begin(ctx BaseModuleContext) {
@@ -988,8 +990,8 @@ func (mod *Module) DepsMutator(actx android.BottomUpMutatorContext) {
 			{Mutator: "rust_libraries", Variation: "dylib"}}...),
 		dylibDepTag, deps.Dylibs...)
 
-	if deps.Rustlibs != nil {
-		autoDep := mod.compiler.(autoDeppable).autoDep()
+	if deps.Rustlibs != nil && !mod.compiler.Disabled() {
+		autoDep := mod.compiler.(autoDeppable).autoDep(ctx)
 		actx.AddVariationDependencies(
 			append(commonDepVariations, []blueprint.Variation{
 				{Mutator: "rust_libraries", Variation: autoDep.variation}}...),

rust/test.go

@@ -114,7 +114,7 @@ func (test *testDecorator) compilerFlags(ctx ModuleContext, flags Flags) Flags {
 	return flags
 }
 
-func (test *testDecorator) autoDep() autoDep {
+func (test *testDecorator) autoDep(ctx BaseModuleContext) autoDep {
 	return rlibAutoDep
 }