diff --git a/android/filegroup.go b/android/filegroup.go index 3b866550d..6cc9232b6 100644 --- a/android/filegroup.go +++ b/android/filegroup.go @@ -24,6 +24,7 @@ import ( "android/soong/ui/metrics/bp2build_metrics_proto" "github.com/google/blueprint" + "github.com/google/blueprint/proptools" ) func init() { @@ -141,8 +142,14 @@ func (fg *fileGroup) ConvertWithBp2build(ctx TopDownMutatorContext) { attrs) } else { if fg.ShouldConvertToProtoLibrary(ctx) { + pkgToSrcs := partitionSrcsByPackage(ctx.ModuleDir(), bazel.MakeLabelList(srcs.Value.Includes)) + if len(pkgToSrcs) > 1 { + ctx.ModuleErrorf("TODO: Add bp2build support for multiple package .protosrcs in filegroup") + return + } + pkg := SortedKeys(pkgToSrcs)[0] attrs := &ProtoAttrs{ - Srcs: srcs, + Srcs: bazel.MakeLabelListAttribute(pkgToSrcs[pkg]), Strip_import_prefix: fg.properties.Path, } @@ -151,13 +158,39 @@ func (fg *fileGroup) ConvertWithBp2build(ctx TopDownMutatorContext) { // TODO(b/246997908): we can remove this tag if we could figure out a solution for this bug. "manual", } + if pkg != ctx.ModuleDir() { + // Since we are creating the proto_library in a subpackage, create an import_prefix relative to the current package + if rel, err := filepath.Rel(ctx.ModuleDir(), pkg); err != nil { + ctx.ModuleErrorf("Could not get relative path for %v %v", pkg, err) + } else if rel != "." { + attrs.Import_prefix = &rel + // Strip the package prefix + attrs.Strip_import_prefix = proptools.StringPtr("") + } + } + ctx.CreateBazelTargetModule( bazel.BazelTargetModuleProperties{Rule_class: "proto_library"}, CommonAttributes{ - Name: fg.Name() + convertedProtoLibrarySuffix, + Name: fg.Name() + "_proto", + Dir: proptools.StringPtr(pkg), Tags: bazel.MakeStringListAttribute(tags), }, attrs) + + // Create an alias in the current dir. The actual target might exist in a different package, but rdeps + // can reliabily use this alias + ctx.CreateBazelTargetModule( + bazel.BazelTargetModuleProperties{Rule_class: "alias"}, + CommonAttributes{ + Name: fg.Name() + convertedProtoLibrarySuffix, + // TODO(b/246997908): we can remove this tag if we could figure out a solution for this bug. + Tags: bazel.MakeStringListAttribute(tags), + }, + &bazelAliasAttributes{ + Actual: bazel.MakeLabelAttribute("//" + pkg + ":" + fg.Name() + "_proto"), + }, + ) } // TODO(b/242847534): Still convert to a filegroup because other unconverted diff --git a/android/module.go b/android/module.go index b98201906..0120a544e 100644 --- a/android/module.go +++ b/android/module.go @@ -1021,6 +1021,11 @@ type CommonAttributes struct { Applicable_licenses bazel.LabelListAttribute Testonly *bool + + // Dir is neither a Soong nor Bazel target attribute + // If set, the bazel target will be created in this directory + // If unset, the bazel target will default to be created in the directory of the visited soong module + Dir *string } // constraintAttributes represents Bazel attributes pertaining to build constraints, diff --git a/android/mutator.go b/android/mutator.go index 2ec051e59..6bcac93c1 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -17,6 +17,7 @@ package android import ( "android/soong/bazel" "android/soong/ui/metrics/bp2build_metrics_proto" + "path/filepath" "github.com/google/blueprint" ) @@ -757,6 +758,27 @@ func (t *topDownMutatorContext) CreateBazelTargetAliasInDir( mod.base().addBp2buildInfo(info) } +// Returns the directory in which the bazel target will be generated +// If ca.Dir is not nil, use that +// Otherwise default to the directory of the soong module +func dirForBazelTargetGeneration(t *topDownMutatorContext, ca *CommonAttributes) string { + dir := t.OtherModuleDir(t.Module()) + if ca.Dir != nil { + dir = *ca.Dir + // Restrict its use to dirs that contain an Android.bp file. + // There are several places in bp2build where we use the existence of Android.bp/BUILD on the filesystem + // to curate a compatible label for src files (e.g. headers for cc). + // If we arbritrarily create BUILD files, then it might render those curated labels incompatible. + if exists, _, _ := t.Config().fs.Exists(filepath.Join(dir, "Android.bp")); !exists { + t.ModuleErrorf("Cannot use ca.Dir to create a BazelTarget in dir: %v since it does not contain an Android.bp file", dir) + } + + // Set ca.Dir to nil so that it does not get emitted to the BUILD files + ca.Dir = nil + } + return dir +} + func (t *topDownMutatorContext) CreateBazelConfigSetting( csa bazel.ConfigSettingAttributes, ca CommonAttributes, @@ -851,7 +873,7 @@ func (t *topDownMutatorContext) createBazelTargetModule( constraintAttributes := commonAttrs.fillCommonBp2BuildModuleAttrs(t, enabledProperty) mod := t.Module() info := bp2buildInfo{ - Dir: t.OtherModuleDir(mod), + Dir: dirForBazelTargetGeneration(t, &commonAttrs), BazelProps: bazelProps, CommonAttrs: commonAttrs, ConstraintAttrs: constraintAttributes, diff --git a/android/proto.go b/android/proto.go index cebbd59cd..aad521ba0 100644 --- a/android/proto.go +++ b/android/proto.go @@ -15,6 +15,7 @@ package android import ( + "path/filepath" "strings" "android/soong/bazel" @@ -156,12 +157,12 @@ func ProtoRule(rule *RuleBuilder, protoFile Path, flags ProtoFlags, deps Paths, // Bp2buildProtoInfo contains information necessary to pass on to language specific conversion. type Bp2buildProtoInfo struct { Type *string - Name string Proto_libs bazel.LabelList } type ProtoAttrs struct { Srcs bazel.LabelListAttribute + Import_prefix *string Strip_import_prefix *string Deps bazel.LabelListAttribute } @@ -172,6 +173,35 @@ var includeDirsToProtoDeps = map[string]string{ "external/protobuf/src": "//external/protobuf:libprotobuf-proto", } +// Partitions srcs by the pkg it is in +// srcs has been created using `TransformSubpackagePaths` +// This function uses existence of Android.bp/BUILD files to create a label that is compatible with the package structure of bp2build workspace +func partitionSrcsByPackage(currentDir string, srcs bazel.LabelList) map[string]bazel.LabelList { + getPackageFromLabel := func(label string) string { + // Remove any preceding // + label = strings.TrimPrefix(label, "//") + split := strings.Split(label, ":") + if len(split) == 1 { + // e.g. foo.proto + return currentDir + } else if split[0] == "" { + // e.g. :foo.proto + return currentDir + } else { + return split[0] + } + } + + pkgToSrcs := map[string]bazel.LabelList{} + for _, src := range srcs.Includes { + pkg := getPackageFromLabel(src.Label) + list := pkgToSrcs[pkg] + list.Add(&src) + pkgToSrcs[pkg] = list + } + return pkgToSrcs +} + // Bp2buildProtoProperties converts proto properties, creating a proto_library and returning the // information necessary for language-specific handling. func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs bazel.LabelListAttribute) (Bp2buildProtoInfo, bool) { @@ -197,54 +227,73 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz } } - info.Name = m.Name() + "_proto" + name := m.Name() + "_proto" + + depsFromFilegroup := protoLibraries if len(directProtoSrcs.Includes) > 0 { - attrs := ProtoAttrs{ - Srcs: bazel.MakeLabelListAttribute(directProtoSrcs), - } - attrs.Deps.Append(bazel.MakeLabelListAttribute(protoLibraries)) + pkgToSrcs := partitionSrcsByPackage(ctx.ModuleDir(), directProtoSrcs) + for _, pkg := range SortedStringKeys(pkgToSrcs) { + srcs := pkgToSrcs[pkg] + attrs := ProtoAttrs{ + Srcs: bazel.MakeLabelListAttribute(srcs), + } + attrs.Deps.Append(bazel.MakeLabelListAttribute(depsFromFilegroup)) - for axis, configToProps := range m.GetArchVariantProperties(ctx, &ProtoProperties{}) { - for _, rawProps := range configToProps { - var props *ProtoProperties - var ok bool - if props, ok = rawProps.(*ProtoProperties); !ok { - ctx.ModuleErrorf("Could not cast ProtoProperties to expected type") - } - if axis == bazel.NoConfigAxis { - info.Type = props.Proto.Type - - if !proptools.BoolDefault(props.Proto.Canonical_path_from_root, canonicalPathFromRootDefault) { - // an empty string indicates to strips the package path - path := "" - attrs.Strip_import_prefix = &path + for axis, configToProps := range m.GetArchVariantProperties(ctx, &ProtoProperties{}) { + for _, rawProps := range configToProps { + var props *ProtoProperties + var ok bool + if props, ok = rawProps.(*ProtoProperties); !ok { + ctx.ModuleErrorf("Could not cast ProtoProperties to expected type") } + if axis == bazel.NoConfigAxis { + info.Type = props.Proto.Type - for _, dir := range props.Proto.Include_dirs { - if dep, ok := includeDirsToProtoDeps[dir]; ok { - attrs.Deps.Add(bazel.MakeLabelAttribute(dep)) - } else { - ctx.PropertyErrorf("Could not find the proto_library target for include dir", dir) + if !proptools.BoolDefault(props.Proto.Canonical_path_from_root, canonicalPathFromRootDefault) { + // an empty string indicates to strips the package path + path := "" + attrs.Strip_import_prefix = &path } + + for _, dir := range props.Proto.Include_dirs { + if dep, ok := includeDirsToProtoDeps[dir]; ok { + attrs.Deps.Add(bazel.MakeLabelAttribute(dep)) + } else { + ctx.PropertyErrorf("Could not find the proto_library target for include dir", dir) + } + } + } else if props.Proto.Type != info.Type && props.Proto.Type != nil { + ctx.ModuleErrorf("Cannot handle arch-variant types for protos at this time.") } - } else if props.Proto.Type != info.Type && props.Proto.Type != nil { - ctx.ModuleErrorf("Cannot handle arch-variant types for protos at this time.") } } + + tags := ApexAvailableTagsWithoutTestApexes(ctx.(TopDownMutatorContext), ctx.Module()) + + // Since we are creating the proto_library in a subpackage, create an import_prefix relative to the current package + if rel, err := filepath.Rel(ctx.ModuleDir(), pkg); err != nil { + ctx.ModuleErrorf("Could not get relative path for %v %v", pkg, err) + } else if rel != "." { + attrs.Import_prefix = &rel + } + + ctx.CreateBazelTargetModule( + bazel.BazelTargetModuleProperties{Rule_class: "proto_library"}, + CommonAttributes{Name: name, Dir: proptools.StringPtr(pkg), Tags: tags}, + &attrs, + ) + + l := "" + if pkg == ctx.ModuleDir() { // same package that the original module lives in + l = ":" + name + } else { + l = "//" + pkg + ":" + name + } + protoLibraries.Add(&bazel.Label{ + Label: l, + }) } - - tags := ApexAvailableTagsWithoutTestApexes(ctx.(TopDownMutatorContext), ctx.Module()) - - ctx.CreateBazelTargetModule( - bazel.BazelTargetModuleProperties{Rule_class: "proto_library"}, - CommonAttributes{Name: info.Name, Tags: tags}, - &attrs, - ) - - protoLibraries.Add(&bazel.Label{ - Label: ":" + info.Name, - }) } info.Proto_libs = protoLibraries diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 4d1d171e4..8ee0439c0 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -1949,3 +1949,46 @@ func TestPrettyPrintSelectMapEqualValues(t *testing.T) { actual, _ := prettyPrintAttribute(lla, 0) android.AssertStringEquals(t, "Print the common value if all keys in an axis have the same value", `[":libfoo.impl"]`, actual) } + +// If CommonAttributes.Dir is set, the bazel target should be created in that dir +func TestCreateBazelTargetInDifferentDir(t *testing.T) { + t.Parallel() + bp := ` + custom { + name: "foo", + dir: "subdir", + } + ` + registerCustomModule := func(ctx android.RegistrationContext) { + ctx.RegisterModuleType("custom", customModuleFactoryHostAndDevice) + } + // Check that foo is not created in root dir + RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{ + Description: "foo is not created in root dir because it sets dir explicitly", + Blueprint: bp, + Filesystem: map[string]string{ + "subdir/Android.bp": "", + }, + ExpectedBazelTargets: []string{}, + }) + // Check that foo is created in `subdir` + RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{ + Description: "foo is created in `subdir` because it sets dir explicitly", + Blueprint: bp, + Filesystem: map[string]string{ + "subdir/Android.bp": "", + }, + Dir: "subdir", + ExpectedBazelTargets: []string{ + MakeBazelTarget("custom", "foo", AttrNameToString{}), + }, + }) + // Check that we cannot create target in different dir if it is does not an Android.bp + RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{ + Description: "foo cannot be created in `subdir` because it does not contain an Android.bp file", + Blueprint: bp, + Dir: "subdir", + ExpectedErr: fmt.Errorf("Cannot use ca.Dir to create a BazelTarget in dir: subdir since it does not contain an Android.bp file"), + }) + +} diff --git a/bp2build/bzl_conversion_test.go b/bp2build/bzl_conversion_test.go index 9d9f456d4..402d4b013 100644 --- a/bp2build/bzl_conversion_test.go +++ b/bp2build/bzl_conversion_test.go @@ -94,6 +94,7 @@ custom = rule( # bazel_module end "bool_prop": attr.bool(), "bool_ptr_prop": attr.bool(), + "dir": attr.string(), "embedded_prop": attr.string(), "int64_ptr_prop": attr.int(), # nested_props start @@ -126,6 +127,7 @@ custom_defaults = rule( "arch_paths_exclude": attr.string_list(), "bool_prop": attr.bool(), "bool_ptr_prop": attr.bool(), + "dir": attr.string(), "embedded_prop": attr.string(), "int64_ptr_prop": attr.int(), # nested_props start @@ -158,6 +160,7 @@ custom_test_ = rule( "arch_paths_exclude": attr.string_list(), "bool_prop": attr.bool(), "bool_ptr_prop": attr.bool(), + "dir": attr.string(), "embedded_prop": attr.string(), "int64_ptr_prop": attr.int(), # nested_props start diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 6b17fc40b..e425b364f 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -2434,11 +2434,17 @@ cc_library { }), MakeBazelTarget("cc_library_shared", "a", AttrNameToString{ "dynamic_deps": `[":libprotobuf-cpp-lite"]`, "whole_archive_deps": `[":a_cc_proto_lite"]`, - }), MakeBazelTargetNoRestrictions("proto_library", "a_fg_proto_bp2build_converted", AttrNameToString{ + }), MakeBazelTargetNoRestrictions("proto_library", "a_fg_proto_proto", AttrNameToString{ "srcs": `["a_fg.proto"]`, "tags": `[ "apex_available=//apex_available:anyapex", "manual", + ]`, + }), MakeBazelTargetNoRestrictions("alias", "a_fg_proto_bp2build_converted", AttrNameToString{ + "actual": `"//.:a_fg_proto_proto"`, + "tags": `[ + "apex_available=//apex_available:anyapex", + "manual", ]`, }), MakeBazelTargetNoRestrictions("filegroup", "a_fg_proto", AttrNameToString{ "srcs": `["a_fg.proto"]`, @@ -2476,11 +2482,17 @@ cc_library { }), MakeBazelTarget("cc_library_shared", "a", AttrNameToString{ "dynamic_deps": `[":libprotobuf-cpp-lite"]`, "whole_archive_deps": `[":a_cc_proto_lite"]`, - }), MakeBazelTargetNoRestrictions("proto_library", "a_fg_proto_bp2build_converted", AttrNameToString{ + }), MakeBazelTargetNoRestrictions("proto_library", "a_fg_proto_proto", AttrNameToString{ "srcs": `["a_fg.proto"]`, "tags": `[ "apex_available=//apex_available:anyapex", "manual", + ]`, + }), MakeBazelTargetNoRestrictions("alias", "a_fg_proto_bp2build_converted", AttrNameToString{ + "actual": `"//.:a_fg_proto_proto"`, + "tags": `[ + "apex_available=//apex_available:anyapex", + "manual", ]`, }), MakeBazelTargetNoRestrictions("filegroup", "a_fg_proto", AttrNameToString{ "srcs": `["a_fg.proto"]`, @@ -4903,3 +4915,67 @@ cc_library_shared { }, }) } + +// Bazel enforces that proto_library and the .proto file are in the same bazel package +func TestGenerateProtoLibraryInSamePackage(t *testing.T) { + tc := Bp2buildTestCase{ + Description: "cc_library depends on .proto files from multiple packages", + ModuleTypeUnderTest: "cc_library", + ModuleTypeUnderTestFactory: cc.LibraryFactory, + Blueprint: ` +cc_library_static { + name: "foo", + srcs: [ + "foo.proto", + "bar/bar.proto", // Different package because there is a bar/Android.bp + "baz/subbaz/baz.proto", // Different package because there is baz/subbaz/Android.bp + ], +} +` + simpleModuleDoNotConvertBp2build("cc_library", "libprotobuf-cpp-lite"), + Filesystem: map[string]string{ + "bar/Android.bp": "", + "baz/subbaz/Android.bp": "", + }, + } + + // We will run the test 3 times and check in the root, bar and baz/subbaz directories + // Root dir + tc.ExpectedBazelTargets = []string{ + MakeBazelTarget("cc_library_static", "foo", AttrNameToString{ + "local_includes": `["."]`, + "deps": `[":libprotobuf-cpp-lite"]`, + "implementation_whole_archive_deps": `[":foo_cc_proto_lite"]`, + }), + MakeBazelTarget("proto_library", "foo_proto", AttrNameToString{ + "srcs": `["foo.proto"]`, + }), + MakeBazelTarget("cc_lite_proto_library", "foo_cc_proto_lite", AttrNameToString{ + "deps": `[ + ":foo_proto", + "//bar:foo_proto", + "//baz/subbaz:foo_proto", + ]`, + }), + } + runCcLibraryTestCase(t, tc) + + // bar dir + tc.Dir = "bar" + tc.ExpectedBazelTargets = []string{ + MakeBazelTarget("proto_library", "foo_proto", AttrNameToString{ + "srcs": `["//bar:bar.proto"]`, + "import_prefix": `"bar"`, + }), + } + runCcLibraryTestCase(t, tc) + + // baz/subbaz dir + tc.Dir = "baz/subbaz" + tc.ExpectedBazelTargets = []string{ + MakeBazelTarget("proto_library", "foo_proto", AttrNameToString{ + "srcs": `["//baz/subbaz:baz.proto"]`, + "import_prefix": `"baz/subbaz"`, + }), + } + runCcLibraryTestCase(t, tc) +} diff --git a/bp2build/filegroup_conversion_test.go b/bp2build/filegroup_conversion_test.go index 7ce559d9b..cb2e2076a 100644 --- a/bp2build/filegroup_conversion_test.go +++ b/bp2build/filegroup_conversion_test.go @@ -137,12 +137,19 @@ filegroup { path: "proto", }`, ExpectedBazelTargets: []string{ - MakeBazelTargetNoRestrictions("proto_library", "foo_bp2build_converted", AttrNameToString{ + MakeBazelTargetNoRestrictions("proto_library", "foo_proto", AttrNameToString{ "srcs": `["proto/foo.proto"]`, "strip_import_prefix": `"proto"`, "tags": `[ "apex_available=//apex_available:anyapex", "manual", + ]`, + }), + MakeBazelTargetNoRestrictions("alias", "foo_bp2build_converted", AttrNameToString{ + "actual": `"//.:foo_proto"`, + "tags": `[ + "apex_available=//apex_available:anyapex", + "manual", ]`, }), MakeBazelTargetNoRestrictions("filegroup", "foo", AttrNameToString{ @@ -170,3 +177,27 @@ filegroup { ]`}), }}) } + +func TestFilegroupWithProtoInDifferentPackage(t *testing.T) { + runFilegroupTestCase(t, Bp2buildTestCase{ + Description: "filegroup with .proto in different package", + Filesystem: map[string]string{ + "subdir/Android.bp": "", + }, + Blueprint: ` +filegroup { + name: "foo", + srcs: ["subdir/foo.proto"], +}`, + Dir: "subdir", // check in subdir + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("proto_library", "foo_proto", AttrNameToString{ + "srcs": `["//subdir:foo.proto"]`, + "import_prefix": `"subdir"`, + "strip_import_prefix": `""`, + "tags": `[ + "apex_available=//apex_available:anyapex", + "manual", + ]`}), + }}) +} diff --git a/bp2build/testing.go b/bp2build/testing.go index 140b214cf..18ae82de9 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -336,6 +336,8 @@ type customProps struct { Api *string // File describing the APIs of this module Test_config_setting *bool // Used to test generation of config_setting targets + + Dir *string // Dir in which the Bazel Target will be created } type customModule struct { @@ -461,6 +463,10 @@ type customBazelModuleAttributes struct { Api bazel.LabelAttribute } +func (m *customModule) dir() *string { + return m.props.Dir +} + func (m *customModule) ConvertWithBp2build(ctx android.TopDownMutatorContext) { if p := m.props.One_to_many_prop; p != nil && *p { customBp2buildOneToMany(ctx, m) @@ -508,7 +514,7 @@ func (m *customModule) ConvertWithBp2build(ctx android.TopDownMutatorContext) { Rule_class: "custom", } - ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name()}, attrs) + ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: m.Name(), Dir: m.dir()}, attrs) if proptools.Bool(m.props.Test_config_setting) { m.createConfigSetting(ctx) diff --git a/python/bp2build.go b/python/bp2build.go index 60cabc410..41e9f4974 100644 --- a/python/bp2build.go +++ b/python/bp2build.go @@ -73,7 +73,6 @@ func (m *PythonLibraryModule) makeArchVariantBaseAttributes(ctx android.TopDownM if !partitionedSrcs["proto"].IsEmpty() { protoInfo, _ := android.Bp2buildProtoProperties(ctx, &m.ModuleBase, partitionedSrcs["proto"]) - protoLabel := bazel.Label{Label: ":" + protoInfo.Name} pyProtoLibraryName := m.Name() + "_py_proto" ctx.CreateBazelTargetModule(bazel.BazelTargetModuleProperties{ @@ -82,7 +81,7 @@ func (m *PythonLibraryModule) makeArchVariantBaseAttributes(ctx android.TopDownM }, android.CommonAttributes{ Name: pyProtoLibraryName, }, &bazelPythonProtoLibraryAttributes{ - Deps: bazel.MakeSingleLabelListAttribute(protoLabel), + Deps: bazel.MakeLabelListAttribute(protoInfo.Proto_libs), }) attrs.Deps.Add(bazel.MakeLabelAttribute(":" + pyProtoLibraryName))