Skip to content

nixos/udisks2: don't enable by default#186028

Merged
dasJ merged 2 commits intoNixOS:masterfrom
helsinki-systems:disable-udisks2-by-default
Aug 11, 2022
Merged

nixos/udisks2: don't enable by default#186028
dasJ merged 2 commits intoNixOS:masterfrom
helsinki-systems:disable-udisks2-by-default

Conversation

@ajs124
Copy link
Member

@ajs124 ajs124 commented Aug 11, 2022

Description of changes

This was enabled by default in 18a7ce7
with the reason that it would be "useful regardless of the desktop
environment.", which I'm not arguing against.

The reason why this should not be enabled by default is that there are a
lot of systems that NixOS runs on that are not desktop systems.
Users on such systems most likely do not want or need this feature and
could even consider this an antifeature.
Furthermore, it is surprising to them to find out that they have this
enabled on their systems.
They might be even more surprised to find that they have polkit enabled
by default, which was a default that was flipped in
a813be0, for some discussion as to why
see #156858.

Evidently, this default is not only surprising to users, but also module
developers, as most if not all modules for desktop environments
already explicity set services.udisks2.enable = true; which they don't
need to right now.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

This was enabled by default in 18a7ce7
with the reason that it would be "useful regardless of the desktop
environment.", which I'm not arguing against.

The reason why this should not be enabled by default is that there are a
lot of systems that NixOS runs on that are not desktop systems.
Users on such systems most likely do not want or need this feature and
could even consider this an antifeature.
Furthermore, it is surprising to them to find out that they have this
enabled on their systems.
They might be even more surprised to find that they have polkit enabled
by default, which was a default that was flipped in
a813be0. For some discussion as to why
see NixOS#156858.

Evidently, this default is not only surprising to users, but also module
developers, as most if not all modules for desktop environments already
explicity set services.udisks2.enable = true; which they don't need to
right now.
@github-actions github-actions bot 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/` labels Aug 11, 2022
@ajs124 ajs124 requested review from Lassulus, dasJ and mweinelt August 11, 2022 00:49
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 11, 2022
Copy link
Member

@fpletz fpletz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 since I have it disabled by default on my systems for years.

After looking through the desktop manager modules, I noticed that the ones that do not specify udisks2 and most likely are using it (MATE, lxqt) enable gvfs which at least can use udisks2. My suggestion would be to enable udisks2 in the services.gvfs module for minimal impact on existing desktop users.

@bobby285271 bobby285271 added the 12.approvals: 3+ This PR was reviewed and approved by three or more persons. label Aug 11, 2022
can be used by gvfs and is disabled by default after
f763710
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/lightdm-dwm-not-working/21147/6

@alyssais
Copy link
Member

alyssais commented Sep 2, 2022

This has broken some NixOS tests. At least cagebreak. It would probably be good to check tests for other desktop / graphical stuff and see if they also need to explicitly enable udisks after this change.

jlesquembre added a commit to jlesquembre/dotfiles that referenced this pull request Oct 8, 2022
udisks2 was enabled by default, but now it's not.
See:
NixOS/nixpkgs#186028
nix-community/home-manager#3153
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: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants