Hi Dylan,
On 12/03/2024 4:16 am, Dylan Aïssi wrote:
Hello Arnaud,
Le jeu. 7 mars 2024 à 04:44, Arnaud Rebillout<arna...@debian.org> a écrit :
-https://salsa.debian.org/arnaudr/pipewire-module-xrdp
-https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1037111
You are right, it's the first pipewire module outside the main package.
So, let's try to clear the way:
What about using "libpipewire-0.3-module-xrdp" for binary package
name? It makes it consistent with other modules packages we have
(libpipewire-0.3-modules and libpipewire-0.3-modules-x11 ) and
this shows that it's a libpipewire-0.3 plugin. Moreover, this name
scheme is close to the one use for gstreamer plugin
(i.e. gstreamer1.0-blabla).
Ack, sounds good to me.
In the build log, dpkg-shlibdeps warns about unresolvable references
to symbols from libpipewire-0.3, maybe only a false positives since
it is a plugin but worth investigating.
I don't know if it's false positive or not (I'm really not familiar with
dynamic linking).
When I look at the x11 module:
$ apt show libpipewire-0.3-modules-x11 | grep ^Depends:
Depends: libc6 (>= 2.4), libcanberra0t64 (>= 0.2), libpipewire-0.3-0 (=
1.0.3-1.1), libx11-6 (>= 2:1.7.0), libxfixes3 (>= 1:6.0.0),
libcanberra-pulse
When I look at the xrdp module:
$ dpkg-deb --info libpipewire-0.3-module-xrdp_0.1-2_amd64.deb | grep
Depends:
Depends: libc6 (>= 2.17), pipewire-bin
I wonder why there's no dependency on libpipewire-0.3.0, and if it's
something that needs to be fixed or not. I also guess that if there was
such a dependency, there would be no dpkg-shlibdeps warning?
I would add a dependency on "pipewire" to be sure its services
are installed. I don't know if this module is useful without a session
manager otherwise you would also need to add wireplumber as
dependency.
I spend some time thinking about this. I think that I should actually
split in two binary packages:
- libpipewire-0.3-module-xrdp would _only_ install the .so. That's in
line with libpipewire-0.3-modules and libpipewire-0.3-modules-x11, and
it's probably simple to come up with the right dependencies.
- pipewire-xrdp (name taken after the name of the xdg/autostart script)
would install the integration bits, ie.
/etc/xdg/autostart/pipewire-xrdp.desktop and
/usr/libexec/pipewire-module-xrdp/load_pw_modules.sh.
With this split, it gives some rope for users. If needed they can
install the module only, and then come up with another integration
(maybe they don't want to use a xdg/autostart script for example).
I also think that, for the principle of least surprise, going forward
it's nice if users know that packages named libpipewire-*-modules-* only
install modules, as the name suggest, and don't install anything else.
But maybe I'm over thinking... What do you think?
Some comments about the upstream load_pw_modules.sh
it refers to the Debian wiki which is nice :-) but not sure this is the
best reference for pipewire. For example, it uses configuration
that doesn't exist, I was asked by upstream to update our wiki about
that :-)
https://wiki.debian.org/PipeWire?action=diff&rev2=49&rev1=48
It is an upstream bug, but I guess they need to refresh their
load_pw_modules.sh.
These are just a few suggestions after a quick review.
Ack, thanks for the review, I will open a bug upstream.
I CCed Dylan, as I'm not sure that the address
pkg-utopia-maintain...@lists.alioth.debian.org reaches anyone out there.
Please tell me if there's another point of contact for the Utopia team.
Some discussions happen here, so it's fine.
I actually just found out about that mailing list archives on
alioth.debian.net, I didn't even know it existed :D
Additionally, I'd be happy to move this repo under the utopia-team Salsa
group for maintenance going forward.
I don't have enough permissions to add you to the group, I guess we will
need help from someone else.
Ok, no rush, let's finish the packaging first.
Best,
Arnaud
_______________________________________________
Pkg-utopia-maintainers mailing list
Pkg-utopia-maintainers@alioth-lists.debian.net
https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-utopia-maintainers
