librdkafka: add 1.9.2 + modernize #12022
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- restore cmake_find_package usage, since find modules have precedence over config files (important for zlib, openssl and libcurl). - drop CONAN_PKG:: logic in old patches, to prepare for conan v2 transition - robust link of zstd targets (zstd::zstd won't exist in CMakeDeps generator, it's either zstd::libzstd_shared or zstd::libzstd_static)
4 tasks
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
uilianries
previously approved these changes
Aug 3, 2022
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
seems that PKG_CONFIG_PATH is not automatically set to generators folder in conan v2 yet
SpaceIm
changed the title
This comment has been minimized.
This comment has been minimized.
What? We can see that libs are properly installed in the log: |
This comment has been minimized.
This comment has been minimized.
|
Can't reproduce locally with latest hooks. I'm running conan 1.51.0, it's conan 1.49.0 in CI? |
SpaceIm
commented
Aug 3, 2022
This was referenced Aug 4, 2022
This comment has been minimized.
This comment has been minimized.
recipes/librdkafka/all/conanfile.py
Outdated
Comment on lines
81
to
82
| self.folders.source = "src" | ||
| self.folders.build = "build" |
There was a problem hiding this comment.
These should be the defaults 🤔 were they explicitly required?
There was a problem hiding this comment.
Good question
signature of cmake_layout is cmake_layout(conanfile, generator=None, src_folder=".")
So the default layout of cmake_layout:
def layout(self):
cmake_layout(self)build/
<package_id>/
build/
<build_type>/
build_artifacts_1
...
build_artifacts_n
generators/
patches/
source_code_file_1
...
source_code_file_n
source/
patches/
source_code_file_1
...
source_code_file_n
- source code and patches folder are mixed up, as well as build folder and source code (not a fully out of source build).
- and less important:
build/<build_type>is unnecessarily long (might lead to path length issue on Windows). Though, default value ofcmake_layoutis"build"for multi config generators.
So I propose instead this layout:
def layout(self):
cmake_layout(self, src_folder="src")
self.folders.build = "build"build/
<package_id>/
build/
build_artifacts_1
...
build_artifacts_n
generators/
patches/
src/
source_code_file_1
...
source_code_file_n
source/
patches/
src/
source_code_file_1
...
source_code_file_n
It would even be better with generators folder outside of build folder:
def layout(self):
cmake_layout(self, src_folder="src")
self.folders.build = "build"
self.folders.generators = "generators"build/
<package_id>/
build/
build_artifacts_1
...
build_artifacts_n
generators/
patches/
src/
source_code_file_1
...
source_code_file_n
source/
patches/
src/
source_code_file_1
...
source_code_file_n
|
All green in build 22 (
|
prince-chrismc
approved these changes
Aug 5, 2022
SSE4
approved these changes
Aug 5, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cmake_find_packagein generators, since find modules have precedence over config files (important for zlib, openssl and libcurl).CONAN_PKG::logic in old patches, to prepare for conan v2 transitionCMakeDepsgenerator, it's eitherzstd::libzstd_sharedorzstd::libzstd_static)