Support dynamic linkage of tests

[pemensik]

If libuv is not fetched, try to find and provide libuv parameters using pkg-config. Makes dynamic linking to shared libuv library from the system.

Expects just default header-included mode. It should be possible to link dynamically to libuv also on built libraries. I don't think it is possible now. But since even shared build does not have library versioning, I am not sure it is ready for Linux distributions in that mode.

[skypjack]

LGTM. @stefanofiorentino any concerns?

[pemensik]

Example how I want to use it: https://github.com/pemensik/uvw/blob/fedora/uvw.spec

[stefanofiorentino]

@pemensik I recall we are trying to get rid of pkg-config by completely support CMake includes. Would be this thing a good thing for you, or you need a specific pkg-config support? We now have a static-only linking support for the libuvw.a in pkg-config, without versioning. I don't think it would be improved in such a way to be fully-supported to official distributions. If you have any concerns about this, and have ideas on this topic, please propose a PR and I would be very glad to discuss it with you.

[pemensik]

@stefanofiorentino If libuv had working cmake module support, it would be fine to me. It seems libuv already supports pkg-config configuration, but I have seen no attempt to provide also cmake package for find_package(). Existing libuv package on Fedora provides pkg-config file but not cmake module, it would require modification of libuv package first. I have never prepared new cmake library file myself, perhaps https://github.com/nlohmann/json/tree/develop/cmake might be used as an example. But libuv would have to use also such files IMO. pkg-config is sufficient for me, because it is always available on Linux distribution.

Linux distributions always require linking to shared libraries. Only header-only libraries or shared libraries are allowed in Fedora, static library would require special exception to be allowed inside.

[pemensik]

I have adjusted the change a bit. I think it does not have to assume every unix has PkgConfig. It might just try to find the cmake package. If there is, try to use its parameters.

I think the main question should be how do you expect people to use uvw library in their projects? For example json library in flamethrower is quite pleasant to be used. cpp-httplib is more similar to your project with existing package, but it does not have dependency library.

I have no system with cmake and compiler but without pkg-config installed, I cannot test it works properly without it. How is it expected user should supply libuv parameters if he wants FETCH_LIBUV=OFF? That build configuration seems a bit unresolved. I expect you do not use that possibility often?

[stefanofiorentino]

How is it expected user should supply libuv parameters if he wants FETCH_LIBUV=OFF? That build configuration seems a bit unresolved. I expect you do not use that possibility often?

I agree with you. Every uvw release is tightly coupled with its specific libuv version. So 99% of time the current uvw release is not working with a simple checkout+build (within the FETCH_LIBUV=OFF build configuration). I think it just won't work anyway. If you need a pkg-config configuration file, specific to shared library, please let me know and I will provide you with a track to follow to create a new PR.

NOTE: sudo make install behavior is changing in a concurrent PR to encapsulate libuv inside a uvw subfolder and avoid to pollute the system libs directory.

[pemensik]

In fact, libuv has quite stable ABI. It never had changed so version since it is packaged on Fedora. That means I think only minimal version of libuv, containing all expected constants, is required. Upgrading libuv forward without upgrade of uvw should work just fine. Then I think there is no need for static copy if shared libuv is available. After compiled into application, even replacing libuv with older version library should work without breaking the application. Version dependency seems to be present only on build time.

[stefanofiorentino]

@pemensik sorry I lost track of this. We just merged into the master an enhanced support for CMake. Could you please try that and see if it is enough for your purposes?

[stefanofiorentino]

@skypjack here we need you. In the experimental HEAD https://github.com/skypjack/uvw/commit/277b7c77a3b35cf8834ab900771f6f13e56a9b2f we have decommissioned the pkg-config way of using this lib. As this PR is stalled, I guess we can decline it. @pemensik anything against this decision?

[skypjack]

I agree @stefanofiorentino let's see if @pemensik has anything to add to the discussion though. I'll close the PR immediately after Christmas otherwise. 👍

[pemensik]

I am sorry for delay @skypjack . I would like to prepare changed version of this proposal after Christmas. Because libuv merged change to export libuv via cmake, I would like to use it for shared library detection instead of pkg-config. But I think no release yet contains that change. I had not found yet enough time, I would have to prepare snapshot of libuv for testing first. I would like to prepare some change first.

Merry Chistmas 🎄!

[pemensik]

What do you think about my changes? I provide alternative dependency, which is already handled if defined by tests. It would work with cmake module provided by libuv, but uses also pkg-config provided by existing releases. Happy New Year!

[skypjack]

I think the idea behind FETCH_LIBUV is that you'll download and link it on your own if turned off. It's probably better to add a second option in this case, something like FIND_LIBUV or similar. Does it make sense?

[pemensik]

