Reuse gtest on system

[pemensik]

Try to use gtest on the system if found.

[pemensik]

This will use gtest detected by pkg-config and avoid downloading it during build. Is required on Fedora package, which does not have internet access during build and check phases.

[yhirose]

@sum01, @jimmy-park, could you review the change? It looks good to me, but I would like to check with CMake experts. :)

[jimmy-park]

@pemensik FetchContent_Declare(FIND_PACKAGE_ARGS) will not work on older versions of CMake (it requires CMake 3.24+). Related suggestion #1468 (comment) was mentioned by @Tachi107 recently. I think find_package(GTest) before FetchContent is more appropriate in this case. I will soon open a pull request for an updated test/CMakeLists.txt which addresses this issue also.

[Tachi107]

@pemensik alternatively you could use Meson, which already uses the system-installed GTest version, produces a more portable pkg-config file, etc ;)

[pemensik]

@jimmy-park feel free to push into my branch, I have allowed that. Pushed modification, is that what you meant?

@Tachi107 my project is using cmake, which requires cpp-httplib. If I build it with meson, I will not have cmake module to use. Just pkg-config module, which requires modification. It is unfortunate used build system causes built output to be different. Especially when both variants generate exclusive content and no build system generates both ways compatible with any of them.

[Tachi107]

@Tachi107 my project is using cmake, which requires cpp-httplib. If I build it with meson, I will not have cmake module to use. Just pkg-config module, which requires modification. It is unfortunate used build system causes built output to be different. Especially when both variants generate exclusive content and no build system generates both ways compatible with any of them.

It's not that hard to generate a CMake Config file from Meson, but really the issue is that CMake's find_package() does not provide a way to automatically use .pc files, which are the closest thing we have to a standard for C and C++ dependency discovery. I'll eventually send a patch to add this extra functionality to the meson.build file, but in the mean time I'd suggest you to just use drop this Find Module in your CMake project; it tries to find cpp-httplib using a CMake Config file, and falls back to pkg-config otherwise.

[jimmy-park]

@pemensik yes, that's what I thought. I will add a small change to move some variables.

[jimmy-park]

@pemensik I couldn't push to this branch because of permission denial. Could you please check it again? or my git settings have some problem.

[pemensik]

Allow edits and access to secrets by maintainers is checked. Not sure how it is supposed to work. Do you have write access into this repository?

[Tachi107]

No, yhirose is the only one with write access. Maybe you can give @jimmy-park write access to your fork, though

[sum01]

I've never used GTest with Cmake, but just at a glance, the docs seem to specify different variables based on the version.

For example:

v3.14	Latest release (v3.26)
GTEST_FOUND	GTest_FOUND
GTest::Main	GTest::Main is deprecated, now uses GTest::gtest_main

Even though this is kind of silly, won't this cause some issues if the builder has an older Cmake version?

[jimmy-park]

@sum01 hmm.. apparently they deprecated those targets since version 3.20. I'm not sure this workaround works on older versions.

find_package(GTest)

if(GTest_FOUND OR GTEST_FOUND)
    if(TARGET GTest::Main AND NOT TARGET GTest::gtest_main)
        add_library(GTest::gtest_main INTERFACE IMPORTED)
        target_link_libraries(GTest::gtest_main INTERFACE GTest::Main)
    endif()
else()
    # FetchContent routine
endif()

[eli-schwartz]

If I build it with meson, I will not have cmake module to use. Just pkg-config module, which requires modification.

What is wrong with:

include(FindPkgConfig)
pkg_search_module(HTTPLIB IMPORTED_TARGET GLOBAL cpp-httplib)

Then simply link to PkgConfig::HTTPLIB.

I've never used GTest with Cmake, but just at a glance, the docs seem to specify different variables based on the version.

What is wrong with:

include(FindPkgConfig)
pkg_search_module(GTEST_MAIN IMPORTED_TARGET GLOBAL gtest_main)

Then simply link to PkgConfig::GTEST_MAIN.

[sum01]

While PkgConf is nice, I guess since it's not commonly installed on things like Windows (and maybe macOS?) it Just requires more workarounds. That's why the built-in modules that come with Cmake are useful.

Just more things to keep in mind stemming from trying to do multi-platform Cmake I suppose.

[eli-schwartz]

Well, at the very least it should be a solution for fedora... For @pemensik's cmake project which depends on httplib, for example.

Although really, cmake isn't commonly installed either -- but people who need it, install it, and the same applies to pkg-config. It's trivially available in macOS via e.g. homebrew, and automatically wrapped on Windows by vcpkg.

[pemensik]

I think the option would be to generate also pkg-config file from cmake build. For example by configure_file cmake statement and a template. That would make pkg-config generated always, maybe just with minor differences. I think generating cmake config from Meson would be more difficult. But at least one way present always would be useful.

But the main topic of this PR is different, maybe separate issue should be created for cmake module vs pkg-config file and discuss it there.

[sum01]

Although really, cmake isn't commonly installed either -- but people who need it, install it, and the same applies to pkg-config. It's trivially available in macOS via e.g. homebrew, and automatically wrapped on Windows by vcpkg.

@eli-schwartz

