skypjack / uvw

Header-only, event based, tiny and easy to use libuv wrapper in modern C++ - now available as also shared/static library!
MIT License
1.82k stars 207 forks source link

Add static lib when building with system's static libuv #266

Closed darktohka closed 1 year ago

darktohka commented 2 years ago

When building with -DFETCH_LIBUV=OFF, -DFIND_LIBUV=ON and -DBUILD_UVW_LIBS=ON and a static build of libuv, the build fails with the following error:

-- Checking for module 'libuv>=1.44.1'
--   Found libuv, version 1.44.1
-- libuv 1.44.1 found via pkg-config
-- Configuring done
CMake Error at src/CMakeLists.txt:76 (target_link_libraries):
-- Generating done
  Target "uvw" links to:

    uv::uv-static

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

This can be fixed by adding the uv-static target when building uvw as a static library.

skypjack commented 2 years ago

LGTM. @stefanofiorentino ? @darktohka can you point your PR to the v3 branch, please? It will replace master as a whole soon. Thanks.

stefanofiorentino commented 2 years ago

ciao @darktohka, this line is not totally convincing me. Distribution packages does usually not provide the static version of libs. Have you a custom environment? (aka did you manually install libuv?)

darktohka commented 2 years ago

Yes, I've compiled and installed libuv as a static library. pkgconfig can detect the static variant as well.

stefanofiorentino commented 2 years ago

Yes, I've compiled and installed libuv as a static library. pkgconfig can detect the static variant as well.

So as it is a custom setup, and not the one that normally comes with package management systems, I think you should keep the mod on your side a patch. Or do you think we can find a way to generalize the behavior without aliasing the uv-static to a possible dynamic lib?

darktohka commented 2 years ago

How about this?

We can instead search for libuv-static through pkgconfig, which is guaranteed to be a static library.

stefanofiorentino commented 2 years ago

I'm still not fully convinced by the introduction of a new variable only for this special case. But I've to admit I need to dig deeper into this.

0EVSG commented 2 years ago

Stumbled upon this because I just created a local FreeBSD package of uvw for testing:

  1. It's not uncommon for system packages to provide both static and dynamic libraries, see e.g. ubuntu or FreeBSD. Names may differ, like libuv.a, libuv_a.a and PkgConfig names also (libuv, libuv-static).
  2. CMake pkg_check_modules already has facilities for detecting both static and shared libraries in one go.
  3. From a packagers POV, uvw should be able to create both static and shared libraries in the same build (with FETCH_LIBUV disabled). Header files certainly installed too, for header-only consumers.
skypjack commented 1 year ago

uvw should be able to create both static and shared libraries in the same build

Do you mean from the same build tree or from the same build command? As in start the build and return with both the libraries compiled at once?

0EVSG commented 1 year ago

Do you mean from the same build tree or from the same build command? As in start the build and return with both the libraries compiled at once?

Exactly. As in setup one build directory with options set for what to build - headers, dynamic lib, static lib, or any combination of them. Fail right there in case the corresponding libuv libraries are not found. Then just build and install, usually in two steps, using make or ninja. I may have overlooked something, but from the CMakeLists.txt it looked like dynamic xor static to me.

Unfortunately I can't dive more into this, just learned that libuv only has timers with ms granularity, which doesn't cut it for me. Good luck anyway!

stefanofiorentino commented 1 year ago

@darktohka can we close this? @skypjack I think it's outdated at this point, right?

skypjack commented 1 year ago

Yeah, definitely. We can close this one and open a new PR from current master eventually if there is something else to do.

stefanofiorentino commented 1 year ago

@darktohka feel free to rebase and re-open it if you're still in the need of this modification. We improved this build setup lately, so maybe you can give it a try.