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

Fix java path separator bug on Windows #6054

Merged
merged 1 commit into from Aug 14, 2019

Conversation

Xjs
Copy link
Contributor

@Xjs Xjs commented Aug 12, 2019

java_grpc_library targets based on java_proto_library targets with more than one proto_library dependency can't be built on Windows, starting somewhere before 1.22. A minimal reproducing example is available on https://github.com/Xjs/bazel-grpc-java-win-repro (see README.md there for output on my machine as well as steps to reproduce).

The issue stems from protoc configuring a different path separator for Windows at compile time in combination with java_grpc_library always using :.

This PR uses the path separator available from the bazel context's host_configuration to resolve the issue.

@thelinuxfoundation
Copy link

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
CLA GitHub bot

@Xjs
Copy link
Contributor Author

Xjs commented Aug 12, 2019

Hey @thelinuxfoundation bot, I signed the CLA.

@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Aug 13, 2019
@ejona86 ejona86 self-requested a review August 13, 2019 20:01
@ejona86 ejona86 self-assigned this Aug 13, 2019
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Aug 13, 2019
@ejona86
Copy link
Member

ejona86 commented Aug 13, 2019

Thank you for the helpful description. Made it easy to see what's going on. Sorry for the bug!

Copy link
Member

@dapengzhang0 dapengzhang0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dapengzhang0 dapengzhang0 merged commit 9389873 into grpc:master Aug 14, 2019
@Xjs Xjs deleted the path_separator_windows_master branch August 15, 2019 08:59
@lock lock bot locked as resolved and limited conversation to collaborators Nov 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants