From c71f266060d2e5ae8e40399b206990775e0a6bfb Mon Sep 17 00:00:00 2001 From: Will Schmitt Date: Sun, 11 Aug 2024 13:52:58 -0700 Subject: [PATCH 1/3] fix: join `--descriptor_set_in` with host path separator Fixes #670. As described in #670, protoc splits the arguments to `--proto_path` and `--descriptor_set_in` using an OS-specific path-separator. On posix, this is `:`, but on Windows this is `;`. The protobuf library takes the approach for its bazel rules to join on `ctx.configuration.host_path_separator`, so I've taken the same approach here as well. --- ts/private/ts_proto_library.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ts/private/ts_proto_library.bzl b/ts/private/ts_proto_library.bzl index ccd08927..77884c77 100644 --- a/ts/private/ts_proto_library.bzl +++ b/ts/private/ts_proto_library.bzl @@ -47,7 +47,7 @@ def _protoc_action(ctx, proto_info, outputs): args.add_joined(["--connect-query_out", ctx.bin_dir.path], join_with = "=") args.add("--descriptor_set_in") - args.add_joined(proto_info.transitive_descriptor_sets, join_with = ":") + args.add_joined(proto_info.transitive_descriptor_sets, join_with = ctx.configuration.host_path_separator) args.add_all(proto_info.direct_sources) From f00e0460e94bac7900b84a82cba8b202f271b05d Mon Sep 17 00:00:00 2001 From: Will Schmitt Date: Mon, 4 Nov 2024 14:24:09 -0800 Subject: [PATCH 2/3] fixup: add simple unit test for ts_proto_library This only simply tests a basic message that depends on a message in another package, but for the scope of the current issue is appropriate. It's probably worth testing grpc functionality, too, but the examples package provides coverage. --- ts/test/BUILD.bazel | 3 +++ ts/test/ts_proto_library/BUILD.bazel | 27 +++++++++++++++++++++++ ts/test/ts_proto_library/bar.proto | 4 ++++ ts/test/ts_proto_library/foo.proto | 8 +++++++ ts/test/ts_proto_library_test.bzl | 33 ++++++++++++++++++++++++++++ 5 files changed, 75 insertions(+) create mode 100644 ts/test/ts_proto_library/BUILD.bazel create mode 100644 ts/test/ts_proto_library/bar.proto create mode 100644 ts/test/ts_proto_library/foo.proto create mode 100644 ts/test/ts_proto_library_test.bzl diff --git a/ts/test/BUILD.bazel b/ts/test/BUILD.bazel index 9d73d734..84528371 100644 --- a/ts/test/BUILD.bazel +++ b/ts/test/BUILD.bazel @@ -4,6 +4,7 @@ load(":flags_test.bzl", "ts_project_flags_test_suite") load(":mock_transpiler.bzl", "mock") load(":transpiler_tests.bzl", "transpiler_test_suite") load(":ts_project_test.bzl", "ts_project_test_suite") +load(":ts_proto_library_test.bzl", "ts_proto_library_test_suite") transpiler_test_suite() @@ -11,6 +12,8 @@ ts_project_test_suite(name = "ts_project_test") ts_project_flags_test_suite(name = "ts_project_flags_test") +ts_proto_library_test_suite(name = "ts_proto_library_test") + # Ensure that when determining output location, the `root_dir` attribute is only removed once. ts_project( name = "rootdir_works_with_repeated_directory", diff --git a/ts/test/ts_proto_library/BUILD.bazel b/ts/test/ts_proto_library/BUILD.bazel new file mode 100644 index 00000000..3b153d3a --- /dev/null +++ b/ts/test/ts_proto_library/BUILD.bazel @@ -0,0 +1,27 @@ +"Proto libraries for ts_proto_library tests." +# These are a concrete package rather than using `write_file` in the test file, +# since protoc would otherwise not find the proto files in the descriptor +# database. + +load("//ts:proto.bzl", "ts_proto_library") + +proto_library( + name = "bar_proto", + srcs = [ + ":bar.proto", + ], + tags = ["manual"], + visibility = ["//ts/test:__subpackages__"], +) + +proto_library( + name = "foo_proto", + srcs = [ + ":foo.proto", + ], + tags = ["manual"], + visibility = ["//ts/test:__subpackages__"], + deps = [ + ":bar_proto", + ], +) diff --git a/ts/test/ts_proto_library/bar.proto b/ts/test/ts_proto_library/bar.proto new file mode 100644 index 00000000..f04174d3 --- /dev/null +++ b/ts/test/ts_proto_library/bar.proto @@ -0,0 +1,4 @@ +syntax = "proto3"; +package bar; + +message Bar {} diff --git a/ts/test/ts_proto_library/foo.proto b/ts/test/ts_proto_library/foo.proto new file mode 100644 index 00000000..fb064e0a --- /dev/null +++ b/ts/test/ts_proto_library/foo.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; +package foo; + +import "ts/test/ts_proto_library/bar.proto"; + +message Foo { + bar.Bar bar = 1; +} diff --git a/ts/test/ts_proto_library_test.bzl b/ts/test/ts_proto_library_test.bzl new file mode 100644 index 00000000..b29a72da --- /dev/null +++ b/ts/test/ts_proto_library_test.bzl @@ -0,0 +1,33 @@ +"UnitTest for ts_proto_library" + +load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("@npm//:defs.bzl", "npm_link_all_packages") +load("@rules_proto//proto:defs.bzl", "proto_library") +load("//ts:proto.bzl", "ts_proto_library") + +def ts_proto_library_test_suite(name): + """Test suite including all tests and data. + + Args: + name: The name of the test suite. + """ + + ts_proto_library( + name = "ts_proto_library_with_dep", + # The //examples package is the root pnpm package for the repo, so we + # borrow from the proto/grpc example to provide the required + # ts_proto_library npm dependencies. + node_modules = "//examples/proto_grpc:node_modules", + proto = "//ts/test/ts_proto_library:foo_proto", + proto_srcs = ["//ts/test/ts_proto_library:foo.proto"], + # This is disabled to avoid checking in the output files, which are + # implicitly inputs for the copy_file tests. + copy_files = False, + tags = ["manual"], + ) + + build_test( + name = "ts_proto_library_with_dep_test", + targets = [":ts_proto_library_with_dep"], + ) From ad33d2f19b154a3b403b7684ec49465bef6820d0 Mon Sep 17 00:00:00 2001 From: Will Schmitt Date: Tue, 12 Nov 2024 10:34:16 -0800 Subject: [PATCH 3/3] fixup: remove unused imports --- ts/test/ts_proto_library/BUILD.bazel | 2 -- ts/test/ts_proto_library_test.bzl | 3 --- 2 files changed, 5 deletions(-) diff --git a/ts/test/ts_proto_library/BUILD.bazel b/ts/test/ts_proto_library/BUILD.bazel index 3b153d3a..b8377383 100644 --- a/ts/test/ts_proto_library/BUILD.bazel +++ b/ts/test/ts_proto_library/BUILD.bazel @@ -3,8 +3,6 @@ # since protoc would otherwise not find the proto files in the descriptor # database. -load("//ts:proto.bzl", "ts_proto_library") - proto_library( name = "bar_proto", srcs = [ diff --git a/ts/test/ts_proto_library_test.bzl b/ts/test/ts_proto_library_test.bzl index b29a72da..83203c3f 100644 --- a/ts/test/ts_proto_library_test.bzl +++ b/ts/test/ts_proto_library_test.bzl @@ -1,9 +1,6 @@ "UnitTest for ts_proto_library" load("@bazel_skylib//rules:build_test.bzl", "build_test") -load("@bazel_skylib//rules:write_file.bzl", "write_file") -load("@npm//:defs.bzl", "npm_link_all_packages") -load("@rules_proto//proto:defs.bzl", "proto_library") load("//ts:proto.bzl", "ts_proto_library") def ts_proto_library_test_suite(name):