[Pkg-utopia-maintainers] Request for review: pipewire-module-xrdp

[Arnaud Rebillout]

On 13/03/2024 4:24 am, Dylan Aïssi wrote:
> Hi Arnaud,
>
> Le mar. 12 mars 2024 à 11:33, Arnaud Rebillout <arnaudr at debian.org> a écrit :
>> Hi Dylan,
>>
>> On 12/03/2024 4:16 am, Dylan Aïssi wrote:
>>
>> 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 checked all other modules with "ldd -r" they are all linked to
> libpipewire-0.3.so.0,
> libpipewire-0.3-module-xrdp is the only one not linked and complaining about
> undefined symbols. Adding "-lpipewire-0.3" to LDFLAGS in src/Makefile.am
> is enough to fix that, but not sure if it 's correct, maybe something
> to check with upstream?

Checking with other modules on my system here, it seems that NOT linking 
happens in other places:

$ ldd -r /usr/lib/xorg/modules/libshadow.so | grep -v ^undefined
     linux-vdso.so.1 (0x00007fffa2678000)
     libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f245fa84000)
     /lib64/ld-linux-x86-64.so.2 (0x00007f245fc8c000)
$ ldd -r /usr/lib/xorg/modules/libshadow.so | grep ^undefined | wc -l
9

$ ldd -r /usr/lib/apache2/modules/mod_dir.so | grep -v ^undefined
     linux-vdso.so.1 (0x00007fffc7938000)
     libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f02b0825000)
     /lib64/ld-linux-x86-64.so.2 (0x00007f02b0a27000)
$ ldd -r /usr/lib/apache2/modules/mod_dir.so | grep ^undefined | wc -l
17

But that doesn't really answer the question...

So the ldflags used for the module pipewire-module-xrdp are: -module 
-avoid-version

For background, pipewire-module-xrdp was seemingly created from 
pulseaudio-module-xrdp [1] as a starting point. The values that are used 
for the ldflags come from there "as is". In turn, it seems that it comes 
from this piece of documentation: 
https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/Modules/, 
section "Update on July 30 2008".

Could it be that there's no good reason for those ldflags, it was just 
carried over from the pulseaudio module to the pipewire module?

According to 
https://www.gnu.org/software/automake/manual/html_node/Libtool-Modules.html, 
with "-module" automake will not complain if a library name doesn't 
start with "lib". And indeed, pulseaudio modules are named 
"module-xxx.so", maybe that's why it was used in this old documentation?

According to 
https://www.gnu.org/software/libtool/manual/html_node/Link-mode.html, 
"-avoid-version" is what prevents creating all the symlinks. Thanks to 
this flag, we only get a ".so", so that's what we want.

Now I tried to build the module with the following ldflags: $(PW_LIBS) 
$(SPA_LIBS) -avoid-version. Result: no more warning from dpkg-shlibdeps. 
The module can be loaded, it all works.

Despite all of that, I still don't know what should be the "correct" 
ldflags for an external module. I can ask upstream, but I think other 
DDs are more likely to know.


[1] https://github.com/neutrinolabs/pulseaudio-module-xrdp/


>
>> 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?
> I really like this suggestion, it makes perfect sense! And it aligns with
> recent discussion I have seen about Debian library packaging and
> dependencies :-)

Sounds good! I'll be doing that. I'll ping back when there's some 
progress, as I opened a few issues upstream, and I'll wait for feedback 
and eventual patches to be merged.

Thanks for your review!

Arnaud