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 #14037

Closed
wants to merge 0 commits into from
Closed

Conversation

Link1J
Copy link
Contributor

@Link1J Link1J commented Oct 14, 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.

  • Which triplets are supported/not supported?
    x86-windows and x64-windows are supported. arm-windows and arm64-windows may work on arm computers, I have no way to test. It may be possible to support all triplets as the cppwinrt.exe app only needs a header only library to build, but we also need the Windows' SDK for the new process.

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

@Link1J Link1J marked this pull request as ready for review October 14, 2020 14:26
@NancyLi1013 NancyLi1013 added the category:port-update The issue is with a library, which is requesting update new revision label Oct 15, 2020
@@ -1,21 +1,49 @@
include(vcpkg_common_functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
include(vcpkg_common_functions)

This function is no longer needed. Please remove it.

DEBUG_CONFIGURATION Release
TARGET cppwinrt
OPTIONS /p:CppWinRTBuildVersion=${VERSION}
)
Copy link
Contributor

Choose a reason for hiding this comment

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

You have added CMakeLists.txt, why does msbuild still require?

)
vcpkg_install_cmake()

file(COPY ${CURRENT_PACKAGES_DIR}/share DESTINATION ${CURRENT_PACKAGES_DIR}/debug)
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the files in this directory ${CURRENT_PACKAGES_DIR}/share? Why do you copy these files to /debug direcroty?

Comment on lines 48 to 13
file(COPY ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/cppwinrt)
file(RENAME ${CURRENT_PACKAGES_DIR}/share/cppwinrt/LICENSE ${CURRENT_PACKAGES_DIR}/share/cppwinrt/copyright)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
file(COPY ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/cppwinrt)
file(RENAME ${CURRENT_PACKAGES_DIR}/share/cppwinrt/LICENSE ${CURRENT_PACKAGES_DIR}/share/cppwinrt/copyright)
file(INSTALL ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright)


vcpkg_fixup_cmake_targets()

file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you remove all the files in /debug directory?

Comment on lines 1857 to 1864

# cppwinrt.exe does not build on arm - this is known by cppwinrt and is even shown in its build files
cppwinrt:arm64-windows = fail
cppwinrt:arm-uwp = fail

# cppwinrt will not work on non windows platforms
cppwinrt:x64-linux = skip
cppwinrt:x64-osx = skip
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# cppwinrt.exe does not build on arm - this is known by cppwinrt and is even shown in its build files
cppwinrt:arm64-windows = fail
cppwinrt:arm-uwp = fail
# cppwinrt will not work on non windows platforms
cppwinrt:x64-linux = skip
cppwinrt:x64-osx = skip

If this port cannot support these triplets, you can add Supports: !(arm|osx|linux) to vcpkg.json file. There is no need to add these codes in ci.baseline.txt.

@Link1J
Copy link
Contributor Author

Link1J commented Oct 16, 2020

Okay, I have made the recommended changes and a few more.

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

2 participants