Why should you link it on your own if libuv is mandatory dependency? The point of uvw usage it avoiding libuv specific code, right? No application can work without libuv. Why should user care for manual libuv linking if his system has all required code available? And he/she is using only uvw directly. I avoided making missing libuv fatal, it would still allow you link manually. But I expect most of users having libuv development files available and in correct location would like it used.

In case cmake -DBUILD_TESTING=ON -DFETCH_LIBUV=OFF, manual linking would not work. Building of tests would fail. I think FIND_LIBUV is possible but I fail to see a reason for it being separate. Do you think -DFIND_LIBUV=ON should set -DFETCH_LIBUV=OFF if found?

Should it work similar to FIND_GTEST_PACKAGE?

[stefanofiorentino]

Do you think -DFIND_LIBUV=ON should set -DFETCH_LIBUV=OFF if found?

They appear exclusive to me.

[skypjack]

Why should you link it on your own if libuv is mandatory dependency?

As far as I remember, this was introduced because of users with multiple instances of libuv available. For example, one that works system wide and one that is meant to be used for a specific project (i.e. because the target uses that version of the library).

[pemensik]

As far as I remember, this was introduced because of users with multiple instances of libuv available. For example, one that works system wide and one that is meant to be used for a specific project (i.e. because the target has uses that version of the library).

I think libuv_DIR variable can be used to specify correct libuv instance module. I admit that would work only for cmake module, not for pkg-config. Some pkg-config variables can be used to select correct pkg-config module as well.

[pemensik]

Removed required flag when pkg-config is found. If FIND_LIBUV is enabled and libuv of minimal version is found, disable fetching by default. Can be still overridden explicitly. Added version check to both cmake and pkg-config search, it should accept only working version, otherwise should fetch recent version. I think it might be ON by default, but that can be changed later.

[pemensik]

I have verified it builds on Fedora 33 with correct package and on RHEL8 also, which has previous version 1.41.0. Both configured and built fine with cmake .. -DFIND_LIBUV=ON -DBUILD_TESTING=ON -DFIND_GTEST_PACKAGE=ON. RHEL8 of course used fetched library.

[skypjack]

Re-running tests, they timed out. @stefanofiorentino any comments? It's a bit convoluted maybe but I don't see many alternatives if we want both find and fetch or nothing. 🤷‍♂️

[stefanofiorentino]

Re-running tests, they timed out. @stefanofiorentino any comments? It's a bit convoluted maybe but I don't see many alternatives if we want both find and fetch or nothing. 🤷‍♂️

To be honest, I would prefer to leave to the lib's client the linking activities. I'm not sure either the header-only mode is fully preserved or not. Need time to check, sorry for this.

[skypjack]

No worries at all @stefanofiorentino let's sync later during the week 👍 I see your point. Thanks.

[stefanofiorentino]

I would keep the two flags FIND and FETCH in a logically XOR (error otherwise). Furthermore, I'd like to be more verbose and change FIND_LIBUV in FIND_SYSTEM_LIBUV or even FIND_SYSTEM_PROVIDED_LIBUV. Is that too much?

[pemensik]

I would keep the two flags FIND and FETCH in a logically XOR (error otherwise). Furthermore, I'd like to be more verbose and change FIND_LIBUV in FIND_SYSTEM_LIBUV or even FIND_SYSTEM_PROVIDED_LIBUV. Is that too much?

If FIND_LIBUV and FETCH_LIBUV makes an error in all cases where they have equal value, what is purpose of separate variables then? If variable cannot be different than FIND_LIBUV == NOT FETCH_LIBUV, why is the second variable even supported?

I would like to keep variable name short. Unless you use cmake-gui it is annoying when you have to type long name. It could be explained better in a comment, right? In fact I would like having just USE_SYSTEM_LIBRARIES=ON and it would try finding both gtest_package and libuv. If gtest is found, it may enable also building tests automatically. I have to use now 3 different variables for distribution package running both tests and using shared library. I expect other distributions have similar requirements.

[skypjack]

He's not wrong actually. A single variable (let's say UVW_REQUIRE_LIBUV) is enough. You can have it empty (and we just ignore it, no find nor fetch) or set to a specific version (in this case, we can search locally and then fetch in case of errors). We don't really need to make it complex with more variables and a bunch of ambiguous corner cases to work around. Is there any obvious problem that I don't see in the simplified version?

[stefanofiorentino]

If gtest is found, it may enable also building tests automatically.

I don't agree with this. BUILD_TESTING is a de-facto standard sort-of..

