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

[cppwinrt] Update to version 2.0.201008.2 #14092

Merged
merged 5 commits into from
Oct 27, 2020

Conversation

Link1J
Copy link
Contributor

@Link1J Link1J commented Oct 16, 2020

Updates C++/WinRT (cppwinrt) to version 2.0.201008.2. This required a new way of installing it because the repository no longer includes pre-built header files. This put more restrictions on what triplets are supported, limiting it to Windows only. It also now requires a version of the Windows SDK to be installed. With the large change needed to get the header files, a CMakeLists.txt was added, as to allow easier usage to with cmake.

This is a second attempt at doing this update. The first attempt was #14037

Which triplets are supported/not supported?
x86-windows, x64-windows, arm-uwp, x64-uwp, arm64-windows, and x64-windows-static are supported.

Does your PR follow the maintainer guide? Yes, including a manifest file.

@Link1J Link1J marked this pull request as ready for review October 16, 2020 19:34
@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Oct 19, 2020
@JackBoosY JackBoosY added requires:discussion info:reviewed Pull Request changes follow basic guidelines labels Oct 19, 2020
ports/cppwinrt/vcpkg.json Outdated Show resolved Hide resolved
ports/cppwinrt/portfile.cmake Outdated Show resolved Hide resolved
ports/cppwinrt/portfile.cmake Outdated Show resolved Hide resolved
ports/cppwinrt/portfile.cmake Outdated Show resolved Hide resolved
ports/cppwinrt/portfile.cmake Outdated Show resolved Hide resolved
ports/cppwinrt/CMakeLists.txt Outdated Show resolved Hide resolved
@BillyONeal BillyONeal added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines requires:discussion labels Oct 20, 2020
@Link1J
Copy link
Contributor Author

Link1J commented Oct 20, 2020

Is cppwinrt using a latest Windows SDK on the system fine? It may be an issue as two developers may not have the same latest Windows SDK installed.

@BillyONeal
Copy link
Member

Is cppwinrt using a latest Windows SDK on the system fine? It may be an issue as two developers may not have the same latest Windows SDK installed.

You mean due to the -input sdk ?

@Link1J
Copy link
Contributor Author

Link1J commented Oct 20, 2020

Correct.

@BillyONeal
Copy link
Member

@Link1J I see... hmmmm I'm not sure. Normally it would be fine but I want to talk to the binary caching expert(s).

@BillyONeal
Copy link
Member

Now that cppwinrt is in the Windows SDK itself would the right thing be to remove/deprecate this port entirely?

@BillyONeal
Copy link
Member

@ras0219 / @ras0219-msft is going to ask around and see what needs to be done here.

@BillyONeal
Copy link
Member

In the future we might want some sort of feature which consumes .winmds and runs the cppwinrt tools against them. However, for the system .winmds, as this port provides, the generated content would be identical to that already included in the PSDK. As as a result for now we should change this port to be empty, and emit a message explaining that the user needs a recent enough Windows SDK (similar to our atlmfc port). Would you be willing to make such changes @Link1J ?

@Link1J
Copy link
Contributor Author

Link1J commented Oct 22, 2020

I am.

@BillyONeal
Copy link
Member

@Link1J Thanks again for your contribution! This should land shortly....

@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

LGTM.

@BillyONeal BillyONeal merged commit 839f533 into microsoft:master Oct 27, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

@JackBoosY
Copy link
Contributor

Issue #23249 requests to update this port and use the github repo instead of Windows SDK.
@BillyONeal what do you think about?

@BillyONeal
Copy link
Member

I believe the cppwinrt owners still want the Windows SDK to be the "official" deployment mechanism. I can ask

@kennykerr
Copy link

Using the version of https://github.com/microsoft/cppwinrt in the Windows SDK is discouraged. The Windows SDK ships very infrequently, so you can often end up with a very old version. I recommend you grab the latest version here:

https://www.nuget.org/packages/Microsoft.Windows.CppWinRT/

This is the only cppwinrt package we publish and maintain.

@walbourn
Copy link
Member

@BillyONeal I'm working on an improved cppwinrt port that pulls down the cppwinrt compiler from the published NuGet and then generates the projections from the installed Windows SDK. This should result in a much more useful port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants