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

'cmake' generator use the package name for target 'CONAN_PKG::' #6288

Merged
merged 5 commits into from Jan 8, 2020

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Jan 2, 2020

Changelog: Fix: Generators cmake and cmake_multi use the name of the package instead of cpp_info.name (this change is to be reverted in 1.22)
Docs: omit

closes #6269

This PR is a patch to keep previous behavior (and future one) related to the name used by the generators. In this PR the cmake generator will use the value in cpp_info.names["cmake"] or the package name (it won't use cpp_info.name). With this workaround we don't need to change any recipe in conan-center-index right now to match the desired behavior, we can wait until Conan v1.22.

Read the rationale in this comment: #6269 (comment)

@jgsogo jgsogo added this to the 1.21.1 milestone Jan 2, 2020
jgsogo added a commit to jgsogo/conan that referenced this pull request Jan 2, 2020
@jgsogo
Copy link
Contributor Author

jgsogo commented Jan 2, 2020

I've added here (#6292) a PR to develop to be sure that these changes are not deployed in v1.22 (I will forget about it)

def get_name(self, generator):
return self.names.get(generator, self.name)
def get_name(self, generator, pkg_name=None):
fallback_name = self.name if generator not in ["cmake", "cmake_multi"] else pkg_name # FIXME: Remove in v1.22 (https://github.com/conan-io/conan/issues/6269#issuecomment-570182130)
Copy link
Member

Choose a reason for hiding this comment

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

This should mimic the behavior that will be implemented by conan-center-index when recipes are fixed, so it should affect probably cmake_paths generator too.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Check comment, change something if necessary or leave, and merge. Thanks.

self.assertNotIn("my_pkg", content["conanbuildinfo_multi.cmake"])
self.assertNotIn("MY_PKG", content["conanbuildinfo_multi.cmake"])
self.assertIn('add_library(CONAN_PKG::MyPkG INTERFACE IMPORTED)',
self.assertNotIn("mypkg", content["conanbuildinfo_multi.cmake"])
Copy link
Member

Choose a reason for hiding this comment

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

It is obvious that these don't exist in content, as the package name is my_pkg. I am not sure what this assert is checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally true, the one that could appear is MyPkG (the cpp_info.name). Changed it, after CI runs, I'll merge it.

@memsharded memsharded merged commit 1bdbf18 into conan-io:release/1.21.1 Jan 8, 2020
@jgsogo jgsogo deleted the fix/cmake-conan_pkg-name branch January 8, 2020 12:32
jgsogo added a commit to jgsogo/conan that referenced this pull request Jan 28, 2020
memsharded pushed a commit that referenced this pull request Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants