Skip to content

nixos/display-managers/auto: remove#78583

Merged
worldofpeace merged 1 commit intoNixOS:masterfrom
worldofpeace:remove-auto-displaymanager
Jan 30, 2020
Merged

nixos/display-managers/auto: remove#78583
worldofpeace merged 1 commit intoNixOS:masterfrom
worldofpeace:remove-auto-displaymanager

Conversation

@worldofpeace
Copy link
Copy Markdown
Contributor

Motivation for this change

This module allows root autoLogin, so we would break that for users, but they shouldn't be using it anyways. This gives the impression like auto is some special display manager, when it's just lightdm and special pam rules to allow root autoLogin. It was created for NixOS's testing
so I believe this is where it belongs.

cc @hedning @jtojnar as we discussed this #53843

Things done

I need to check the tests in NixOS still work properly that might rely on auto being enabled but don't have the correct imports or something.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace
Copy link
Copy Markdown
Contributor Author

Screenshot of the release note rendered

Screenshot from 2020-01-26 17 42 06

It describes things someone could do if they really actually needed root autologin without this module.

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: xfce The Xfce Desktop Environment 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 Jan 26, 2020
@worldofpeace worldofpeace force-pushed the remove-auto-displaymanager branch from 2763c7a to 558d16e Compare January 26, 2020 23:16
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is better than before but I would much rather do something like

imports = [
  (import ./common/auto-login.nix { user = "alice"; })
];

at each individual modules.

But it would not be idiomatic and I am not sure if it would not affect the performance negatively.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This whole thing could really turn into a common testing lib which I think we actually might need.
But I don't think I really want to improve this more just yet, I'd like to see this in 20.03.

@worldofpeace
Copy link
Copy Markdown
Contributor Author

@jtojnar Changes pushed. I believe I got all the display-manager -> display manager.

@worldofpeace worldofpeace force-pushed the remove-auto-displaymanager branch 2 times, most recently from e60c2ce to 5834008 Compare January 29, 2020 01:34
@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 Jan 29, 2020
This module allows root autoLogin, so we would break that for users, but
they shouldn't be using it anyways. This gives the impression like auto
is some special display manager, when it's just lightdm and special pam
rules to allow root autoLogin. It was created for NixOS's testing
so I believe this is where it belongs.
@worldofpeace worldofpeace force-pushed the remove-auto-displaymanager branch from 5834008 to c95612a Compare January 30, 2020 00:06
@worldofpeace worldofpeace merged commit a8ea4f0 into NixOS:master Jan 30, 2020
@worldofpeace worldofpeace deleted the remove-auto-displaymanager branch January 30, 2020 00:06
@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: xfce The Xfce Desktop Environment 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-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 30, 2020
anna328p pushed a commit to anna328p/nixpkgs that referenced this pull request Feb 2, 2020
…anager

nixos/display-managers/auto: remove
@nixos-discourse
Copy link
Copy Markdown

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

https://discourse.nixos.org/t/january-2020-in-nixos/5771/1

@NixOS NixOS deleted a comment from nixos-discourse Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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