fix: join `--descriptor_set_in` with host path separator

[willjschmitt]

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.

---

Changes are visible to end-users: no

Test plan

• Manual testing; please provide instructions so we can reproduce: https://github.com/AchilleaResearch/rules_ts_667/tree/repro_670 demonstrates the fix as a patch. bazel build //:foo_ts_pb on Windows with and without the patch https://github.com/AchilleaResearch/rules_ts_667/blob/repro_670/MODULE.bazel#L47

[aspect-workflows[bot]]

Test

All tests were cache hits

150 tests (100.0%) were fully cached saving 9s.

---

Buildifier      Format

[jbedard]

Can you rebase this to kickoff CI again? I'm not sure why it's red atm.

This is simple enough (and a noop on *nix) I think we can merge it, but if you have a chance to add some tests along with those for https://github.com/aspect-build/rules_ts/pull/668 that would be great.

[willjschmitt]

@jbedard sorry for the delay, I was out on baby bonding leave and just am getting back in. I rebased, but I don't think it's auto kicking off a build. I manually ran bazel test //... on linux and ran bazel test //ts/test:ts_proto_library_with_dep_test on windows, cherry picking #668, although unfortunately, due to https://github.com/aspect-build/rules_js/issues/1739, a patch to handle bsdtar on windows is still needed. The Windows build is failing but getting past this and the issues of #667 at least

I did borrow from the node_modules in //examples/proto_grpc, since the repo root is not the pnpm workspace root, which might not be acceptable, so let me know if 1) the existing examples essentially are already the test coverage we are looking for, or 2) the pnpm workspace can/should be promoted to repo root, which I figured wouldn't be desirable

[jbedard]

Looks like we just have a buildifier error, otherwise LGTM and thanks for the tests 👍

[willjschmitt]

Sorry about that! Removed the extraneous imports, so we should be good to go now