From 7dc6bcbd5880d1190334d55fda8d4646a19a33ba Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Thu, 10 Aug 2023 12:51:59 -0400 Subject: [PATCH] fix protos in another dir + a module that uses it Protos in another directory were using import prefix, which was prepending the repository-relative path with the value, instead, we want to strip the prefix of the directory of the module the protos were used in such that they can be referenced at the appropriate relative path. Reference on import_prefix: https://bazel.build/reference/be/protocol-buffer#proto_library.import_prefix Test: b build //packages/modules/adb:libfastdeploy_host Change-Id: If050b0f5fc5103bd9cc5a99703bd604325aa4204 --- android/allowlists/allowlists.go | 1 - android/proto.go | 19 ++++--- bp2build/cc_library_conversion_test.go | 79 ++++++++++++++++++++++++-- 3 files changed, 87 insertions(+), 12 deletions(-) diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index a1d7fbb82..b921e4116 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -964,7 +964,6 @@ var ( "libdebuggerd_handler", // depends on unconverted module libdebuggerd_handler_core "libdebuggerd_handler_core", "libdebuggerd_handler_fallback", // depends on unconverted module libdebuggerd "libdexfiled", // depends on unconverted modules: dexfile_operator_srcs, libartbased, libartpalette - "libfastdeploy_host", // depends on unconverted modules: libandroidfw, libusb, AdbWinApi "libgmock_main_ndk", // depends on unconverted modules: libgtest_ndk_c++ "libgmock_ndk", // depends on unconverted modules: libgtest_ndk_c++ "libnativehelper_lazy_mts_jni", "libnativehelper_mts_jni", // depends on unconverted modules: libnativetesthelper_jni, libgmock_ndk diff --git a/android/proto.go b/android/proto.go index aad521ba0..49b3733d2 100644 --- a/android/proto.go +++ b/android/proto.go @@ -230,6 +230,7 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz name := m.Name() + "_proto" depsFromFilegroup := protoLibraries + var canonicalPathFromRoot bool if len(directProtoSrcs.Includes) > 0 { pkgToSrcs := partitionSrcsByPackage(ctx.ModuleDir(), directProtoSrcs) @@ -250,7 +251,8 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz if axis == bazel.NoConfigAxis { info.Type = props.Proto.Type - if !proptools.BoolDefault(props.Proto.Canonical_path_from_root, canonicalPathFromRootDefault) { + canonicalPathFromRoot = proptools.BoolDefault(props.Proto.Canonical_path_from_root, canonicalPathFromRootDefault) + if !canonicalPathFromRoot { // an empty string indicates to strips the package path path := "" attrs.Strip_import_prefix = &path @@ -271,11 +273,14 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz 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 + moduleDir := ctx.ModuleDir() + if !canonicalPathFromRoot { + // Since we are creating the proto_library in a subpackage, set the import_prefix relative to the current package + if rel, err := filepath.Rel(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( @@ -285,7 +290,7 @@ func Bp2buildProtoProperties(ctx Bp2buildMutatorContext, m *ModuleBase, srcs baz ) l := "" - if pkg == ctx.ModuleDir() { // same package that the original module lives in + if pkg == moduleDir { // same package that the original module lives in l = ":" + name } else { l = "//" + pkg + ":" + name diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index e425b364f..e5ae73e48 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -4930,6 +4930,9 @@ cc_library_static { "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 ], + proto: { + canonical_path_from_root: true, + } } ` + simpleModuleDoNotConvertBp2build("cc_library", "libprotobuf-cpp-lite"), Filesystem: map[string]string{ @@ -4963,8 +4966,7 @@ cc_library_static { tc.Dir = "bar" tc.ExpectedBazelTargets = []string{ MakeBazelTarget("proto_library", "foo_proto", AttrNameToString{ - "srcs": `["//bar:bar.proto"]`, - "import_prefix": `"bar"`, + "srcs": `["//bar:bar.proto"]`, }), } runCcLibraryTestCase(t, tc) @@ -4973,8 +4975,77 @@ cc_library_static { tc.Dir = "baz/subbaz" tc.ExpectedBazelTargets = []string{ MakeBazelTarget("proto_library", "foo_proto", AttrNameToString{ - "srcs": `["//baz/subbaz:baz.proto"]`, - "import_prefix": `"baz/subbaz"`, + "srcs": `["//baz/subbaz:baz.proto"]`, + }), + } + runCcLibraryTestCase(t, tc) +} + +// Bazel enforces that proto_library and the .proto file are in the same bazel package +func TestGenerateProtoLibraryInSamePackageNotCanonicalFromRoot(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 + ], + proto: { + canonical_path_from_root: false, + } +} +` + 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"]`, + "strip_import_prefix": `""`, + }), + 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"]`, + "strip_import_prefix": `""`, + "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"]`, + "strip_import_prefix": `""`, + "import_prefix": `"baz/subbaz"`, }), } runCcLibraryTestCase(t, tc)