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

[Bugfix] CMakeDeps and options for linking #9980

Conversation

franramirez688
Copy link
Contributor

@franramirez688 franramirez688 commented Nov 8, 2021

Changelog: Bugfix: CMakeDeps generated *-data.cmake was not including properly the set of link flags.
Changelog: Bugfix: CMakeDeps was not populating INTERFACE_LINK_OPTIONS to each target.
Closes: #9936
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.

@@ -215,8 +218,8 @@ def join_paths_single_var(values):
# linker flags without magic: trying to mess with - and / =>
# https://github.com/conan-io/conan/issues/8811
# frameworks should be declared with cppinfo.frameworks not "-framework Foundation"
self.sharedlinkflags_list = join_flags(";", cpp_info.sharedlinkflags)
self.exelinkflags_list = join_flags(";", cpp_info.exelinkflags)
self.sharedlinkflags_list = join_flags(";", cpp_info.sharedlinkflags, as_string=True)
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit confusing. The self.xxxx_list variable was created to explicitly represent it contains a list of flags, not a unique string. Maybe we want to return to the sharedlinkflags variable? This was added because some CMake was complaining about receiving a string and it always needed a list, but better double check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but the thing is that it's still a list of flags but passed as a string. I mean, internally, CMake is reading it as a list of flags -> "oneflag;otherflag;-z flag".
I don't know what it was doing before but this is the recommended way to pass it, isn't it? For me, what could be confusing is the parameter as_string, any suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the as_string and quote the final string in the assignation directly.

@franramirez688 franramirez688 force-pushed the bugfix/cmakedeps_and_sharedlinkflags branch from ca455d8 to b1f9c53 Compare November 15, 2021 16:47
@@ -215,8 +218,8 @@ def join_paths_single_var(values):
# linker flags without magic: trying to mess with - and / =>
# https://github.com/conan-io/conan/issues/8811
# frameworks should be declared with cppinfo.frameworks not "-framework Foundation"
self.sharedlinkflags_list = join_flags(";", cpp_info.sharedlinkflags)
self.exelinkflags_list = join_flags(";", cpp_info.exelinkflags)
self.sharedlinkflags_list = join_flags(";", cpp_info.sharedlinkflags, as_string=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the as_string and quote the final string in the assignation directly.

conan/tools/cmake/cmakedeps/templates/target_data.py Outdated Show resolved Hide resolved
conan/tools/cmake/cmakedeps/templates/target_data.py Outdated Show resolved Hide resolved
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.

[bug] CMakeDeps generated .cmake config files doesn't put linker flags in quotation
3 participants