From 447f6c99c9eef8f814552f70697dfe4a42bda5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20M=C3=A1rquez=20P=C3=A9rez=20Mu=C3=B1=C3=ADz=20D?= =?UTF-8?q?=C3=ADaz=20P=C3=BAras=20Thaureaux?= Date: Tue, 31 Aug 2021 20:30:36 +0000 Subject: [PATCH] Bp2Build common properties auto-handling Introduce `commonAttributes` & `fillCommonBp2BuildModuleAttrs used in CreateBazelTargetModule Adapt `bp2BuildInfo` to use `commonAttrs` instead of `Name`. And thus also all downstream users of `CreateBazelTargetModule`. As initial user, the Soong `required` property will be translated to Bazel's `data`. Bug: 198146582, 196091467 Test: build_converstion_test.go:TestCommonBp2BuildModuleAttrs Test: go test Test: mixed_{libc,droid}.sh Change-Id: Ib500e40f7e2cb48c459f1ebe3188962fc41ec124 --- android/filegroup.go | 2 +- android/module.go | 52 ++++++++++-- android/mutator.go | 20 +++-- apex/apex.go | 2 +- apex/key.go | 2 +- bp2build/build_conversion.go | 14 +++- bp2build/build_conversion_test.go | 134 +++++++++++++++++++++++++++++- bp2build/conversion.go | 5 +- bp2build/testing.go | 19 ++--- cc/library.go | 4 +- cc/library_headers.go | 2 +- cc/object.go | 2 +- etc/prebuilt_etc.go | 2 +- genrule/genrule.go | 2 +- java/app.go | 2 +- python/binary.go | 7 +- python/library.go | 7 +- sh/sh_binary.go | 2 +- 18 files changed, 229 insertions(+), 51 deletions(-) diff --git a/android/filegroup.go b/android/filegroup.go index 4db165f87..2cf556750 100644 --- a/android/filegroup.go +++ b/android/filegroup.go @@ -72,7 +72,7 @@ func FilegroupBp2Build(ctx TopDownMutatorContext) { Bzl_load_location: "//build/bazel/rules:filegroup.bzl", } - ctx.CreateBazelTargetModule(fg.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, CommonAttributes{Name: fg.Name()}, attrs) } type fileGroupProperties struct { diff --git a/android/module.go b/android/module.go index c9b01a00a..327e6ae20 100644 --- a/android/module.go +++ b/android/module.go @@ -852,6 +852,16 @@ type commonProperties struct { UnconvertedBp2buildDeps []string `blueprint:"mutated"` } +// CommonAttributes represents the common Bazel attributes from which properties +// in `commonProperties` are translated/mapped; such properties are annotated in +// a list their corresponding attribute. It is embedded within `bp2buildInfo`. +type CommonAttributes struct { + // Soong nameProperties -> Bazel name + Name string + // Data mapped from: Required + Data bazel.LabelListAttribute +} + type distProperties struct { // configuration to distribute output files from this module to the distribution // directory (default: $OUT/dist, configurable with $DIST_DIR) @@ -1072,6 +1082,34 @@ func InitCommonOSAndroidMultiTargetsArchModule(m Module, hod HostOrDeviceSupport m.base().commonProperties.CreateCommonOSVariant = true } +func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutatorContext) { + // Assert passed-in attributes include Name + name := attrs.Name + if len(name) == 0 { + ctx.ModuleErrorf("CommonAttributes in fillCommonBp2BuildModuleAttrs expects a `.Name`!") + } + + mod := ctx.Module().base() + props := &mod.commonProperties + + depsToLabelList := func(deps []string) bazel.LabelListAttribute { + return bazel.MakeLabelListAttribute(BazelLabelForModuleDeps(ctx, deps)) + } + + data := &attrs.Data + + required := depsToLabelList(props.Required) + archVariantProps := mod.GetArchVariantProperties(ctx, &commonProperties{}) + for axis, configToProps := range archVariantProps { + for config, _props := range configToProps { + if archProps, ok := _props.(*commonProperties); ok { + required.SetSelectValue(axis, config, depsToLabelList(archProps.Required).Value) + } + } + } + data.Append(required) +} + // A ModuleBase object contains the properties that are common to all Android // modules. It should be included as an anonymous field in every module // struct definition. InitAndroidModule should then be called from the module's @@ -1183,15 +1221,15 @@ type ModuleBase struct { // A struct containing all relevant information about a Bazel target converted via bp2build. type bp2buildInfo struct { - Name string - Dir string - BazelProps bazel.BazelTargetModuleProperties - Attrs interface{} + Dir string + BazelProps bazel.BazelTargetModuleProperties + CommonAttrs CommonAttributes + Attrs interface{} } // TargetName returns the Bazel target name of a bp2build converted target. func (b bp2buildInfo) TargetName() string { - return b.Name + return b.CommonAttrs.Name } // TargetPackage returns the Bazel package of a bp2build converted target. @@ -1211,8 +1249,8 @@ func (b bp2buildInfo) BazelRuleLoadLocation() string { } // BazelAttributes returns the Bazel attributes of a bp2build converted target. -func (b bp2buildInfo) BazelAttributes() interface{} { - return b.Attrs +func (b bp2buildInfo) BazelAttributes() []interface{} { + return []interface{}{&b.CommonAttrs, b.Attrs} } func (m *ModuleBase) addBp2buildInfo(info bp2buildInfo) { diff --git a/android/mutator.go b/android/mutator.go index b361c51ce..4b373778e 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -15,10 +15,11 @@ package android import ( - "android/soong/bazel" "reflect" "sync" + "android/soong/bazel" + "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -268,7 +269,7 @@ type TopDownMutatorContext interface { // factory method, just like in CreateModule, but also requires // BazelTargetModuleProperties containing additional metadata for the // bp2build codegenerator. - CreateBazelTargetModule(string, bazel.BazelTargetModuleProperties, interface{}) + CreateBazelTargetModule(bazel.BazelTargetModuleProperties, CommonAttributes, interface{}) } type topDownMutatorContext struct { @@ -514,17 +515,18 @@ func registerDepsMutatorBp2Build(ctx RegisterMutatorsContext) { } func (t *topDownMutatorContext) CreateBazelTargetModule( - name string, bazelProps bazel.BazelTargetModuleProperties, + commonAttrs CommonAttributes, attrs interface{}) { - + commonAttrs.fillCommonBp2BuildModuleAttrs(t) + mod := t.Module() info := bp2buildInfo{ - Name: name, - Dir: t.OtherModuleDir(t.Module()), - BazelProps: bazelProps, - Attrs: attrs, + Dir: t.OtherModuleDir(mod), + BazelProps: bazelProps, + CommonAttrs: commonAttrs, + Attrs: attrs, } - t.Module().base().addBp2buildInfo(info) + mod.base().addBp2buildInfo(info) } func (t *topDownMutatorContext) AppendProperties(props ...interface{}) { diff --git a/apex/apex.go b/apex/apex.go index 5294b6c17..0a2e35b92 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -3325,5 +3325,5 @@ func apexBundleBp2BuildInternal(ctx android.TopDownMutatorContext, module *apexB Bzl_load_location: "//build/bazel/rules:apex.bzl", } - ctx.CreateBazelTargetModule(module.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name()}, attrs) } diff --git a/apex/key.go b/apex/key.go index 468bb8a22..e2695d7a0 100644 --- a/apex/key.go +++ b/apex/key.go @@ -240,5 +240,5 @@ func apexKeyBp2BuildInternal(ctx android.TopDownMutatorContext, module *apexKey) Bzl_load_location: "//build/bazel/rules:apex_key.bzl", } - ctx.CreateBazelTargetModule(module.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name()}, attrs) } diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 4a0eeea51..c05a62b07 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -411,7 +411,7 @@ type bp2buildModule interface { TargetPackage() string BazelRuleClass() string BazelRuleLoadLocation() string - BazelAttributes() interface{} + BazelAttributes() []interface{} } func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget { @@ -419,7 +419,8 @@ func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget { bzlLoadLocation := m.BazelRuleLoadLocation() // extract the bazel attributes from the module. - props := extractModuleProperties([]interface{}{m.BazelAttributes()}) + attrs := m.BazelAttributes() + props := extractModuleProperties(attrs, true) delete(props.Attrs, "bp2build_available") @@ -482,14 +483,14 @@ func getBuildProperties(ctx bpToBuildContext, m blueprint.Module) BazelAttribute // TODO: this omits properties for blueprint modules (blueprint_go_binary, // bootstrap_go_binary, bootstrap_go_package), which will have to be handled separately. if aModule, ok := m.(android.Module); ok { - return extractModuleProperties(aModule.GetProperties()) + return extractModuleProperties(aModule.GetProperties(), false) } return BazelAttributes{} } // Generically extract module properties and types into a map, keyed by the module property name. -func extractModuleProperties(props []interface{}) BazelAttributes { +func extractModuleProperties(props []interface{}, checkForDuplicateProperties bool) BazelAttributes { ret := map[string]string{} // Iterate over this android.Module's property structs. @@ -503,6 +504,11 @@ func extractModuleProperties(props []interface{}) BazelAttributes { if isStructPtr(propertiesValue.Type()) { structValue := propertiesValue.Elem() for k, v := range extractStructProperties(structValue, 0) { + if existing, exists := ret[k]; checkForDuplicateProperties && exists { + panic(fmt.Errorf( + "%s (%v) is present in properties whereas it should be consolidated into a commonAttributes", + k, existing)) + } ret[k] = v } } else { diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index e904627c2..f14574c06 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -15,10 +15,12 @@ package bp2build import ( - "android/soong/android" "fmt" "strings" "testing" + + "android/soong/android" + "android/soong/python" ) func TestGenerateSoongModuleTargets(t *testing.T) { @@ -1215,3 +1217,133 @@ func TestGlobExcludeSrcs(t *testing.T) { } } } + +func TestCommonBp2BuildModuleAttrs(t *testing.T) { + testCases := []bp2buildTestCase{ + { + description: "Required into data test", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, + blueprint: `filegroup { + name: "reqd", +} + +filegroup { + name: "fg_foo", + required: ["reqd"], + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`filegroup( + name = "fg_foo", + data = [":reqd"], +)`, + `filegroup( + name = "reqd", +)`, + }, + }, + { + description: "Required via arch into data test", + moduleTypeUnderTest: "python_library", + moduleTypeUnderTestFactory: python.PythonLibraryFactory, + moduleTypeUnderTestBp2BuildMutator: python.PythonLibraryBp2Build, + blueprint: `python_library { + name: "reqdx86", + bazel_module: { bp2build_available: false, }, +} + +python_library { + name: "reqdarm", + bazel_module: { bp2build_available: false, }, +} + +python_library { + name: "fg_foo", + arch: { + arm: { + required: ["reqdarm"], + }, + x86: { + required: ["reqdx86"], + }, + }, + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`py_library( + name = "fg_foo", + data = select({ + "//build/bazel/platforms/arch:arm": [":reqdarm"], + "//build/bazel/platforms/arch:x86": [":reqdx86"], + "//conditions:default": [], + }), + srcs_version = "PY3", +)`, + }, + }, + { + description: "Required appended to data test", + moduleTypeUnderTest: "python_library", + moduleTypeUnderTestFactory: python.PythonLibraryFactory, + moduleTypeUnderTestBp2BuildMutator: python.PythonLibraryBp2Build, + blueprint: `python_library { + name: "reqd", + srcs: ["src.py"], +} + +python_library { + name: "fg_foo", + data: ["data.bin"], + required: ["reqd"], + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{ + `py_library( + name = "fg_foo", + data = [ + "data.bin", + ":reqd", + ], + srcs_version = "PY3", +)`, + `py_library( + name = "reqd", + srcs = ["src.py"], + srcs_version = "PY3", +)`, + }, + filesystem: map[string]string{ + "data.bin": "", + "src.py": "", + }, + }, + { + description: "All props-to-attrs at once together test", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, + blueprint: `filegroup { + name: "reqd" +} +filegroup { + name: "fg_foo", + required: ["reqd"], + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{ + `filegroup( + name = "fg_foo", + data = [":reqd"], +)`, + `filegroup( + name = "reqd", +)`, + }, + filesystem: map[string]string{}, + }, + } + + for _, test := range testCases { + runBp2BuildTestCaseSimple(t, test) + } +} diff --git a/bp2build/conversion.go b/bp2build/conversion.go index 0a86a79c5..d34a4ba9b 100644 --- a/bp2build/conversion.go +++ b/bp2build/conversion.go @@ -1,12 +1,13 @@ package bp2build import ( - "android/soong/android" - "android/soong/cc/config" "fmt" "reflect" "strings" + "android/soong/android" + "android/soong/cc/config" + "github.com/google/blueprint/proptools" ) diff --git a/bp2build/testing.go b/bp2build/testing.go index 1e7e53c9f..6c322eefe 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -135,17 +135,14 @@ func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi android.FailIfErrored(t, errs) } if actualCount, expectedCount := len(bazelTargets), len(tc.expectedBazelTargets); actualCount != expectedCount { - t.Errorf("%s: Expected %d bazel target, got %d; %v", - tc.description, expectedCount, actualCount, bazelTargets) + t.Errorf("%s: Expected %d bazel target, got `%d``", + tc.description, expectedCount, actualCount) } else { for i, target := range bazelTargets { if w, g := tc.expectedBazelTargets[i], target.content; w != g { t.Errorf( - "%s: Expected generated Bazel target to be '%s', got '%s'", - tc.description, - w, - g, - ) + "%s: Expected generated Bazel target to be `%s`, got `%s`", + tc.description, w, g) } } } @@ -312,7 +309,7 @@ func customBp2BuildMutator(ctx android.TopDownMutatorContext) { Rule_class: "custom", } - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name()}, attrs) } } @@ -331,19 +328,19 @@ func customBp2BuildMutatorFromStarlark(ctx android.TopDownMutatorContext) { Rule_class: "my_library", Bzl_load_location: "//build/bazel/rules:rules.bzl", } - ctx.CreateBazelTargetModule(baseName, myLibraryProps, attrs) + ctx.CreateBazelTargetModule(myLibraryProps, android.CommonAttributes{Name: baseName}, attrs) protoLibraryProps := bazel.BazelTargetModuleProperties{ Rule_class: "proto_library", Bzl_load_location: "//build/bazel/rules:proto.bzl", } - ctx.CreateBazelTargetModule(baseName+"_proto_library_deps", protoLibraryProps, attrs) + ctx.CreateBazelTargetModule(protoLibraryProps, android.CommonAttributes{Name: baseName + "_proto_library_deps"}, attrs) myProtoLibraryProps := bazel.BazelTargetModuleProperties{ Rule_class: "my_proto_library", Bzl_load_location: "//build/bazel/rules:proto.bzl", } - ctx.CreateBazelTargetModule(baseName+"_my_proto_library_deps", myProtoLibraryProps, attrs) + ctx.CreateBazelTargetModule(myProtoLibraryProps, android.CommonAttributes{Name: baseName + "_my_proto_library_deps"}, attrs) } } diff --git a/cc/library.go b/cc/library.go index de9d01ede..b181a1601 100644 --- a/cc/library.go +++ b/cc/library.go @@ -345,7 +345,7 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) { Bzl_load_location: "//build/bazel/rules:full_cc_library.bzl", } - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name()}, attrs) } // cc_library creates both static and/or shared libraries for a device and/or @@ -2434,7 +2434,7 @@ func ccSharedOrStaticBp2BuildMutatorInternal(ctx android.TopDownMutatorContext, Bzl_load_location: fmt.Sprintf("//build/bazel/rules:%s.bzl", modType), } - ctx.CreateBazelTargetModule(module.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name()}, attrs) } // TODO(b/199902614): Can this be factored to share with the other Attributes? diff --git a/cc/library_headers.go b/cc/library_headers.go index cabeb0156..51c1eb828 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -147,5 +147,5 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { Bzl_load_location: "//build/bazel/rules:cc_library_headers.bzl", } - ctx.CreateBazelTargetModule(module.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name()}, attrs) } diff --git a/cc/object.go b/cc/object.go index 4ec2b19a2..d8bb08fac 100644 --- a/cc/object.go +++ b/cc/object.go @@ -206,7 +206,7 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { Bzl_load_location: "//build/bazel/rules:cc_object.bzl", } - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name()}, attrs) } func (object *objectLinker) appendLdflags(flags []string) { diff --git a/etc/prebuilt_etc.go b/etc/prebuilt_etc.go index 85abf5998..81896bd68 100644 --- a/etc/prebuilt_etc.go +++ b/etc/prebuilt_etc.go @@ -704,5 +704,5 @@ func prebuiltEtcBp2BuildInternal(ctx android.TopDownMutatorContext, module *Preb Bzl_load_location: "//build/bazel/rules:prebuilt_etc.bzl", } - ctx.CreateBazelTargetModule(module.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name()}, attrs) } diff --git a/genrule/genrule.go b/genrule/genrule.go index 96b610bac..4dd21351a 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -928,7 +928,7 @@ func genruleBp2Build(ctx android.TopDownMutatorContext) { } // Create the BazelTargetModule. - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name()}, attrs) } var Bool = proptools.Bool diff --git a/java/app.go b/java/app.go index bc0f488d0..6554d6693 100755 --- a/java/app.go +++ b/java/app.go @@ -1431,5 +1431,5 @@ func androidAppCertificateBp2BuildInternal(ctx android.TopDownMutatorContext, mo Bzl_load_location: "//build/bazel/rules:android_app_certificate.bzl", } - ctx.CreateBazelTargetModule(module.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name()}, attrs) } diff --git a/python/binary.go b/python/binary.go index afcc53a65..bf6167c3c 100644 --- a/python/binary.go +++ b/python/binary.go @@ -37,7 +37,6 @@ func registerPythonBinaryComponents(ctx android.RegistrationContext) { type bazelPythonBinaryAttributes struct { Main string Srcs bazel.LabelListAttribute - Data bazel.LabelListAttribute Deps bazel.LabelListAttribute Python_version string } @@ -85,7 +84,6 @@ func PythonBinaryBp2Build(ctx android.TopDownMutatorContext) { attrs := &bazelPythonBinaryAttributes{ Main: main, Srcs: baseAttrs.Srcs, - Data: baseAttrs.Data, Deps: baseAttrs.Deps, Python_version: python_version, } @@ -95,7 +93,10 @@ func PythonBinaryBp2Build(ctx android.TopDownMutatorContext) { Rule_class: "py_binary", } - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{ + Name: m.Name(), + Data: baseAttrs.Data, + }, attrs) } type BinaryProperties struct { diff --git a/python/library.go b/python/library.go index 19fa59a3f..d136a4efb 100644 --- a/python/library.go +++ b/python/library.go @@ -45,7 +45,6 @@ func PythonLibraryHostFactory() android.Module { type bazelPythonLibraryAttributes struct { Srcs bazel.LabelListAttribute - Data bazel.LabelListAttribute Deps bazel.LabelListAttribute Srcs_version string } @@ -91,7 +90,6 @@ func pythonLibBp2Build(ctx android.TopDownMutatorContext, modType string) { baseAttrs := m.makeArchVariantBaseAttributes(ctx) attrs := &bazelPythonLibraryAttributes{ Srcs: baseAttrs.Srcs, - Data: baseAttrs.Data, Deps: baseAttrs.Deps, Srcs_version: python_version, } @@ -101,7 +99,10 @@ func pythonLibBp2Build(ctx android.TopDownMutatorContext, modType string) { Rule_class: "py_library", } - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{ + Name: m.Name(), + Data: baseAttrs.Data, + }, attrs) } func PythonLibraryFactory() android.Module { diff --git a/sh/sh_binary.go b/sh/sh_binary.go index db66ae21a..b22a5b750 100644 --- a/sh/sh_binary.go +++ b/sh/sh_binary.go @@ -548,7 +548,7 @@ func ShBinaryBp2Build(ctx android.TopDownMutatorContext) { Rule_class: "sh_binary", } - ctx.CreateBazelTargetModule(m.Name(), props, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name()}, attrs) } var Bool = proptools.Bool