Merge changes Ie5f793a0,I9b9674ba

* changes:
  bp2build: support Starlark rules and load statements.
  Make bp2buildMutators registration local to TestContext.

android/filegroup.go

@@ -23,7 +23,7 @@ import (
 
 func init() {
 	RegisterModuleType("filegroup", FileGroupFactory)
-	RegisterBp2BuildMutator("filegroup", bp2buildMutator)
+	RegisterBp2BuildMutator("filegroup", FilegroupBp2Build)
 }
 
 // https://docs.bazel.build/versions/master/be/general.html#filegroup
@@ -51,7 +51,7 @@ func (bfg *bazelFilegroup) Name() string {
 func (bfg *bazelFilegroup) GenerateAndroidBuildActions(ctx ModuleContext) {}
 
 // TODO: Create helper functions to avoid this boilerplate.
-func bp2buildMutator(ctx TopDownMutatorContext) {
+func FilegroupBp2Build(ctx TopDownMutatorContext) {
 	if m, ok := ctx.Module().(*fileGroup); ok {
 		name := "__bp2build__" + m.base().BaseModuleName()
 		ctx.CreateModule(BazelFileGroupFactory, &bazelFilegroupAttributes{

android/testing.go

@@ -89,7 +89,7 @@ func (ctx *TestContext) RegisterBp2BuildMutator(moduleType string, m func(TopDow
 	f := func(ctx RegisterMutatorsContext) {
 		ctx.TopDown(mutatorName, m)
 	}
-	bp2buildMutators = append(bp2buildMutators, f)
+	ctx.bp2buildMutators = append(ctx.bp2buildMutators, f)
 }
 
 func (ctx *TestContext) Register() {
@@ -100,7 +100,7 @@ func (ctx *TestContext) Register() {
 
 // RegisterForBazelConversion prepares a test context for bp2build conversion.
 func (ctx *TestContext) RegisterForBazelConversion() {
-	RegisterMutatorsForBazelConversion(ctx.Context.Context, bp2buildMutators)
+	RegisterMutatorsForBazelConversion(ctx.Context.Context, ctx.bp2buildMutators)
 }
 
 func (ctx *TestContext) ParseFileList(rootDir string, filePaths []string) (deps []string, errs []error) {

bazel/properties.go

@@ -31,4 +31,7 @@ type Properties struct {
 type BazelTargetModuleProperties struct {
 	// The Bazel rule class for this target.
 	Rule_class string
+
+	// The target label for the bzl file containing the definition of the rule class.
+	Bzl_load_location string
 }

bp2build/build_conversion.go

@@ -18,6 +18,7 @@ import (
 	"android/soong/android"
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/google/blueprint"
@@ -29,8 +30,62 @@ type BazelAttributes struct {
 }
 
 type BazelTarget struct {
-	name    string
+	name            string
-	content string
+	content         string
+	ruleClass       string
+	bzlLoadLocation string
+}
+
+// IsLoadedFromStarlark determines if the BazelTarget's rule class is loaded from a .bzl file,
+// as opposed to a native rule built into Bazel.
+func (t BazelTarget) IsLoadedFromStarlark() bool {
+	return t.bzlLoadLocation != ""
+}
+
+// BazelTargets is a typedef for a slice of BazelTarget objects.
+type BazelTargets []BazelTarget
+
+// String returns the string representation of BazelTargets, without load
+// statements (use LoadStatements for that), since the targets are usually not
+// adjacent to the load statements at the top of the BUILD file.
+func (targets BazelTargets) String() string {
+	var res string
+	for i, target := range targets {
+		res += target.content
+		if i != len(targets)-1 {
+			res += "\n\n"
+		}
+	}
+	return res
+}
+
+// LoadStatements return the string representation of the sorted and deduplicated
+// Starlark rule load statements needed by a group of BazelTargets.
+func (targets BazelTargets) LoadStatements() string {
+	bzlToLoadedSymbols := map[string][]string{}
+	for _, target := range targets {
+		if target.IsLoadedFromStarlark() {
+			bzlToLoadedSymbols[target.bzlLoadLocation] =
+				append(bzlToLoadedSymbols[target.bzlLoadLocation], target.ruleClass)
+		}
+	}
+
+	var loadStatements []string
+	for bzl, ruleClasses := range bzlToLoadedSymbols {
+		loadStatement := "load(\""
+		loadStatement += bzl
+		loadStatement += "\", "
+		ruleClasses = android.SortedUniqueStrings(ruleClasses)
+		for i, ruleClass := range ruleClasses {
+			loadStatement += "\"" + ruleClass + "\""
+			if i != len(ruleClasses)-1 {
+				loadStatement += ", "
+			}
+		}
+		loadStatement += ")"
+		loadStatements = append(loadStatements, loadStatement)
+	}
+	return strings.Join(android.SortedUniqueStrings(loadStatements), "\n")
 }
 
 type bpToBuildContext interface {
@@ -104,8 +159,8 @@ func propsToAttributes(props map[string]string) string {
 	return attributes
 }
 
-func GenerateSoongModuleTargets(ctx bpToBuildContext, codegenMode CodegenMode) map[string][]BazelTarget {
+func GenerateSoongModuleTargets(ctx bpToBuildContext, codegenMode CodegenMode) map[string]BazelTargets {
-	buildFileToTargets := make(map[string][]BazelTarget)
+	buildFileToTargets := make(map[string]BazelTargets)
 	ctx.VisitAllModules(func(m blueprint.Module) {
 		dir := ctx.ModuleDir(m)
 		var t BazelTarget
@@ -127,22 +182,44 @@ func GenerateSoongModuleTargets(ctx bpToBuildContext, codegenMode CodegenMode) m
 	return buildFileToTargets
 }
 
+// Helper method to trim quotes around strings.
+func trimQuotes(s string) string {
+	if s == "" {
+		// strconv.Unquote would error out on empty strings, but this method
+		// allows them, so return the empty string directly.
+		return ""
+	}
+	ret, err := strconv.Unquote(s)
+	if err != nil {
+		// Panic the error immediately.
+		panic(fmt.Errorf("Trying to unquote '%s', but got error: %s", s, err))
+	}
+	return ret
+}
+
 func generateBazelTarget(ctx bpToBuildContext, m blueprint.Module) BazelTarget {
 	// extract the bazel attributes from the module.
 	props := getBuildProperties(ctx, m)
 
 	// extract the rule class name from the attributes. Since the string value
 	// will be string-quoted, remove the quotes here.
-	ruleClass := strings.Replace(props.Attrs["rule_class"], "\"", "", 2)
+	ruleClass := trimQuotes(props.Attrs["rule_class"])
 	// Delete it from being generated in the BUILD file.
 	delete(props.Attrs, "rule_class")
 
+	// extract the bzl_load_location, and also remove the quotes around it here.
+	bzlLoadLocation := trimQuotes(props.Attrs["bzl_load_location"])
+	// Delete it from being generated in the BUILD file.
+	delete(props.Attrs, "bzl_load_location")
+
 	// Return the Bazel target with rule class and attributes, ready to be
 	// code-generated.
 	attributes := propsToAttributes(props.Attrs)
 	targetName := targetNameForBp2Build(ctx, m)
 	return BazelTarget{
-		name: targetName,
+		name:            targetName,
+		ruleClass:       ruleClass,
+		bzlLoadLocation: bzlLoadLocation,
 		content: fmt.Sprintf(
 			bazelTarget,
 			ruleClass,

bp2build/build_conversion_test.go

@@ -268,18 +268,185 @@ func TestGenerateBazelTargetModules(t *testing.T) {
 	}
 }
 
-func TestModuleTypeBp2Build(t *testing.T) {
+func TestLoadStatements(t *testing.T) {
 	testCases := []struct {
-		moduleTypeUnderTest        string
+		bazelTargets           BazelTargets
-		moduleTypeUnderTestFactory android.ModuleFactory
+		expectedLoadStatements string
-		bp                         string
-		expectedBazelTarget        string
-		description                string
 	}{
 		{
-			description:                "filegroup with no srcs",
+			bazelTargets: BazelTargets{
-			moduleTypeUnderTest:        "filegroup",
+				BazelTarget{
-			moduleTypeUnderTestFactory: android.FileGroupFactory,
+					name:            "foo",
+					ruleClass:       "cc_library",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+			},
+			expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_library")`,
+		},
+		{
+			bazelTargets: BazelTargets{
+				BazelTarget{
+					name:            "foo",
+					ruleClass:       "cc_library",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+				BazelTarget{
+					name:            "bar",
+					ruleClass:       "cc_library",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+			},
+			expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_library")`,
+		},
+		{
+			bazelTargets: BazelTargets{
+				BazelTarget{
+					name:            "foo",
+					ruleClass:       "cc_library",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+				BazelTarget{
+					name:            "bar",
+					ruleClass:       "cc_binary",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+			},
+			expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_binary", "cc_library")`,
+		},
+		{
+			bazelTargets: BazelTargets{
+				BazelTarget{
+					name:            "foo",
+					ruleClass:       "cc_library",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+				BazelTarget{
+					name:            "bar",
+					ruleClass:       "cc_binary",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+				BazelTarget{
+					name:            "baz",
+					ruleClass:       "java_binary",
+					bzlLoadLocation: "//build/bazel/rules:java.bzl",
+				},
+			},
+			expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_binary", "cc_library")
+load("//build/bazel/rules:java.bzl", "java_binary")`,
+		},
+		{
+			bazelTargets: BazelTargets{
+				BazelTarget{
+					name:            "foo",
+					ruleClass:       "cc_binary",
+					bzlLoadLocation: "//build/bazel/rules:cc.bzl",
+				},
+				BazelTarget{
+					name:            "bar",
+					ruleClass:       "java_binary",
+					bzlLoadLocation: "//build/bazel/rules:java.bzl",
+				},
+				BazelTarget{
+					name:      "baz",
+					ruleClass: "genrule",
+					// Note: no bzlLoadLocation for native rules
+				},
+			},
+			expectedLoadStatements: `load("//build/bazel/rules:cc.bzl", "cc_binary")
+load("//build/bazel/rules:java.bzl", "java_binary")`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		actual := testCase.bazelTargets.LoadStatements()
+		expected := testCase.expectedLoadStatements
+		if actual != expected {
+			t.Fatalf("Expected load statements to be %s, got %s", expected, actual)
+		}
+	}
+
+}
+
+func TestGenerateBazelTargetModules_OneToMany_LoadedFromStarlark(t *testing.T) {
+	testCases := []struct {
+		bp                       string
+		expectedBazelTarget      string
+		expectedBazelTargetCount int
+		expectedLoadStatements   string
+	}{
+		{
+			bp: `custom {
+    name: "bar",
+}`,
+			expectedBazelTarget: `my_library(
+    name = "bar",
+)
+
+my_proto_library(
+    name = "bar_my_proto_library_deps",
+)
+
+proto_library(
+    name = "bar_proto_library_deps",
+)`,
+			expectedBazelTargetCount: 3,
+			expectedLoadStatements: `load("//build/bazel/rules:proto.bzl", "my_proto_library", "proto_library")
+load("//build/bazel/rules:rules.bzl", "my_library")`,
+		},
+	}
+
+	dir := "."
+	for _, testCase := range testCases {
+		config := android.TestConfig(buildDir, nil, testCase.bp, nil)
+		ctx := android.NewTestContext(config)
+		ctx.RegisterModuleType("custom", customModuleFactory)
+		ctx.RegisterBp2BuildMutator("custom_starlark", customBp2BuildMutatorFromStarlark)
+		ctx.RegisterForBazelConversion()
+
+		_, errs := ctx.ParseFileList(dir, []string{"Android.bp"})
+		android.FailIfErrored(t, errs)
+		_, errs = ctx.ResolveDependencies(config)
+		android.FailIfErrored(t, errs)
+
+		bazelTargets := GenerateSoongModuleTargets(ctx.Context.Context, Bp2Build)[dir]
+		if actualCount := len(bazelTargets); actualCount != testCase.expectedBazelTargetCount {
+			t.Fatalf("Expected %d bazel target, got %d", testCase.expectedBazelTargetCount, actualCount)
+		}
+
+		actualBazelTargets := bazelTargets.String()
+		if actualBazelTargets != testCase.expectedBazelTarget {
+			t.Errorf(
+				"Expected generated Bazel target to be '%s', got '%s'",
+				testCase.expectedBazelTarget,
+				actualBazelTargets,
+			)
+		}
+
+		actualLoadStatements := bazelTargets.LoadStatements()
+		if actualLoadStatements != testCase.expectedLoadStatements {
+			t.Errorf(
+				"Expected generated load statements to be '%s', got '%s'",
+				testCase.expectedLoadStatements,
+				actualLoadStatements,
+			)
+		}
+	}
+}
+
+func TestModuleTypeBp2Build(t *testing.T) {
+	testCases := []struct {
+		moduleTypeUnderTest                string
+		moduleTypeUnderTestFactory         android.ModuleFactory
+		moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext)
+		bp                                 string
+		expectedBazelTarget                string
+		description                        string
+	}{
+		{
+			description:                        "filegroup with no srcs",
+			moduleTypeUnderTest:                "filegroup",
+			moduleTypeUnderTestFactory:         android.FileGroupFactory,
+			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
 			bp: `filegroup {
 	name: "foo",
 	srcs: [],
@@ -291,9 +458,10 @@ func TestModuleTypeBp2Build(t *testing.T) {
 )`,
 		},
 		{
-			description:                "filegroup with srcs",
+			description:                        "filegroup with srcs",
-			moduleTypeUnderTest:        "filegroup",
+			moduleTypeUnderTest:                "filegroup",
-			moduleTypeUnderTestFactory: android.FileGroupFactory,
+			moduleTypeUnderTestFactory:         android.FileGroupFactory,
+			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
 			bp: `filegroup {
 	name: "foo",
 	srcs: ["a", "b"],
@@ -307,9 +475,10 @@ func TestModuleTypeBp2Build(t *testing.T) {
 )`,
 		},
 		{
-			description:                "genrule with command line variable replacements",
+			description:                        "genrule with command line variable replacements",
-			moduleTypeUnderTest:        "genrule",
+			moduleTypeUnderTest:                "genrule",
-			moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
+			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
 			bp: `genrule {
     name: "foo",
     out: ["foo.out"],
@@ -332,9 +501,10 @@ func TestModuleTypeBp2Build(t *testing.T) {
 )`,
 		},
 		{
-			description:                "genrule using $(locations :label)",
+			description:                        "genrule using $(locations :label)",
-			moduleTypeUnderTest:        "genrule",
+			moduleTypeUnderTest:                "genrule",
-			moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
+			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
 			bp: `genrule {
     name: "foo",
     out: ["foo.out"],
@@ -357,9 +527,10 @@ func TestModuleTypeBp2Build(t *testing.T) {
 )`,
 		},
 		{
-			description:                "genrule using $(location) label should substitute first tool label automatically",
+			description:                        "genrule using $(location) label should substitute first tool label automatically",
-			moduleTypeUnderTest:        "genrule",
+			moduleTypeUnderTest:                "genrule",
-			moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
+			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
 			bp: `genrule {
     name: "foo",
     out: ["foo.out"],
@@ -383,9 +554,10 @@ func TestModuleTypeBp2Build(t *testing.T) {
 )`,
 		},
 		{
-			description:                "genrule using $(locations) label should substitute first tool label automatically",
+			description:                        "genrule using $(locations) label should substitute first tool label automatically",
-			moduleTypeUnderTest:        "genrule",
+			moduleTypeUnderTest:                "genrule",
-			moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
+			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
 			bp: `genrule {
     name: "foo",
     out: ["foo.out"],
@@ -409,9 +581,10 @@ func TestModuleTypeBp2Build(t *testing.T) {
 )`,
 		},
 		{
-			description:                "genrule without tools or tool_files can convert successfully",
+			description:                        "genrule without tools or tool_files can convert successfully",
-			moduleTypeUnderTest:        "genrule",
+			moduleTypeUnderTest:                "genrule",
-			moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
+			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
 			bp: `genrule {
     name: "foo",
     out: ["foo.out"],
@@ -436,6 +609,7 @@ func TestModuleTypeBp2Build(t *testing.T) {
 		config := android.TestConfig(buildDir, nil, testCase.bp, nil)
 		ctx := android.NewTestContext(config)
 		ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
+		ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
 		ctx.RegisterForBazelConversion()
 
 		_, errs := ctx.ParseFileList(dir, []string{"Android.bp"})

bp2build/bzl_conversion_test.go

@@ -172,7 +172,7 @@ func TestGenerateSoongModuleBzl(t *testing.T) {
 			content: "irrelevant",
 		},
 	}
-	files := CreateBazelFiles(ruleShims, make(map[string][]BazelTarget), QueryView)
+	files := CreateBazelFiles(ruleShims, make(map[string]BazelTargets), QueryView)
 
 	var actualSoongModuleBzl BazelFile
 	for _, f := range files {

bp2build/conversion.go

@@ -17,7 +17,7 @@ type BazelFile struct {
 
 func CreateBazelFiles(
 	ruleShims map[string]RuleShim,
-	buildToTargets map[string][]BazelTarget,
+	buildToTargets map[string]BazelTargets,
 	mode CodegenMode) []BazelFile {
 	files := make([]BazelFile, 0, len(ruleShims)+len(buildToTargets)+numAdditionalFiles)
 
@@ -43,20 +43,20 @@ func CreateBazelFiles(
 	return files
 }
 
-func createBuildFiles(buildToTargets map[string][]BazelTarget, mode CodegenMode) []BazelFile {
+func createBuildFiles(buildToTargets map[string]BazelTargets, mode CodegenMode) []BazelFile {
 	files := make([]BazelFile, 0, len(buildToTargets))
 	for _, dir := range android.SortedStringKeys(buildToTargets) {
-		content := soongModuleLoad
-		if mode == Bp2Build {
-			// No need to load soong_module for bp2build BUILD files.
-			content = ""
-		}
 		targets := buildToTargets[dir]
 		sort.Slice(targets, func(i, j int) bool { return targets[i].name < targets[j].name })
-		for _, t := range targets {
+		content := soongModuleLoad
-			content += "\n\n"
+		if mode == Bp2Build {
-			content += t.content
+			content = targets.LoadStatements()
 		}
+		if content != "" {
+			// If there are load statements, add a couple of newlines.
+			content += "\n\n"
+		}
+		content += targets.String()
 		files = append(files, newFile(dir, "BUILD.bazel", content))
 	}
 	return files

bp2build/conversion_test.go

@@ -55,7 +55,7 @@ func sortFiles(files []BazelFile) {
 }
 
 func TestCreateBazelFiles_QueryView_AddsTopLevelFiles(t *testing.T) {
-	files := CreateBazelFiles(map[string]RuleShim{}, map[string][]BazelTarget{}, QueryView)
+	files := CreateBazelFiles(map[string]RuleShim{}, map[string]BazelTargets{}, QueryView)
 	expectedFilePaths := []filepath{
 		{
 			dir:      "",
@@ -85,7 +85,7 @@ func TestCreateBazelFiles_QueryView_AddsTopLevelFiles(t *testing.T) {
 }
 
 func TestCreateBazelFiles_Bp2Build_AddsTopLevelFiles(t *testing.T) {
-	files := CreateBazelFiles(map[string]RuleShim{}, map[string][]BazelTarget{}, Bp2Build)
+	files := CreateBazelFiles(map[string]RuleShim{}, map[string]BazelTargets{}, Bp2Build)
 	expectedFilePaths := []filepath{
 		{
 			dir:      "",

bp2build/testing.go

@@ -137,3 +137,29 @@ func customBp2BuildMutator(ctx android.TopDownMutatorContext) {
 		})
 	}
 }
+
+// A bp2build mutator that uses load statements and creates a 1:M mapping from
+// module to target.
+func customBp2BuildMutatorFromStarlark(ctx android.TopDownMutatorContext) {
+	if m, ok := ctx.Module().(*customModule); ok {
+		baseName := "__bp2build__" + m.Name()
+		ctx.CreateModule(customBazelModuleFactory, &customBazelModuleAttributes{
+			Name: proptools.StringPtr(baseName),
+		}, &bazel.BazelTargetModuleProperties{
+			Rule_class:        "my_library",
+			Bzl_load_location: "//build/bazel/rules:rules.bzl",
+		})
+		ctx.CreateModule(customBazelModuleFactory, &customBazelModuleAttributes{
+			Name: proptools.StringPtr(baseName + "_proto_library_deps"),
+		}, &bazel.BazelTargetModuleProperties{
+			Rule_class:        "proto_library",
+			Bzl_load_location: "//build/bazel/rules:proto.bzl",
+		})
+		ctx.CreateModule(customBazelModuleFactory, &customBazelModuleAttributes{
+			Name: proptools.StringPtr(baseName + "_my_proto_library_deps"),
+		}, &bazel.BazelTargetModuleProperties{
+			Rule_class:        "my_proto_library",
+			Bzl_load_location: "//build/bazel/rules:proto.bzl",
+		})
+	}
+}

genrule/genrule.go

@@ -47,7 +47,7 @@ func RegisterGenruleBuildComponents(ctx android.RegistrationContext) {
 		ctx.BottomUp("genrule_tool_deps", toolDepsMutator).Parallel()
 	})
 
-	android.RegisterBp2BuildMutator("genrule", bp2buildMutator)
+	android.RegisterBp2BuildMutator("genrule", GenruleBp2Build)
 }
 
 var (
@@ -794,7 +794,7 @@ func BazelGenruleFactory() android.Module {
 	return module
 }
 
-func bp2buildMutator(ctx android.TopDownMutatorContext) {
+func GenruleBp2Build(ctx android.TopDownMutatorContext) {
 	if m, ok := ctx.Module().(*Module); ok {
 		name := "__bp2build__" + m.Name()
 		// Bazel only has the "tools" attribute.