Merge changes Ie5f793a0,I9b9674ba
* changes:
bp2build: support Starlark rules and load statements.
Make bp2buildMutators registration local to TestContext.
diff --git a/android/filegroup.go b/android/filegroup.go
index fd4a2fe..3d1bbc5 100644
--- a/android/filegroup.go
+++ b/android/filegroup.go
@@ -23,7 +23,7 @@
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) 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{
diff --git a/android/testing.go b/android/testing.go
index 76172d1..5640c29 100644
--- a/android/testing.go
+++ b/android/testing.go
@@ -89,7 +89,7 @@
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 @@
// 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) {
diff --git a/bazel/properties.go b/bazel/properties.go
index ac0047b..79956e1 100644
--- a/bazel/properties.go
+++ b/bazel/properties.go
@@ -31,4 +31,7 @@
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
}
diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go
index a7c3adb..1af1d60 100644
--- a/bp2build/build_conversion.go
+++ b/bp2build/build_conversion.go
@@ -18,6 +18,7 @@
"android/soong/android"
"fmt"
"reflect"
+ "strconv"
"strings"
"github.com/google/blueprint"
@@ -29,8 +30,62 @@
}
type BazelTarget struct {
- name string
- content string
+ name 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 @@
return attributes
}
-func GenerateSoongModuleTargets(ctx bpToBuildContext, codegenMode CodegenMode) map[string][]BazelTarget {
- buildFileToTargets := make(map[string][]BazelTarget)
+func GenerateSoongModuleTargets(ctx bpToBuildContext, codegenMode CodegenMode) map[string]BazelTargets {
+ buildFileToTargets := make(map[string]BazelTargets)
ctx.VisitAllModules(func(m blueprint.Module) {
dir := ctx.ModuleDir(m)
var t BazelTarget
@@ -127,22 +182,44 @@
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,
diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go
index 367e13f..66ed42d 100644
--- a/bp2build/build_conversion_test.go
+++ b/bp2build/build_conversion_test.go
@@ -268,18 +268,185 @@
}
}
-func TestModuleTypeBp2Build(t *testing.T) {
+func TestLoadStatements(t *testing.T) {
testCases := []struct {
- moduleTypeUnderTest string
- moduleTypeUnderTestFactory android.ModuleFactory
- bp string
- expectedBazelTarget string
- description string
+ bazelTargets BazelTargets
+ expectedLoadStatements string
}{
{
- description: "filegroup with no srcs",
- moduleTypeUnderTest: "filegroup",
- moduleTypeUnderTestFactory: android.FileGroupFactory,
+ bazelTargets: BazelTargets{
+ BazelTarget{
+ 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 @@
)`,
},
{
- description: "filegroup with srcs",
- moduleTypeUnderTest: "filegroup",
- moduleTypeUnderTestFactory: android.FileGroupFactory,
+ description: "filegroup with srcs",
+ moduleTypeUnderTest: "filegroup",
+ moduleTypeUnderTestFactory: android.FileGroupFactory,
+ moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
bp: `filegroup {
name: "foo",
srcs: ["a", "b"],
@@ -307,9 +475,10 @@
)`,
},
{
- description: "genrule with command line variable replacements",
- moduleTypeUnderTest: "genrule",
- moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ description: "genrule with command line variable replacements",
+ moduleTypeUnderTest: "genrule",
+ moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
bp: `genrule {
name: "foo",
out: ["foo.out"],
@@ -332,9 +501,10 @@
)`,
},
{
- description: "genrule using $(locations :label)",
- moduleTypeUnderTest: "genrule",
- moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ description: "genrule using $(locations :label)",
+ moduleTypeUnderTest: "genrule",
+ moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
bp: `genrule {
name: "foo",
out: ["foo.out"],
@@ -357,9 +527,10 @@
)`,
},
{
- description: "genrule using $(location) label should substitute first tool label automatically",
- moduleTypeUnderTest: "genrule",
- moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ description: "genrule using $(location) label should substitute first tool label automatically",
+ moduleTypeUnderTest: "genrule",
+ moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
bp: `genrule {
name: "foo",
out: ["foo.out"],
@@ -383,9 +554,10 @@
)`,
},
{
- description: "genrule using $(locations) label should substitute first tool label automatically",
- moduleTypeUnderTest: "genrule",
- moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ description: "genrule using $(locations) label should substitute first tool label automatically",
+ moduleTypeUnderTest: "genrule",
+ moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
bp: `genrule {
name: "foo",
out: ["foo.out"],
@@ -409,9 +581,10 @@
)`,
},
{
- description: "genrule without tools or tool_files can convert successfully",
- moduleTypeUnderTest: "genrule",
- moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ description: "genrule without tools or tool_files can convert successfully",
+ moduleTypeUnderTest: "genrule",
+ moduleTypeUnderTestFactory: genrule.GenRuleFactory,
+ moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
bp: `genrule {
name: "foo",
out: ["foo.out"],
@@ -436,6 +609,7 @@
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"})
diff --git a/bp2build/bzl_conversion_test.go b/bp2build/bzl_conversion_test.go
index 8cdee65..f2a4058 100644
--- a/bp2build/bzl_conversion_test.go
+++ b/bp2build/bzl_conversion_test.go
@@ -172,7 +172,7 @@
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 {
diff --git a/bp2build/conversion.go b/bp2build/conversion.go
index 2025ef7..62cd8d4 100644
--- a/bp2build/conversion.go
+++ b/bp2build/conversion.go
@@ -17,7 +17,7 @@
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 @@
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 += "\n\n"
- content += t.content
+ content := soongModuleLoad
+ if mode == Bp2Build {
+ 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
diff --git a/bp2build/conversion_test.go b/bp2build/conversion_test.go
index 7f75b14..ec5f27e 100644
--- a/bp2build/conversion_test.go
+++ b/bp2build/conversion_test.go
@@ -55,7 +55,7 @@
}
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_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: "",
diff --git a/bp2build/testing.go b/bp2build/testing.go
index 4c31d2d..5e6481b 100644
--- a/bp2build/testing.go
+++ b/bp2build/testing.go
@@ -137,3 +137,29 @@
})
}
}
+
+// 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",
+ })
+ }
+}
diff --git a/genrule/genrule.go b/genrule/genrule.go
index 66e5652..ddfb459 100644
--- a/genrule/genrule.go
+++ b/genrule/genrule.go
@@ -47,7 +47,7 @@
ctx.BottomUp("genrule_tool_deps", toolDepsMutator).Parallel()
})
- android.RegisterBp2BuildMutator("genrule", bp2buildMutator)
+ android.RegisterBp2BuildMutator("genrule", GenruleBp2Build)
}
var (
@@ -794,7 +794,7 @@
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.