Skip to content

Support zstd also via pkg-config #2121

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 29, 2025
Merged

Support zstd also via pkg-config #2121

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2fe697
Support zstd also via pkg-config
DarthGandalf Mar 28, 2025
cf30acf
Find zstd with pkg-config also in non-required case
DarthGandalf Mar 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,26 @@ elseif(HTTPLIB_USE_BROTLI_IF_AVAILABLE)
endif()

if(HTTPLIB_REQUIRE_ZSTD)
find_package(zstd REQUIRED)
find_package(zstd)
if(NOT zstd_FOUND)
find_package(PkgConfig REQUIRED)
pkg_check_modules(zstd REQUIRED IMPORTED_TARGET libzstd)
add_library(zstd::libzstd ALIAS PkgConfig::zstd)
endif()
Comment on lines -162 to +167
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR appears to be "obviously broken", because it is obvious that attempting to do find_package(cpp-httplib) doesn't include the same changes, thus, it fails.

Aside: maybe about time for the cmake files to generate a pkgconfig file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an unfortunate side-effect of having to manually handle these things in Cmake. While it's not technically unfixable, it's a bit annoying.

You'd realistically need to check if found after find_dependency calls. If not found, run the pkgconf equivalent in the correct quiet/normal mode, create the aliases, etc.

I don't think it's "obviously broken". It'd just fail to find it in the specific case where you built with a pkgconf dependency then tried to find it with find_package. Has no bearing on actually building/using the library, just with post-build cmake integration into other projects.

But I guess I can look into it..

Also, like discussed in other issues before, there's no clean way to generate pkgconfig files as it's not a cmake feature at all. It'd just end up as another file you manually manage like a cmake.in file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zstd project doesn't distribute a find_package() module at all, so the default case involves building with a pkgconf dependency.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for a .pc is much more straightforward because you already know what type of dependency to find so all you have to do is say yes or no do you embed the specific string in the requires field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eli-schwartz that may add support of usage via pkg_check_modules(cpp-httplib) or however it would be called, but wouldn't fix find_package(httplib) unless it's rewritten to use pkg-config, if I understand what you mean

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also allow usage from autoconf or meson. ;)

Meson can execute find_package(httplib) as a dependency finder but it is quite slow and fragile as it involves generating a dummy project which does so, and then using the cmake debug traces to determine what compile arguments were resolved by find_package(httplib). Using a buildsystem-agnostic interchange format allows divorcing oneself from the unreliability of a cmake config file...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All build systems suck, it's not really worth talking about.
Even this project has like 3 I think? Cmake, Meson, and Makefile.
If you want a .pc file then write one, I doubt the PR would be denied.

Also, as I said, Cmake doesn't make generating them any less painless then the current cmake.in config system. It'd have the same problems (and probably more janky).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So erm maybe if all build systems suck that's a reason to support pkg-config (which is just a standard, not a set of competing standards)???

Because, you know. Then I can find cpp-httplib and use it in my code without having to deal with buildsystem proliferation, since I'm not required to run a build system in order to find the compile flags for cpp-httplib. I can even use:

gcc my-code.cpp -o my-program $(pkg-config --cflags --libs cpp-httplib)

"Look ma, no build system!"

set(HTTPLIB_IS_USING_ZSTD TRUE)
elseif(HTTPLIB_USE_ZSTD_IF_AVAILABLE)
find_package(zstd QUIET)
if(NOT zstd_FOUND)
find_package(PkgConfig QUIET)
if(PKG_CONFIG_FOUND)
pkg_check_modules(zstd QUIET IMPORTED_TARGET libzstd)

if(TARGET PkgConfig::zstd)
add_library(zstd::libzstd ALIAS PkgConfig::zstd)
endif()
endif()
endif()
# Both find_package and PkgConf set a XXX_FOUND var
set(HTTPLIB_IS_USING_ZSTD ${zstd_FOUND})
endif()

Expand Down