Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: --descriptor_set_in input files should be joined by ; on Windows rather than : in ts_proto_library #670

Closed
willjschmitt opened this issue Aug 11, 2024 · 0 comments · Fixed by #671
Labels
bug Something isn't working

Comments

@willjschmitt
Copy link
Contributor

What happened?

Building a ts_proto_library with a proto library that has transitive dependencies fails to build with a "<entire-descriptor_set_in-arg>: No such file or directory" error

Inspecting the arguments provided to protoc, it looks like the paths are separated with : characters, which makes sense, since it's the typical construction on Linux, but it turns out the Windows protoc build wants to see them ;-separated. This is reinforced looking at some examples of how descriptor_set_in is joined in the protobuf repo: https://github.com/protocolbuffers/protobuf/blob/b9d1cfff8ca6814723889bade011f3fa4675d46d/protobuf.bzl#L294-L298 (other examples: https://github.com/search?q=repo%3Aprotocolbuffers%2Fprotobuf+--descriptor_set_in&type=code), where they string join on ctx.configuration.host_path_separator.

I don't see where the use of a system path separator for path delimiting is documented (the docs just say "delimited"), but the parsing by this character is defined here: https://github.com/protocolbuffers/protobuf/blob/b9d1cfff8ca6814723889bade011f3fa4675d46d/src/google/protobuf/compiler/command_line_interface.cc#L2134-L2157, with the path separator conditionally defined to : or ; at https://github.com/protocolbuffers/protobuf/blob/b9d1cfff8ca6814723889bade011f3fa4675d46d/src/google/protobuf/compiler/command_line_interface.cc#L950-L954

The only other use of this delimiter is in --proto_path, but I don't see any uses of --proto_path in this repo

Version

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

Output of bazel --version: 7.1.1

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

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

How to reproduce

No response

Any other information?

No response

@willjschmitt willjschmitt added the bug Something isn't working label Aug 11, 2024
willjschmitt added a commit to willjschmitt/rules_ts that referenced this issue Aug 11, 2024
Fixes aspect-build#670.

As described in aspect-build#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.
willjschmitt added a commit to willjschmitt/rules_ts that referenced this issue Nov 4, 2024
Fixes aspect-build#670.

As described in aspect-build#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant