From ad8b18b87260a83aeadb28d637f77474158fcdb1 Mon Sep 17 00:00:00 2001 From: Ivan Lozano Date: Thu, 31 Oct 2019 19:38:29 -0700 Subject: [PATCH] Enforce correct rust library file names. rustc expects libraries and proc_macro filenames to conform to a particular format, alphanumeric with underscores and lib${crate_name}.*. Enforce this with a check when getStem() is called. This makes the crate_name property required for proc_macros and libraries. This also removes the notion of a default crate name derived from the module name. It's not needed for binaries, so this won't impact them. Bug: 143579265 Test: m -j crosvm.experimental Change-Id: I2770cf7d02dd4291c3d240d58d242b940098dcee --- rust/builder.go | 4 +++- rust/compiler.go | 6 ++++-- rust/library.go | 30 ++++++++++++++++++++++++++++++ rust/library_test.go | 37 +++++++++++++++++++++++++++++++++++++ rust/proc_macro.go | 7 +++++++ rust/rust.go | 6 +----- rust/rust_test.go | 23 +++++++---------------- 7 files changed, 89 insertions(+), 24 deletions(-) diff --git a/rust/builder.go b/rust/builder.go index 2a7643d62..d9e36dbbe 100644 --- a/rust/builder.go +++ b/rust/builder.go @@ -88,7 +88,9 @@ func transformSrctoCrate(ctx android.ModuleContext, main android.Path, rustcFlags = append(rustcFlags, flags.GlobalRustFlags...) rustcFlags = append(rustcFlags, flags.RustFlags...) rustcFlags = append(rustcFlags, "--crate-type="+crate_type) - rustcFlags = append(rustcFlags, "--crate-name="+crate_name) + if crate_name != "" { + rustcFlags = append(rustcFlags, "--crate-name="+crate_name) + } if targetTriple != "" { rustcFlags = append(rustcFlags, "--target="+targetTriple) linkFlags = append(linkFlags, "-target "+targetTriple) diff --git a/rust/compiler.go b/rust/compiler.go index 3f028350a..85e8ba6aa 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -18,9 +18,10 @@ import ( "fmt" "path/filepath" + "github.com/google/blueprint/proptools" + "android/soong/android" "android/soong/rust/config" - "github.com/google/blueprint/proptools" ) func getEdition(compiler *baseCompiler) string { @@ -64,7 +65,7 @@ type BaseCompilerProperties struct { // list of C static library dependencies Static_libs []string `android:"arch_variant"` - // crate name (defaults to module name); if library, this must be the expected extern crate name + // crate name, required for libraries. This must be the expected extern crate name used in source Crate_name string `android:"arch_variant"` // list of features to enable for this crate @@ -207,6 +208,7 @@ func (compiler *baseCompiler) getStemWithoutSuffix(ctx BaseModuleContext) string return stem } + func (compiler *baseCompiler) relativeInstallPath() string { return String(compiler.Properties.Relative_install_path) } diff --git a/rust/library.go b/rust/library.go index 273a3ce16..adf6e954c 100644 --- a/rust/library.go +++ b/rust/library.go @@ -15,6 +15,9 @@ package rust import ( + "regexp" + "strings" + "android/soong/android" ) @@ -354,6 +357,33 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa return outputFile } +func (library *libraryDecorator) getStem(ctx ModuleContext) string { + stem := library.baseCompiler.getStemWithoutSuffix(ctx) + validateLibraryStem(ctx, stem, library.crateName()) + + return stem + String(library.baseCompiler.Properties.Suffix) +} + +var validCrateName = regexp.MustCompile("[^a-zA-Z0-9_]+") + +func validateLibraryStem(ctx BaseModuleContext, filename string, crate_name string) { + if crate_name == "" { + ctx.PropertyErrorf("crate_name", "crate_name must be defined.") + } + + // crate_names are used for the library output file, and rustc expects these + // to be alphanumeric with underscores allowed. + if validCrateName.MatchString(crate_name) { + ctx.PropertyErrorf("crate_name", + "library crate_names must be alphanumeric with underscores allowed") + } + + // Libraries are expected to begin with "lib" followed by the crate_name + if !strings.HasPrefix(filename, "lib"+crate_name) { + ctx.ModuleErrorf("Invalid name or stem property; library filenames must start with lib") + } +} + func LibraryMutator(mctx android.BottomUpMutatorContext) { if m, ok := mctx.Module().(*Module); ok && m.compiler != nil { switch library := m.compiler.(type) { diff --git a/rust/library_test.go b/rust/library_test.go index 66bcd20fb..9f9f374b9 100644 --- a/rust/library_test.go +++ b/rust/library_test.go @@ -77,3 +77,40 @@ func TestDylibPreferDynamic(t *testing.T) { t.Errorf("missing prefer-dynamic flag for libfoo dylib, rustcFlags: %#v", libfooDylib.Args["rustcFlags"]) } } + +func TestValidateLibraryStem(t *testing.T) { + testRustError(t, "crate_name must be defined.", ` + rust_library_host { + name: "libfoo", + srcs: ["foo.rs"], + }`) + + testRustError(t, "library crate_names must be alphanumeric with underscores allowed", ` + rust_library_host { + name: "libfoo-bar", + srcs: ["foo.rs"], + crate_name: "foo-bar" + }`) + + testRustError(t, "Invalid name or stem property; library filenames must start with lib", ` + rust_library_host { + name: "foobar", + srcs: ["foo.rs"], + crate_name: "foo_bar" + }`) + testRustError(t, "Invalid name or stem property; library filenames must start with lib", ` + rust_library_host { + name: "foobar", + stem: "libfoo", + srcs: ["foo.rs"], + crate_name: "foo_bar" + }`) + testRustError(t, "Invalid name or stem property; library filenames must start with lib", ` + rust_library_host { + name: "foobar", + stem: "foo_bar", + srcs: ["foo.rs"], + crate_name: "foo_bar" + }`) + +} diff --git a/rust/proc_macro.go b/rust/proc_macro.go index 1a247d9b3..0da87dad1 100644 --- a/rust/proc_macro.go +++ b/rust/proc_macro.go @@ -77,3 +77,10 @@ func (procMacro *procMacroDecorator) compile(ctx ModuleContext, flags Flags, dep TransformSrctoProcMacro(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) return outputFile } + +func (procMacro *procMacroDecorator) getStem(ctx ModuleContext) string { + stem := procMacro.baseCompiler.getStemWithoutSuffix(ctx) + validateLibraryStem(ctx, stem, procMacro.crateName()) + + return stem + String(procMacro.baseCompiler.Properties.Suffix) +} diff --git a/rust/rust.go b/rust/rust.go index ec3b59086..612e25727 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -225,11 +225,7 @@ func DefaultsFactory(props ...interface{}) android.Module { } func (mod *Module) CrateName() string { - if mod.compiler != nil && mod.compiler.crateName() != "" { - return mod.compiler.crateName() - } - // Default crate names replace '-' in the name to '_' - return strings.Replace(mod.BaseModuleName(), "-", "_", -1) + return mod.compiler.crateName() } func (mod *Module) CcLibrary() bool { diff --git a/rust/rust_test.go b/rust/rust_test.go index eb04e7257..599af098c 100644 --- a/rust/rust_test.go +++ b/rust/rust_test.go @@ -129,44 +129,33 @@ func TestLinkPathFromFilePath(t *testing.T) { } } -// Test default crate names from module names are generated correctly. -func TestDefaultCrateName(t *testing.T) { - ctx := testRust(t, ` - rust_library_host_dylib { - name: "fizz-buzz", - srcs: ["foo.rs"], - }`) - module := ctx.ModuleForTests("fizz-buzz", "linux_glibc_x86_64_dylib").Module().(*Module) - crateName := module.CrateName() - expectedResult := "fizz_buzz" - - if crateName != expectedResult { - t.Errorf("CrateName() returned the wrong default crate name; expected '%#v', got '%#v'", expectedResult, crateName) - } -} - // Test to make sure dependencies are being picked up correctly. func TestDepsTracking(t *testing.T) { ctx := testRust(t, ` rust_library_host_static { name: "libstatic", srcs: ["foo.rs"], + crate_name: "static", } rust_library_host_shared { name: "libshared", srcs: ["foo.rs"], + crate_name: "shared", } rust_library_host_dylib { name: "libdylib", srcs: ["foo.rs"], + crate_name: "dylib", } rust_library_host_rlib { name: "librlib", srcs: ["foo.rs"], + crate_name: "rlib", } rust_proc_macro { name: "libpm", srcs: ["foo.rs"], + crate_name: "pm", } rust_binary_host { name: "fizz-buzz", @@ -208,11 +197,13 @@ func TestProcMacroDeviceDeps(t *testing.T) { rust_library_host_rlib { name: "libbar", srcs: ["foo.rs"], + crate_name: "bar", } rust_proc_macro { name: "libpm", rlibs: ["libbar"], srcs: ["foo.rs"], + crate_name: "pm", } rust_binary { name: "fizz-buzz",