xxhash: Add header_only option #25795
Conversation
Define XXH_STATIC_LINKING_ONLY in consumers when the package is not a shared library.
Conan v1 pipeline ✔️Warning Conan Center will stop receiving updates for Conan 1.x packages soon - please see announcement. All green in build 1 (
Conan v2 pipeline ✔️
All green in build 1 ( |
There was a problem hiding this comment.
Thank you for your suggestion.
I understand your point of view about performance benefit, but it's a feature/build to be discussed with the upstream first.
Conan tries to keep as closer as possible to the upstream implementation, and there is no such header-only option supported right now in the upstream.
The XXH_STATIC_LINKING_ONLY is always defined as well.
The Makefile build both static and shared library, but does not offer a header-only option.
The unofficial CMakeLists.txt has support for both static and shared library, but not for header-only.
Plus, checking other package managers, I did not find any of them offering such approach, so I would prefer not integrating this change to the current recipe as we will be maintaining a non-official build.
Please, discuss with the upstream, trying to integrate a header-only option there first.
It needs to be defined for the consumers as well. When this macro is not defined, the internal types are not defined in the header. The implementation defines it because it needs access to those types. Consumers need access to those types in order to allocate them statically. As the naming suggest, this macro should only be defined when linking statically, which is what this PR does.
Header-only usage is something explicitly supported by upstream. See here. It's presumably not part of the build scripts because header-only usage doesn't actually require any build steps. So to be clear, before accepting this, upstream needs a CMake option that builds nothing and installs just the header? |
@uilianries are you able to clarify this? |
Summary
Changes to recipe: xxhash/0.8.2
Motivation
XXH_STATIC_LINKING_ONLYenables static allocation of hash states. This is important for performance reasons.Closes #25778.
Details
header_onlyoption (default: false), defineXXH_INLINE_ALLin consumers in header-only mode.XXH_STATIC_LINKING_ONLYin consumers when the package is not a shared library.