-
Notifications
You must be signed in to change notification settings - Fork 140
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
CI: complete the Windows CI coverage #832
base: master
Are you sure you want to change the base?
Conversation
8ff7abb
to
26b27de
Compare
@keith okay, the install issue is resolved, but now the problem is that |
are the values stable enough to pass directly? like |
I suppose that they should be - the installer is installing into a user specific location - |
@keith I'm not sure how to make the appropriate changes for this, so a bit of guidance would be appreciated. |
I was thinking that if you set that in the |
Extend the Windows CI to include the tests as well. This will mostly complete the Windows support (the remaining pieces remain in supporting the Windows ARM64 build host).
I'm not too fond of the approach, but it does get further:
|
is that error one you're thinking the CI setup should have solved? |
Well, that seems like something that general Windows support should have figured out. I'm not sure what is going on, but it seems that we are missing something in the image possibly? Or we are not invoking things properly. |
this makes it sound like maybe the windows setup doesn't have the SDK? bazelbuild/bazel#13261 is that what you're thinking? should that be solved by https://github.com/bazelbuild/continuous-integration/blob/master/buildkite/setup-windows.ps1#L137 ? |
Yeah, exactly, it seems like that is what is needed. |
is it possible it's setup correctly but the env var isn't set? i assume that link is the current runner |
@compnerd Wondering where we stand with this. We are going to be making some changes to the worker soon, and I want to make sure I don't regress Windows support. |
@brentleyjones I think that @keith is right and that the Windows SDK is missing. Once that is installed, I think that this should just work out of the box now. The admin requirements are removed upstream and we now install to the user's directory. We no longer need to modify system files so that is also not an issue. As long as we have the |
Extend the Windows CI to include the tests as well. This will mostly
complete the Windows support (the remaining pieces remain in supporting
the Windows ARM64 build host).