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

nixos/pam: add service generation for swaylock #89019

Closed
wants to merge 1 commit into from
Closed

nixos/pam: add service generation for swaylock #89019

wants to merge 1 commit into from

Conversation

SeTSeR
Copy link
Contributor

@SeTSeR SeTSeR commented May 27, 2020

Motivation for this change

According to swaywm/sway#3631, pam requires presence of /etc/pam.d/swaylock file for correct unlocking when using swaylock as a screensaver.

Things done

Added an option to generate /etc/pam.d/swaylock for swaylock.

Related: #57602

  • 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.

@primeos
Copy link
Member

primeos commented May 27, 2020

Are you using swaylock with a different compositor than Sway? In b9851a9 we moved that option to the Sway module (see also the comment in pam.nix), but for users that use a different compositor this isn't ideal (need to set the option manually - might be a good idea to at least mention this in longDescription of swaylock).

@SeTSeR
Copy link
Contributor Author

SeTSeR commented May 27, 2020

I use Sway as compositor, but I enable it via home-manager, maybe that's the reason, why this option was not set

@primeos
Copy link
Member

primeos commented May 27, 2020

Yes, that's the case then. IMO the home-manager module should recommend to also enable Sway system-wide (programs.sway.enable) so that all relevant options are set:

    security.pam.services.swaylock = {};
    hardware.opengl.enable = mkDefault true;
    fonts.enableDefaultFonts = mkDefault true;
    programs.dconf.enable = mkDefault true;
    # To make a Sway session available if a display manager like SDDM is enabled:
    services.xserver.displayManager.sessionPackages = [ swayPackage ];

(A default system configuration most likely lacks the first and last option.)

And this shouldn't cause any conflicts (though I'm not familiar with the home-manager module).

primeos added a commit that referenced this pull request May 27, 2020
@primeos
Copy link
Member

primeos commented May 27, 2020

I've added a short note to swaylock in 023e11a but unfortunately that will probably not help that much, especially for home-manager.

@SeTSeR I'll close this PR as I don't think there's anything we could/should change in Nixpkgs, but feel free to re-open if you disagree ;) Might be a good idea to open a home-manager PR/issue to document this (maybe a hint in the enable option of the home-manager module that the NixOS module for Sway should also be enabled?).

@SeTSeR
Copy link
Contributor Author

SeTSeR commented May 30, 2020

Forwarded issue to home-manager: nix-community/home-manager#1288

@primeos primeos mentioned this pull request May 30, 2020
10 tasks
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

2 participants