From fc46bc1ee437f76771fa09af4b2836176b17c3f8 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Fri, 19 Feb 2021 11:06:17 -0500 Subject: [PATCH] Refactor BazelTargetModule This eliminates the need to remove quotes, delete attributes, and re-checking that name has correct prefix. Additionally, this allows assignment directly to the BazelTargetModuleProperties struct, which allows defaulting unused fields and clarity of which field is being set. Test: go test soong tests Test: ran ./build/bazel/scripts/milestone-2/demo.sh Change-Id: Ia9bfcce76234c793a4ddd5f29a661150f83341c9 --- android/filegroup.go | 4 ++-- android/module.go | 27 +++++++++++++++++++++---- android/mutator.go | 20 ++++++++++++------ bazel/properties.go | 26 ++---------------------- bp2build/build_conversion.go | 37 +++++++--------------------------- bp2build/testing.go | 39 ++++++++++++++++++------------------ cc/library_headers.go | 11 +++++----- cc/object.go | 11 +++++----- genrule/genrule.go | 6 ++++-- sh/sh_binary.go | 6 ++++-- 10 files changed, 85 insertions(+), 102 deletions(-) diff --git a/android/filegroup.go b/android/filegroup.go index 674a196cb..c3bf6f8d9 100644 --- a/android/filegroup.go +++ b/android/filegroup.go @@ -57,9 +57,9 @@ func FilegroupBp2Build(ctx TopDownMutatorContext) { Srcs: BazelLabelForModuleSrcExcludes(ctx, fg.properties.Srcs, fg.properties.Exclude_srcs), } - props := bazel.NewBazelTargetModuleProperties(fg.Name(), "filegroup", "") + props := bazel.BazelTargetModuleProperties{Rule_class: "filegroup"} - ctx.CreateBazelTargetModule(BazelFileGroupFactory, props, attrs) + ctx.CreateBazelTargetModule(BazelFileGroupFactory, fg.Name(), props, attrs) } type fileGroupProperties struct { diff --git a/android/module.go b/android/module.go index bf74cad07..58675d466 100644 --- a/android/module.go +++ b/android/module.go @@ -507,13 +507,17 @@ type Module interface { type BazelTargetModule interface { Module - BazelTargetModuleProperties() *bazel.BazelTargetModuleProperties + bazelTargetModuleProperties() *bazel.BazelTargetModuleProperties + SetBazelTargetModuleProperties(props bazel.BazelTargetModuleProperties) + + RuleClass() string + BzlLoadLocation() string } // InitBazelTargetModule is a wrapper function that decorates BazelTargetModule // with property structs containing metadata for bp2build conversion. func InitBazelTargetModule(module BazelTargetModule) { - module.AddProperties(module.BazelTargetModuleProperties()) + module.AddProperties(module.bazelTargetModuleProperties()) InitAndroidModule(module) } @@ -524,11 +528,26 @@ type BazelTargetModuleBase struct { Properties bazel.BazelTargetModuleProperties } -// BazelTargetModuleProperties getter. -func (btmb *BazelTargetModuleBase) BazelTargetModuleProperties() *bazel.BazelTargetModuleProperties { +// bazelTargetModuleProperties getter. +func (btmb *BazelTargetModuleBase) bazelTargetModuleProperties() *bazel.BazelTargetModuleProperties { return &btmb.Properties } +// SetBazelTargetModuleProperties setter for BazelTargetModuleProperties +func (btmb *BazelTargetModuleBase) SetBazelTargetModuleProperties(props bazel.BazelTargetModuleProperties) { + btmb.Properties = props +} + +// RuleClass returns the rule class for this Bazel target +func (b *BazelTargetModuleBase) RuleClass() string { + return b.bazelTargetModuleProperties().Rule_class +} + +// BzlLoadLocation returns the rule class for this Bazel target +func (b *BazelTargetModuleBase) BzlLoadLocation() string { + return b.bazelTargetModuleProperties().Bzl_load_location +} + // Qualified id for a module type qualifiedModuleName struct { // The package (i.e. directory) in which the module is defined, without trailing / diff --git a/android/mutator.go b/android/mutator.go index c38719334..b0230011d 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -283,7 +283,7 @@ type TopDownMutatorContext interface { // factory method, just like in CreateModule, but also requires // BazelTargetModuleProperties containing additional metadata for the // bp2build codegenerator. - CreateBazelTargetModule(ModuleFactory, bazel.BazelTargetModuleProperties, interface{}) BazelTargetModule + CreateBazelTargetModule(ModuleFactory, string, bazel.BazelTargetModuleProperties, interface{}) BazelTargetModule } type topDownMutatorContext struct { @@ -513,17 +513,25 @@ func registerDepsMutatorBp2Build(ctx RegisterMutatorsContext) { func (t *topDownMutatorContext) CreateBazelTargetModule( factory ModuleFactory, + name string, bazelProps bazel.BazelTargetModuleProperties, attrs interface{}) BazelTargetModule { - if !strings.HasPrefix(*bazelProps.Name, bazel.BazelTargetModuleNamePrefix) { + if strings.HasPrefix(name, bazel.BazelTargetModuleNamePrefix) { panic(fmt.Errorf( - "bp2build error: the bazel target module name must start with '%s': %s", + "The %s name prefix is added automatically, do not set it manually: %s", bazel.BazelTargetModuleNamePrefix, - *bazelProps.Name, - )) + name)) + } + name = bazel.BazelTargetModuleNamePrefix + name + nameProp := struct { + Name *string + }{ + Name: &name, } - return t.CreateModule(factory, &bazelProps, attrs).(BazelTargetModule) + b := t.CreateModule(factory, &nameProp, attrs).(BazelTargetModule) + b.SetBazelTargetModuleProperties(bazelProps) + return b } func (t *topDownMutatorContext) AppendProperties(props ...interface{}) { diff --git a/bazel/properties.go b/bazel/properties.go index 8055306b2..a4df4bc83 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -14,11 +14,6 @@ package bazel -import ( - "fmt" - "strings" -) - type bazelModuleProperties struct { // The label of the Bazel target replacing this Soong module. Label string @@ -37,32 +32,15 @@ type Properties struct { // BazelTargetModuleProperties contain properties and metadata used for // Blueprint to BUILD file conversion. type BazelTargetModuleProperties struct { - Name *string - // The Bazel rule class for this target. - Rule_class string + Rule_class string `blueprint:"mutated"` // The target label for the bzl file containing the definition of the rule class. - Bzl_load_location string + Bzl_load_location string `blueprint:"mutated"` } const BazelTargetModuleNamePrefix = "__bp2build__" -func NewBazelTargetModuleProperties(name string, ruleClass string, bzlLoadLocation string) BazelTargetModuleProperties { - if strings.HasPrefix(name, BazelTargetModuleNamePrefix) { - panic(fmt.Errorf( - "The %s name prefix is added automatically, do not set it manually: %s", - BazelTargetModuleNamePrefix, - name)) - } - name = BazelTargetModuleNamePrefix + name - return BazelTargetModuleProperties{ - Name: &name, - Rule_class: ruleClass, - Bzl_load_location: bzlLoadLocation, - } -} - // Label is used to represent a Bazel compatible Label. Also stores the original bp text to support // string replacement. type Label struct { diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 41ad409cc..a4c4a0dd3 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -19,7 +19,6 @@ import ( "android/soong/bazel" "fmt" "reflect" - "strconv" "strings" "github.com/google/blueprint" @@ -178,13 +177,14 @@ func GenerateBazelTargets(ctx CodegenContext) (map[string]BazelTargets, CodegenM switch ctx.Mode() { case Bp2Build: - if _, ok := m.(android.BazelTargetModule); !ok { + if b, ok := m.(android.BazelTargetModule); !ok { // Only include regular Soong modules (non-BazelTargetModules) into the total count. totalModuleCount += 1 return + } else { + t = generateBazelTarget(bpCtx, m, b) + ruleClassCount[t.ruleClass] += 1 } - t = generateBazelTarget(bpCtx, m) - ruleClassCount[t.ruleClass] += 1 case QueryView: // Blocklist certain module types from being generated. if canonicalizeModuleType(bpCtx.ModuleType(m)) == "package" { @@ -208,36 +208,13 @@ func GenerateBazelTargets(ctx CodegenContext) (map[string]BazelTargets, CodegenM return buildFileToTargets, metrics } -// 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, b android.BazelTargetModule) BazelTarget { + ruleClass := b.RuleClass() + bzlLoadLocation := b.BzlLoadLocation() -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 := 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") - delete(props.Attrs, "bp2build_available") // Return the Bazel target with rule class and attributes, ready to be diff --git a/bp2build/testing.go b/bp2build/testing.go index 97749154e..cb50fc8a4 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -136,9 +136,11 @@ func customBp2BuildMutator(ctx android.TopDownMutatorContext) { String_list_prop: m.props.String_list_prop, } - props := bazel.NewBazelTargetModuleProperties(m.Name(), "custom", "") + props := bazel.BazelTargetModuleProperties{ + Rule_class: "custom", + } - ctx.CreateBazelTargetModule(customBazelModuleFactory, props, attrs) + ctx.CreateBazelTargetModule(customBazelModuleFactory, m.Name(), props, attrs) } } @@ -153,26 +155,23 @@ func customBp2BuildMutatorFromStarlark(ctx android.TopDownMutatorContext) { baseName := m.Name() attrs := &customBazelModuleAttributes{} - myLibraryProps := bazel.NewBazelTargetModuleProperties( - baseName, - "my_library", - "//build/bazel/rules:rules.bzl", - ) - ctx.CreateBazelTargetModule(customBazelModuleFactory, myLibraryProps, attrs) + myLibraryProps := bazel.BazelTargetModuleProperties{ + Rule_class: "my_library", + Bzl_load_location: "//build/bazel/rules:rules.bzl", + } + ctx.CreateBazelTargetModule(customBazelModuleFactory, baseName, myLibraryProps, attrs) - protoLibraryProps := bazel.NewBazelTargetModuleProperties( - baseName+"_proto_library_deps", - "proto_library", - "//build/bazel/rules:proto.bzl", - ) - ctx.CreateBazelTargetModule(customBazelModuleFactory, protoLibraryProps, attrs) + protoLibraryProps := bazel.BazelTargetModuleProperties{ + Rule_class: "proto_library", + Bzl_load_location: "//build/bazel/rules:proto.bzl", + } + ctx.CreateBazelTargetModule(customBazelModuleFactory, baseName+"_proto_library_deps", protoLibraryProps, attrs) - myProtoLibraryProps := bazel.NewBazelTargetModuleProperties( - baseName+"_my_proto_library_deps", - "my_proto_library", - "//build/bazel/rules:proto.bzl", - ) - ctx.CreateBazelTargetModule(customBazelModuleFactory, myProtoLibraryProps, attrs) + myProtoLibraryProps := bazel.BazelTargetModuleProperties{ + Rule_class: "my_proto_library", + Bzl_load_location: "//build/bazel/rules:proto.bzl", + } + ctx.CreateBazelTargetModule(customBazelModuleFactory, baseName+"_my_proto_library_deps", myProtoLibraryProps, attrs) } } diff --git a/cc/library_headers.go b/cc/library_headers.go index e5a555729..03450c6aa 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -129,13 +129,12 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { Deps: headerLibLabels, } - props := bazel.NewBazelTargetModuleProperties( - module.Name(), - "cc_library_headers", - "//build/bazel/rules:cc_library_headers.bzl", - ) + props := bazel.BazelTargetModuleProperties{ + Rule_class: "cc_library_headers", + Bzl_load_location: "//build/bazel/rules:cc_library_headers.bzl", + } - ctx.CreateBazelTargetModule(BazelCcLibraryHeadersFactory, props, attrs) + ctx.CreateBazelTargetModule(BazelCcLibraryHeadersFactory, module.Name(), props, attrs) } func (m *bazelCcLibraryHeaders) Name() string { diff --git a/cc/object.go b/cc/object.go index d92e1109f..3a7af9715 100644 --- a/cc/object.go +++ b/cc/object.go @@ -150,13 +150,12 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { Local_include_dirs: localIncludeDirs, } - props := bazel.NewBazelTargetModuleProperties( - m.Name(), - "cc_object", - "//build/bazel/rules:cc_object.bzl", - ) + props := bazel.BazelTargetModuleProperties{ + Rule_class: "cc_object", + Bzl_load_location: "//build/bazel/rules:cc_object.bzl", + } - ctx.CreateBazelTargetModule(BazelObjectFactory, props, attrs) + ctx.CreateBazelTargetModule(BazelObjectFactory, m.Name(), props, attrs) } func (object *objectLinker) appendLdflags(flags []string) { diff --git a/genrule/genrule.go b/genrule/genrule.go index 9fa6c484f..c743fc329 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -853,10 +853,12 @@ func GenruleBp2Build(ctx android.TopDownMutatorContext) { Tools: tools, } - props := bazel.NewBazelTargetModuleProperties(m.Name(), "genrule", "") + props := bazel.BazelTargetModuleProperties{ + Rule_class: "genrule", + } // Create the BazelTargetModule. - ctx.CreateBazelTargetModule(BazelGenruleFactory, props, attrs) + ctx.CreateBazelTargetModule(BazelGenruleFactory, m.Name(), props, attrs) } func (m *bazelGenrule) Name() string { diff --git a/sh/sh_binary.go b/sh/sh_binary.go index 58f8cf69e..3a5c7de99 100644 --- a/sh/sh_binary.go +++ b/sh/sh_binary.go @@ -514,9 +514,11 @@ func ShBinaryBp2Build(ctx android.TopDownMutatorContext) { Srcs: srcs, } - props := bazel.NewBazelTargetModuleProperties(m.Name(), "sh_binary", "") + props := bazel.BazelTargetModuleProperties{ + Rule_class: "sh_binary", + } - ctx.CreateBazelTargetModule(BazelShBinaryFactory, props, attrs) + ctx.CreateBazelTargetModule(BazelShBinaryFactory, m.Name(), props, attrs) } func (m *bazelShBinary) Name() string {