From cb847638af5a56969929ddeae57d407fc8ad98c0 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 22 Aug 2023 19:24:07 +0000 Subject: [PATCH 1/2] Translate python_libray.pkg_path to proto.import_prefix If a python_library uses a pkg_path foo/bar, then the proto srcs in that libray need to import the dep .proto as foo/bar/proto.proto. This behavior is restricted to python modules. To implement this is in bp2build, this CL creates a new interface with a single method `PkgPath`. Only python module structs implement this interface, and this method is only available during bp2build Test: Added a bp2build unit test Test: TH Change-Id: If8d207c0b321f75337a053795826b283a5eaaf46 --- android/proto.go | 12 +++++++ bp2build/python_library_conversion_test.go | 40 ++++++++++++++++++++++ python/python.go | 8 +++++ 3 files changed, 60 insertions(+) diff --git a/android/proto.go b/android/proto.go index 6887900c4..b9365a70f 100644 --- a/android/proto.go +++ b/android/proto.go @@ -281,6 +281,13 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz } } + if p, ok := m.module.(PkgPathInterface); ok && p.PkgPath(ctx) != nil { + // python_library with pkg_path + // proto_library for this module should have the pkg_path as the import_prefix + attrs.Import_prefix = p.PkgPath(ctx) + attrs.Strip_import_prefix = proptools.StringPtr("") + } + tags := ApexAvailableTagsWithoutTestApexes(ctx.(TopDownMutatorContext), ctx.Module()) moduleDir := ctx.ModuleDir() @@ -333,6 +340,11 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz return info, true } +// PkgPathInterface is used as a type assertion in bp2build to get pkg_path property of python_library_host +type PkgPathInterface interface { + PkgPath(ctx BazelConversionContext) *string +} + var ( protoIncludeDirGeneratedSuffix = ".include_dir_bp2build_generated_proto" protoIncludeDirsBp2buildKey = NewOnceKey("protoIncludeDirsBp2build") diff --git a/bp2build/python_library_conversion_test.go b/bp2build/python_library_conversion_test.go index a53371d2b..595acd2e4 100644 --- a/bp2build/python_library_conversion_test.go +++ b/bp2build/python_library_conversion_test.go @@ -348,3 +348,43 @@ func TestPythonLibraryWithProtobufs(t *testing.T) { }, }) } + +func TestPythonLibraryWithProtobufsAndPkgPath(t *testing.T) { + t.Parallel() + runBp2BuildTestCaseWithPythonLibraries(t, Bp2buildTestCase{ + Description: "test python_library protobuf with pkg_path", + Filesystem: map[string]string{ + "dir/foo.proto": "", + "dir/bar.proto": "", // bar contains "import dir/foo.proto" + "dir/Android.bp": ` +python_library { + name: "foo", + pkg_path: "dir", + srcs: [ + "foo.proto", + "bar.proto", + ], + bazel_module: {bp2build_available: true}, +}`, + }, + Dir: "dir", + ExpectedBazelTargets: []string{ + MakeBazelTarget("proto_library", "foo_proto", AttrNameToString{ + "import_prefix": `"dir"`, + "strip_import_prefix": `""`, + "srcs": `[ + "foo.proto", + "bar.proto", + ]`, + }), + MakeBazelTarget("py_proto_library", "foo_py_proto", AttrNameToString{ + "deps": `[":foo_proto"]`, + }), + MakeBazelTarget("py_library", "foo", AttrNameToString{ + "srcs_version": `"PY3"`, + "imports": `[".."]`, + "deps": `[":foo_py_proto"]`, + }), + }, + }) +} diff --git a/python/python.go b/python/python.go index 6c837a888..7d77ca772 100644 --- a/python/python.go +++ b/python/python.go @@ -197,6 +197,14 @@ func (p *PythonLibraryModule) getPkgPath() string { return String(p.properties.Pkg_path) } +// PkgPath is the "public" version of `getPkgPath` that is only available during bp2build +func (p *PythonLibraryModule) PkgPath(ctx android.BazelConversionContext) *string { + if ctx.Config().BuildMode != android.Bp2build { + ctx.ModuleErrorf("PkgPath is only supported in bp2build mode") + } + return p.properties.Pkg_path +} + func (p *PythonLibraryModule) getBaseProperties() *BaseProperties { return &p.properties } From e8a90c57e0500bc25aba0e665cd3032c496d9571 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Thu, 24 Aug 2023 00:30:13 +0000 Subject: [PATCH 2/2] Handle proto.include_dirs for java The proto_library(s) created for include_dirs will be added to transitive_deps This also fixes an existing bug for java_library containing .protos in srcs via filegroups. ``` java_library { name: "foo", srcs: ["foo.proto", "foo_filegroup"], } ``` At ToT, foo_filegroup was missing from the equivalent proto_library in bp2build workspace. Bug: 285140726 Test: allowlisted pandora-proto-java and built that Change-Id: I2657d8cdef2e47434bc3e0d09a074c8e27299afc --- java/proto.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/java/proto.go b/java/proto.go index c732d9842..2ed7b27e9 100644 --- a/java/proto.go +++ b/java/proto.go @@ -143,7 +143,14 @@ func protoFlags(ctx android.ModuleContext, j *CommonProperties, p *android.Proto } type protoAttributes struct { - Deps bazel.LabelListAttribute + Deps bazel.LabelListAttribute + + // A list of proto_library targets that the proto_library in `deps` depends on + // This list is overestimation. + // Overestimation is necessary since Soong includes other protos via proto.include_dirs and not + // a specific .proto file module explicitly. + Transitive_deps bazel.LabelListAttribute + Sdk_version bazel.StringAttribute Java_version bazel.StringAttribute } @@ -176,11 +183,11 @@ func bp2buildProto(ctx android.Bp2buildMutatorContext, m *Module, protoSrcs baze ctx.PropertyErrorf("proto.type", "cannot handle conversion at this time: %q", typ) } - protoLabel := bazel.Label{Label: ":" + m.Name() + "_proto"} protoAttrs := &protoAttributes{ - Deps: bazel.MakeSingleLabelListAttribute(protoLabel), - Java_version: bazel.StringAttribute{Value: m.properties.Java_version}, - Sdk_version: bazel.StringAttribute{Value: m.deviceProperties.Sdk_version}, + Deps: bazel.MakeLabelListAttribute(protoInfo.Proto_libs), + Transitive_deps: bazel.MakeLabelListAttribute(protoInfo.Transitive_proto_libs), + Java_version: bazel.StringAttribute{Value: m.properties.Java_version}, + Sdk_version: bazel.StringAttribute{Value: m.deviceProperties.Sdk_version}, } name := m.Name() + suffix