-
Notifications
You must be signed in to change notification settings - Fork 21
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
Tools don't run under windows #78
Comments
I don't understand runfiles rules enough to fix this, but am happy to test out potential fixes, I have a windows environment ready to go. |
You're right, there are some issues running this under Windows right now. There's a bug that registers the toolchains incorrectly, which is what you're hitting here. I took a look and noticed also, though, that there's another issue: Bazel under Windows doesn't populate the runfiles symlinks because historically Windows didn't support symlinks, and even now that it does, symlinks are slow and make builds take a long time. Because of this, our solution for shimming protoc can't work without enabling runfiles via adding the build option bazel --windows_enable_symlinks test //... --enable_runfiles The relevant options can be added to bazelrc. A better solution certainly needs to be worked out in the long term, but I believe this essentially eliminates the possibility of using scripting to shim protoc, so we'll need a new approach. I'll be pushing up a few fixes shortly. Thanks for the report. |
Thank you! We already have both I believe it is possible to work without --enable_runfiles, although I don't know the detail of your repositories, by parsing the manifest, and this can be done natively on windows in a bat file (e.g. https://github.com/aspect-build/bazel-lib/blob/main/lib/windows_utils.bzl) or in a native exe. Anyhow, this is low priority for us as we are unlikely to be able to run with --noenable_runfiles anytime soon. Thank you so much for the quick fix! I'll integrate it into rules_lint and see how it works. |
Wow, I did not expect anybody to have already implemented runfiles manifest parsing in the Batch command interpreter. My very first thought was that this wouldn't be worth the effort, but all in all, the resulting code actually looks somewhat succinct and straightforward. Maybe that's the way to go after all. |
Yeah, I am impressed too! The fix on your PR works great, BTW, we have buf linting now working on Windows in rules_lint :-) Feel free to close this or leave it open as you wish. I'm happy 👍 |
This should resolve the toolchain registration issue from #78. Also, it adds a batch script version of the shell script shim used to invoke protoc for tests; however, this requires `--enable_runfiles` to be explicitly passed or added to `.bazelrc` for a project, as Bazel on Windows does not populate runfiles by default. In the future, we will need a better approach to supporting Windows that uses the runfiles manifest instead. This is tricky though, and will require some careful thought.
I can see that, on Windows, executable versions of tools such as buf.exe exist. But running them in the toolchain fails. This is impacting integrations such as rules_lint which refuse to lint protobuf files on windows (aspect-build/rules_lint#294)
If I edit toolchain.bzl and change line 61:
from: cli = str(Label("//:"+ cmd)),
to: cli = str(Label"//:" + cmd + ext)),
Then it gets further, but fails with:
The text was updated successfully, but these errors were encountered: