Skip to content

[WIP] pkgs: pulseaudio -> pressureaudio; nixos: override when running a daemon#35374

Closed
oxij wants to merge 26 commits intoNixOS:masterfrom
oxij:pkgs/pressureaudio-by-default
Closed

[WIP] pkgs: pulseaudio -> pressureaudio; nixos: override when running a daemon#35374
oxij wants to merge 26 commits intoNixOS:masterfrom
oxij:pkgs/pressureaudio-by-default

Conversation

@oxij
Copy link
Member

@oxij oxij commented Feb 23, 2018

Motivation for this change

An idea that struck me when writing #35292 (comment) (see #35355 (comment) for in-depth explanation of the motivation behind every part of that comment).

With this we link against pressureaudio by default to provide libpulse API over pure ALSA to apps that refuse to properly work over ALSA themselves (e.g. Firefox).

NixOS then overrides this with pulseaudioLib via LD_LIBRARY_PATH when user enables the daemon service (pressureaudio and pulseaudioLib are link-time-compatible).

This way users that don't run PulseAudio daemon and don't want to link against pulseaudioLib for religious and/or security reasons don't need to make any overrides, while PulseAudio daemon users stay happy.

This also significantly reduces closure size when not running PulseAudio daemon since pressureaudio (apulse) is tiny in comparison to pulseaudioLib and requires only alsaLib and glib.

It is a mass rebuild, but none of my PRs to staging got merged this month, so.

Things not done
  • Not tested with hardware.pulseaudio.enable set. I refuse to set it for ethical reasons. But the change seems trivial enough that it should work.
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of some pkgs that depend on this change using nix-shell -p nox --run "nox-review wip" Seems to work.
  • Tested execution of many binary files of the changed packages. Seem to work.
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels Feb 23, 2018
@oxij
Copy link
Member Author

oxij commented Feb 23, 2018

@GrahamcOfBorg Hooray!

I wonder what gets rebuilt on darwin. Is pulseaudio even supposed to work on darwin?

@oxij
Copy link
Member Author

oxij commented Feb 23, 2018 via email

@oxij oxij force-pushed the pkgs/pressureaudio-by-default branch from 50d255f to f4c0a02 Compare February 23, 2018 07:03
@oxij oxij requested a review from ttuegel as a code owner February 23, 2018 07:03
@GrahamcOfBorg GrahamcOfBorg added the 6.topic: qt/kde Object-oriented framework for GUI creation label Feb 23, 2018
@oxij oxij force-pushed the pkgs/pressureaudio-by-default branch from f4c0a02 to 698b9ce Compare February 23, 2018 07:09
oxij added 6 commits February 23, 2018 07:56
This satisfies both people that want to purge every single bit of
PulseAudio from their systems for ethical, religious, and/or security
reasons and those who want to run the daemon for convenience reasons.

This also significantly reduces closure size when not running
PulseAudio daemon since `pressureaudio` (`apulse`) is tiny in
comparison to `pulseaudioLib` and requires only `alsaLib` and `glib`.
@oxij oxij force-pushed the pkgs/pressureaudio-by-default branch from 44d2d67 to fe9eb72 Compare February 23, 2018 07:56
@oxij
Copy link
Member Author

oxij commented Feb 23, 2018

I've set config.pulseaudio = true in my config, all that's left is to rebuild everything and see what will happen when the whole system uses fake pulseaudio.

@oxij oxij changed the title pkgs: pulseaudio -> pressureaudio; nixos: override when running a daemon [WIP] pkgs: pulseaudio -> pressureaudio; nixos: override when running a daemon Feb 23, 2018
@vcunat
Copy link
Member

vcunat commented Feb 23, 2018

System-wide LD_LIBRARY_PATH turned out to be a fragile approach, IMHO, and we'd like to get rid of it for OpenGL. I wouldn't go this way.

@oxij
Copy link
Member Author

oxij commented Feb 23, 2018 via email

@vcunat
Copy link
Member

vcunat commented Feb 24, 2018

Probably replace it with glvnd, as that provides indirection to load the right libGL anyway. (We may still need some custom hacks, e.g. to find libGL on non-nixos, etc.)

@oxij
Copy link
Member Author

oxij commented Feb 24, 2018 via email

@7c6f434c
Copy link
Member

I think the problem with LD_LIBRARY_PATH is that some programs randomly decide to change this variable in a weird way but expect that they will still be able to find all the system libraries including libGL. Special-case side-channel variables are much less likely to be manipulated.

@oxij
Copy link
Member Author

oxij commented Feb 24, 2018 via email

@7c6f434c
Copy link
Member

I think Steam alone disproves the numerical bound you give.

I mean, if no one would want to either write or use garbage software, we wouldn't have the current discussion.

Also. there are setuid applications.

@oxij
Copy link
Member Author

oxij commented Feb 24, 2018 via email

@oxij
Copy link
Member Author

oxij commented Mar 6, 2018

Status report.

As I said in #35292 (comment)

Something very related I made: https://github.com/oxij/libcardiacarrest
It describes the problems with PulseAudio in some detail (including direct pointers at the crazy code in the lib) and provides what seems to be a working solution for all of this shit.
Will switch #35374 to using that after I come back to my senses from making the thing.

I switched this to libcardiacarrest and now rebuilding everything the nth time for libcardiacarrest v4+. On my laptop I'm running on v2 without any problems after applying patches from #36377, so it is very usable approach. But some apps do some crazy shit with PA API and fail to check for errors, so I had to fix some and add more PA shit to libcardiacarrest to fix others.

On v4 even official freedesktop PA tools gracefully fail as they should, so it's time to get excited. Will publish after I confirm Xfce and KDE work.

@oxij
Copy link
Member Author

oxij commented Mar 12, 2018

Status: Done. See #36881 for the version with libcardiacarrest instead of libpressureaudio. Keeping this open till that gets merged (also to remind myself to PR the rest of the changes here eventually).

@lukateras
Copy link
Member

Can this be closed in favor of #36881?

@oxij
Copy link
Member Author

oxij commented Jun 15, 2018 via email

@flokli
Copy link
Member

flokli commented Mar 30, 2019

Closing, due to the same reasons as expressed in #36881 (comment).

This is a controversial change, and needs to go through an RFC.

Keeping this PR open doesn't really help, it has bitrotten and doesn't apply cleanly anyhow.

@flokli flokli closed this Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: qt/kde Object-oriented framework for GUI creation 8.has: clean-up This PR removes packages or removes other cruft 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants