mbox series

[BlueZ,v3,0/8] Remove support for external plugins

Message ID 20240125-rm-ext-plugins-v3-0-d141f7870bb6@gmail.com (mailing list archive)
Headers show
Series Remove support for external plugins | expand

Message

Emil Velikov via B4 Relay Jan. 25, 2024, 12:07 a.m. UTC
Hello everyone,

Here's v3 fixing a small bug with the previous patches, which was
tripping the CI.

Link to the previous revision can be found below.

Thanks
Emil

- Link to v2: https://lore.kernel.org/r/20240124-rm-ext-plugins-v2-0-5244906f05ff@gmail.com

---
Emil Velikov (8):
      configure, README: introduce --enable-external-plugins
      obexd: factor out external plugin support
      bluetoothd: remove external-dummy plugin
      bluetoothd: convert external sixaxis plugin to builtin
      bluetoothd: factor out external plugin support
      bluetoothd: don't export internal API
      bluetoothd: change plugin loading alike obexd
      android: export only (android) entrypoint from the modules

 Makefile.am              |  17 ++----
 Makefile.obexd           |   2 +
 Makefile.plugins         |   8 +--
 README                   |  13 +++++
 android/Makefile.am      |   3 +
 android/hal-audio.c      |   1 +
 android/hal-bluetooth.c  |   1 +
 android/hal-sco.c        |   1 +
 configure.ac             |  10 ++++
 obexd/src/obexd.h        |   2 +-
 obexd/src/plugin.c       |  93 +++++++++++++++++++++----------
 obexd/src/plugin.h       |   4 ++
 plugins/external-dummy.c |  28 ----------
 src/btd.h                |   2 +-
 src/plugin.c             | 140 +++++++++++++++++++++++++++++------------------
 src/plugin.h             |  16 ++++++
 16 files changed, 209 insertions(+), 132 deletions(-)
---
base-commit: a9d1f6f6a625607de6c3f5b7a40a3aac5f36c02b
change-id: 20240116-rm-ext-plugins-ba0b852a492b

Best regards,

Comments

Szymon Janc Jan. 25, 2024, 1:02 p.m. UTC | #1
Hi,

On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay
<devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
>
> Hello everyone,
>
> Here's v3 fixing a small bug with the previous patches, which was
> tripping the CI.
>
> Link to the previous revision can be found below.

Just a comment that external plugins support was added to avoid udev
dependency (from sixaxis) in bluetoothd.
(not that I have strong opinion on this, just a note, I don't remember
exactly why it was done, maybe Marcel recalls?)

>
> Thanks
> Emil
>
> - Link to v2: https://lore.kernel.org/r/20240124-rm-ext-plugins-v2-0-5244906f05ff@gmail.com
>
> ---
> Emil Velikov (8):
>       configure, README: introduce --enable-external-plugins
>       obexd: factor out external plugin support
>       bluetoothd: remove external-dummy plugin
>       bluetoothd: convert external sixaxis plugin to builtin
>       bluetoothd: factor out external plugin support
>       bluetoothd: don't export internal API
>       bluetoothd: change plugin loading alike obexd
>       android: export only (android) entrypoint from the modules
>
>  Makefile.am              |  17 ++----
>  Makefile.obexd           |   2 +
>  Makefile.plugins         |   8 +--
>  README                   |  13 +++++
>  android/Makefile.am      |   3 +
>  android/hal-audio.c      |   1 +
>  android/hal-bluetooth.c  |   1 +
>  android/hal-sco.c        |   1 +
>  configure.ac             |  10 ++++
>  obexd/src/obexd.h        |   2 +-
>  obexd/src/plugin.c       |  93 +++++++++++++++++++++----------
>  obexd/src/plugin.h       |   4 ++
>  plugins/external-dummy.c |  28 ----------
>  src/btd.h                |   2 +-
>  src/plugin.c             | 140 +++++++++++++++++++++++++++++------------------
>  src/plugin.h             |  16 ++++++
>  16 files changed, 209 insertions(+), 132 deletions(-)
> ---
> base-commit: a9d1f6f6a625607de6c3f5b7a40a3aac5f36c02b
> change-id: 20240116-rm-ext-plugins-ba0b852a492b
>
> Best regards,
> --
> Emil Velikov <emil.l.velikov@gmail.com>
>
>


--
pozdrawiam
Szymon K. Janc
Emil Velikov Jan. 25, 2024, 1:48 p.m. UTC | #2
On Thu, 25 Jan 2024 at 13:02, Szymon Janc <szymon.janc@codecoup.pl> wrote:
>
> Hi,
>
> On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay
> <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> >
> > Hello everyone,
> >
> > Here's v3 fixing a small bug with the previous patches, which was
> > tripping the CI.
> >
> > Link to the previous revision can be found below.
>
> Just a comment that external plugins support was added to avoid udev
> dependency (from sixaxis) in bluetoothd.
> (not that I have strong opinion on this, just a note, I don't remember
> exactly why it was done, maybe Marcel recalls?)
>

