From a99348dca45cb08c0876dc6c06b9c11096a7523f Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 1 Aug 2023 23:10:05 +0000 Subject: [PATCH] cc Bp2build support for genrules that generate .proto file If `srcs` contains a gensrcs/genrule module, the current bp2build module will put it in the catch-all `srcs` attribute. This is reserved for .cpp sources, so if the genrule produces a .proto/.aidl/... file, this will fail. This handles genrules that produce .proto files. To implement this, this creates an additional partition that detects if the other module is a genrule/gensrc that produces .proto files. If true, it will append it to the proto partition. This CL does not handle - genrule that produce .c/.aidl/.yacc/.... files. They will continue to be partitioned into the catch-all partition - java modules Test: unit tests Test: TH Bug: 293205700 Change-Id: Ib720fdf3a053ff5dd256e6faa632e3fa7776966d --- bp2build/cc_library_static_conversion_test.go | 35 ++++++++++++++ cc/bp2build.go | 47 +++++++++++++++++-- genrule/genrule.go | 30 ++++++++---- 3 files changed, 98 insertions(+), 14 deletions(-) diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 03e9cd060..c1eed9688 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -2247,3 +2247,38 @@ cc_library_static{ ]`, })}}) } + +func TestCcLibraryWithProtoInGeneratedSrcs(t *testing.T) { + runCcLibraryStaticTestCase(t, Bp2buildTestCase{ + Description: "cc_library with a .proto file generated from a genrule", + ModuleTypeUnderTest: "cc_library_static", + ModuleTypeUnderTestFactory: cc.LibraryStaticFactory, + Blueprint: soongCcLibraryPreamble + ` +cc_library_static { + name: "mylib", + generated_sources: ["myprotogen"], +} +genrule { + name: "myprotogen", + out: ["myproto.proto"], +} +` + simpleModuleDoNotConvertBp2build("cc_library", "libprotobuf-cpp-lite"), + ExpectedBazelTargets: []string{ + MakeBazelTarget("cc_library_static", "mylib", AttrNameToString{ + "local_includes": `["."]`, + "deps": `[":libprotobuf-cpp-lite"]`, + "implementation_whole_archive_deps": `[":mylib_cc_proto_lite"]`, + }), + MakeBazelTarget("cc_lite_proto_library", "mylib_cc_proto_lite", AttrNameToString{ + "deps": `[":mylib_proto"]`, + }), + MakeBazelTarget("proto_library", "mylib_proto", AttrNameToString{ + "srcs": `[":myprotogen"]`, + }), + MakeBazelTargetNoRestrictions("genrule", "myprotogen", AttrNameToString{ + "cmd": `""`, + "outs": `["myproto.proto"]`, + }), + }, + }) +} diff --git a/cc/bp2build.go b/cc/bp2build.go index 9e485d6d0..9d90a5b5d 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -48,6 +48,8 @@ const ( genrulePartition = "genrule" + protoFromGenPartition = "proto_gen" + hdrPartition = "hdr" stubsSuffix = "_stub_libs_current" @@ -126,6 +128,41 @@ func (m *Module) convertTidyAttributes(ctx android.BaseMutatorContext, moduleAtt } } +// Returns an unchanged label and a bool indicating whether the dep is a genrule that produces .proto files +func protoFromGenPartitionMapper(pathCtx android.BazelConversionContext) bazel.LabelMapper { + return func(ctx bazel.OtherModuleContext, label bazel.Label) (string, bool) { + mod, exists := ctx.ModuleFromName(label.OriginalModuleName) + if !exists { + return label.Label, false + } + gen, isGen := mod.(*genrule.Module) + if !isGen { + return label.Label, false + } + if containsProto(ctx, gen.RawOutputFiles(pathCtx)) { + // Return unmodified label + true + // True will ensure that this module gets dropped from `srcs` of the generated cc_* target. `srcs` is reserved for .cpp files + return label.Label, true + } + return label.Label, false + } +} + +// Returns true if srcs contains only .proto files +// Raises an exception if there is a combination of .proto and non .proto srcs +func containsProto(ctx bazel.OtherModuleContext, srcs []string) bool { + onlyProtos := false + for _, src := range srcs { + if strings.HasSuffix(src, ".proto") { + onlyProtos = true + } else if onlyProtos { + // This is not a proto file, and we have encountered a .proto file previously + ctx.ModuleErrorf("TOOD: Add bp2build support combination of .proto and non .proto files in outs of genrule") + } + } + return onlyProtos +} + // groupSrcsByExtension partitions `srcs` into groups based on file extension. func groupSrcsByExtension(ctx android.BazelConversionPathContext, srcs bazel.LabelListAttribute) bazel.PartitionToLabelListAttribute { // Convert filegroup dependencies into extension-specific filegroups filtered in the filegroup.bzl @@ -159,10 +196,11 @@ func groupSrcsByExtension(ctx android.BazelConversionPathContext, srcs bazel.Lab // contains .l or .ll files we will need to find a way to add a // LabelMapper for these that identifies these filegroups and // converts them appropriately - lSrcPartition: bazel.LabelPartition{Extensions: []string{".l"}}, - llSrcPartition: bazel.LabelPartition{Extensions: []string{".ll"}}, - rScriptSrcPartition: bazel.LabelPartition{Extensions: []string{".fs", ".rscript"}}, - xsdSrcPartition: bazel.LabelPartition{LabelMapper: android.XsdLabelMapper(xsdConfigCppTarget)}, + lSrcPartition: bazel.LabelPartition{Extensions: []string{".l"}}, + llSrcPartition: bazel.LabelPartition{Extensions: []string{".ll"}}, + rScriptSrcPartition: bazel.LabelPartition{Extensions: []string{".fs", ".rscript"}}, + xsdSrcPartition: bazel.LabelPartition{LabelMapper: android.XsdLabelMapper(xsdConfigCppTarget)}, + protoFromGenPartition: bazel.LabelPartition{LabelMapper: protoFromGenPartitionMapper(ctx)}, // C++ is the "catch-all" group, and comprises generated sources because we don't // know the language of these sources until the genrule is executed. cppSrcPartition: bazel.LabelPartition{Extensions: []string{".cpp", ".cc", ".cxx", ".mm"}, LabelMapper: addSuffixForFilegroup("_cpp_srcs"), Keep_remainder: true}, @@ -594,6 +632,7 @@ func (ca *compilerAttributes) finalize(ctx android.BazelConversionPathContext, i partitionedHdrs := partitionHeaders(ctx, exportHdrs) ca.protoSrcs = partitionedSrcs[protoSrcPartition] + ca.protoSrcs.Append(partitionedSrcs[protoFromGenPartition]) ca.aidlSrcs = partitionedSrcs[aidlSrcPartition] for p, lla := range partitionedSrcs { diff --git a/genrule/genrule.go b/genrule/genrule.go index 69ba1e94e..8a8d605bb 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -1012,16 +1012,7 @@ func (m *Module) ConvertWithBp2build(ctx android.TopDownMutatorContext) { Tags: tags, }, attrs) } else { - // The Out prop is not in an immediately accessible field - // in the Module struct, so use GetProperties and cast it - // to the known struct prop. - var outs []string - for _, propIntf := range m.GetProperties() { - if props, ok := propIntf.(*genRuleProperties); ok { - outs = props.Out - break - } - } + outs := m.RawOutputFiles(ctx) for _, out := range outs { if out == bazelName { // This is a workaround to circumvent a Bazel warning where a genrule's @@ -1096,6 +1087,25 @@ type ccHeaderLibraryAttrs struct { Export_includes []string } +// RawOutputFfiles returns the raw outputs specified in Android.bp +// This does not contain the fully resolved path relative to the top of the tree +func (g *Module) RawOutputFiles(ctx android.BazelConversionContext) []string { + if ctx.Config().BuildMode != android.Bp2build { + ctx.ModuleErrorf("RawOutputFiles is only supported in bp2build mode") + } + // The Out prop is not in an immediately accessible field + // in the Module struct, so use GetProperties and cast it + // to the known struct prop. + var outs []string + for _, propIntf := range g.GetProperties() { + if props, ok := propIntf.(*genRuleProperties); ok { + outs = props.Out + break + } + } + return outs +} + var Bool = proptools.Bool var String = proptools.String