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

Fix support for toolchains that set CMAKE_FIND_ROOT_PATH_MODE_* variables to ONLY #11753

Conversation

traversaro
Copy link
Contributor

The vcpkg toolchain in scripts/buildsystems/vcpkg.cmake adds the installation prefixes to CMAKE_FIND_ROOT_PATH, but that directory is not searched directly by find_package, but it is prepended to all the usual search paths (for example the one specified by CMAKE_PREFIX_PATH). To make sure that find_package search in the directory specified in CMAKE_FIND_ROOT_PATH even if CMAKE_FIND_ROOT_PATH is set to ONLY, it is necessary to also add / to CMAKE_PREFIX_PATH.

  • What does your PR fix?

This PR ensure that:

C:\src\vcpkg-wasm>vcpkg.exe install spdlog:wasm32-emscripten

works fine, and in general is necessary for any wasm32-emscripten port that find its dependencies via find_package, as discussed in #11323 (comment) . It probably also fixes #8216, but I did not tested it.

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    This fix was developed for ports of the wasm32-emscripten triplets, but it probably also applies to other triplets, but it should not affect any officially supported triplet.

  • Does your PR follow the maintainer guide?
    Yes.

@PhoebeHui
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PhoebeHui PhoebeHui added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed requires:discussion labels Jun 9, 2020
@vicroms
Copy link
Member

vicroms commented Jun 26, 2020

Need to test that this does not affect other triplets.

@traversaro
Copy link
Contributor Author

Need to test that this does not affect other triplets.

Hi @vicroms, I did a few tests and also other triplets were working correctly, which kind of tests do you have in mind? There is something I can do? Thanks!

@traversaro traversaro force-pushed the fix-toolchains-that-set-path-mode-only branch from aa680a9 to 36419bf Compare July 4, 2020 10:50
@traversaro
Copy link
Contributor Author

To test with a recent version of vcpkg, I rebased the branch on the top of existing master.

@ras0219-msft
Copy link
Contributor

This LGTM. Thanks for continuing to improve wasm, looking forward to someday having it in CI :D

@ras0219-msft ras0219-msft merged commit c916aba into microsoft:master Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[spdlog] arm-linux build failure
5 participants