Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config.sound: Disable sound by default #35319

Closed
wants to merge 1 commit into from

Conversation

adisbladis
Copy link
Member

Motivation for this change

In #35292 @fpletz brought up the imo very good idea to disable sound by default.

Per my proposal here: #35292 (comment) I enable sound if services.xserver.enable is true.
This gives most users the desired effect, desktop users get working sound and server users will not get alsaUtils or any ALSA services.

If you feel like you dont belong in either category it's simple enough to enable.

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 all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@aristidb
Copy link
Contributor

I didn't notice this PR until now. I have the competing proposal here: #35355

Technical suggestions:

  1. Try using mkDefault so the manual isn't rebuilt whenever services.xserver.enable is. (Maybe academic for this particular option, but still.)
  2. I think even this change should be only if versionAtLeast stateVersion "18.03"

(And of course I prefer the simplicity of just completely disabling sound by default and putting it in the example config, that's why #35355 .)

@fpletz
Copy link
Member

fpletz commented Feb 22, 2018

Hmm, after thinking about this for a while this is IMHO not a good idea. Having more of these cross module dependencies makes the whole module machinery we have in NixOS very confusing and convoluted. X11 itself is not directly bound to sound support in any way to this doesn't make a lot of sense.

If people really want this, @aristidb's suggestions should be implemented though. 👍

@adisbladis adisbladis closed this Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants