{"action":"created","issue":{"url":"https://api.github.com/repos/NixOS/nixpkgs/issues/36881","repository_url":"https://api.github.com/repos/NixOS/nixpkgs","labels_url":"https://api.github.com/repos/NixOS/nixpkgs/issues/36881/labels{/name}","comments_url":"https://api.github.com/repos/NixOS/nixpkgs/issues/36881/comments","events_url":"https://api.github.com/repos/NixOS/nixpkgs/issues/36881/events","html_url":"https://github.com/NixOS/nixpkgs/pull/36881","id":304535815,"number":36881,"title":"[Request For Testing] pkgs: libpulseaudio -> libcardiacarrest; nixos: override when running PA daemon","user":{"login":"oxij","id":391919,"avatar_url":"https://avatars2.githubusercontent.com/u/391919?v=4","gravatar_id":"","url":"https://api.github.com/users/oxij","html_url":"https://github.com/oxij","followers_url":"https://api.github.com/users/oxij/followers","following_url":"https://api.github.com/users/oxij/following{/other_user}","gists_url":"https://api.github.com/users/oxij/gists{/gist_id}","starred_url":"https://api.github.com/users/oxij/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/oxij/subscriptions","organizations_url":"https://api.github.com/users/oxij/orgs","repos_url":"https://api.github.com/users/oxij/repos","events_url":"https://api.github.com/users/oxij/events{/privacy}","received_events_url":"https://api.github.com/users/oxij/received_events","type":"User","site_admin":false},"labels":[{"id":731735423,"url":"https://api.github.com/repos/NixOS/nixpkgs/labels/10.rebuild-darwin:%2011-100","name":"10.rebuild-darwin: 11-100","color":"fbca04","default":false},{"id":731734523,"url":"https://api.github.com/repos/NixOS/nixpkgs/labels/10.rebuild-linux:%20501+","name":"10.rebuild-linux: 501+","color":"b60205","default":false},{"id":60265212,"url":"https://api.github.com/repos/NixOS/nixpkgs/labels/6.topic:%20nixos","name":"6.topic: nixos","color":"fef2c0","default":false},{"id":290122988,"url":"https://api.github.com/repos/NixOS/nixpkgs/labels/8.has:%20clean-up","name":"8.has: clean-up","color":"009800","default":false},{"id":290122884,"url":"https://api.github.com/repos/NixOS/nixpkgs/labels/8.has:%20module%20(update)","name":"8.has: module (update)","color":"009800","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":13,"created_at":"2018-03-12T20:53:15Z","updated_at":"2018-03-13T17:19:20Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/NixOS/nixpkgs/pulls/36881","html_url":"https://github.com/NixOS/nixpkgs/pull/36881","diff_url":"https://github.com/NixOS/nixpkgs/pull/36881.diff","patch_url":"https://github.com/NixOS/nixpkgs/pull/36881.patch"},"body":"###### What?\r\n\r\n`libcardiacarrest` (https://github.com/oxij/libcardiacarrest) is a trivial implementation of `libpulse*` PulseAudio library API that unconditionally (but gracefully) fails to connect to the PulseAudio daemon and does **nothing** else. See https://github.com/oxij/libcardiacarrest/blob/master/README.org for more info.\r\n\r\nWith this we switch to linking against `libcardiacarrest` instead of the vanilla `libpulseaudio` by default. NixOS then overrides this with the vanilla `libpulseaudio` via `LD_LIBRARY_PATH` when user enables the daemon service.\r\n\r\n###### Motivation\r\n\r\n- If you do run PulseAudio daemon you shouldn't notice anything (except, maybe, for one of the Cons below until things will get fixed there).\r\n\r\nPros:\r\n\r\n- Smaller closure size when not using PulseAudio, much smaller closure size when not running either of PulseAudio, SystemD.\r\n- You can't statically link against `libcardiacarrest` by accident, unlike the vanilla `libpulseaudio` (SDL did this before, see #36377).\r\n- Hence, `libpulseaudio` is no longer an ethical and security liability (see https://github.com/oxij/libcardiacarrest/blob/master/README.org for the case against `libpulseaudio`).\r\n- Since it's a different implementation, it makes some buggy software crash sometimes. Which is good.\r\n\r\nCons:\r\n\r\n- Since it's a different implementation, it makes some buggy software crash sometimes. Which can be inconvenient. But it will print helpful tracing messages when it does so. Also overriding this with the vanilla `libpulseaudio` on app-by-app basis for a temporary workaround is pretty trivial.\r\n- Apps than override `LD_LIBRARY_PATH` instead of appending into it will need fixing when running with PulseAudio daemon. Or we can do something like #31263 (but, hopefully, a bit better). Or you can just override the broken app with the vanilla `libpulseaudio` and forget about it.\r\n\r\n###### Related things\r\n\r\nThis is a better version of what I wanted to have with #35374 which did the same with `libpressureaudio`. The problem with linking against `libpressureaudio` by default is that many apps just start using `apulse` emulation of PA API and completely ignore native ALSA. This hurts performance, ignores cool things you can do with your `asound.conf`, and puts pressure on `apulse` to emulate more of the PA API, which could eventually turn `apulse` into a monster `libpulseaudio` already is. `libcardiacarrest` is tiny (even by comparison to `apulse`, not just to `libpulseaudio`), all it does is it reports to the caller that PulseAudio daemon is unavailable, all the apps I tested then just switch to other output plugins like ALSA or JACK. This closes #35374.\r\n\r\n###### What I did\r\n\r\n- I built a lot (~60% of nixpkgs and counting) of things with this and everything that built before seems to build now.\r\n- I'm running several machines with minimalistic DEs on top of this for a week and everything is perfect.\r\n- I'm running a machine with Xfce DE on top of this, and everything is perfect, but I used it maybe a couple of hours this week, so it may have issues I didn't encounter.\r\n- I tried running KDE with this, everything seems to be perfect, but I'm not a KDE user, so who knows.\r\n- I tried building Gnome3, but it failed even before applying this patch, so, meh.\r\n\r\n###### What I want you to do\r\n\r\n- Running with the actual PulseAudio daemon. I have a report that it works with one of the previous versions of this branch, but I'm not running PulseAudio myself.\r\n- Building everything you use against this. Maybe something breaks. Can I have a hydra job for this?\r\n- Run everything you use under this. Maybe something crashes.\r\n\r\n###### Merge plan\r\n\r\nAfter somebody else can confirm that running with PulseAudio daemon works, I think, in principle, you can merge this immediately because I'm pretty sure it doesn't break anything sufficiently common and overriding with the vanilla `libpulseaudio` on app-by-app basis for uncommon things (while things get fixed, or just forever) is trivial.\r\n\r\nYou could keep it here for a week or two anyway so that people could test it, but the practice shows that people don't test Nixpkgs PRs until they get into `master`, so, inevitably, there are going to be surprises. I'd like this to get into `master` ASAP to be over the surprises ASAP.\r\n\r\nHydra job would be nice, though.\r\n\r\nSo. Study this branch. Study the source of `libcardiacarrest`. Confirm I'm not doing anything stupid or evil. Try this with PulseAudio daemon. Merge."},"comment":{"url":"https://api.github.com/repos/NixOS/nixpkgs/issues/comments/372746815","html_url":"https://github.com/NixOS/nixpkgs/pull/36881#issuecomment-372746815","issue_url":"https://api.github.com/repos/NixOS/nixpkgs/issues/36881","id":372746815,"user":{"login":"abbradar","id":1174810,"avatar_url":"https://avatars0.githubusercontent.com/u/1174810?v=4","gravatar_id":"","url":"https://api.github.com/users/abbradar","html_url":"https://github.com/abbradar","followers_url":"https://api.github.com/users/abbradar/followers","following_url":"https://api.github.com/users/abbradar/following{/other_user}","gists_url":"https://api.github.com/users/abbradar/gists{/gist_id}","starred_url":"https://api.github.com/users/abbradar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abbradar/subscriptions","organizations_url":"https://api.github.com/users/abbradar/orgs","repos_url":"https://api.github.com/users/abbradar/repos","events_url":"https://api.github.com/users/abbradar/events{/privacy}","received_events_url":"https://api.github.com/users/abbradar/received_events","type":"User","site_admin":false},"created_at":"2018-03-13T17:19:20Z","updated_at":"2018-03-13T17:19:20Z","author_association":"MEMBER","body":"> Oops, sorry, I misread that part as being a question about `libpulseaudio`, not about the application. But the rest is still valid.\r\n\r\nJust to be clear -- when application uses `libpulse` (or `libpulse-simple`) and tries to open a connection to it with some default settings, would those libraries try to launch PulseAudio? Not via DBus service launching as this requires a PulseAudio `.service` files installed so that's not applicable when `services.pulseaudio.enable` is disabled. I ask because if no this PR doesn't actually change any defaults and doesn't add security hardening much too -- with PulseAudio disabled applications won't use and almost code from it anyway. If yes on the other hand it does constitute a change of default behavior (okay by me but may be difficult if something relies on it).\r\n\r\n> libglvnd\r\n\r\nI agree that it was made for a different reason, what I meant is that our current usage of `LD_LIBRARY_PATH` is exactly for those cases -- to give applications machine-specific drivers (`libGL`, `sane` and others), and we'd like to move away from it when we can because there are multiple issues with it. BTW some applications also load dynamic libraries from hard-coded `/run/current-system/sw/lib` path (like Qt plugins). I wanted to consolidate all of this into one `/run/dynamic-libraries{,-32}`; PR was somewhere but it's unfinished (maybe I'll continue one day).\r\n\r\n> , if you think FHS problem is too much of inconvenience, ok, I see your point.\r\n\r\nFHS was just an example of problems with `LD_LIBRARY_PATH` so don't think over it much -- it's very similar to non-NixOS case so if it's solved probably FHS issue would be resolved automatically.\r\n\r\n> libpulsevnd\r\n\r\nMay be the solution yes -- I'm not sure if that much work is worth it but technically that should solve what I pointed out. I hope there's something better though -- personally I feel it's too much complexity. Before implementing anything serious however we'd need to find out if it's a behavior change in the end and if it is, should we do it or not."}}
{"actor":{"display_login":"abbradar"}}