Well the main difference is you need Cmake to build with Cmake, which means the built-ins are guarnteed to be available, whereas adding PkgConf to the mix means you need to also make sure the user has that too. Just an unnecessary complication if Cmake already has a module to use.

---

@pemensik I'd suggest implementing some form of this #1493 (comment) to avoid the issues I mentioned in #1493 (comment)

I'm also not sure trying to generate PkgConf files with Cmake is a good idea, if the internet is to be believed (see here) considering the loss of information vs Cmake. You'd probably have better luck writing your own PkgConf file than generating one.

[Tachi107]

@sum01 GTest::Main is guaranteed to always be available when using find_package(GTest) for compatibility reasons (it's defined by CMake's FindGTest built-in module before calling the upstream GTestConfig)

[sum01]

@sum01 GTest::Main is guaranteed to always be available when using find_package(GTest) for compatibility reasons (it's defined by CMake's FindGTest built-in module before calling the upstream GTestConfig)

My impression was that since it's listed as "deprecated since v3.20" it'll eventually disappear, and OP may as well take action now instead of waiting for vX.Y.Z when it's gone.

[eli-schwartz]

I'm also not sure trying to generate PkgConf files with Cmake is a good idea, if the internet is to be believed (see here) considering the loss of information vs Cmake.

What loss of information? The information was never present in cmake to begin with -- cmake doesn't even try to detect e.g. openssl, zlib, brotli via pkg-config in order to write proper "requires" metadata so one pkg-config file can depend on another pkg-config file.

And cmake itself is doing considerably worse and dropping information all over the place. The cmake-generated export Targets file provides INTERFACE_LINK_LIBRARIES for ZLIB::ZLIB, OpenSSL::SSL, OpenSSL::Crypto, Brotli::encoder, Brotli::decoder, etc. -- but never defines them, which is why httplibConfig.cmake.in has to go to some very verbose double-handling of this.

With pkg-config it's very simple, try to find the pkg-config dependency "foo", if it is found then link to PkgConfig::FOO and add "foo" to a list pc_requires. Format the pkg-config file with Requires: @pc_requires@. It's the same information in both places.

If you found a dependency using find_package() instead of pkg_check_modules() then you need to write out its -lfoo flags instead.

You'd probably have better luck writing your own PkgConf file than generating one.

No, you absolutely want to sync it with cmake's found packages state, which means encoding the information about what to include in the pkg-config file as CMakeLists.txt logic.

[yhirose]

@pemensik, what is the status of this pull request? It seems like two comments from @jimmy-park are not resolved, yet.

[yhirose]

@pemensik, @jimmy-park, it's hanging here for a while. Can I merge this pull request, or should I close it? Please let me know.

[pemensik]

Yes, I think those changes are ready to be merged. Remaining questions were just unimportant details, marked them as resolved.

[jimmy-park]

@sum01 hmm.. apparently they deprecated those targets since version 3.20. I'm not sure this workaround works on older versions.

find_package(GTest)

if(GTest_FOUND OR GTEST_FOUND)
    if(TARGET GTest::Main AND NOT TARGET GTest::gtest_main)
        add_library(GTest::gtest_main INTERFACE IMPORTED)
        target_link_libraries(GTest::gtest_main INTERFACE GTest::Main)
    endif()
else()
    # FetchContent routine
endif()

@pemensik How about applying this workaround? I think #1493 (comment) should be handled in this pull request for the backward compatibility.

[yhirose]

@pemensik, please let me know when you apply the above @jimmy-park's comment.

[pemensik]

Is that forward or backward compatibility? I have looked into our gtest package for RHEL8, version 1.8.0 does not yet have any cmake module. RHEL9 version 1.11.0 does have only gtest_main target. Latest release on https://github.com/google/googletest is 1.13.0. The same is in Fedora Rawhide and it produces still only GTest::gtest_main target. I haven't found GTest::Main in the whole google organization. Can you provide link to version without GTest::gtest_main target, @jimmy-park ?

[pemensik]

Oh, found that, it is part of cmake-data package for older versions. RHEL8 has version barely required for modern targets, RHEL7 does not even parse main CMakeLists.txt. So RHEL is not a good distribution to test the legacy target. Okay, it may not hurt providing additional target.

[Tachi107]

Two minor nitpicks

[Tachi107]

Can't you create an ALIAS library instead?

[jimmy-park]

Unfortunately, before CMake 3.18, we can not use ALIAS target for GTest::Main that is a non-global IMPORTED library target.

https://cmake.org/cmake/help/latest/command/add_library.html#alias-libraries:~:text=New%20in%20version%203.18%3A%20An%20ALIAS%20can%20target%20a%20non%2DGLOBAL%20Imported%20Target.%20Such%20alias%20is%20scoped%20to%20the%20directory%20in%20which%20it%20is%20created%20and%20below.%20The%20ALIAS_GLOBAL%20target%20property%20can%20be%20used%20to%20check%20if%20the%20alias%20is%20global%20or%20not.

[yhirose]

It's pretty hard for me to keep track of this thread... So is it now ready for merge?

[jimmy-park]

@yhirose Yes, I think it is okay to merge. I have no further discussion about this pull request.

[yhirose]

Thanks all!