Skip to content
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

cmake: limit pkg-config to UNIX and MSVC+vcpkg by default #14575

Closed
wants to merge 2 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Aug 16, 2024

Limit pkg-config to UNIX and MSVC with vcpkg, by default. Compared to
curl 8.9.1, this unlocks pkg-config on MSVC with vcpkg.

This condition might be updated in the future depending on where
pkg-config can be useful without breaking things. (e.g. to non-cross
MINGW, or all MINGW).

In the meantime everyone is free to override the default and test their
build with pkg-config by setting the CURL_USE_PKGCONFIG=ON CMake
option.


Peeking into CMake's built-in Find modules: Around 10% support
pkg-config, some limited to UNIX, some guarded with package-specific
flags, some unguarded:

$ ls -1 Find* | wc -l
     179
$ grep -l -R HINTS Find* | wc -l
      68
$ grep -l -R PkgConf Find* | wc -l
      19

Though we tested the waters in 8.9.1 via the libidn2 dependency (ON by default with
pkg-config), I'm still concerned that unlocking pkg-config almost everywhere (for all
dependencies this time) might be overshooting the target, causing fallouts and manual
opt-out in more cases than ideal.

I also reckon that there has been a lot of changes already around CMake in this release cycle,
so unlocking pkg-config now may be too much to handle at once.

Thus, this PR limits pkg-config to UNIX and MSVC with vcpkg, by default. Compared to
8.9.1, this unlocks pkg-config on MSVC with vcpkg.

Nobody has been publicly complaining about missing pkg-config on their platform.
If anyone does, or we have a future release with fewer changes all around, we might
unlock this by default for more targets (e.g. non-cross MINGW, or all MINGW).

In the meantime everyone is free to override the default and test their build with
pkg-config by setting the CURL_USE_PKGCONFIG=ON CMake option.

Let me know what you think!

edit: pkg-config support does not break curl under MSYS/MINGW. Ref: #14575 (comment)

@talregev
Copy link
Contributor

talregev commented Aug 17, 2024

Let me know what you think!

I agree. Let's hear more people opinion.

@vszakats
Copy link
Member Author

/cc @lazka

@lazka
Copy link

lazka commented Aug 19, 2024

I can only speak for MSYS2/MINGW, and using pkg-config there shouldn't be a problem. We've only switched away from autotools a few months ago, so we were using pkg-config before that. But cmake is also fine, as long as it builds :)

@vszakats
Copy link
Member Author

I can only speak for MSYS2/MINGW, and using pkg-config there shouldn't be a problem. We've only switched away from autotools a few months ago, so we were using pkg-config before that. But cmake is also fine, as long as it builds :)

Thanks! I've seen you've switched to CMake :) This is still about CMake, and whether or not to allow looking up dependencies via pkg-config (within the CMake build), by default. Or put it differently, could it break something for MSYS/MINGW badly if we enabled it by default? [ You'll be able to test that with -DCURL_USE_PKGCONFIG=ON with 8.10.0; with current master it's still enabled for MINGW, but I plan to disable it. ]

@lazka
Copy link

lazka commented Aug 19, 2024

I just did a rebuild of current master with CURL_USE_PKGCONFIG=ON with our configs for openssl/gnutls/schannel, and all looks good.

@vszakats
Copy link
Member Author

Thank you @lazka!

@vszakats vszakats closed this in c555ab4 Aug 19, 2024
@vszakats vszakats deleted the cm-pkg-config-reduct branch August 19, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants