Fix incorrect path for CMake components with identical framework names

[T1T4N]

Bugfix: Incorrect path found for a different component with the same framework name This PR is for issue #10140 – detailed description of the problem (and the solution) can be found there.

Short summary: If multiple components in a recipe have self.cpp_info.components[component].frameworks defined as an identical value, Conan's cmake_find_package generator will incorrectly return the path to the first found component for all of them because the value is cached, even if each components points to a different path.

NOTE: This change is in line with many other Conan macros which properly clear a value before using it.

• [x] Refer to the issue that supports this Pull Request: #10140
• [x] If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
• [x] I've read the Contributing guide.
• [x] 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.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: franramirez688
:x: T1T4N
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[T1T4N]

@memsharded Thanks for the quick reply!

I already identified that CMakeDeps is using the exact same CMake macro text, so I'll update it there as well. I'll need some time to get acquainted with your project's unit tests, but it shouldn't be hard to have this behavior testable. Stay tuned for new commits.

[T1T4N]

@memsharded I added a commit that addresses the issue in CMakeDeps as well.

I have trouble writing a proper test for this, as the bug exists in a function that is called on runtime by CMake. The generated files clearly mark it as "Will be filled later", so it is not enough to simply test the generated <name>-Target-debug.cmake or <name>-debug-data.cmake files.

Can you point me to an example (integration?) test in the codebase where CMake is actually executed based on an input CMakeLists.txt file?

[Jihadist]

@memsharded sorry for ping. Are there any chances for pr?

[T1T4N]

Hello @memsharded,

I have created an example project to reproduce the issue: CONAN-10140.zip

This project doesn't depend on any macOS details, instead it manually declares a few components. It should be much simpler and easier to understand the issue and test the fix. It is written using the new Conan v2 generators (CMakeDeps, CMakeToolchain) but as previously said, it affects the older generators as well, as all of them use conan_find_apple_frameworks where the bug is.

Final structure of the example package

❯ tree ~/.conan/data/testproject-10140/0.1/jenkins/testing/package/<pkg-id>

├── ComponentA
│   ├── include
│   │   └── ExampleComponentsSameFrameworkName.h
│   └── libComponent.a
├── ComponentB
│   ├── include
│   │   └── ExampleComponentsSameFrameworkName.h
│   └── libComponent.a
├── ComponentC
│   ├── include
│   │   └── ExampleComponentsSameFrameworkName.h
│   └── libComponent.a
├── conaninfo.txt
└── conanmanifest.txt

To encounter the issue a simple conan create is enough. The CMakeLists.txt of the test package will try to use the main package and it will additionally check if the paths for each component are correctly configured (they're not).

To test the bugfix, simply export ENABLE_BUGFIX=1 before running conan create. The test package's conanfile.py will then patch cmakedeps_macros.cmake after it is generated in the generate step. Using the fixed macro results in correct values for each of the components' variables.

As I said previously, I couldn't add any tests as I'm not that familiar with your codebase, but I'm hoping that this small project will be enough to result in a proper fix.

[franramirez688]

Hi @T1T4N

Thank you very much for taking the time to perform that test locally. For sure, we can help! Let me convert it into a Conan test and push it into your branch.

[T1T4N]

@franramirez688 Thank you very much for your reply!

Please ping me when you reproduce the issue locally and in case you need any further details. Also, please note that I just updated the PR to reflect all the changes contained in the example project.

[franramirez688]

Hi @T1T4N

Sorry, I had to revert the flags NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH. There were a lot of tests broken because of that (we're mixing system frameworks and our own ones and it seems to be a problem). I'm going to open a new issue to have a look at this. Anyway, it was a separate issue. Thanks again!

[T1T4N]

Hello @franramirez688,

Thanks for your reply! Should we now proceed forward with the original change: NO_CMAKE_PATH CMAKE_FIND_ROOT_PATH_BOTH ?

I see that your revert commit doesn't include it, and as that is part of the fix the new test should fail without it.

[franramirez688]

Hello @franramirez688,

Thanks for your reply! Should we now proceed forward with the original change: NO_CMAKE_PATH CMAKE_FIND_ROOT_PATH_BOTH ?

Yes, please.

I see that your revert commit doesn't include it, and as that is part of the fix the new test should fail without it.

Yeah, sorry, I did not realize it 😵‍💫

[franramirez688]

@T1T4N I just restored it! Thanks for your comment because I did not remember it 😭

[franramirez688]

@T1T4N @memsharded @jcar87 and I have been looking at the issue, and it seems that the bug could be related to another thing. Let me have a look deeper to get what's going on there. I'll update you both later.

[franramirez688]

So sorry for the late response.

Let's wait for this issue https://github.com/conan-io/conan/issues/13018 to be tackled. IMHO, we have to give a better approach to this issue.