-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
opencascade: relocatable shared libs on macOS + bump requirements + modernize #8869
opencascade: relocatable shared libs on macOS + bump requirements + modernize #8869
Conversation
I detected other pull requests that are modifying opencascade/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
@@ -428,11 +431,7 @@ def _register_components(modules_dict): | |||
system_libs = target_deps.get("system_libs", []) | |||
frameworks = target_deps.get("frameworks", []) | |||
|
|||
self.cpp_info.components[conan_component_target_name].names["cmake_find_package"] = target_lib | |||
self.cpp_info.components[conan_component_target_name].names["cmake_find_package_multi"] = target_lib | |||
self.cpp_info.components[conan_component_target_name].builddirs.append(self._cmake_module_subfolder) |
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.
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.
No. I guess because cmake file is in build_modules. And it's good, you don't want to add this folder to builddirs
(because buildirs
are prepended to CMAKE_MODULE_PATH
in cmake
generator, and we don't want consumer to be able to include this cmake file, it's an internal detail of cmake_find_package
and cmake_find_package_multi
generator).
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.
Well, take a look at the last lines in https://c3i.jfrog.io/c3i/misc/logs/pr/8869/1-configs/windows-visual_studio/opencascade/7.5.0//4eb25c38fccb68e6473f428d27ae7648cf3422c9-build.txt
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.
ok, so it' just a warning now, but this hook is wrong, you don't have to put cmake files listed in build_modules, in builddirs as well.
From hook description:
It is only allowed to put build files in builddirs because the generators might be able to include them when needed, but only if they are located in well known paths.
No, generators use absolute path of the cmake file thanks to build_modules
values, they don't need builddirs
to append something in CMAKE_MODULE_PATH
. builddirs is needed if you want to append something to CMAKE_MODULE_PATH
in cmake or CMakeToolchain generators, that's all. Basically builddirs is for cmake module files which are packaged and may be included directly by consumers in their CMakeLists.
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.
I think we first have to make this discussion in https://github.com/conan-io/hooks/, and fix the hooks, before spreading this all over CCI
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
damned |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This recipe is so hard to build in CI of CCI. |
Specify library name and version: lib/1.0
This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!
conan-center hook activated.