-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[blingfire] Use Ninja rather than MSBuild in BlingFire to support UWP. #31331
Conversation
FYI - I'm also working with upstream to eliminate the need for the patch long-term: |
Let's wait for the reply from the upstream. |
Note: I will be converting your PR to draft status. please revert to "ready for review" after the upstream agrees to this change. Thanks. |
@Adela0814 - Can we go ahead with this PR now and then remove the patch once upstream is able to figure out a better approach, so that using this port in uwp apps can be unblocked in the meantime? Thanks! |
- add_compile_options("/O2" "/W4" "/GS" "/Gy" "/guard:cf" "/Gm-" "/Zc:inline" "/fp:precise" "/GF" "/EHsc" "/ZH:SHA_256") | ||
+ add_compile_options("/W4" "/GS" "/Gy" "/guard:cf" "/Gm-" "/Zc:inline" "/fp:precise" "/GF" "/EHsc" "/ZH:SHA_256") |
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.
How is this change connected to ninja?
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.
Ninja appears to add a conflicting /Ox switch itself. Removing WINDOWS_USE_MSBUILD causes this port to break without this patch.
Fixes #31330.
./vcpkg x-add-version --all
and committing the result.