[Bug]: ts_proto_library protoc cannot execute generated plugin js_binary *.bat file wrappers with forward-slashes

[willjschmitt]

What happened?

Building a ts_proto_library (with or without connect/query enabled) on Windows, we get an error suggesting 'bazel-out' is not recognized as an internal or external command, operable program or batch file.:

bazel build :foo_ts_pb --verbose_failures
INFO: Analyzed target //:foo_ts_pb (0 packages loaded, 0 targets configured).
ERROR: C:/users/willschmitt/documents/achillea/rules_ts_repro/BUILD.bazel:15:17: Generating .js/.d.ts from //:foo_ts_pb failed: (Exit 1): protoc.exe failed: error executing ProtocGenEs command (from target //:foo_ts_pb)
  cd /d C:/users/willschmitt/_bazel_willschmitt/hqqsioi3/execroot/_main
  SET BAZEL_BINDIR=bazel-out/x64_windows-fastbuild/bin
  bazel-out\x64_windows-opt-exec-ST-d57f47055a04\bin\external\protobuf~\protoc.exe --plugin=protoc-gen-es=bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/_foo_ts_pb.gen_es.bat --es_opt=keep_empty_files=true --es_opt=target=js+dts --es_out=bazel-out/x64_windows-fastbuild/bin --plugin=protoc-gen-connect-es=bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/_foo_ts_pb.gen_connect_es.bat --connect-es_opt=keep_empty_files=true --connect-es_opt=target=js+dts --connect-es_out=bazel-out/x64_windows-fastbuild/bin --descriptor_set_in bazel-out/x64_windows-fastbuild/bin/foo_proto-descriptor-set.proto.bin foo.proto
# Configuration: a5a9a62db26d025cbc2fa4b4fbccebfc6b077137210c76db063d38b8ff442d14
# Execution platform: @@platforms//host:host
'DOSKEY' is not recognized as an internal or external command,
operable program or batch file.
'bazel-out' is not recognized as an internal or external command,
operable program or batch file.
--es_out: protoc-gen-es: Plugin failed with status code 1.
Target //:foo_ts_pb failed to build
INFO: Elapsed time: 1.155s, Critical Path: 0.09s
INFO: 2 processes: 2 internal.
ERROR: Build did NOT complete successfully

which suggests the forward slashes are leading to a failure to find and execute, truncating the rest of the path. Digging in, running the command directly from a shell and toying with the paths, it looks like it's only a problem with the plugin paths and the rest of the toolchain handles the proto, out-dir, and descriptor paths just fine with forward slashes. Manually running the following fails:

cd /d C:/users/willschmitt/_bazel_willschmitt/hqqsioi3/execroot/_main
SET BAZEL_BINDIR=bazel-out/x64_windows-fastbuild/bin
bazel-out\x64_windows-opt-exec-ST-d57f47055a04\bin\external\protobuf~\protoc.exe ^
    --plugin=protoc-gen-es=bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/_foo_ts_pb.gen_es.bat ^
    --es_opt=keep_empty_files=true ^
    --es_opt=target=js+dts ^
    --es_out=bazel-out/x64_windows-fastbuild/bin ^
    --plugin=protoc-gen-connect-es=bazel-out/x64_windows-opt-exec-ST-d57f47055a04/bin/_foo_ts_pb.gen_connect_es.bat ^
    --connect-es_opt=keep_empty_files=true ^
    --connect-es_opt=target=js+dts ^
    --connect-es_out=bazel-out/x64_windows-fastbuild/bin ^
    --descriptor_set_in bazel-out/x64_windows-fastbuild/bin/foo_proto-descriptor-set.proto.bin ^
    foo.proto

while this succeeds to execute:

cd /d C:/users/willschmitt/_bazel_willschmitt/hqqsioi3/execroot/_main
SET BAZEL_BINDIR=bazel-out/x64_windows-fastbuild/bin
bazel-out\x64_windows-opt-exec-ST-d57f47055a04\bin\external\protobuf~\protoc.exe ^
    --plugin=protoc-gen-es=bazel-out\x64_windows-opt-exec-ST-d57f47055a04\bin\_foo_ts_pb.gen_es.bat ^
    --es_opt=keep_empty_files=true ^
    --es_opt=target=js+dts ^
    --es_out=bazel-out/x64_windows-fastbuild/bin ^
    --plugin=protoc-gen-connect-es=bazel-out\x64_windows-opt-exec-ST-d57f47055a04\bin\_foo_ts_pb.gen_connect_es.bat ^
    --connect-es_opt=keep_empty_files=true ^
    --connect-es_opt=target=js+dts ^
    --connect-es_out=bazel-out/x64_windows-fastbuild/bin ^
    --descriptor_set_in bazel-out/x64_windows-fastbuild/bin/foo_proto-descriptor-set.proto.bin ^
    foo.proto

