I wonder if this could use more tweaking to avoid cross-build issues. pkg-config is happy to pick up native packages in cross-builds, which breaks them, requiring builders to manually disable options pkg-config happens to pick up.

This was seen when doing Windows builds from Linux, and I wonder if pkg-config is more clever when doing other kinds of cross-builds (my guess: it's not).

Enclosing the pkg-config stuff in this might help?:

if(NOT CMAKE_CROSSCOMPILING)
  find_package(PkgConfig QUIET)
  pkg_search_module(...)
endif()

That would break Unix→Unix cross-builds, so if pkg-config is known to work OK with those (or we just don't want to break compatibility), this might be an alternative (also allowing non-Unix→Unix, for better or worse):

if(UNIX OR NOT CMAKE_CROSSCOMPILING)
  find_package(PkgConfig QUIET)
  pkg_search_module(...)
endif()

edit: Or, is there a CMake setting to control pkg-config behaviour in cross-builds? E.g. disabling it, or setting a custom prefix? Perhaps CMAKE_PREFIX_PATH?

@vszakats Are you planning to merge it next version window?

Yes, in the next one, we have time to discuss.

I hoped you would say no :D

I tried also adding c-ares:

D:\a\curl\curl\lib\asyn-ares.c(332,13): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
D:\a\curl\curl\lib\asyn-ares.c(332,13): warning C4996: 'ares_getsock': Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]

and mbedtls

D:\a\curl\curl\lib\vtls\cipher_suite.c(193,3): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
D:\a\curl\curl\lib\vtls\cipher_suite.c(193,3): warning C4310: cast truncates constant value [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
... the same warning between these lines: 193-718. not continuously. 
D:\a\curl\curl\lib\vtls\cipher_suite.c(718,3): warning C4310: cast truncates constant value [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
D:\a\curl\curl\lib\vtls\mbedtls.c(1016,17): warning C4013: 'mbedtls_ssl_get_version_number' undefined; assuming extern returning int [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj

Also found a patch in vcpkg for mbedtls:
https://github.com/microsoft/vcpkg/blob/master/ports/curl/fix-mbedtls.patch

Can you take a look?

The mbedtls patch was copied from 0c4b4c1, and should be fine.

In general, yes, such fallouts are a sign that the new builds/tests increase code coverage by revealing issues. I suggest opening a separate PR for them, and continue there.

As for pkg-config, merging it early in the next cycle, allows us to gather more feedback and experience before the next release. I think it'd be nice to bring cmake dependency defaults and detection capabilities closer to ./configure. This is one such step, but some of those are breaking.

I will create a PRs for each one.

Brainstorming: The internet suggests that these envs should be set for cross-compiling, used by pkg-config to do the correct thing:
PKG_CONFIG_DIR, PKG_CONFIG_LIBDIR, PKG_CONFIG_SYSROOT_DIR.
For ./configure this is already used in CI. How is this expected by people using CMake, I don't know. It's also an option to require (some of) these envs to enable pkg-config in cross-builds. Or put out a log message, if missing.

Brainstorming: The internet suggests that these envs should be set for cross-compiling, used by pkg-config to do the correct thing: PKG_CONFIG_DIR, PKG_CONFIG_LIBDIR, PKG_CONFIG_SYSROOT_DIR. For ./configure this is already used in CI. How is this expected by people using CMake, I don't know. It's also an option to require (some of) these envs to enable pkg-config in cross-builds. Or put out a log message, if missing.

Why not implement the same logic that is on autotool and do the same inside the cmake?

We can use autotools logic as an inspiration, but if there is a CMake "best practice" for this, it'd be nice to follow.

Experiments show that export PKG_CONFIG_LIBDIR= prevents mis-detections in cross-builds. It doesn't seem like common practice in CMake, but a way to handle this is:

if(NOT(CMAKE_CROSSCOMPILING AND "$ENV{PKG_CONFIG_LIBDIR}" STREQUAL ""))
  find_package(PkgConfig QUIET)
  pkg_search_module(...)
endif()

According to https://cmake.org/cmake/help/v3.30/variable/CMAKE_CROSSCOMPILING.html, CMAKE_CROSSCOMPILING is true set when setting CMAKE_SYSTEM_NAME manually.

Since nobody uses this solution, I wonder if there is something better than this. Or, is this not an issue for most?

I also tried -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=OFF / set(PKG_CONFIG_USE_CMAKE_PREFIX_PATH OFF) but it didn't have the desired effect.

edit: FWIW I've seen this issue and tested these with Libgcrypt + libssh2 in the Debian → Windows cross-build.

@vszakats I think is better to post this msg on discussions that more people can react and talk about it.
You can tag me as well on that discussion.

Feel free to open a discussion. Feedback may definitely help.

I know that official Find modules in CMake use the same pattern, but I consider this a bad pattern:
Use pc files to determine the installation prefix, but ignore the pc files for detecting library names, include paths etc.

The canonical CMake pattern is to use input variables to guide the search for libs and include paths.

I do understand that this approach was or is useful for some contexts, in particular transitions.
IMO this PR is not needed, and probably not beneficial, for vcpkg.

@dg0yt Would something like in #14140 (comment) make it better for vcpkg? Also, which vcpkg platforms do you mean, as not beneficial for? (for UNIX targets this proposal doesn't change anything, only for non-UNIX, aka Windows)

To be clear, I don't say it is bad for vcpkg. I say there is zero benefit. vcpkg cannot not be the reason to proliferate the pattern. MSYS2 etc. are a different story.

Regardless of windows or build type (debug, release): vcpkg generally sets PKG_CONFIG_PATH correctly to find valid pkg-config files. In the past vcpkg relied on pkg-config for building GDAL with nmake/MSVC, and at the moment ffmpeg is a port which relies on pkg-config for many dependencies also on Windows.

The flaw with the CMake HINTS approach is that it ignores the key feature of pkg-config files: the content. The location of the pc file is just a very poor hint. Problems arise in the following cases:

• The name of the library is correct in the pc file content but doesn't match the find modules find_library pattern.
  In particular on Windows, libs sometimes carry extra suffixes for debug builds or for static libs.
• The find_library pattern lacks the NAMES_PER_DIR option.
• The headers are not in the expected subpath.
  In particular in vcpkg, headers for the debug build are in <prefix>/../include.

Depending on how search locations and cache variables play together. This may lead to a mix of debug and release binaries, or to mismatch of headers and binaries. This may explode at configuration time, at build time, or at runtime.

Edit: In addition, the content can provide the actual link libs for static linkage. This valuable information is lost when only looking at the location.

@dg0yt Thanks for the info. I understand pkg-config support in CMake is "somewhat haphasard", based on your description. Also, that vcpkg does the right thing with PKG_CONFIG_PATH.

How it works with MSYS2, cross-builds targeting Windows, and other "random" Windows cases, remain the questions.

How it works with MSYS2, cross-builds targeting Windows, and other "random" Windows cases, remain the questions.

Basically you identified the main problem: Finding the right pc files for non-native builds. Users who don't know how to setup pkg-config for a cross build are most likely to stumble over the automatism. CMake users might be unaware of pkg-config interfering with CMake dependency lookup. Simply installing pkg-config may change a working environment into an environment with problems.

IMHO the key is to use a fairly up-to-date CMake, and to leverage CMake's original search patterns, e.g. for find_libary:

• for particular packages: <PackageName>_ROOT (since 3.12; also as an environment variable since 3.27)
• for all libs: CMAKE_LIBRARY_PATH
• general: CMAKE_PREFIX_PATH
• etc.

It comes down to two solutions:

1. be defensive and try to guard pkg-config logic under conditions.
2. let it run and expect CMake envs/users to have things (like PKG_CONFIG_PATH) configured accurately to have the expected results.

We can kick off with either and switch to the other when issues are reported. The conditions can also be fine-tuned to reach the best experience for most.

(I'd probably start defensive.)

For a defensive approach:

The user might provide -DCMAKE_DISABLE_FIND_PACKAGE_PkgConfig=1. Then there wouldn't be pkg_check_modules, and the resulting guard would be PKG_CONFIG_FOUND.

The picture in CMake's official modules is "mixed", but that pattern exists:

find_package(PkgConfig QUIET)
if(PKG_CONFIG_FOUND)
  pkg_check_modules(PC_CURL QUIET libcurl)
  ...

Note that CMAKE_DISABLE_FIND_PACKAGE_PkgConfig=1 breaks configuration at the moment. The proposed pattern would add a benefit to UNIX users.

Adding if(PKG_CONFIG_FOUND) would be useful regardless of this particular PR, wouldn't it?

-DCMAKE_DISABLE_FIND_PACKAGE_PkgConfig=1, based on GitHub search, is not commonly used. But, would be a plus if it worked with curl.

The PKG_CONFIG_LIBDIR= trick seems more discoverable, but also somewhat hard to figure out. (PKG_CONFIG_PATH= didn't prevent a mis-detection in my Linux cross build test.)

Adding if(PKG_CONFIG_FOUND) would be useful regardless of this particular PR, wouldn't it?

Yes.

-DCMAKE_DISABLE_FIND_PACKAGE_PkgConfig=1, based on GitHub search, is not commonly used. But, would be plus if it worked with curl.

Maybe not seen very often, but it is a general CMake pattern. CMake know-how for CMake users.

The PKG_CONFIG_LIBDIR= trick seems more discoverable, but also some hard to figure out. (PKG_CONFIG_PATH= didn't prevent a mis-detection in my Linux cross build test.)

But it requires pkg-config tool know-how in order to AVOID use pkg-config.
PKG_CONFIG_LIBDIR overrides pkg-config's builtin search dirs.
PKG_CONFIG_PATH is searched first.

@vszakats
When compile gnutls with curl in vcpkg, it not detected correctly the nettle dependency when compile cmake with curl.
Can you add both gnutls and nettle with pkg-config (in the defensive like you want to do for all pkg-config) also for windows:

When compile gnutls with curl in vcpkg, it not detected correctly the nettle dependency when compile cmake with curl.

This is the problem of transitive usage requirements (in particular for static library linkage). Again, this is encoded in the content of the pc files. There is no Find module in CURL that makes use of the content. This PR only changes which locations to search with find_....

(Does CURL actually directly depend on nettle? If it does not, it should not have any explicit references to "nettle".)

And that's why we continue to patch CURL in vcpkg.

@dg0yt I don't mind if you solve the real problem here in curl,
but we can add also in defensive pkg-config also for gnutls like other libs.

@talregev: I suggest sticking to the original point of this PR.

The nettle reference is not ideal (it is indeed not a direct curl dependency), but I have no idea or time to fix it. Feel free to open a PR to improve it, or report a bug with the necessary info. I'm not a GnuTLS user and that was apparently the best approach at the time for an initial GnuTLS detection support in our CMake.

Correction: nettle is a direct dependency of curl. (when using the GnuTLS backend)

Correction: nettle is a direct dependency of curl. (when using the GnuTLS backend)

can we use find package nettle when using gnutls?

Yes, it was the first option I tried, but find_package(nettle REQUIRED) doesn't work:
https://github.com/curl/curl/actions/runs/10122645736/job/27995045993?pr=14285#step:6:47

Yes, it was the first option I tried, but find_package(nettle REQUIRED) doesn't work: https://github.com/curl/curl/actions/runs/10122645736/job/27995045993?pr=14285#step:6:47

I understand, that why I suggested with pkg-config.
I tried and it working. I will create a PR when vcpkg will updated in curl ci.

How would nettle be detected when pkg-config isn't available?

How would nettle be detected when pkg-config isn't available?

find_library?

Yes, it looks it will need a custom find module, with find_library and pkg-config (pkg-config alone is not enough).

you may also want to add this?
#14199

Reminder: Please do

find_package(PkgConfig QUIET)
if(PKG_CONFIG_FOUND)
  pkg_check_modules(PC_CURL QUIET libcurl)
  ...

so that users can opt out of it globally with CMAKE_DISABLE_FIND_PACKAGE_PkgConfig.

Speaking of if(PKG_CONFIG_FOUND), I've tested it and found it redundant. We require CMake 3.7 and this version (and newer) do a similar check inside pkg_check_modules() using if (PKG_CONFIG_EXECUTABLE).

This has the benefit that all the expected output variables will be initialized. With the manual/local solution they won't be. Either way, adding an extra local check doesn't seem to change the outcome of the detection.

(Just realized that I missed to push this commit to the nettle PR)

I also tested -DCMAKE_DISABLE_FIND_PACKAGE_PkgConfig=1, and it breaks CMake's built-in detection:

CMake Error at /usr/local/Cellar/cmake/3.30.0/share/cmake/Modules/FindGnuTLS.cmake:46 (PKG_CHECK_MODULES):
  Unknown CMake command "PKG_CHECK_MODULES".
Call Stack (most recent call first):
  CMakeLists.txt:543 (find_package)

What am I missing?

CMake Error at /usr/local/Cellar/cmake/3.30.0/share/cmake/Modules/FindGnuTLS.cmake:46
...

(Workaround if not needing gnutls: CMAKE_DISABLE_FIND_PACKAGE_GnuTLS.)

What am I missing?

This:

The picture in CMake's official modules is "mixed

"Mixed" includes "broken".

FTR I still think it is an anti-pattern to use FindPkgConfig.cmake only to determine prefix hints.

• It ignores available information about binary names, e.g. debug suffix.
• It is not efficient: Each call to pkg_check_modules running pkg-config twice, normal and --static.
• It ignores available information about transitive usage requirements.
• WRT transitive usage requirements: pkg-config doesn't just look for a module file. It actually calculates a solution to a dependency satisfaction problem encoded in Requires[.private]. Including version constraints.
  (BTW both pkg-config and pkgconf pull cflags from Requires.private even when not asking for --static.)
• The installation of a pkg-config program can change a working environment into a misconfigured environment.

(Disclaimer: I spent more time on pkgconf than on cmake this year.)

CMAKE_DISABLE_FIND_PACKAGE_GnuTLS is a workaround for a workaround, both not very discoverable.
I understand you say that CMake is buggy, but then why expose more bugs and force everyone to work it around?
It leaves me unconvinced we need an extra PKG_CONFIG_FOUND check.

The anti-pattern you mention looks like a separate issue from the rest discussed here.

I agree that using pkg-config detection as a path hint is sloppy and wrong. I don't know where it originally comes from, but it looks widespread practice. It caused problems in libssh2 with Libgcrypt which I'm trying to fix here: libssh2/libssh2#1420.

Some things learned there I applied to the recent nettle and libssh PRs in curl: e8acb2e 669ce42

As for what CMake does inside pkg_check_modules, we can't do much in curl. If there is a bug, it should be reported to CMake. In curl we can avoid common issues and/or allow a discoverable/documented mean to override. In curl we also deal with a limited set of dependencies, so we probably see much less of these issue than you see in context of the whole vcpkg project.

I'm abandoning this effort.

@vszakats Don't give up because one man opinion. @dg0yt have great advices but we don't have to use all of them.
I still found this PR useful, and it will help me to add more tests here on curl.
Please consider to open this PR, and work your way.

Thank you again for your hard work.