search

oneapi-src / level-zero

oneAPI Level Zero Specification Headers and Loader
https://spec.oneapi.com/versions/latest/elements/l0/source/index.html
MIT License
211 stars 90 forks source link

spdlog: Use system library if found #154

Closed frantisekz closed 2 months ago

frantisekz commented 3 months ago

Distributions prefer to use system libraries if present, this will make level-zero use the system spdlog if one is found, falls back to the bundled one otherwise.

lisanna-dettwyler commented 2 months ago

We'll accept this if you place the lookup behind a cmake flag that distros can enable if they want to. We want the bundled headers to be used by default.

LecrisUT commented 2 months ago

We'll accept this if you place the lookup behind a cmake flag that distros can enable if they want to. We want the bundled headers to be used by default.

It is customizable with CMAKE_DISABLE_FIND_PACKAGE_spdlog, but the default would still be the opposite. Could you elaborate on what your concerns are with having default in one way or the other? Typically I see projects implement fallthrough mechanism, or more modern is to rely on FetchContent(FIND_PACKAGE_ARGS) which gives maximum control to users, developers and packagers. (It can work offline with local/submodule third-party sources)

lisanna-dettwyler commented 2 months ago

It is customizable with CMAKE_DISABLE_FIND_PACKAGE_spdlog, but the default would still be the opposite. Could you elaborate on what your concerns are with having default in one way or the other? Typically I see projects implement fallthrough mechanism, or more modern is to rely on FetchContent(FIND_PACKAGE_ARGS) which gives maximum control to users, developers and packagers. (It can work offline with local/submodule third-party sources)

If we default to system headers, at best nothing changes, but at worst a breaking change gets introduced.

LecrisUT commented 2 months ago

but at worst a breaking change gets introduced.

spdlog is responsible to make sure they don't have breaking changes in the same major version as they are using COMPATIBILITY SameMajorVersion https://github.com/gabime/spdlog/blob/c3aed4b68373955e1cc94307683d44dca1515d2b/CMakeLists.txt#L356

Version checking is handled by simply adding a 1.x.y version for the find_package argument

frantisekz commented 2 months ago

We'll accept this if you place the lookup behind a cmake flag that distros can enable if they want to. We want the bundled headers to be used by default.

I've added this behavior, if SYSTEM_SPDLOG flag isn't specified, the project defaults to use the bundled spdlog.

lisanna-dettwyler commented 2 months ago

One last thing, please re-fetch and add signed-off-by line to the commit.

frantisekz commented 2 months ago

One last thing, please re-fetch and add signed-off-by line to the commit.

Aaand done :)