Skip to content

config.sound: Disable sound by default#35319

Closed
adisbladis wants to merge 1 commit intoNixOS:masterfrom
adisbladis:sound_disable
Closed

config.sound: Disable sound by default#35319
adisbladis wants to merge 1 commit intoNixOS:masterfrom
adisbladis:sound_disable

Conversation

@adisbladis
Copy link
Copy Markdown
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.

@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Feb 22, 2018
@aristidb
Copy link
Copy Markdown
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
Copy Markdown
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

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants