Avoid Rust source provider rule duplication

Until now, source provider modules duplicated the rule to generate the
source for each variant. Add a inter-variant dependency between the
source and the other variants (e.g. rlib, dylib) to avoid this
duplication. Add documentation on this behaviour.

Bug: 162588681
Test: m
Change-Id: I41c9e2220f8875245415e17374852e540dfd47ec

rust/bindgen.go

@@ -169,7 +169,9 @@ func (b *bindgenDecorator) SourceProviderProps() []interface{} {
 
 // rust_bindgen generates Rust FFI bindings to C libraries using bindgen given a wrapper header as the primary input.
 // Bindgen has a number of flags to control the generated source, and additional flags can be passed to clang to ensure
-// the header and generated source is appropriately handled.
+// the header and generated source is appropriately handled. It is recommended to add it as a dependency in the
+// rlibs, dylibs or rustlibs property. It may also be added in the srcs property for external crates, using the ":"
+// prefix.
 func RustBindgenFactory() android.Module {
 	module, _ := NewRustBindgen(android.HostAndDeviceSupported)
 	return module.Init()

rust/bindgen_test.go

@@ -41,7 +41,7 @@ func TestRustBindgen(t *testing.T) {
 			export_include_dirs: ["static_include"],
 		}
 	`)
-	libbindgen := ctx.ModuleForTests("libbindgen", "android_arm64_armv8-a").Output("bindings.rs")
+	libbindgen := ctx.ModuleForTests("libbindgen", "android_arm64_armv8-a_source").Output("bindings.rs")
 	// Ensure that the flags are present and escaped
 	if !strings.Contains(libbindgen.Args["flags"], "'--bindgen-flag.*'") {
 		t.Errorf("missing bindgen flags in rust_bindgen rule: flags %#v", libbindgen.Args["flags"])
@@ -73,7 +73,7 @@ func TestRustBindgenCustomBindgen(t *testing.T) {
 		}
 	`)
 
-	libbindgen := ctx.ModuleForTests("libbindgen", "android_arm64_armv8-a").Output("bindings.rs")
+	libbindgen := ctx.ModuleForTests("libbindgen", "android_arm64_armv8-a_source").Output("bindings.rs")
 
 	// The rule description should contain the custom binary name rather than bindgen, so checking the description
 	// should be sufficient.

rust/library.go

@@ -15,6 +15,7 @@
 package rust
 
 import (
+	"fmt"
 	"regexp"
 	"strings"
 
@@ -77,6 +78,8 @@ type LibraryMutatedProperties struct {
 	VariantIsShared bool `blueprint:"mutated"`
 	// This variant is a static library
 	VariantIsStatic bool `blueprint:"mutated"`
+	// This variant is a source provider
+	VariantIsSource bool `blueprint:"mutated"`
 
 	// This variant is disabled and should not be compiled
 	// (used for SourceProvider variants that produce only source)
@@ -103,6 +106,7 @@ type libraryInterface interface {
 	static() bool
 	shared() bool
 	sysroot() bool
+	source() bool
 
 	// Returns true if the build options for the module have selected a particular build type
 	buildRlib() bool
@@ -115,6 +119,7 @@ type libraryInterface interface {
 	setDylib()
 	setShared()
 	setStatic()
+	setSource()
 
 	// Set libstd linkage
 	setRlibStd()
@@ -158,6 +163,10 @@ func (library *libraryDecorator) staticStd(ctx *depsContext) bool {
 	return library.static() || library.MutatedProperties.VariantIsStaticStd
 }
 
+func (library *libraryDecorator) source() bool {
+	return library.MutatedProperties.VariantIsSource
+}
+
 func (library *libraryDecorator) buildRlib() bool {
 	return library.MutatedProperties.BuildRlib && BoolDefault(library.Properties.Rlib.Enabled, true)
 }
@@ -210,13 +219,17 @@ func (library *libraryDecorator) setStatic() {
 	library.MutatedProperties.VariantIsDylib = false
 }
 
+func (library *libraryDecorator) setSource() {
+	library.MutatedProperties.VariantIsSource = true
+}
+
 func (library *libraryDecorator) autoDep(ctx BaseModuleContext) autoDep {
 	if library.rlib() || library.static() {
 		return rlibAutoDep
 	} else if library.dylib() || library.shared() {
 		return dylibAutoDep
 	} else {
-		panic("autoDep called on library" + ctx.ModuleName() + "that has no enabled variants.")
+		panic(fmt.Errorf("autoDep called on library %q that has no enabled variants.", ctx.ModuleName()))
 	}
 }
 
@@ -518,40 +531,67 @@ func validateLibraryStem(ctx BaseModuleContext, filename string, crate_name stri
 	}
 }
 
+// LibraryMutator mutates the libraries into variants according to the
+// build{Rlib,Dylib} attributes.
 func LibraryMutator(mctx android.BottomUpMutatorContext) {
-	if m, ok := mctx.Module().(*Module); ok && m.compiler != nil {
-		switch library := m.compiler.(type) {
-		case libraryInterface:
-			if library.buildRlib() && library.buildDylib() {
-				variants := []string{"rlib", "dylib"}
+	// Only mutate on Rust libraries.
+	m, ok := mctx.Module().(*Module)
+	if !ok || m.compiler == nil {
+		return
+	}
+	library, ok := m.compiler.(libraryInterface)
+	if !ok {
+		return
+	}
+
+	var variants []string
+	// The source variant is used for SourceProvider modules. The other variants (i.e. rlib and dylib)
+	// depend on this variant. It must be the first variant to be declared.
+	sourceVariant := false
 	if m.sourceProvider != nil {
-					variants = append(variants, "")
+		variants = append(variants, "source")
+		sourceVariant = true
+	}
+	if library.buildRlib() {
+		variants = append(variants, rlibVariation)
+	}
+	if library.buildDylib() {
+		variants = append(variants, dylibVariation)
+	}
+
+	if len(variants) == 0 {
+		return
 	}
 	modules := mctx.CreateLocalVariations(variants...)
 
-				rlib := modules[0].(*Module)
-				dylib := modules[1].(*Module)
-				rlib.compiler.(libraryInterface).setRlib()
-				dylib.compiler.(libraryInterface).setDylib()
-				if m.sourceProvider != nil {
-					// This library is SourceProvider generated, so the non-library-producing
-					// variant needs to disable it's compiler and skip installation.
-					sourceProvider := modules[2].(*Module)
-					sourceProvider.compiler.SetDisabled()
+	// The order of the variations (modules) matches the variant names provided. Iterate
+	// through the new variation modules and set their mutated properties.
+	for i, v := range modules {
+		switch variants[i] {
+		case rlibVariation:
+			v.(*Module).compiler.(libraryInterface).setRlib()
+		case dylibVariation:
+			v.(*Module).compiler.(libraryInterface).setDylib()
+		case "source":
+			v.(*Module).compiler.(libraryInterface).setSource()
+			// The source variant does not produce any library.
+			// Disable the compilation steps.
+			v.(*Module).compiler.SetDisabled()
 		}
-			} else if library.buildRlib() {
-				modules := mctx.CreateLocalVariations("rlib")
-				modules[0].(*Module).compiler.(libraryInterface).setRlib()
-			} else if library.buildDylib() {
-				modules := mctx.CreateLocalVariations("dylib")
-				modules[0].(*Module).compiler.(libraryInterface).setDylib()
 	}
 
-			if m.sourceProvider != nil {
-				// Alias the non-library variant to the empty-string variant.
-				mctx.AliasVariation("")
+	// If a source variant is created, add an inter-variant dependency
+	// between the other variants and the source variant.
+	if sourceVariant {
+		sv := modules[0]
+		for _, v := range modules[1:] {
+			if !v.Enabled() {
+				continue
 			}
+			mctx.AddInterVariantDependency(sourceDepTag, v, sv)
 		}
+		// Alias the source variation so it can be named directly in "srcs" properties.
+		mctx.AliasVariation("source")
 	}
 }
 

rust/protobuf_test.go

@@ -29,7 +29,7 @@ func TestRustProtobuf(t *testing.T) {
 		}
 	`)
 	// Check that there's a rule to generate the expected output
-	_ = ctx.ModuleForTests("librust_proto", "android_arm64_armv8-a_dylib").Output("buf.rs")
+	_ = ctx.ModuleForTests("librust_proto", "android_arm64_armv8-a_source").Output("buf.rs")
 
 	// Check that libprotobuf is added as a dependency.
 	librust_proto := ctx.ModuleForTests("librust_proto", "android_arm64_armv8-a_dylib").Module().(*Module)

rust/rust.go

@@ -88,7 +88,6 @@ type Module struct {
 	subAndroidMkOnce map[SubAndroidMkProvider]bool
 
 	outputFile android.OptionalPath
-	generatedFile android.OptionalPath
 }
 
 func (mod *Module) OutputFiles(tag string) (android.Paths, error) {
@@ -687,12 +686,20 @@ func (mod *Module) GenerateAndroidBuildActions(actx android.ModuleContext) {
 		flags, deps = mod.clippy.flags(ctx, flags, deps)
 	}
 
-	// SourceProvider needs to call GenerateSource() before compiler calls compile() so it can provide the source.
-	// TODO(b/162588681) This shouldn't have to run for every variant.
+	// SourceProvider needs to call GenerateSource() before compiler calls
+	// compile() so it can provide the source. A SourceProvider has
+	// multiple variants (e.g. source, rlib, dylib). Only the "source"
+	// variant is responsible for effectively generating the source. The
+	// remaining variants relies on the "source" variant output.
 	if mod.sourceProvider != nil {
-		generatedFile := mod.sourceProvider.GenerateSource(ctx, deps)
-		mod.generatedFile = android.OptionalPathForPath(generatedFile)
+		if mod.compiler.(libraryInterface).source() {
+			mod.sourceProvider.GenerateSource(ctx, deps)
 			mod.sourceProvider.setSubName(ctx.ModuleSubDir())
+		} else {
+			sourceMod := actx.GetDirectDepWithTag(mod.Name(), sourceDepTag)
+			sourceLib := sourceMod.(*Module).compiler.(*libraryDecorator)
+			mod.sourceProvider.setOutputFile(sourceLib.sourceProvider.Srcs()[0])
+		}
 	}
 
 	if mod.compiler != nil && !mod.compiler.Disabled() {
@@ -743,6 +750,7 @@ var (
 	dylibDepTag         = dependencyTag{name: "dylib", library: true}
 	procMacroDepTag     = dependencyTag{name: "procMacro", proc_macro: true}
 	testPerSrcDepTag    = dependencyTag{name: "rust_unit_tests"}
+	sourceDepTag        = dependencyTag{name: "source"}
 )
 
 type autoDep struct {
@@ -751,8 +759,10 @@ type autoDep struct {
 }
 
 var (
-	rlibAutoDep  = autoDep{variation: "rlib", depTag: rlibDepTag}
-	dylibAutoDep = autoDep{variation: "dylib", depTag: dylibDepTag}
+	rlibVariation  = "rlib"
+	dylibVariation = "dylib"
+	rlibAutoDep    = autoDep{variation: rlibVariation, depTag: rlibDepTag}
+	dylibAutoDep   = autoDep{variation: dylibVariation, depTag: dylibDepTag}
 )
 
 type autoDeppable interface {
@@ -1000,11 +1010,11 @@ func (mod *Module) DepsMutator(actx android.BottomUpMutatorContext) {
 
 	actx.AddVariationDependencies(
 		append(rlibDepVariations, []blueprint.Variation{
-			{Mutator: "rust_libraries", Variation: "rlib"}}...),
+			{Mutator: "rust_libraries", Variation: rlibVariation}}...),
 		rlibDepTag, deps.Rlibs...)
 	actx.AddVariationDependencies(
 		append(commonDepVariations, []blueprint.Variation{
-			{Mutator: "rust_libraries", Variation: "dylib"}}...),
+			{Mutator: "rust_libraries", Variation: dylibVariation}}...),
 		dylibDepTag, deps.Dylibs...)
 
 	if deps.Rustlibs != nil && !mod.compiler.Disabled() {

rust/source_provider.go

@@ -43,6 +43,7 @@ type SourceProvider interface {
 	SourceProviderProps() []interface{}
 	SourceProviderDeps(ctx DepsContext, deps Deps) Deps
 	setSubName(subName string)
+	setOutputFile(outputFile android.Path)
 }
 
 func (sp *BaseSourceProvider) Srcs() android.Paths {
@@ -95,3 +96,7 @@ func (sp *BaseSourceProvider) SourceProviderDeps(ctx DepsContext, deps Deps) Dep
 func (sp *BaseSourceProvider) setSubName(subName string) {
 	sp.subName = subName
 }
+
+func (sp *BaseSourceProvider) setOutputFile(outputFile android.Path) {
+	sp.OutputFile = outputFile
+}