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

Add compatibility for using uvw with older libuv versions #240

Closed jagerman closed 3 years ago

jagerman commented 3 years ago

One thing that is fairly important for our project is to be able to be packaged and run on moderately older Linux distributions -- we build packages for Ubuntu versions between 18.04 and current and Debian 10 and up. We try, when possible, to link against system libraries whenever possible because it keeps the onus for security updates in dependent libraries on the distribution itself (and, secondarily, keeps binaries smaller). One of the obstacles I've run into in converting our codebase from libuv to uvw, however, is that uvw is currently tightly coupled to the latest libuv version—even if I don't need newer capabilities from libuv—which effectively makes it impossible to use with an older, system-provided libuv.

Hence this PR: this adds version macros around the features in newer libuv versions so that they are only available if the libuv headers in use are sufficiently up-to-date; that lets one use current uvw (with fixes, features, etc.) with an older libuv (as long as you don't need the features only available with a newer libuv version, of course). It also adds the required libuv version into the docs for the gated features.

I went back as far as libuv 1.16.0 (because that introduced OS-dependent fs open macros which would be a nuisance to support with earlier libuv versions), which allows successful builds on Ubuntu 18.04 and newer (and also coincides with when Ubuntu started shipping a C++17 capable compiler).

stefanofiorentino commented 3 years ago

During the packaging phase, maybe you can just stick to the stable uvw's tag referring to the specific uv version in your distro?

jagerman commented 3 years ago

During the packaging phase, maybe you can just stick to the stable uvw's tag referring to the specific uv version in your distro?

That would mean that I can't use anything added or fixed in uvw since 2018 that doesn't depend on libuv, which seems highly undesirable.

stefanofiorentino commented 3 years ago

That would mean that I can't use anything added or fixed in uvw since 2018 that doesn't depend on libuv, which seems highly undesirable.

I see your point, and agreed. Up to me looks hard to maintain, though. Let's wait @skypjack for further discussion.

jagerman commented 3 years ago

Up to me looks hard to maintain, though.

Maintenance shouldn't be too hard -- if libuv 1.42.0 adds some feature uv_foo_bar then in the uvw update it gets gated as:

if LIBUV_VERSION_AT_LEAST(1,42,0)

void fooBar() { / ... / }

endif

That said there may be cases where this is more complicated because something deeper changes, but at least walking back through the updates in this PR I didn't encounter anything between 1.41.0 back to 1.16.0 that required anything more than just ifdef-ing out the relevant uvw implementations.

(Keep in mind that this PR looks bigger because it's adding version gates for 25 versions at once).

stefanofiorentino commented 3 years ago

The reasoning is often the opposite (i.e. deprecation), seems strange to me adding new features under ifdef blocks. Anyway maybe it's just a matter of setting the right CI/CD test chains..

skypjack commented 3 years ago

I see your point, and agreed. Up to me looks hard to maintain, though.

I agree that it's harder to maintain, though for example:

That would mean that I can't use anything added or fixed in uvw since 2018 that doesn't depend on libuv, which seems highly undesirable.

If we changed how events are indexed, one couldn't switch to the new approach without updating to the latest version of libuv.

As long as libuv adds up it shouldn't be that hard. Probably things become trickier if they change the API of a member in vX.Y and update the signature in vU.V, but still. Nothing prevents us from doing this for a while, then remove all ifdefs if it turns to be impossible to maintain. This is an open source project and we are working on it in our free time, without any kind of support nor sponsorization. So, well, we'll do our best until something is doable, then we'll just get rid of it or ask people to maintain it if possible. 🤷‍♂️

Seems reasonable?

skypjack commented 3 years ago

@jagerman it would be great if we could test 2/3 versions on the CI though. Not all of them though, it isn't reqlly required imho and it would consume all CI minutes with a single commit. Do you know any way to do/automate that with GH?

jagerman commented 3 years ago

it would be great if we could test 2/3 versions on the CI though. Not all of them though, it isn't reqlly required imho and it would consume all CI minutes with a single commit. Do you know any way to do/automate that with GH?

I added a couple of builds here that use the system-installed libuv from Ubuntu bionic (1.18.0) and focal (1.34.2), and are both very fast builds (especially since they don't have to build libuv at all).

(I also pushed some small test suite & static lib fixes).

slycelote commented 3 years ago

I agree that supporting multiple libuv versions would be highly desirable. @skypjack Did you close the PR accidentally, or was it decided that maintenance overhead would be too high?

skypjack commented 3 years ago

I've honestly no idea why this was closed. 😅 Myabe it happened because I deleted experimental? In any case, the maintenance burden is pretty high here. Not sure what to do with this request.