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.84k stars 209 forks source link

Give the option to build uvw as lib without fetching the libuv dependency #231

Closed stefanofiorentino closed 3 years ago

stefanofiorentino commented 3 years ago

I often found myself commenting the fetch_libuv() call when the BUILD_UVW_LIBS flag is ON (working offline for too many reasons and git-repo build setup on new projects). Adding a FETCH_LIBUV_DEPS option (defaulted to ON) shall suffice. This will hurt you @skypjack in any way?

skypjack commented 3 years ago

This will hurt you @skypjack in any way?

Indeed no. It makes perfectly sense actually.

stefanofiorentino commented 3 years ago

Ok, the LIB_TEST requires the libuv.so target to be present, I'll find a workaround to it asap.


target_compile_definitions(${TEST_NAME} PRIVATE TARGET_LIB_SO="$<TARGET_FILE:uv>")
stefanofiorentino commented 3 years ago

I'm not 100% sure this can be a suitable approach. Anyway, I will try to push it to experimental. Feel free to drop it while rebasing/squashing.

skypjack commented 3 years ago

I'm not 100% sure this can be a suitable approach.

Can you elaborate? I looked into the commit and it seems good, but I'm missing something probably...

stefanofiorentino commented 3 years ago

Off course, I had to

skypjack commented 3 years ago

The first point seems reasonable if we want to test it. 🙂 What about making the link configurable and set uv_a as its default instead? This would give to the user the possibility of using a different name if needed.

stefanofiorentino commented 3 years ago

The issue is that with a non-internal uv_a target you have to link to dl too. I'll dive into this asap.

stefanofiorentino commented 3 years ago

Committed a version to experimental at https://github.com/stefanofiorentino/uvw/commit/36df393352fabd11ccec6d794c427ca205c5152e.

My VSCode configuration is:

"cmake.configureArgs": ["-DFETCH_LIBUV=OFF", "-DLINK_LIB_NAMES=uv_a dl", "-DBUILD_UVW_LIBS=ON", "-DBUILD_TESTING=ON"]

I think my current setup with git-repo would hugely benefit from this modification. Although it is out of topic overhere, this solutions isn't too general. In fact, other build configurations would regrettably need the libuv custom directories of both headers and libs.

skypjack commented 3 years ago

Well, in fact uwv is a header-only wrapper, there is no reason for which it should provide libuv too. We fetch it because of the testsuite, but still. I wouldn't care much about this. Users are always expected to provide and use their own version of libuv. Therefore, something that goes in this direction seems good to me overall. We can adjust the how eventually. Thoughts?

stefanofiorentino commented 3 years ago

Users are always expected to provide and use their own version of libuv.

Almost false.. Users are always expected to provide and use their compatible version of libuv. Too often, from libuv version to version, headers and ABI are broken ;)

skypjack commented 3 years ago

Yeah, good point, but for that they have to complain to libuv, not uvw!! :) We cut releases by specifying the reference version of libuv, more so what can we do? Any idea is welcome.