-
Notifications
You must be signed in to change notification settings - Fork 1.1k
qbs: add optional support for cmake_file_name property #18866
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
base: develop2
Are you sure you want to change the base?
Conversation
As was requested by user migrating from Conan 1.x here https://bugreports.qt.io/projects/QBS/issues/QBS-1849.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @ABBAPOH
I am a bit confused about reusing the cmake_file_name
property, as it is intended for CMake. If the qbs files need to differ from the package name, maybe it would be better to define a qbs_file_name
new property? Because there are many cmake_file_name
defined out there, that would change the qbs file name, who guarantees that the qbs file name is always aligned with the CMake one? I think it might not be the case, am I missing something?
def use_cmake_file_name(self): | ||
return self._use_cmake_file_name | ||
|
||
@use_cmake_file_name.setter | ||
def use_cmake_file_name(self, value): | ||
self._use_cmake_file_name = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, probably no need of new setter/getter, and the QBSDeps.set_property()
is the way to go
""" Handles a single package, can create multiple modules in case of several components | ||
""" | ||
def __init__(self, conanfile, dep, build_bindirs): | ||
def __init__(self, conanfile, dep, build_bindirs, use_cmake_file_name): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably no need of the new argument, if we are going to use properties, maybe the approach should be similar to CMakeDeps and allow a local QBSDeps.set_property() that can override the upstream property, and also allows defining the property locally. This would be general for any property that the generator might use in the future too.
A user specifically asked for usage of cmake_file_name rather than the normal flow (pkg_config_name or dep.ref.name). My understanding is that they relied on cmake_file_name from Conan 1.x JSON generator and ask this feature to ease transition to Conan 2.x. It is indeed quite a daunting task to replace all entries |
I am not sure if I understand. Conan 1 never used the And even if that was the case, it is simply too incorrect to depend on a So this would only make sense as a |
As was requested by user migrating from Conan 1.x here https://bugreports.qt.io/projects/QBS/issues/QBS-1849.
Changelog: (Feature): Describe here your pull request
Docs: omitted
develop
branch, documenting this one.