Repository navigation
Move from sdk-provided cppwinrt-1.x to cppwinrt-2.x nuget package - #2246
Conversation
|
Rafael Rivera (@riverar) |
Andrey Nekrasov (yuyoyuppe)
left a comment
There was a problem hiding this comment.
Thank you so much for the contribution! I've tested it locally and it works.
| <ProjectGuid>{D29DDD63-E2CF-4657-9FD5-2AEDE4257E5D}</ProjectGuid> | ||
| <RootNamespace>actionrunner</RootNamespace> | ||
| <WindowsTargetPlatformVersion>10.0</WindowsTargetPlatformVersion> | ||
| <WindowsTargetPlatformVersion>10.0.17134.0</WindowsTargetPlatformVersion> |
There was a problem hiding this comment.
What is the reason to set 10.0.17134.0 in stead of 10.0.18362.0?
There was a problem hiding this comment.
Aligns with the README: Microsoft PowerToys requires Windows 10 1803 (build 17134) or later. But we can change to whatever you want the min target to be.
There was a problem hiding this comment.
Since we have been testing and building the release with 10.0.18362.0 let's set it as the default.
There was a problem hiding this comment.
I'm okay with bumping up to 10.0.17763.0 (EOL November 10, 2020), to re-align with the consumer Windows 10 support lifecycle, but anything beyond will cause misalignment with product lifecycles.
Changes if left alone:
- None
Changes if bumped to 10.0.17763.0:
- Minimum supported OS will need to change in README
- This will signal that PowerToys does not support Windows 10 Education or Enterprise lifecycles (17134 is still supported until late Q4)
Changes needed if bumped to 10.0.18362.0:
- Minimum supported OS will need to change in README
- This will signal that PowerToys does not follow any Windows 10 product lifecycle.
Official builds should not be using the 10.0.18362.0 SDK.
I'm not familiar with the team hierarchy here yet (apologies) so tagging Clint Rutkas (@crutkas) for double-check.
There was a problem hiding this comment.
Let me re-check what are we using in the piepline to build the release.
There was a problem hiding this comment.
Our build environment is 1809 with VS 16.4.3.
Clint Rutkas (@crutkas) can you confirm is it OK to set it as minimum version supported?
There was a problem hiding this comment.
I'm fine moving to RS5 as min version looking at telem.
There was a problem hiding this comment.
Clint Rutkas (@crutkas) So, to confirm, you do not want to support Windows 10 Education or Enterprise, right? So PowerToys moving forward will only support consumer lifecycle.
There was a problem hiding this comment.
I misunderstood, yes, we should support those. I thought RS5 flip was May timeframe ,not Nov. We currently support Rs4+ so lets keep that until nov.
Seraphima Zykova (SeraphimaZykova)
left a comment
There was a problem hiding this comment.
I checked out tests and all of them worked as expected. Thanks!
Yevhenii Holovachov (yevhenii44-zz)
left a comment
There was a problem hiding this comment.
Tested and it works as expected. Thank you for contribution
c52c43d to
8ccf9cf
Compare
|
Clint Rutkas (@crutkas) do you want to merge this for 0.17 or do we merge it right after it so we have more time for testing? |
8ccf9cf to
6f446de
Compare
Summary of the Pull Request
Moves PowerToys off SDK-provided cppwinrt (1.x) to nuget-provided cppwinrt (2.x). (This also resolves 17134 and 19041 SDK compilation issues.)
References
#1907 - PowerToys doesn't compile with SDKs higher than 10.0.18362.0
PR Checklist
Detailed Description of the Pull Request / Additional comments
src/Directory.Build.propsintroduced to set important global properties (e.g. for cppwinrt compilation success)Microsoft.Windows.CppWinRTnuget added to relevant projects. This will rewire project internals at runtime to execute the bundledcppwinrt.exe, generate projection headers, and more. This means identical looking projection headers will be generated for every project. This duplicitous work appears to be per cppwinrt design.Validation Steps Performed
Basic smoke test, app launches and modules appear to work.