-
Notifications
You must be signed in to change notification settings - Fork 282
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 windows Buildkit CI #1649
Fix windows Buildkit CI #1649
Conversation
- windows buildkite needs batch_commands instead of shell_commands
This is needed to handle targets (with //) that are passed as args
4addcac
to
d0416b1
Compare
Hi, @crt-31, thanks for PR. You are right. Unfortunately windows support was not a priority. Thanks for taking this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to get a proper diff for presubmit.yml?
d0416b1
to
3a5826b
Compare
@simuons, should be proper diff now. RTR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @crt-31 !
Apparently windows CI hasn't been running for a long time (or ever?)
The issue seems to be that for windows, buildkite needs "batch_commands" instead of "shell_commands".
(Update)
Also made minor fixes to tests regarding line endings. I originally (#1529) had the system work with autocrlf=true, but this isn't handled by BazelCI. And after thinking bout it, its probably best to keep autocrlf=false, since that is what is distributed to users.. even windows users.
Motivation
Make windows CI work so we don't accidently break windows support.