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

Move to latest cppwinrt package across all projects #3868

Merged
merged 8 commits into from
Nov 14, 2023

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Nov 9, 2023

Change

Move all projects that use C++/WinRT, WIL, and C#/WinRT to the latest release. Then fix all of the build breaks that this caused.

This also re-enables a test that needed a newer version of C++/WinRT to work.

After many attempts to debug the issue with the tests that crash on x64 only, I can only assume that there is some perfect collection of OS and dependency versions that will hopefully change at some point and allow us to re-enable them. I have tried so many attempts at debugging it and ran it on a similar server VM to no avail. Given that it is only affecting the inproc tests in x64, I can only assume that it something in codegen somewhere. Will have to try to enable them later.

Validation

Local build and unit tests succeeded; PR regression tests should cover the rest.

Microsoft Reviewers: Open in CodeFlow

@JohnMcPMS JohnMcPMS requested a review from a team as a code owner November 9, 2023 03:24
yao-msft
yao-msft previously approved these changes Nov 9, 2023
Copy link
Contributor

@yao-msft yao-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@@ -160,24 +160,27 @@ namespace AppInstaller::Logging
}
}

std::ostream& operator<<(std::ostream& out, const std::chrono::system_clock::time_point& time)
namespace std
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding to the std namespace seems weird

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what properties the latest C++/WinRT is forcing on us, but they ended up breaking most of these operator<< overloads that were in the global namespace. Leveraging ADL to overcome this requires putting these into std (or std::chrono here).

@JohnMcPMS JohnMcPMS merged commit 897ddec into microsoft:master Nov 14, 2023
8 checks passed
@JohnMcPMS JohnMcPMS deleted the update-cppwinrt branch November 14, 2023 22:46
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.

3 participants