A single variable (let's say UVW_REQUIRE_LIBUV) is enough.

Agreed on this.

or set to a specific version

I'm wondering how many times the exact version of libuv will be found on systems (~0 ??). Or I misunderstood something?

[skypjack]

I'm wondering how many times the exact version of libuv will be found on systems (~0 ??). Or I misunderstood something?

In theory, nothing prevents me from having multiple versions of libuv installed, as long as I also have enough soft links to please my system. 😅 Therefore, technically speaking it can be ~0 as well as all the times. I don't want to base the choice on my own experience. 🙂 That being said, we can also an UVW_REQUIRE_LIBUV that finds-or-fetches libuv (any version), then refine it to support the versions if required. At least, this should get rid of the ambiguity between FIND_LIBUV and FETCH_LIBUV and also address the vast majority of cases. @pemensik does it make sense to you?

[stefanofiorentino]

this should get rid of the ambiguity between FIND_LIBUV and FETCH_LIBUV

And that is already a good enough outcome to me.

[pemensik]

I think libuv is mandatory, I do not really see your point why UVW_REQUIRE_LIBUV should ever be OFF. If you have custom library location, okay, allow them used. If those libuv versions export cmake module, there should be simple way with libuv_ROOT or libuv_DIR variable. If custom compiler flags are passed to select correct library, okay. But it would never work without libuv at all, right? I don't understand why we don't solve it by optionally forced path to libuv build for overriding manual builds. Most people would use just single version, which is the most easy to install.

[stefanofiorentino]

From the description @skypjack made, UVW_REQUIRE_LIBUV looks like the already present BUILD_UVW_LIBS. Maybe it adds only the logic to have a specific version to be linked. Is that right?

@pemensik do you know uvw also support header-only inclusion? If BUILD_UVW_LIBS:BOOL=FALSE and BUILD_TESTING:BOOL=FALSE, users just add the ${libuv_SOURCE_DIR} to the include_directories and #include <uvw.hpp>. Did you miss this case?

@all I guess we should enumerate the possible combination and apply De Morgan's law ;)

[skypjack]

Yeah, my suggestion to make it optional is due to the header-only nature of the library. I wouldn't expect it to also export libuv as a dependency in this case. Let's try to sum up it though. I agree.

[pemensik]

I thought it is possible to add libuv as dependency of uvw and then just use find_package(uvw) from end project using uwv. Without explicitly mentioning find_package(libuv) in that end project. But it does not seem it is possible. I think cmake expect interface libraries consisting of only headers to not depend on normal library. I do not think it allows setting required combination without -DUVW_BUILD_LIBS=ON. I have found easier reusing of pkg-config subsystem to earn cmake compatible target, makes it a bit simpler.

So okay, let's keep libuv optional in uvw, because there does not seem to be nice and simple way to configure it. Copying find_package or pkg-config from uvw CMakeLists.txt seems simpler and better understood.

I have used this minimal client build:

# CMakeLists.txt
cmake_minimum_required(VERSION 3.13)

project(uvw_test)

find_package(uvw)
find_package(libuv)

add_executable(test test.cpp)
target_link_libraries(test uvw::uvw uv)

// test.cpp
#include <uvw.hpp>
#include <iostream>

uvw::Utilities utils;

int main(int argc, char *argv[])
{
        std::cout << "Test of uvw, " << uvw::Utilities::OS::hostname() << std::endl;
        return 0;
}

Tuned it with -Duvw_DIR=/tmp/uvw-.../cmake/uvw to find tested instance. It would be nice if uv could be omitted from it, but there seems too many variants each with some problem. I am somehow satisfied with -DFIND_LIBUV=ON turning off default fetch of libuv. Flamethrower is using libuv explicitly anyway. I just wanted it to use uvw as external library, not bundled sources.

Would you have any other examples how uvw library is used by the end projects?

[stefanofiorentino]

Would you have any other examples how uvw library is used by the end projects?

I prefer the static linkage, so I always add_subdirectory to the fetched by cmake or fetched by the repo-tool. A second approach is include the cmake file generated by install target of the uvw project (you will have kind-of automatically included the specific libuv target version in your ${LIBDIR}/uvw/ subfolder). Did you know it?

[stefanofiorentino]

A third approach can be to export a hybrid target: uvw shared lib with linked the static uv_a so you'll depend only on headers.

We decided months ago to not offer this weird solution, though.

[skypjack]

Time to sum up and to close this one. My mobile app refuses to show new messages since the thread is too long. 😅 Did we find an agreement in the end? I got a little lost in all the intertwined discussions. Is anyone able to put down what we are missing, please?

[pemensik]

I kind of like my current proposal. Should I rename FIND_LIBUV to UVW_REQUIRE_LIBUV? I would prefer keeping it just simple boolean, not explicit version.

If uvw had also soversion, I would prefer building it like normal library. That is required by Fedora shared library guidelines. It would require increasing that version on any incompatible change to API. Then it would be possible adding libuv as public linked target to uvw and have it propagated. Header-only uvw does not allow it from cmake it seems. But let's stick to simplest solution and leave similar attempts to later.