pulseaudioFull: fix wrapGApp wrapping, fix vm tests

[Yarny0]

Since 7a2605e the pulseaudio build recipe incorporates the wrapGAppsHook wrapper setup-hook if advancedBluetoothCodecs are enabled. This wrapper setup-hook -- like most wrappers -- wraps binaries in $out/bin by first renaming them, then placing a wrapper script where the original binary was.

Unfortunatelly, pulseaudio doesn't like its binary moved around after installation: It records the binaries path during installation time in https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/blob/e5ad31e873eed62bc580a86a61177047f9e8c491/meson.build#L154 , then checks the path in /proc/self/exe and complains

Jun 16 19:06:48 nixosb pulseaudio[2219]: W: [.pulseaudio-wra] main.c: /proc/self/exe does not point to /nix/store/bqfyzxwpxa2ydmyvh3j32xrm4chxbj22-pulseaudio-15.0/bin/pulseaudio, cannot self execute. Are you playing games?

if they don't match: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/blob/e5ad31e873eed62bc580a86a61177047f9e8c491/src/daemon/main.c#L577

Somehow, this also results in a real bug: pacmd fails to connect to the pulseaudio server, see #177915 .

Description of changes

To fix this issue, the commit at hand changes the installation directory for binaries to $out/.bin-unwrapped. After the installation, $out/bin is created by hand and populated with symlinks to files in $out/.bin-unwrapped. wrapGAppsHook doesn't know or care about the .bin-unwrapped directory; it just sees all the symlinks in bin, renames them and places wrapper scripts beside them.

