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

[libvpx] Add cmake config file #11795

Merged
merged 4 commits into from
Jun 11, 2020

Conversation

mcmtroffaes
Copy link
Contributor

This allows libvpx to be used with

find_package(libvpx CONFIG REQUIRED)
target_link_libraries(main PRIVATE libvpx::libvpx)

Describe the pull request

  • What does your PR fix? See above.

  • Which triplets are supported/not supported? Have you updated the CI baseline? Tested on all supported windows triplets. Should work also on linux/darwin provided the same library naming convention is used (vpx base name for release, vpxd base name for debug). If not, it is easy to add alternate names.

  • Does your PR follow the maintainer guide? Yes.

This allows libvpx to be used with

find_package(libvpx CONFIG REQUIRED)
target_link_libraries(main PRIVATE libvpx::libvpx)

Tested on all supported windows triplets.
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@traversaro
Copy link
Contributor

Unofficial targets created by vcpkg ports are typically prefixed by unofficial (see

if (NOT TARGET unofficial::iconv::libiconv)
), could it make sense to do the same here?

@mcmtroffaes
Copy link
Contributor Author

That is a splendid suggestion @traversaro - I'll go ahead and make that change.

@mcmtroffaes mcmtroffaes marked this pull request as draft June 9, 2020 09:13
@mcmtroffaes mcmtroffaes marked this pull request as ready for review June 9, 2020 09:43
@mcmtroffaes
Copy link
Contributor Author

I've done the change suggested by @traversaro, and I've also added a target guard in case the config gets loaded more than once (similar to what iconv is doing), since that seemed good practice. With those changes, the library can be used as follows:

find_package(unofficial-libvpx CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::libvpx::libvpx)

@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jun 10, 2020
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Please update the version info. See documentation.

@mcmtroffaes
Copy link
Contributor Author

Thanks for the review. I think I've addressed all comments now. Let me know if anything else is needed.

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jun 11, 2020
@dan-shaw dan-shaw merged commit 6e67cb1 into microsoft:master Jun 11, 2020
@dan-shaw
Copy link
Contributor

Thanks for the PR!

@mcmtroffaes mcmtroffaes deleted the feature/libvpx-cmake-config branch June 11, 2020 12:16
JangBoo pushed a commit to JangBoo/vcpkg that referenced this pull request Jun 18, 2020
* [libvpx] Add cmake config file

This allows libvpx to be used with

find_package(libvpx CONFIG REQUIRED)
target_link_libraries(main PRIVATE libvpx::libvpx)

Tested on all supported windows triplets.

* [libvpx] Rename cmake config file to make clear it is unofficial

* [libvpx] Add cmake config target guard

* [libvpx] Update version
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 info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants