nixos: disable sound by default, if stateVersion >= 18.03 #35355

aristidb · 2018-02-22T19:18:16Z

Motivation for this change

This change was suggested by @fpletz in #35292. Sound is disabled by default (for stateVersion > 18.03), and nixos-generate-config now puts a helpful comment telling you how to enable it. This is consistent with X11 being disabled by default.

Things done

I'm still building the branch, so not ready yet with everything. I wanted to create the PR already to enable discussion.

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 · 2018-02-22T19:18:43Z

CC @fpletz @vcunat @grahamc

grahamc · 2018-02-22T19:24:26Z

nixos/modules/services/audio/alsa.nix

@@ -78,7 +77,9 @@ in

  ###### implementation

-  config = mkIf config.sound.enable {
+  config = {
+      sound.enable = mkDefault !(versionAtLeast config.system.stateVersion "18.03")


you need a ; at the end of this line

grahamc · 2018-02-22T19:24:46Z

nixos/modules/services/audio/alsa.nix

-  config = mkIf config.sound.enable {
+  config = {
+      sound.enable = mkDefault !(versionAtLeast config.system.stateVersion "18.03")
+    } // mkIf config.sound.enable {


I'm not sure, but you might need to wrap the mkIf config.sound.enable { ... } in (...).

grahamc · 2018-02-22T19:40:38Z

Try testing it like this: HOME=/homeless-shelter NIX_PATH=nixpkgs=$(pwd) nix-instantiate ./nixos/release.nix -A options --option restrict-eval true --option build-timeout 1800 --argstr system x86_64-linux --show-trace

oxij · 2018-02-22T19:42:28Z

Agree in principle, strongly against in practice: - Breaks existing configurations. - Headless and speakerless systems disable it anyway. - Saves only about 2.3Mb (can be made ~828K if we separate outputs of `alsa-utils`) anyway.

grahamc · 2018-02-22T19:44:32Z

How does this break existing configurations? Are people deleting the do-not-touch stateVersion from their configuration? If so, I'm okay with that and we should move forward.

oxij · 2018-02-22T19:47:28Z

Also, what is #35319?

aristidb · 2018-02-22T19:52:16Z

@oxij Thanks for point out the other PR that I missed.

oxij · 2018-02-22T19:52:44Z

How does this break existing configurations? Are people deleting the `do-not-touch` `stateVersion` from their configuration? If so, I'm okay with that and we should move forward.

I use and continuously update the same `configuration.nix` from around 7 years ago (and "reinstall" by simply copying system closure to a new HDD). My configuration does not contain such a line.

grahamc · 2018-02-22T19:54:45Z

I understand. I'm okay with this level of breakage, as the stateVersion has been around for several years now, is well documented, and long time users have had several opportunities (and will have another opportunity) to catch this problem via the release notes before their configuration breaks.

oxij · 2018-02-22T20:12:03Z

I understand. I'm okay with this level of breakage, as the stateVersion has been around for several years now, is well documented, and long time users have had several opportunities (and will have another opportunity) to catch this problem via the release notes before their configuration breaks.

Ugh, can't we at the very least generate a warning if you don't have `stateVersion` set? Surely I'm not alone here. Now, as to the implementation. ``` + # Enable sound. + # sound.enable = true; + # hardware.pulseaudio.enable = true; + ``` `hardware.pulseaudio.enable = true;` line is out of scope of this PR and belongs to the discussion of #35292. Lets not repeat all of that here. "Hey, its just a comment" is not an argument. I'm absolutely serious. Please, remove that line. ``` - config = mkIf config.sound.enable { + config = { + sound.enable = mkDefault (!versionAtLeast config.system.stateVersion "18.03"); + } // mkIf config.sound.enable { ``` This is ugly and probably won't work (lacks parens). Why don't you just put that expression into the `default = ` where it belongs? The manual build can then be made deterministic again by defining `defaultText`.

aristidb · 2018-02-22T20:14:51Z

@oxij Your aggressive style is not appreciated.

oxij · 2018-02-22T20:20:49Z

How is any of the above aggressive?

aristidb · 2018-02-22T20:20:56Z

I've fixed the error of how I was using mkIf in 473cfed (my nixos module system skills are rusty :(), and the missing defaultText in f372898. Fortunately, this will all be squash-merged, so nobody will have to look at my erroneous intermediate commits.

Now I'm waiting for @fpletz and @vcunat to say what they think about getting this into 18.03.

oxij · 2018-02-22T20:34:21Z

... and then most of my substantial comments were ignored =/ See oxij@42a814e instead.

oxij · 2018-02-22T21:47:34Z

Same thing, but with warnings and no sneaky changes: #35366.

Closes NixOS#35355.

grahamc · 2018-02-22T22:01:43Z

How is any of the above aggressive?

Jan,

Let me start with recognizing that you don't care for PulseAudio and
possibly the entire FreeDesktop software universe. You have made your
opinion of Poettering quite clear. I appreciate your position on
this, and respect your choice.

As an organizational goal, the NixOS system defaults should do a nice
job representing a common, well supported base system to work off of.
It is especially nice when this default also matches what most users
are already using, and what upstream software writers are expecting.

In the case of Firefox, indeed -- upstream expects PulseAudio and
we're inclined to follow that lead. NixOS does not have the user base
or contributor base to put up a meaningful resistance to far-reaching
ecosystem changes like this.

In the case of users, because NixOS has no sort of metrics collection
mechanism, it is much harder to tell. However, anecdotally, it is
a very common option to enable -- present in most public
configurations and tutorials I've come across.

It seems to be a very common opinion that NixOS should in fact enable
PulseAudio if sound is enabled by default, and indeed I also support
that position.

However, my position on PulseAudio is not what I'm writing this for,
it is more about your approach to communicating about the issue. Your
behavior on these recent pull requests have distressed, upset, and
frustrated fellow members of the community.

Firstly: it is not appropriate to compare a configuration setting
about sound to the Stasi, or a re-framing of a poem about the
systematic extermination of people.

Secondly: a much easier way to explain the PRs around enabling
PulseAudio is that it is considered an improvement to the defaults.
Not an attack upon you, not conspiracy against you.

People have different opinions from you, and that is okay. This is a
beautiful part of the NixOS configuration mechanism!

When people do things you disagree with, it is not stupid. When people
do things you disagree with, it is not crazy. It is not appropriate to
question a fellow contributor's mental fitness because you don't care
for the work they've done.

Franz is not lying when he interprets his experience and observations
and concludes that most NixOS users are using PulseAudio. It is not
appropriate to accuse well-meaning core contributors of lying.

Finally, nobody is forcing you to use any software. You can choose to
not use NixOS. In a lot of ways, you already have -- by making SLNOS.
NixOS has no intentions of making SLNOS hard to maintain, and at the
same time NixOS is not beholden to making it easy or convenient for
SLNOS to exist.

If you truly believe NixOS contributors are crazy, lying, and
conspiring against you, then I wholeheartedly recommend you avoid
such a toxic environment. Otherwise, please tone down your rhetoric
and cease with the personal attacks.

Thank you,
Graham

oxij · 2018-02-23T00:01:40Z

=/ Well.

Let me start with recognizing that you don't care for PulseAudio and possibly the entire FreeDesktop software universe. ...

I do care about PulseAudio, I care about it so much that I read its source and, as you see, now put a lot of effort into trying to prevent it ever running on as many machines as possible. Anyway. I apologize if I offended anyone, as that was not my intention (never ever felt such an intention in my life). But, please try to look at what happens to NixOS from my point of view. Lets forget about issues with systemd, use-flags and SLNOS even. So, I follow GitHub issues and look into `git log -p --grep pulseaudio` (seriously, just look at the log) and see this constant push for PulseAudio by default. I ask myself "Why is that?" I see three possibilities: I simply don't understand something, incompetence, malice. So I ask technical questions like "why do you even need it? and why do you need to link against libpulseaudio in apps that support ALSA when they would work via ALSA->PA adapter anyway?`. And get no answers. Instead I get ignored and PRs get merged. So I make a PR that goes the other direction (#35041), and get 👎s that cite same factually incorrect technical arguments. Note that in the recently merged #35005 and ignored #35041 I'm exactly concerned that people will use arguments of #35292 for PulseAudio by default. Then, just as #35005 gets merged, I see #35292 pop up after the long discussion of #29250 (and by the same author as #29250!) using exactly the arguments I was concerned about. I thought I were dreaming, but I were not. Then this PR gets split off out of #35292, and instead of doing what it states, it also sneaks in a change that suggests you need to run PulseAudio by default to get sound. Yes, I got triggered by this. Would you not be?

upstream software

Right. Do you remember #29250 where I said

$ cd nixpkgs/pkgs $ ag alsaLib | cut -d ':' -f1 | sort | uniq > alsa $ ag libpulse | cut -d ':' -f1 | sort | uniq > pulse $ cat alsa pulse | sort | uniq -d > both $ wc -l alsa pulse both 272 alsa 139 pulse 87 both So, ~185 expressions mentioning ALSA without libpulse, ~52 expressions mentioning libpulse without ALSA. I think we have a clear winner in "better supported by applications" category.

What are your counterarguments? Sure, there is n=1 of firefox, but after what they did with their 'Mr. Robot "study"' and having read PA code myself I now can't interpret their

nobody wants to support it [PulseAudio] and we don't have time to do it

as anything but malice (sorry to all the honest firefox devs, but what else do you expect me to do here?).

a very common option to enable, so lets do it by default

I agree that it is a common option, but - it is much more common to leave everything to the defaults, - most people that do enable PA simply don't know better, it's highly improbable that they actually need any of its features ALSA doesn't provide already, but they expose themselves to a huge security liability. Just as an experiment, run ``` $ grep 'gstreamer.*bad' pkgs ``` Every app that has `gstreamer-plugins-bad` and feeds them from the network (e.g. `liferea`) is a security liability (I planned to PR SLNOS fix for this eventually, but now I wonder if I should even try). They are called "-bad" for a reason, look it up if you don't believe me. If you look at PA's internals, you'll understand my concern with this whole thing. _It's exactly the same problem._ Personally, I think that making or even suggesting to users to run PA by default and especially to connect it to networking software like `firefox` is simply ethically unacceptable. Should I've said "NSA", "KGB", "an advanced persistent threat" instead of "Stasi"? I don't care about the pleasantries. I care about technical details. Connecting PA to untrusted data sources is madness and anyone willingly pushing it is either underinformed, crazy or malicious. No offense intended.

Your behavior on these recent pull requests have distressed, upset, and frustrated fellow members of the community.

Again, I apologize. Mental institutions part, indeed, was a bit too much. But the sequence of "let's enable pulseaudio by default" and "let's disable sound by default unless pulseaudio is enabled" got me flipping my tea table here, so, I think, you have to understand and pardon that paragraph.

Firstly: it is not appropriate to compare a configuration setting about sound to the Stasi, or a re-framing of a poem about the systematic extermination of people.

I disagree. I think it was appropriate. That poem is about social responsibility. Not connecting PA to the net in a social responsibility to our users.

Secondly: ...

Addressed above.

When people do things you disagree with, it is not stupid. ...

Addressed above.

Franz is not lying when he interprets his experience and observations and concludes that most NixOS users are using PulseAudio. It is not appropriate to accuse well-meaning core contributors of lying.

Franz, I apologize, I did not mean that you were lying. I was writing that part just after I flipped the tea table, and, indeed, it turned out to be ambiguous. I purely meant that I think that Mozilla are deliberately (again, sorry to all the honest mozilla devs) misleading with their interpretation of the usage stats they collect, nothing else. Note, that my general statistical observations from that part still are not addressed. Thus, I think, that even if we ignore all of the above, the argument "normal users want this by default" is not as clear as you present it to be. Just sayin'.

SLNOS

Unrelated. This is a purely technical discussion.

Otherwise, please tone down your rhetoric and cease with the personal attacks.

I did not personally attack anyone, I never personally attack anyone, I attacked their patches that I find malicious. Surely, most of the "advanced persistent threats" and nixpkgs contributors are good people that pursue noble goals (state security and convenience for users respectively). But I don't care if they are good people or if their goals are noble when by following their goals they expose normal people to security and privacy threats. I think this is simply ethically unacceptable.

edolstra · 2018-02-23T10:24:11Z

Hm, this is not really the intended use of stateVersion. From the description:

        Every once in a while, a new NixOS release may change
        configuration defaults in a way incompatible with stateful
        data. For instance, if the default version of PostgreSQL
        changes, the new version will probably be unable to read your
        existing databases. To prevent such breakage, you can set the
        value of this option to the NixOS release with which you want
        to be compatible. The effect is that NixOS will option
        defaults corresponding to the specified release (such as using
        an older version of PostgreSQL).

So this is only intended for options that have some corresponding on-disk state. AFAICT this is not the case for sound. In any case stateVersion is a necessary evil that only exists because we can't just upgrade Postgres databases or change SSH host keys. It's not necessary for things like whether sound is enabled. (If the user discovers that sound is suddenly disabled, they can just enable it.)

I had some vague recollection that we also had a configVersion option setting to control the defaults for non-state-related options, but I can't find it so maybe it was only discussed.

edolstra · 2018-02-23T10:45:09Z

nixos/modules/installer/tools/nixos-generate-config.pl

@@ -603,6 +603,10 @@ sub multiLineList {
  # Enable CUPS to print documents.
  # services.printing.enable = true;

+  # Enable sound.
+  # sound.enable = true;
+  # hardware.pulseaudio.enable = true;


BTW, shouldn't hardware.pulseaudio.enable = true imply sound.enable = true? Or perhaps the other way around, if we want Pulseaudio to be the default sound system.

oxij · 2018-02-23T16:06:47Z

BTW, shouldn't `hardware.pulseaudio.enable = true` imply `sound.enable = true`? Or perhaps the other way around, if we want Pulseaudio to be the default sound system.

They are independent. `sound.enable = true` makes `asound.conf` and runs ALSA services, `hardware.pulseaudio.enable = true` makes its own `asound.conf` with ALSA->PA adapter and runs PA services.

vcunat · 2018-02-23T20:15:46Z

Maybe the defaults would better be solved along these lines:

{
  # X, pulseaudio, ...
  imports = [ <nixpkgs/nixos/modules/profiles/desktop.nix> ];
}

oxij · 2018-02-24T10:08:50Z

Probably a good idea to point to this #35292 (comment) here too.

abbradar · 2018-02-24T15:32:42Z

Linking e349ccc#commitcomment-27757112 here as I'm fairly concerned about making this change unconditionally -- it would break every NixOS desktop's user sound after an update, which IMO is not justified by a small closure size benefit in default server setups.

oxij · 2018-02-24T22:22:11Z

it would break every NixOS desktop's user sound after an update, which IMO is not justified by a small closure size benefit in default server setups.

Exactly my original concern in the parent thread and above here. Thanks for chipping in. I still think this should be reverted.

abbradar · 2018-02-24T22:27:02Z

To be clear I was okay with this change as long as it was conditional on some variable which specifies how "modern" is current NixOS configuration. However I also agree with @edolstra that stateVersion is unfit for this so we have no good way of determining.

Offtopic: @oxij made a good point that we should warn if stateVersion is not set. Instead we should have a special value that says "I want bleeding edge state assumptions and don't mind consequences".

oxij · 2018-02-24T22:38:12Z

Offtopic: @oxij made a good point that we should warn if `stateVersion` is not set.

Did you look at #35366?

abbradar · 2018-02-24T22:41:48Z

@oxij Yeah, I actually wanted to propose you to split stateVersion changes into a separate PR.

oxij · 2018-09-10T17:54:04Z

Let me remind you that after this was reverted the ALSA module is now broken by default. If you don't want to revert the original change, let's make a warning at the very least. See #46490.

nixos: disable sound by default, if stateVersion > 18.03

ea48c41

aristidb added 6.topic: nixos 2.status: work-in-progress 8.has: module (update) labels Feb 22, 2018

GrahamcOfBorg added 8.has: changelog 8.has: documentation labels Feb 22, 2018

aristidb mentioned this pull request Feb 22, 2018

Pulseaudio by default, if sound is enabled #35292

Closed

7 tasks

grahamc reviewed Feb 22, 2018

View reviewed changes

aristidb added 2 commits February 22, 2018 20:33

nixos: fix syntax error in alsa module

58b07ac

nixos: fix another syntax error (local instantiation succeeds now)

3af0ff2

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Feb 22, 2018

aristidb mentioned this pull request Feb 22, 2018

config.sound: Disable sound by default #35319

Closed

8 tasks

aristidb closed this Feb 22, 2018

aristidb reopened this Feb 22, 2018

nixos/alsa: fix wrong use of mkIf

473cfed

nixos/alsa: fix missing defaultText

f372898

oxij mentioned this pull request Feb 22, 2018

nixos: properly disable sound by default, if stateVersion > 18.03 #35366

Closed

6 tasks

oxij pushed a commit to oxij/nixpkgs that referenced this pull request Feb 22, 2018

nixos: disable sound by default, if stateVersion > 18.03

f0816cf

Closes NixOS#35355.

fpletz added this to the 18.03 milestone Feb 22, 2018

fpletz removed the 2.status: work-in-progress label Feb 22, 2018

fpletz changed the title ~~nixos: disable sound by default, if stateVersion > 18.03~~ nixos: disable sound by default, if stateVersion >= 18.03 Feb 22, 2018

fpletz merged commit a43e33d into master Feb 22, 2018

fpletz deleted the sound-disabled-by-default branch February 22, 2018 22:06

oxij mentioned this pull request Feb 23, 2018

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

Closed

9 tasks

edolstra reviewed Feb 23, 2018

View reviewed changes

oxij mentioned this pull request Mar 13, 2018

[Request For Testing] pkgs: libpulseaudio -> libcardiacarrest; nixos: override when running PA daemon #36881

Closed

oxij mentioned this pull request Apr 16, 2018

thunderbird: enable official branding #36449

Closed

8 tasks

oxij mentioned this pull request Jun 9, 2018

Move from GitHub long-term #41448

Closed

etu mentioned this pull request Feb 22, 2019

installer: Enable pulseaudio in all graphical iso's #56167

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos: disable sound by default, if stateVersion >= 18.03 #35355

nixos: disable sound by default, if stateVersion >= 18.03 #35355

aristidb commented Feb 22, 2018

aristidb commented Feb 22, 2018

grahamc Feb 22, 2018

grahamc Feb 22, 2018

grahamc commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

grahamc commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

aristidb commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

grahamc commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

aristidb commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

aristidb commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

oxij commented Feb 22, 2018

grahamc commented Feb 22, 2018

oxij commented Feb 23, 2018 via email •

edited

edolstra commented Feb 23, 2018

edolstra Feb 23, 2018

oxij commented Feb 23, 2018 via email

vcunat commented Feb 23, 2018

oxij commented Feb 24, 2018

abbradar commented Feb 24, 2018

oxij commented Feb 24, 2018 via email

abbradar commented Feb 24, 2018

oxij commented Feb 24, 2018 via email

abbradar commented Feb 24, 2018

oxij commented Sep 10, 2018

nixos: disable sound by default, if stateVersion >= 18.03 #35355

nixos: disable sound by default, if stateVersion >= 18.03 #35355

Conversation

aristidb commented Feb 22, 2018

Motivation for this change

Things done

aristidb commented Feb 22, 2018

grahamc Feb 22, 2018

Choose a reason for hiding this comment

grahamc Feb 22, 2018

Choose a reason for hiding this comment

grahamc commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

grahamc commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

aristidb commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

grahamc commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

aristidb commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

aristidb commented Feb 22, 2018

oxij commented Feb 22, 2018 via email

oxij commented Feb 22, 2018

grahamc commented Feb 22, 2018

oxij commented Feb 23, 2018 via email • edited

edolstra commented Feb 23, 2018

edolstra Feb 23, 2018

Choose a reason for hiding this comment

oxij commented Feb 23, 2018 via email

vcunat commented Feb 23, 2018

oxij commented Feb 24, 2018

abbradar commented Feb 24, 2018

oxij commented Feb 24, 2018 via email

abbradar commented Feb 24, 2018

oxij commented Feb 24, 2018 via email

abbradar commented Feb 24, 2018

oxij commented Sep 10, 2018

oxij commented Feb 23, 2018 via email •

edited