Skip to content

Commit

Permalink
fix: join --descriptor_set_in with host path separator (#671)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
willjschmitt authored Nov 12, 2024
1 parent 70e6ba9 commit 105f294
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 1 deletion.
2 changes: 1 addition & 1 deletion ts/private/ts_proto_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions ts/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ 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()

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",
Expand Down
25 changes: 25 additions & 0 deletions ts/test/ts_proto_library/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"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.

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",
],
)
4 changes: 4 additions & 0 deletions ts/test/ts_proto_library/bar.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
syntax = "proto3";
package bar;

message Bar {}
8 changes: 8 additions & 0 deletions ts/test/ts_proto_library/foo.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto3";
package foo;

import "ts/test/ts_proto_library/bar.proto";

message Foo {
bar.Bar bar = 1;
}
30 changes: 30 additions & 0 deletions ts/test/ts_proto_library_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"UnitTest for ts_proto_library"

load("@bazel_skylib//rules:build_test.bzl", "build_test")
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"],
)

0 comments on commit 105f294

Please sign in to comment.