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 Windows build on GitHub CI #23

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

eliemichel
Copy link
Contributor

Commit 54ad644 removed a test because the required version of Windows SDK to build Dawn is 22621. However, GitHub Action Windows runner uses SDK version 20348 by default.

This PR uses fbactions/setup-winsdk@v2 to install the target version of Windows SDK, but this is not enough so we also add env variable to force SDK detection. Both env variables and cmake variable seem to be required, the former are used by DXC's FindD3D12 script and others are for CMake's generic SDK detection.

Example of successful CI run: https://github.com/eliemichel/dawn-prebuilt/actions/runs/11861028908/job/33057504211

Copy link

👋 Thanks for your contribution! Your PR has been imported to Gerrit.
Please visit https://dawn-review.googlesource.com/c/dawn/+/215215 to see it and CC yourself on the change.
After iterating on feedback, please comment on the Gerrit review to notify reviewers.
All reviews are handled within Gerrit, any comments on the GitHub PR may be missed.
You can continue to upload commits to this PR, and they will be automatically imported
into Gerrit.

@Kangz
Copy link
Collaborator

Kangz commented Feb 10, 2025

When it passes CI lets me know so we can merge the Gerrit CL associated with it.

@eliemichel
Copy link
Contributor Author

The check that does not pass here is not related to this PR. Artifacts are correctly generated for tasks that did succeed, so this PR is still helpful as it enables generating binaries for Windows (even if not all tasks succeed)

@Kangz
Copy link
Collaborator

Kangz commented Mar 17, 2025

Thanks @eliemichel, yeah we want to take this in but it has to be reviewed on https://dawn-review.googlesource.com/c/dawn/+/215215 where there already are a couple comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants