openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
3.95k stars 3.46k forks source link

Question : libwebsockets -enbling support for libevent #21157

Open snaami opened 1 year ago

snaami commented 1 year ago

Hello All,

I see that libwebsockets-full doesn't enable libevent2 support by default see (https://github.com/openwrt/packages/blob/master/libs/libwebsockets/Makefile#L87). by adding the ( -DLWS_WITH_LIBEVENT=ON)

Do you think it would be beneficial to enable it by default in the config, there are many application relays on it out there and many of them use libevent as event loop?

Thanks,

karlp commented 1 year ago

Seems reasonable to me, it wasn't an option in the past when -full was created, but would seem sane to add it now.

brada4 commented 1 year ago

Maybe enable event plugins an build available ones that in turn pull larger dependencies? https://libwebsockets.org/git/libwebsockets/tree/lib/event-libs

mhei commented 1 year ago

I'm also wondering how this is supposed to work? -full is linking against libuv, standard (none-full) libwebsockets against libubox. Having support for other event backend sounds good, but would result in more (build) variants? So this is not just enabling it...

snaami commented 1 year ago

I'm also wondering how this is supposed to work? -full is linking against libuv, standard (none-full) libwebsockets against libubox. Having support for other event backend sounds good, but would result in more (build) variants? So this is not just enabling it...

Indeed, this will result in a more build variants a few kbs larger, but as I see it won't affect the functional of the lib itself as libwebsockets take in argument an event_loop ctx and type (libevent, libuv) so it should continue to work as expected even if we enable multiple event lib in the build .

I see this is used in other (forks of openwrt) like in prplos here (https://gitlab.com/prpl-foundation/prplos/feed-prpl/-/blob/prplos/backports/packages_22.03/libwebsockets4/Makefile#L97)

brada4 commented 1 year ago

libwebsockets can plug event modules dynamically now. What I am saying is to not introduce any new dependencies in current package, just let it load those plugins from separate packages, and -full package is just dependency-set to emulate the might it once has?

mhei commented 1 year ago

The different event backends are different shared libraries if I understand correctly. Maybe it is possible to pack each backend into its own "backend" package. If packed all into the -full package, the build system would insist to add dependencies to the -full package. Just 2 minutes thought... I'm not sure yet if that makes sense or is doable.

karlp commented 1 year ago

I'm no longer using this for $dayjob anymore, so if anyone of you want to work on this, feel extremely welcome to take over maintainership on this package too, it's been a lot of pain over the years

snaami commented 1 year ago

The different event backends are different shared libraries if I understand correctly. Maybe it is possible to pack each backend into its own "backend" package. If packed all into the -full package, the build system would insist to add dependencies to the -full package. Just 2 minutes thought... I'm not sure yet if that makes sense or is doable.

I think it is better to have each event-lib packed into its own config maybe something like libwebsockets-event, libwebsockets-uv ...

snaami commented 1 year ago

On Debian they have all of them enabled by default :

**libwebsockets-evlib-ev
libwebsockets-evlib-uv
libwebsockets-evlib-glib**

All of them then depend on core/main libwebsockets package

They use the following configuration for that :

dh_auto_configure -- -DCMAKE_LIBRARY_ARCHITECTURE=${arch} \ -DLIB_SUFFIX=/${arch} -DLWS_WITHOUT_DAEMONIZE=OFF \ -DLWS_WITH_LIBEV=ON -DLWS_WITH_LIBUV=ON \ -DLWS_UNIX_SOCK=ON -DLWS_IPV6=ON \ -DLWS_WITH_ZLIB=ON -DLWS_WITHOUT_EXTENSIONS=OFF \ -DLWS_WITH_GLIB=ON

As we can see, all event-libs are enabled at compile time in the same config.

brada4 commented 1 year ago

Check if libs are separate files just like that, its ok to install them one by one in separate packages. Default should work without plugins for all uses that fit in embedded device.