aspect-build
fix: convert forward slashes to backslashes for protoc plugins on Windows.
Fixes https://github.com/aspect-build/rules_ts/issues/667
This takes the approach rules_proto_grpc does to detect if we are building on Windows via the host_path_separator. protoc will fail to call into the plugins if they are expressed to protoc as forward-slash paths. Alternatively, we could make this more limited like my patch in the linked issue to check .bat file extensions, which is fragile in its own way, but works for the given use case.
Changes are visible to end-users: no
Test plan
- Manual testing; please provide instructions so we can reproduce: check out https://github.com/AchilleaResearch/rules_ts_667 on Windows and
bazel build //:foo_ts_pb. This PR's change is a patch in that repo, so I could also change that to directly use my commit instead, if preferred
There aren't any tests as far as I can tell currently for ts_proto_library, so my manual test is the best I can currently offer, and it's not clear if the Windows test runners are fully active, but I'd be happy to write a test for initial coverage on ts_proto_library if desired
Test
All tests were cache hits
150 tests (100.0%) were fully cached saving 9s.
Buildifier Format
![aspect-workflows[bot] avatar](https://avatars.githubusercontent.com/in/380038?v=4 "Published by a pub.dev verified publisher"){kind=small}
Would you be able to add some initial ts_proto_library tests? It would be great to merge some tests in and then try to reproduce+fix the windows after 🙏
Now that #671 is merged in, I rebased and the same unit test should apply, showing a noop on Linux and now should work on Windows
Any chance this can be re-reviewed based on my last comment in reply to adding test coverage?
Were we never able to get a test failing on windows? But the PR lgtm I think, especially now that we have some better coverage. Sorry for missing your last comment :/