Effectively, this leaves the original binary in .bin-unwrapped unchanged! So pulseaudio will find itself still in its oritinal place, and "users" of the package can call pulseaudio via the wrapper script in bin as usual.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • [ ]and/or package tests
  • [ ]or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • [ ]made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.11 Release Notes (or backporting 22.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

More comments

• I tried nixpkgs-review and it attempted a mass rebuild, which I had to abort. The mass rebuild is likely caused by this pull request also touching libpulseaudio. So it better targets staging...

• I tested a minimal (e.g., non-graphical) NixOS installation with pulseaudioFull and the patch at hand applied. After some compiling, the system booted, pulseaudio started, it was able to play some music with paplay, and pacmd info also worked, i.e., the patch fixes pulseaudioFull: pacmd cannot connect to daemon #177915 .

• The patch, in particular the new .bin-unwrapped directory, feel a bit unorthodox. It attempts to fix a minor bug in the current implementation and I hope it doesn't introduce new ones. I can't test this on my productive system as I cannot rebuild everything, but good testing seems to be needed here.

• notifying maintainer: @lovek323

• notifying bug report participant and author of the commit that introduced wrapGAppsHook: @schmittlauch

[schmittlauch]

Thanks for implementing your idea. I've successfully integrated your modified pulseaudio into my system, audio still works fine. But I still struggle a bit with understanding what happens there:

$ ls -lsh /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/.bin-unwrapped 
insgesamt 488K
 84K -r-xr-xr-x 1 root root  83K  1. Jan 1970  pacat
 44K -r-xr-xr-x 1 root root  43K  1. Jan 1970  pacmd
112K -r-xr-xr-x 1 root root 111K  1. Jan 1970  pactl
4,0K -r-xr-xr-x 1 root root 2,4K  1. Jan 1970  padsp
4,0K -r-xr-xr-x 1 root root 2,1K  1. Jan 1970  pa-info
4,0K lrwxrwxrwx 1 root root    5  1. Jan 1970  pamon -> pacat
4,0K lrwxrwxrwx 1 root root    5  1. Jan 1970  paplay -> pacat
4,0K lrwxrwxrwx 1 root root    5  1. Jan 1970  parec -> pacat
4,0K lrwxrwxrwx 1 root root    5  1. Jan 1970  parecord -> pacat
 44K -r-xr-xr-x 1 root root  44K  1. Jan 1970  pasuspender
 40K -r-xr-xr-x 1 root root  38K  1. Jan 1970  pax11publish
136K -r-xr-xr-x 1 root root 134K  1. Jan 1970  pulseaudio
4,0K -r-xr-xr-x 1 root root 2,7K  1. Jan 1970  start-pulseaudio-x11

$ ls -lsh /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/bin           
insgesamt 208K
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pacat
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pacmd
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pactl
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  padsp
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pa-info
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pamon
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  paplay
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  parec
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  parecord
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pasuspender
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pax11publish
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  pulseaudio
16K -r-xr-xr-x 1 root root 16K  1. Jan 1970  start-pulseaudio-x11

$ file /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/.bin-unwrapped/pulseaudio
/nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/.bin-unwrapped/pulseaudio: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /nix/store/d2bpliayddadf6lx6l1i04w265gqw8n6-glibc-2.34-210/lib/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, with debug_info, not stripped

$ file /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/bin/pulseaudio           
/nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/bin/pulseaudio: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /nix/store/d2bpliayddadf6lx6l1i04w265gqw8n6-glibc-2.34-210/lib/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, not stripped

Didn't wrapGApps use to create wrapper bash scripts? Somehow both pulseaudio files in .bin-unwrapped/ as well as in bin/ are identified as ELF binaries, not scripts. Additionally, none of them is a symlink. The files do differ in size though.

Once I understand what's going on there, I additionally suggest to only enable that symlink-wrapper dance when it's actually needed, based on the same condition like for including wrapGApps.

[schmittlauch]

Okay, update, I now discovered that wrapGAppsHook wrappers are binary for quite a while already, thanks to makeBinaryWrapper. Let's see how long it takes until I figured out the changes in symlinking behavior, please bear with me ^_^

[schmittlauch]

I got it, forgot to check the hidden files:

$ ls -lshA /nix/store/080v37wdjqrcscljklimk7p6zhpqzwsj-pulseaudio-15.0/bin
total 508K
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pacat
 64K -r-xr-xr-x 1 root root  61K Jan  1  1970 .pacat-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pacmd
 24K -r-xr-xr-x 1 root root  21K Jan  1  1970 .pacmd-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pactl
 92K -r-xr-xr-x 1 root root  90K Jan  1  1970 .pactl-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 padsp
4,0K -r-xr-xr-x 1 root root 2,4K Jan  1  1970 .padsp-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pa-info
4,0K -r-xr-xr-x 1 root root 2,1K Jan  1  1970 .pa-info-wrapped
4,0K lrwxrwxrwx 1 root root    5 Jan  1  1970 pamon -> pacat
4,0K lrwxrwxrwx 1 root root    5 Jan  1  1970 paplay -> pacat
4,0K lrwxrwxrwx 1 root root    5 Jan  1  1970 parec -> pacat
4,0K lrwxrwxrwx 1 root root    5 Jan  1  1970 parecord -> pacat
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pasuspender
 24K -r-xr-xr-x 1 root root  23K Jan  1  1970 .pasuspender-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pax11publish
 20K -r-xr-xr-x 1 root root  17K Jan  1  1970 .pax11publish-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pulseaudio
112K -r-xr-xr-x 1 root root 112K Jan  1  1970 .pulseaudio-wrapped
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 start-pulseaudio-x11
4,0K -r-xr-xr-x 1 root root 2,6K Jan  1  1970 .start-pulseaudio-x11-wrapped

But apparently we now have the same binaries in the package twice, diff /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/{.bin-unwrapped/pulseaudio,bin/.pulseaudio-wrapped} does not return any differences.

Can we somehow remove one of the binaries or does the whole perceived path and executable logic not add up anymore?

[Yarny0]

Thanks for your feedback, @schmittlauch .

I additionally suggest to only enable that symlink-wrapper dance when it's actually needed, based on the same condition like for including wrapGApps.

This would certainly simplify the build process without wrapGApps. However, if we can assure ourselfes of the correctness of this symlink-wrapper dance, then I would rather avoid the additional complexity of optional-branches. Package size would only increase by a couple of symlinks, we avoid additional heterogenity in pulseaudio flavours, and bugs in the symlink dance would show up faster. But maybe the maintainer @lovek323 has a preference here?

I got it, forgot to check the hidden files:

$ ls -lshA /nix/store/080v37wdjqrcscljklimk7p6zhpqzwsj-pulseaudio-15.0/bin
total 508K
 16K -r-xr-xr-x 1 root root  16K Jan  1  1970 pacat
 64K -r-xr-xr-x 1 root root  61K Jan  1  1970 .pacat-wrapped
[...]

But apparently we now have the same binaries in the package twice, diff /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0/{.bin-unwrapped/pulseaudio,bin/.pulseaudio-wrapped} does not return any differences.

Hmm, you're looking into two different packages here. In the package /nix/store/4f9zi1250bcm869kk6c3zgqpci9ysgmj-pulseaudio-15.0, the "unwrapped" binaries in bin should indeed be symlinks. diff certianly sees no differences: It follows the symlink and compares the file with itself. I can't explain why the expected symlinks in /nix/store/080v37wdjqrcscljklimk7p6zhpqzwsj-pulseaudio-15.0 are instead real/normal files. Can you provide the build recipe or otherwise explain how to build this one?

[schmittlauch]

The pulseaudioFull from this PR has been running on my machine for several weeks now, I could not not encounter any issues.

And regarding the seemingly-duplicated binaries, that's not an issue as well, sorry about that. The call chain for pulseaudio for example is
bin/pulseaudio ==[is a binary wrapper for]==> bin/.pulseaudio-wrapped ==[is a symlink to]==> .bin-unwrapped/pulseaudio

I just fell victim to diff following the symlinks.

So for me this looks pretty good and I encourage you to convert this to a proper PR and go through with this.

[Yarny0]

Thanks, @schmittlauch , for your feedback and thorough testing!

I just rebased and updated the branch. Unfortunatelly, the branch now conflicts with other changes in staging, namely with fd031c0 .

The core commit "pulseaudioFull: fix wrapGApp wrapping" is still retained, although some shell commands have been shortened (e.g. "mkdir -p" instead of "mkdir --parents") due to feeback in another pull request ( #182360 (review) ).

While testing, I realized that all pulseaudio nixos vm tests currently fail, for various reasons. I fixed and extended the tests with more commits:

• The URL to the sample file was broken.
• One part of the test tries to decect the audio mixer via OCR, but this failed.
• The test user wasn't member of the pulse-audio access group, which is needed to access the system-wide daemon.
• I added tests for pulseaudioFull and for pacmd. This should detect further problems with pacmd.

I tested this branch, based on current nixos-unstable ( 762b003 ):

• All (now four) pulseaudio vm tests pass.
• I also tested audio playback with pulseaudio and pulseaudioFull on my machine (only the user daemon, not the system-wide version), and it works fine.

Due to the conflict with current staging, I simply rebased again, onto staging itself. Resolving the conflict seemed easy as I just concatenated the different preFixup scripts. However, I no longer can test the brach as it would recompile almost everything locally. If there is any way I can further test this pull request, please let me know.

Thanks again, @schmittlauch, for you help with fixing this issue and preparing this branch. I will now turn it into a proper pull request for staging.

Notifying pulseaudio maintainer: @lovek323
Notifying pulseaudio vm test maintainer: @Synthetica9
Notifying author of conflicting commit: @wegank , maybe you can verify that this branch doesn't clash with your changes.

[SuperSandro2000]

@astro this is the exact problem we had on pulsebert

[schmittlauch]

Do we want to backport this to 22.05? I suggest doing so as long as a backport just works, as breaking pactl is a regression introduced into the original 22.05 release and thus worthy fixing.

[SuperSandro2000]

@ofborg eval
@ofborg build pulseaudioFull

[schmittlauch]

I finally managed to build this on my laptop. Pulseaudio is still working fine (audio playback, device switching, Bluetooth audio) and a pacmd info now succeeds.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/596

[hexagonal-sun]

It looks like there has been a change in pulseaudio which breaks hardware.pulseaudio.systemWide = true;. This used to work, however as @Yarny0 has said (and provided a fix for), the pulse-access group appears to be hard-coded in the pulse source (well it's a meson option but I don't think the nix expression of pulse audio allows us to change this).

If this could be merged ASAP that would be great!