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

PowerToys doesn't compile with SDKs higher than 10.0.18362.0 #1907

Closed
riverar opened this issue Apr 3, 2020 · 14 comments
Closed

PowerToys doesn't compile with SDKs higher than 10.0.18362.0 #1907

riverar opened this issue Apr 3, 2020 · 14 comments
Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@riverar
Copy link
Contributor

riverar commented Apr 3, 2020

PowerToys is configured to always build using the very latest SDK but doesn't actually seem to support the cppwinrt bundled with Windows 10 SDK 10.0.18362.0 or higher. (I also tried 10.0.19592.0).

I understand these SDKs are beyond vCurrent but a few points:

  • We've effectively hit Windows 10 Version 2004 (20H1) RTM so SDKs are relatively stable and should be supported by PowerToys by now to be ready in time for Windows vNext release
  • Because the projects are configured to target the latest SDKs on the box, this creates a poor OSS contributor onboarding experience if any of these SDKs are installed. I suggest explicitly setting the min/max platform targets and documenting them.
@enricogior
Copy link
Contributor

Hi @riverar
what error are you getting using 10.0.18362.0?

@riverar
Copy link
Contributor Author

riverar commented Apr 3, 2020

Sample from 381 compile errors:

Error	C3779	'winrt::impl::consume_Windows_Foundation_Collections_IMap<D,K,V>::HasKey': a function that returns 'auto' cannot be used before it is defined	common (common\common)	C:\Sources\PowerToys\src\common\json.h	21	
Error	C3779	'winrt::impl::consume_Windows_Foundation_Collections_IMap<D,K,V>::HasKey': a function that returns 'auto' cannot be used before it is defined	FancyZonesLib	C:\Sources\PowerToys\src\common\json.h	21	
Error	C2039	'shared_lock': is not a member of 'std'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C7568	argument list missing after assumed function template 'shared_lock'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C4430	missing type specifier - int assumed. Note: C++ does not support default-int	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C2144	syntax error: 'unknown-type' should be preceded by ')'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C2144	syntax error: 'unknown-type' should be preceded by ';'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C2238	unexpected token(s) preceding ';'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C2059	syntax error: ')'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C2098	unexpected token after data member 'lock'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	161	
Error	C2988	unrecognizable template declaration/definition	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	162	
Error	C2059	syntax error: '<end Parse>'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	162	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	185	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	185	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	188	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	188	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	189	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	189	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	190	
Error	C2955	'FancyZones::require_write_lock': use of class template requires template argument list	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	190	
Error	C2131	expression did not evaluate to a constant	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	203	
Error	C2131	expression did not evaluate to a constant	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	205	
Error	C2039	'shared_mutex': is not a member of 'std'	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	207	
Error	C3646	'm_lock': unknown override specifier	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	207	
Error	C4430	missing type specifier - int assumed. Note: C++ does not support default-int	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	207	
Error	C2131	expression did not evaluate to a constant	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	208	
Error	C2131	expression did not evaluate to a constant	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	209	
Error	C2131	expression did not evaluate to a constant	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	210	
Error	C2131	expression did not evaluate to a constant	FancyZonesLib	C:\Sources\PowerToys\src\modules\fancyzones\lib\FancyZones.cpp	211	

@enricogior
Copy link
Contributor

We'll investigate it, but it's strange since the whole team is building it using 10.0.18362.0.

@enricogior enricogior added the Area-Build Issues pertaining to the build system, CI, infrastructure, meta label Apr 3, 2020
@riverar
Copy link
Contributor Author

riverar commented Apr 3, 2020

@enricogior Right, these errors appear when using SDKs versioned higher than 10.0.18362.0.

@enricogior
Copy link
Contributor

OK, so we will set for now 10.0.18362.0 as max target, thanks for pointing it out.

@crutkas
Copy link
Member

crutkas commented Apr 5, 2020

@enricogior, we'll have to do this shortly. While i do enjoy ribbing Raf, he is correct here. We'll have to get the project to working with the Win10 2004 SDK. Lemme see what the status of that is.

@riverar
Copy link
Contributor Author

riverar commented Apr 5, 2020

I performed a dry run and ... it's going to take a bit of work to move the project to cppwinrt 2.0 in the new Windows 10 SDK. To avoid stressing everyone out, I suggest this upgrade path:

  • This issue - Fix the minimum/maximum supported platform targets solution-wide:

    The README claims the minimum supported OS is 1803 (build 17134) but it doesn't compile with the 17134 SDK. (It does compile with the 17763 SDK.)

    You have a few options:

    1. If PowerToys follows the consumer-only Windows lifecycle, 17134 is already EOL. The README and solution minimums can be bumped to 1809 (build 17763).
    2. If PowerToys follows both consumer and educational/enterprise Windows lifecycles, 17134 is not EOL and the compilation issues will need to be repaired.
  • New issue - Decouple from SDK-provided cppwinrt, bring in nuget package (1.0.190211.5):

    This should allow you to anchor to cppwinrt 1.0 and allow compilation with the upcoming Windows 10 2004 (build ~19041) SDK. (Testing this tonight.)

  • New issue - Upgrade to cppwinrt 2.0 solution-wide, fix breakage

@yuyoyuppe
Copy link
Contributor

Thanks for reporting this, we might even get away with updating to cppwinrt 2.0 as part of this, since we don't use it extensively, and I suspect most of those errors are caused by a single json.h.

@enricogior
Copy link
Contributor

@riverar
did you have time to check how much work would be required to switch to cppwinrt 2.0?
As @yuyoyuppe suggested, we may consider to skip the intermediate steps and move to cppwinrt 2.0 right away.
Thanks.

@riverar
Copy link
Contributor Author

riverar commented Apr 13, 2020

Hey @enricogior! It wasn't too bad. Mainly just needed to go around to all the pch files and add Windows.Foundation.Collections.h, add some std headers (e.g. <shared_mutex>), turn off conformance mode solution-wide, then clean up some direct allocation bugs.

I have a branch compiling with cppwinrt 2.0 from 19592 SDK. Let me wire up the cppwinrt nuget tomorrow (Monday) then I'll hand it to you and the team.

@enricogior
Copy link
Contributor

@riverar
thank you very much, really appreciated!

@riverar
Copy link
Contributor Author

riverar commented Apr 20, 2020

Got delayed as I hit a few cppwinrt snags. #2246 has the goods. Will figure out how to run the tests tomorrow morning (Monday).

@enricogior
Copy link
Contributor

@riverar
@SeraphimaZ will help with running all the unit and functional tests.

@enricogior enricogior added the Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. label Apr 29, 2020
@enricogior
Copy link
Contributor

Merged, thanks @riverar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

No branches or pull requests

5 participants