From 4da55f59f8538c30513942a6acabdfb88654d8d2 Mon Sep 17 00:00:00 2001 From: Vihang Mehta Date: Wed, 14 Feb 2024 09:41:35 -0800 Subject: [PATCH] Update the strip prefix logic for python libraries (#361) * Fix imports directive for py_library when using stripPrefix Signed-off-by: Vihang Mehta * Tests Signed-off-by: Vihang Mehta --------- Signed-off-by: Vihang Mehta --- .../module_lib/util/nested/prefix/BUILD.in | 1 + .../module_lib/util/nested/prefix/BUILD.out | 28 +++++++++++++++++++ .../module_lib/util/nested/prefix/test.proto | 7 +++++ .../rules_python/proto_py_library_test.go | 3 +- pkg/rule/rules_python/py_library.go | 13 +++++++-- 5 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.in create mode 100644 example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.out create mode 100644 example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/test.proto diff --git a/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.in b/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.in new file mode 100644 index 000000000..9a77193a2 --- /dev/null +++ b/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.in @@ -0,0 +1 @@ +# gazelle:proto_strip_import_prefix /module_lib/util/nested diff --git a/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.out b/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.out new file mode 100644 index 000000000..6ae2b58bc --- /dev/null +++ b/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/BUILD.out @@ -0,0 +1,28 @@ +load("@rules_proto//proto:defs.bzl", "proto_library") +load("@build_stack_rules_proto//rules/py:proto_py_library.bzl", "proto_py_library") +load("@build_stack_rules_proto//rules:proto_compile.bzl", "proto_compile") + +# gazelle:proto_strip_import_prefix /module_lib/util/nested + +proto_library( + name = "prefix_test_proto", + srcs = ["test.proto"], + strip_import_prefix = "/module_lib/util/nested", + visibility = ["//visibility:public"], +) + +proto_compile( + name = "prefix_test_python_compile", + output_mappings = ["test_pb2.py=/prefix/test_pb2.py"], + outputs = ["test_pb2.py"], + plugins = ["@build_stack_rules_proto//plugin/builtin:python"], + proto = "prefix_test_proto", +) + +proto_py_library( + name = "prefix_test_py_library", + srcs = ["test_pb2.py"], + imports = [".."], + visibility = ["//visibility:public"], + deps = ["@com_google_protobuf//:protobuf_python"], +) diff --git a/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/test.proto b/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/test.proto new file mode 100644 index 000000000..3f84cb257 --- /dev/null +++ b/example/golden/testdata/strip_import_prefix/module_lib/util/nested/prefix/test.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package prefix.test; + +message Data { + int64 id = 1; +} diff --git a/pkg/rule/rules_python/proto_py_library_test.go b/pkg/rule/rules_python/proto_py_library_test.go index d48ae9ac4..5c776788c 100644 --- a/pkg/rule/rules_python/proto_py_library_test.go +++ b/pkg/rule/rules_python/proto_py_library_test.go @@ -64,12 +64,13 @@ proto_py_library( Outputs: []string{"test_pb2.py"}, }, }, + Rel: "com/foo/baz/qux/v1", }, want: ` proto_py_library( name = "test_py_library", srcs = ["test_pb2.py"], - imports = ["../.."], + imports = ["../../.."], ) `, }, diff --git a/pkg/rule/rules_python/py_library.go b/pkg/rule/rules_python/py_library.go index 6aea634d2..106c3c277 100644 --- a/pkg/rule/rules_python/py_library.go +++ b/pkg/rule/rules_python/py_library.go @@ -1,6 +1,7 @@ package rules_python import ( + "path/filepath" "strings" "github.com/bazelbuild/bazel-gazelle/config" @@ -71,7 +72,8 @@ func (s *PyLibrary) Visibility() []string { func (s *PyLibrary) ImportsAttr() (imps []string) { // if we have a strip_import_prefix on the proto_library, the python search // path should include the directory N parents above the current package, - // where N is the number of segments in an absolute strip_import_prefix + // where N is the number of segments needed to ascend to the prefix from + // the dir for the current rule. if s.Config.Library.StripImportPrefix() == "" { return } @@ -79,9 +81,14 @@ func (s *PyLibrary) ImportsAttr() (imps []string) { if !strings.HasPrefix(prefix, "/") { return // deal with relative-imports at another time } + prefix = strings.TrimPrefix(prefix, "/") - prefix = strings.TrimSuffix(prefix, "/") - parts := strings.Split(prefix, "/") + rel, err := filepath.Rel(prefix, s.Config.Rel) + if err != nil { + return // the prefix doesn't prefix the current path, shouldn't happen + } + + parts := strings.Split(rel, "/") for i := 0; i < len(parts); i++ { parts[i] = ".." }