diff options
| -rw-r--r-- | android/filegroup.go | 4 | ||||
| -rw-r--r-- | android/testing.go | 4 | ||||
| -rw-r--r-- | bazel/properties.go | 3 | ||||
| -rw-r--r-- | bp2build/build_conversion.go | 89 | ||||
| -rw-r--r-- | bp2build/build_conversion_test.go | 226 | ||||
| -rw-r--r-- | bp2build/bzl_conversion_test.go | 2 | ||||
| -rw-r--r-- | bp2build/conversion.go | 16 | ||||
| -rw-r--r-- | bp2build/conversion_test.go | 4 | ||||
| -rw-r--r-- | bp2build/testing.go | 26 | ||||
| -rw-r--r-- | genrule/genrule.go | 4 |
10 files changed, 329 insertions, 49 deletions
diff --git a/android/filegroup.go b/android/filegroup.go index fd4a2fe49..3d1bbc533 100644 --- a/android/filegroup.go +++ b/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{ diff --git a/android/testing.go b/android/testing.go index 76172d18e..5640c2981 100644 --- a/android/testing.go +++ b/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) { diff --git a/bazel/properties.go b/bazel/properties.go index ac0047b49..79956e1ec 100644 --- a/bazel/properties.go +++ b/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 } diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index a7c3adb65..1af1d605f 100644 --- a/bp2build/build_conversion.go +++ b/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 - 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 @@ func propsToAttributes(props map[string]string) string { 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 @@ 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, diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 367e13f75..66ed42d03 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -268,18 +268,185 @@ func TestGenerateBazelTargetModules(t *testing.T) { } } +func TestLoadStatements(t *testing.T) { + testCases := []struct { + bazelTargets BazelTargets + expectedLoadStatements string + }{ + { + 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 - bp string - expectedBazelTarget string - description string + 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, + 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", - 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 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - 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 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - 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 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - 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 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - 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 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - 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 @@ 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"}) diff --git a/bp2build/bzl_conversion_test.go b/bp2build/bzl_conversion_test.go index 8cdee655c..f2a405854 100644 --- a/bp2build/bzl_conversion_test.go +++ b/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 { diff --git a/bp2build/conversion.go b/bp2build/conversion.go index 2025ef706..62cd8d4b6 100644 --- a/bp2build/conversion.go +++ b/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) { + targets := buildToTargets[dir] + sort.Slice(targets, func(i, j int) bool { return targets[i].name < targets[j].name }) content := soongModuleLoad if mode == Bp2Build { - // No need to load soong_module for bp2build BUILD files. - content = "" + content = targets.LoadStatements() } - targets := buildToTargets[dir] - sort.Slice(targets, func(i, j int) bool { return targets[i].name < targets[j].name }) - for _, t := range targets { + if content != "" { + // If there are load statements, add a couple of newlines. content += "\n\n" - content += t.content } + 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 7f75b14f7..ec5f27e21 100644 --- a/bp2build/conversion_test.go +++ b/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: "", diff --git a/bp2build/testing.go b/bp2build/testing.go index 4c31d2d02..5e6481b32 100644 --- a/bp2build/testing.go +++ b/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", + }) + } +} diff --git a/genrule/genrule.go b/genrule/genrule.go index 66e5652f7..ddfb4596b 100644 --- a/genrule/genrule.go +++ b/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. |