Version

Development (host) and target OS/architectures: Windows 10 64bit

Output of bazel --version: 7.1.1

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:

• rules_ts - 2.4.2
• rules_js - 1.40.0 (patched for bsdtar/libcrypto a la [Bug]: bsdtar.exe fails to run on windows: libcrypto dll not found bazel-contrib/bazel-lib#848 (comment))

Language(s) and/or frameworks involved: Typescript/protobuf/rule

How to reproduce

Build a `ts_proto_library` target on Windows

Any other information?

Workaround patch for v2.4.2, which does a hacky check if the path ends with *.bat and rewrites the forward slashes to backslashes (a la https://bazel.build/rules/windows#paths):

diff --git ts/private/ts_proto_library.bzl ts/private/ts_proto_library.bzl
index 6fa8c30..5954ff9 100644
--- ts/private/ts_proto_library.bzl
+++ ts/private/ts_proto_library.bzl
@@ -8,6 +8,20 @@ load("@rules_proto//proto:proto_common.bzl", proto_toolchains = "toolchains")
 
 _PROTO_TOOLCHAIN_TYPE = "@rules_proto//proto:toolchain_type"
 
+
+def _windows_path_normalize(path):
+    """Changes forward slashs to backslashs for Windows paths.
+
+    Only works for batch files.
+    """
+    # This is a hack to check if we are on Windows, since the offending file
+    # paths are exclusively batch files that are used as the entry point for
+    # protoc plugins that are wrapped by js_binary.
+    if path.endswith(".bat"):
+        return path.replace("/", "\\")
+    return path
+
+
 # buildifier: disable=function-docstring-header
 def _protoc_action(ctx, proto_info, outputs, options = {
     "keep_empty_files": True,
@@ -24,19 +38,19 @@ def _protoc_action(ctx, proto_info, outputs, options = {
     inputs = depset(proto_info.direct_sources, transitive = [proto_info.transitive_descriptor_sets])
 
     args = ctx.actions.args()
-    args.add_joined(["--plugin", "protoc-gen-es", ctx.executable.protoc_gen_es.path], join_with = "=")
+    args.add_joined(["--plugin", "protoc-gen-es", _windows_path_normalize(ctx.executable.protoc_gen_es.path)], join_with = "=")
     for (key, value) in options.items():
         args.add_joined(["--es_opt", key, value], join_with = "=")
     args.add_joined(["--es_out", ctx.bin_dir.path], join_with = "=")
 
     if ctx.attr.gen_connect_es:
-        args.add_joined(["--plugin", "protoc-gen-connect-es", ctx.executable.protoc_gen_connect_es.path], join_with = "=")
+        args.add_joined(["--plugin", "protoc-gen-connect-es", _windows_path_normalize(ctx.executable.protoc_gen_connect_es.path)], join_with = "=")
         for (key, value) in options.items():
             args.add_joined(["--connect-es_opt", key, value], join_with = "=")
         args.add_joined(["--connect-es_out", ctx.bin_dir.path], join_with = "=")
 
     if ctx.attr.gen_connect_query:
-        args.add_joined(["--plugin", "protoc-gen-connect-query", ctx.executable.protoc_gen_connect_query.path], join_with = "=")
+        args.add_joined(["--plugin", "protoc-gen-connect-query", _windows_path_normalize(ctx.executable.protoc_gen_connect_query.path)], join_with = "=")
         for (key, value) in options.items():
             args.add_joined(["--connect-query_opt", key, value], join_with = "=")
         args.add_joined(["--connect-query_out", ctx.bin_dir.path], join_with = "=")

Also looking at rules_proto_grpc it looks like they do a similar path mangling for plugin paths on Windows here: https://github.com/rules-proto-grpc/rules_proto_grpc/blob/9e61a4c3dfc78de163febe7530bd04dabfbcca16/modules/core/internal/protoc.bzl#L79-L83, although they look by looking at ctx.configuration.host_path_separator as the heuristic for Windows