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

#4956 cmake_find_package_multi might need also CMAKE_MODULE_PATH #5021

Merged
merged 2 commits into from Apr 30, 2019

Conversation

Projects
None yet
3 participants
@uilianries
Copy link
Member

commented Apr 23, 2019

cmake_flags.py add also CMAKE_MODULE_PATH when using that generator. Looks like the find_dependency needs it.

closes #4956

Changelog: Fix: Include CMAKE_MODULE_PATH for CMake find_dependency (#4956)
Docs: Omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

#4956 cmake_find_package_multi might need also CMAKE_MODULE_PATH
- cmake_flags.py add also CMAKE_MODULE_PATH when using that generator.
  Looks like the find_dependency needs it.

@ghost ghost assigned uilianries Apr 23, 2019

@ghost ghost added the stage: review label Apr 23, 2019

@uilianries

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

No idea where should I put a new test to check it ...

conans/test/functional/generators/cmake_find_package_multi_test.py

Create a package and consume it by a conan recipe, which uses cmake_find_package_multi and check the cmake output?

@lasote lasote self-requested a review Apr 29, 2019

@lasote lasote assigned lasote and unassigned uilianries Apr 29, 2019

@lasote

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

I think a unit test here mocking the needed stuff (like the generators) should be fine conans/test/unittests/client/build/cmake_test.py

@memsharded memsharded added this to the 1.15 milestone Apr 29, 2019

#4956 Validate cmake_find_package_multi definitions
- CMake should use both CMAKE_PREFIX_PATH and CMAKE_PREFIX_PATH
  for cmake_find_package_multi

Signed-off-by: Uilian Ries <uilianries@gmail.com>

@ghost ghost assigned uilianries Apr 29, 2019

@uilianries

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2019

@lasote please, review the new test.

@lasote lasote merged commit 9687124 into conan-io:develop Apr 30, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Apr 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.