nixos/xautolock: rewrite and add some options #30128

WilliButz · 2017-10-05T13:59:53Z

Things done

xautolock is now run as a user service
added the killer and killertime options to the module
added extraOptions for additional customization

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
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.

Ma27 · 2017-10-05T15:12:46Z

ah this is the change we talked about some time ago? I'll definetely try tondo a review in the nsxt days:)

On 5. Oct 2017, at 4:00 PM, WilliButz ***@***.***> wrote: Things done xautolock is now run as a user service added the killer and killertime options to the module added extraOptions for additional customization Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS) Built on platform(s) NixOS macOS Linux 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. /cc @Ma27 @fpletz You can view, comment on, or merge this pull request online at: #30128 Commit Summary nixos/xautolock: rewrite and add some options File Changes M nixos/modules/services/x11/xautolock.nix (90) Patch Links: https://github.com/NixOS/nixpkgs/pull/30128.patch https://github.com/NixOS/nixpkgs/pull/30128.diff — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

rycee · 2017-10-05T20:48:58Z

nixos/modules/services/x11/xautolock.nix

+          type = types.nullOr types.string;
+
+          description = ''
+            The script to use when manually locking the computer with `xautolock -locknow`.


I believe

`xautolock -locknow`

here should be

<command>xautolock -locknow</command>

since the description is interpreted as docbook code.

Ma27 · 2017-10-07T08:08:33Z

nixos/modules/services/x11/xautolock.nix

+        killer = mkOption {
+          default = null; # default according to `man xautolock` is none
+          example = "systemctl suspend";
+          type = types.string;


must be types.nullOr types.string.
I left this one empty and I got error: The option value `services.xserver.xautolock.killer' in `/home/ma27/Projects/nixpkgs/nixos/modules/services/x11/xautolock.nix' is not a string.

Ma27 · 2017-10-07T08:09:51Z

nixos/modules/services/x11/xautolock.nix

@@ -45,28 +55,80 @@ in
        };

        notifier = mkOption {
-          default = "notify-send 'Locking in 10 seconds'";
-          type = types.string;
+          default = null;


as you added an assertion to ensure that notifier contains a useful script, default = null is not a good idea, right?

Ah, I did this so that you'd have to take care of the script when you set enableNotifier to true, because the previous default notify-send was not mentioned in the manpage and would add another dependency.

Ma27 · 2017-10-07T08:28:40Z

FYI I built a simple test VM using the following expression to confirm this change:

{
  xautolock = { pkgs, ... }: with pkgs; {
    environment.systemPackages = [ pkgs.i3lock ];

    users.extraUsers.vm = {
      password = "vm";
      extraGroups = [ "wheel" ];
      isNormalUser = true;
    };

    services.xserver = {
      enable = true;
      windowManager = {
        i3.enable = true;
        default = "i3";
      };
      desktopManager = {
        xterm.enable = false;
        default = "none";
      };
      xautolock = {
        enable = true;
        enableNotifier = true;
        locker = "i3lock";
        killer = "systemctl suspend";
        notifier = "notify-send 'abc'";
      };
    };
  };
}

Mic92 · 2017-10-07T11:58:45Z

nixos/modules/services/x11/xautolock.nix

-          default = "notify-send 'Locking in 10 seconds'";
-          type = types.string;
+          default = null;
+          example = "notify-send 'Locking in 10 seconds'";


Would this example actually work if libnotify is not installed? Should this not be "${pkgs.libnotify}/bin/notify-send" 'Locking in 10 seconds'.

true, but the example is just for documentation purposes and nothing that actually influences the system, right?

@Mic92 No it actually won't, but I'm not sure what example should be used there, so I kept the previous one. I'd prefix it as you did if this example with notify-send is acceptable, even though it does not notify me on my own system like that. I'm using i3wm and here it probably would be more fitting to use something like i3-nagbar -m 'locking soon'.
I'm open for suggestions :)

I just got it working. I need to have notify-osd running and then notify-send works, which means anyone using notify-send has to enable/run the daemon manually. I think this is not for the xautolock module to handle, right?
If so, I'd just prefix the command in the example :)

Using the following value should do the trick:

example = literalExample ''"${pkgs.libnotify}/bin/notify-send \"Locking in 10 seconds\"''';

Ma27 · 2017-10-08T12:12:33Z

@WilliButz done :-)

grahamc · 2017-10-10T15:20:14Z

@Ma27 can you formalize that in to a proper nixos test that can be run by hydra?

Ma27 · 2018-03-09T19:46:12Z

@grahamc sorry for the delay, I've just seen your comment right now as I talked with @WilliButz about some user services and things like this.
Back in October that wasn't possible since the testing framework didn't support user services properly (see #32845), however I hope that I manage to write a test tonight %)

WilliButz force-pushed the xautolock branch from 6d89314 to 6f99265 Compare October 5, 2017 14:12

rycee reviewed Oct 5, 2017

View reviewed changes

WilliButz force-pushed the xautolock branch from 6f99265 to 15ebb23 Compare October 5, 2017 21:08

Ma27 reviewed Oct 7, 2017

View reviewed changes

WilliButz force-pushed the xautolock branch 2 times, most recently from 760aa18 to 5ed21ee Compare October 7, 2017 11:10

Mic92 reviewed Oct 7, 2017

View reviewed changes

nixos/xautolock: rewrite and add some options

45d9f9c

WilliButz force-pushed the xautolock branch from 5ed21ee to 45d9f9c Compare October 7, 2017 16:11

Ma27 approved these changes Oct 8, 2017

View reviewed changes

globin merged commit 5e8d175 into NixOS:master Oct 10, 2017

WilliButz deleted the xautolock branch March 10, 2018 21:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/xautolock: rewrite and add some options #30128

nixos/xautolock: rewrite and add some options #30128

WilliButz commented Oct 5, 2017

Ma27 commented Oct 5, 2017 via email

rycee Oct 5, 2017

Ma27 Oct 7, 2017

Ma27 Oct 7, 2017

WilliButz Oct 7, 2017

Ma27 commented Oct 7, 2017

Mic92 Oct 7, 2017 •

edited

Ma27 Oct 7, 2017

WilliButz Oct 7, 2017 •

edited

WilliButz Oct 7, 2017

Mic92 Oct 7, 2017

Ma27 commented Oct 8, 2017

grahamc commented Oct 10, 2017

Ma27 commented Mar 9, 2018

nixos/xautolock: rewrite and add some options #30128

nixos/xautolock: rewrite and add some options #30128

Conversation

WilliButz commented Oct 5, 2017

Things done

Ma27 commented Oct 5, 2017 via email

rycee Oct 5, 2017

Choose a reason for hiding this comment

Ma27 Oct 7, 2017

Choose a reason for hiding this comment

Ma27 Oct 7, 2017

Choose a reason for hiding this comment

WilliButz Oct 7, 2017

Choose a reason for hiding this comment

Ma27 commented Oct 7, 2017

Mic92 Oct 7, 2017 • edited

Choose a reason for hiding this comment

Ma27 Oct 7, 2017

Choose a reason for hiding this comment

WilliButz Oct 7, 2017 • edited

Choose a reason for hiding this comment

WilliButz Oct 7, 2017

Choose a reason for hiding this comment

Mic92 Oct 7, 2017

Choose a reason for hiding this comment

Ma27 commented Oct 8, 2017

grahamc commented Oct 10, 2017

Ma27 commented Mar 9, 2018

Mic92 Oct 7, 2017 •

edited

WilliButz Oct 7, 2017 •

edited