Thanks, I may have some ideas why.

About 10 years ago (or so) some distributions were shipping
libudev.so.0 while others libudev.so.1. The ABI break was minimal,
although it was a thing.
I remember us doing all sorts of hacks in Mesa trying to pick the
correct one, esp when your system can have .1 while the game (or its
chroot-like environment) has .0 and vice-versa.

I would imagine a similar issue was observed in bluez - but I can only
speculate.

Over the last 5+ years, literally all supported distributions have
moved for libudev.so.1 and the Steam games (and runtime) has both with
some compat quirks to avoid explosions.

HTH o/
Emil
Luiz Augusto von Dentz Jan. 25, 2024, 6:30 p.m. UTC | #3
Hi Emil,

On Thu, Jan 25, 2024 at 8:51 AM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Thu, 25 Jan 2024 at 13:02, Szymon Janc <szymon.janc@codecoup.pl> wrote:
> >
> > Hi,
> >
> > On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay
> > <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> > >
> > > Hello everyone,
> > >
> > > Here's v3 fixing a small bug with the previous patches, which was
> > > tripping the CI.
> > >
> > > Link to the previous revision can be found below.
> >
> > Just a comment that external plugins support was added to avoid udev
> > dependency (from sixaxis) in bluetoothd.
> > (not that I have strong opinion on this, just a note, I don't remember
> > exactly why it was done, maybe Marcel recalls?)
> >
>
> Thanks, I may have some ideas why.
>
> About 10 years ago (or so) some distributions were shipping
> libudev.so.0 while others libudev.so.1. The ABI break was minimal,
> although it was a thing.
> I remember us doing all sorts of hacks in Mesa trying to pick the
> correct one, esp when your system can have .1 while the game (or its
> chroot-like environment) has .0 and vice-versa.
>
> I would imagine a similar issue was observed in bluez - but I can only
> speculate.
>
> Over the last 5+ years, literally all supported distributions have
> moved for libudev.so.1 and the Steam games (and runtime) has both with
> some compat quirks to avoid explosions.

I was considering applying this week but if you want to respin this
set to sort out the dependency Im fine with, but I don't think it
would hurt to have a libudev dependency provide we have some means to
disable it in case the system don't intend to support sixaxis plugin.
Emil Velikov Jan. 26, 2024, 3:43 p.m. UTC | #4
Hi Luiz,

On Thu, 25 Jan 2024 at 18:30, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Emil,
>
> On Thu, Jan 25, 2024 at 8:51 AM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > On Thu, 25 Jan 2024 at 13:02, Szymon Janc <szymon.janc@codecoup.pl> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay
> > > <devnull+emil.l.velikov.gmail.com@kernel.org> wrote:
> > > >
> > > > Hello everyone,
> > > >
> > > > Here's v3 fixing a small bug with the previous patches, which was
> > > > tripping the CI.
> > > >
> > > > Link to the previous revision can be found below.
> > >
> > > Just a comment that external plugins support was added to avoid udev
> > > dependency (from sixaxis) in bluetoothd.
> > > (not that I have strong opinion on this, just a note, I don't remember
> > > exactly why it was done, maybe Marcel recalls?)
> > >
> >
> > Thanks, I may have some ideas why.
> >
> > About 10 years ago (or so) some distributions were shipping
> > libudev.so.0 while others libudev.so.1. The ABI break was minimal,
> > although it was a thing.
> > I remember us doing all sorts of hacks in Mesa trying to pick the
> > correct one, esp when your system can have .1 while the game (or its
> > chroot-like environment) has .0 and vice-versa.
> >
> > I would imagine a similar issue was observed in bluez - but I can only
> > speculate.
> >
> > Over the last 5+ years, literally all supported distributions have
> > moved for libudev.so.1 and the Steam games (and runtime) has both with
> > some compat quirks to avoid explosions.
>
> I was considering applying this week but if you want to respin this
> set to sort out the dependency Im fine with, but I don't think it
> would hurt to have a libudev dependency provide we have some means to
> disable it in case the system don't intend to support sixaxis plugin.
>

I don't think I follow: what do you mean with "sort out the dependency"?

Sixaxis is no different to midi where it a) pulls a third-party
library (udev vs alsa) and b) it can be disabled at build. Technically
one can dlopen/dlsym libudev.so, although that should probably be
deferred until needed IMHO.

Thanks
Emil