Skip to content

[caf] Add usage and vcpkg-cmake-wrapper#14292

Merged
strega-nil merged 2 commits intomicrosoft:masterfrom
NancyLi1013:dev/NancyLi/fix-caf
Nov 9, 2020
Merged

[caf] Add usage and vcpkg-cmake-wrapper#14292
strega-nil merged 2 commits intomicrosoft:masterfrom
NancyLi1013:dev/NancyLi/fix-caf

Conversation

@NancyLi1013
Copy link
Contributor

@NancyLi1013 NancyLi1013 commented Oct 29, 2020

Describe the pull request

Note: No feature needs to test.

@NancyLi1013 NancyLi1013 added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal labels Oct 29, 2020
@NancyLi1013 NancyLi1013 changed the title [caf] Add usage and vcpkg_cmake_wrapper [caf] Add usage and vcpkg-cmake-wrapper Oct 29, 2020
@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Nov 3, 2020
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

I believe vcpkg_cmake_wrapper is only supposed to be used to override a built in cmake module but cmake has no such findcaf module. I'm not positive though so going to tag this requires:discussion.

@BillyONeal BillyONeal added requires:discussion and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Nov 4, 2020
@BillyONeal
Copy link
Member

@vicroms indicates that this vcpkg_cmake_wrapper is needed for something related to cmake modules Bill doesn't understand :)

@strega-nil says that this is probably fine for now but that we should not be using vcpkg_cmake_wrapper to accomplish this task in the future.

@BillyONeal BillyONeal dismissed their stale review November 5, 2020 23:04

I hope @strega-nil knows how to process this when she's "on rotation" next week 😂

@NancyLi1013
Copy link
Contributor Author

@BillyONeal
Thanks for your further review and investigation about this.

I noticed that there are also other ports that use the similar way to handle this kind of issue. Such as stb.

So I assumed that this can be applied to other ports too.

Just mention, for caf, upstream has supported config.cmake and targets.cmake in current master, but not in current version in vcpkg.

So we can remove these two files when we update the port next time.

@strega-nil strega-nil merged commit 1b5e00a into microsoft:master Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install FindCAF.cmake file to where it is found by cmake

4 participants