Add dunst service #58209

xaverdh · 2019-03-24T15:11:33Z

Motivation for this change

This is a fresh attempt at implementing #19183

Things done

infinisil · 2019-03-25T17:04:38Z

xaverdh · 2019-03-25T17:55:45Z

Good candidate for NixOS/rfcs#42

Yes, the rules part is probably too verbose. I wanted to internalize it more, in order to check for naming conflicts with official section names. But I agree, there is no need to go that deep into individual options for that.
Ok, I simplified this somewhat. It no longer references individual options.

xaverdh · 2019-03-25T20:45:57Z

Other changes:

Use writeText instead of toFile to allow e.g. rules to reference storepaths properly.
Don't add the wrapper to systemPackages by default

~~I still have some problem with running scripts from the systemd services context though.~~ that was just me being stupid..

dotlambda · 2019-03-26T06:45:00Z

nixos/modules/services/x11/dunst.nix

+
+  globalConfig = mkOption {
+    type = types.nullOr (types.attrsOf types.string);
+    default = null;


Why not

type = with types; attrsOf str; default = { };

Anyway, use types.str because

# Deprecated; should not be used because it quietly concatenates # strings, which is usually not what you want. string = separatedString "";

dotlambda · 2019-03-26T06:45:40Z

nixos/modules/services/x11/dunst.nix

+
+options.services.dunst = {
+
+  enable = mkOption {


Suggested change

enable = mkOption {

enable = mkEnableOption {

xaverdh · 2019-06-15T16:20:18Z

I pushed some changes to use the upstream systemd unit file.

aanderse · 2019-07-16T02:26:44Z

ping (triage)

xaverdh · 2019-07-16T11:50:59Z

Reading this code again after some time, I wonder if that is the right way to create the wrapper.
The problem was/is I think, that there is no sane way to reuse the upstream service file without overriding the original dunst derivation. If someone has a suggestion how to do this properly, I would very much like to hear it.

xaverdh · 2019-07-16T12:40:35Z

Ok, I have an alternative implementation here using symlinkJoin (similar to how the sway and way-cooler modules do things). But at the cost of not being able to reuse the upstream systemd service file.

aanderse · 2019-07-17T01:56:55Z

@xaverdh Well it looks like you have satisfied every request in the review of this PR so far... so let us know when you make up your mind because from skimming the comments in this PR I think a merge is in order 😄

xaverdh · 2019-07-17T09:53:00Z

Ok, lets merge this as is then. The implementation can still be switched later on.

aanderse · 2019-07-21T20:48:52Z

(triage) @dotlambda From the comments it looks like final word is yours. Please merge if you are satisfied.

infinisil · 2019-08-09T14:03:25Z

nixos/modules/services/x11/dunst.nix

+    dunst-wrapper = pkgs.dunst.overrideAttrs (oldAttrs: {
+      postInstall = oldAttrs.postInstall + ''
+          wrapProgram $out/bin/dunst \
+            --add-flags ${escapeShellArg wrapper-args}


All this trouble can be avoided by changing the services ExecStart somehow like this: systemd.user.services.dunst.serviceConfig.ExecStart = "${pkgs.dunst}/bin/dunst -config ..."

That actually leads to two ExecStart directives in the service file.

Then you can use e.g. mkOverride 900 on this entry to override the other

That doesn't work either. Maybe because I am using the upstream service file with systemd.packages ?

Ahh right. it's a bit ugly but give ExecStart = [ "" "${pkgs.dunst... a try

That worked, thanks!

But the socket activation seems broken with this approach somehow.

nixos/modules/services/x11/dunst.nix

infinisil · 2019-08-09T14:04:09Z

nixos/modules/services/x11/dunst.nix

+      '';
+    });
+
+    environment.systemPackages = [ dunst-wrapper ];


Is this needed?

infinisil · 2019-08-09T14:08:01Z

nixos/modules/services/x11/dunst.nix

+  in mkIf cfg.enable {
+
+    assertions = flip mapAttrsToList cfg.rules (name: conf: {
+      assertion = ! ( any (ruleName: ruleName == name) reservedSections );


Suggested change

assertion = ! ( any (ruleName: ruleName == name) reservedSections );

assertion = ! elem name reservedSections;

infinisil · 2019-08-09T14:09:17Z

nixos/modules/services/x11/dunst.nix

+  };
+
+  extraCliOptions = mkOption {
+    type = types.str;


I recommend types.listOf types.str, which is better for escaping

infinisil · 2019-08-09T14:10:07Z

nixos/modules/services/x11/dunst.nix

+      urgency_critical = cfg.urgencyConfig.critical;
+    } // cfg.rules;
+
+    iconPath = builtins.concatStringsSep ":" cfg.iconDirs;


Suggested change

iconPath = builtins.concatStringsSep ":" cfg.iconDirs;

iconPath = concatStringsSep ":" cfg.iconDirs;

nixos/modules/services/x11/dunst.nix

infinisil · 2019-08-09T14:13:17Z

nixos/modules/services/x11/dunst.nix

+    reservedSections = [
+      "global" "experimental" "frame" "shortcuts"
+      "urgency_low" "urgency_normal" "urgency_critical"
+    ];


If you put this at the top of the module, you can reuse this list in the description for rules

xaverdh · 2019-08-09T15:39:03Z

Thanks to Infinisil for the extensive review!
I pushed some changes to apply most of your suggestions.

infinisil · 2019-08-09T15:47:51Z

nixos/modules/services/x11/dunst.nix

+
+    iconPath = concatStringsSep ":" cfg.iconDirs;
+
+    dunst-args = "-config ${pkgs.writeText "dunstrc" dunstConfig} -icon_path ${iconPath} ${concatStringsSep " " cfg.extraCliOptions}";


This should then be

{ dunstArgs = [ "-config" (pkgs.writeText "dunstrc" dunstConfig) "-icon_path" iconPath ] ++ extraCliOptions; }

And you can use escapeShellArgs dunstArgs in the ExecStart

infinisil · 2019-08-09T15:49:19Z

nixos/modules/services/x11/dunst.nix

+    '';
+  };
+
+  extraCliOptions = mkOption {


I suggest renaming this to either extraArgs or extraOptions since this is a more common name for this

infinisil · 2019-08-09T15:50:29Z

nixos/modules/services/x11/dunst.nix

+    type = with types; listOf str;
+    default = [];
+    description = ''
+      This gets appended verbatim to the command line of dunst.


I suggest something like "Extra command line options for dunst" (describing what it is, not what it gets used for/does). Same for the other options

rnhmjoj · 2019-09-10T00:03:16Z

Status? Can we get this ready in time for 19.09?

Lassulus · 2019-10-02T22:38:15Z

nixos/modules/services/x11/dunst.nix

+    '';
+  };
+
+  urgencyConfig = {


I would really like examples for this config

dmrauh · 2020-06-18T15:48:21Z

Are there any news regarding this PR's status? I'm really interested in this service 😀

JohnAZoidberg · 2020-06-19T16:32:51Z

nixos/modules/services/x11/dunst.nix

+    systemd.user.services.dunst.serviceConfig.ExecStart = [ "" "${pkgs.dunst}/bin/dunst ${escapeShellArgs dunst-args}" ];
+    # [ "" ... ] is needed to overwrite the ExecStart directive from the upstream service file
+
+    systemd.packages = [ pkgs.dunst ];


Does it need to be installed for the user?

JohnAZoidberg · 2020-06-19T16:40:29Z

It would be good if somebody could try to see if it works how they expect.

And check whether we should add some default config or at least more examples.
Upstream has a big default config file with lots of explanations: https://github.com/dunst-project/dunst/blob/master/dunstrc

RocketPuppy · 2021-02-06T06:16:35Z

I'd really like to get this in, so I picked this up and added some defaults, examples, and fixed merge conflict here: #112100

xaverdh · 2021-02-06T09:10:17Z

closed in favor of #112100

xaverdh requested a review from infinisil as a code owner March 24, 2019 15:11

xaverdh force-pushed the dunst-config branch from 27c360a to e9f91c0 Compare March 24, 2019 15:13

xaverdh force-pushed the dunst-config branch from 3c48d13 to bc960a2 Compare March 25, 2019 16:29

xaverdh force-pushed the dunst-config branch 3 times, most recently from aed0ede to 7a1a0b6 Compare March 25, 2019 17:47

xaverdh force-pushed the dunst-config branch from 7a1a0b6 to 260dc5f Compare March 25, 2019 20:27

xaverdh force-pushed the dunst-config branch from 260dc5f to 67fe0bf Compare March 25, 2019 20:46

dotlambda requested changes Mar 26, 2019

View reviewed changes

xaverdh force-pushed the dunst-config branch from 67fe0bf to 758464f Compare March 26, 2019 07:03

c0bw3b mentioned this pull request May 18, 2019

dunst service: init [wip, stalled] #19183

Closed

7 tasks

xaverdh force-pushed the dunst-config branch from 758464f to 75ebc17 Compare June 15, 2019 16:08

ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-linux: 1-10 labels Jun 15, 2019

xaverdh force-pushed the dunst-config branch from 75ebc17 to f9d1046 Compare June 15, 2019 17:19

infinisil requested changes Aug 9, 2019

View reviewed changes

xaverdh force-pushed the dunst-config branch from f9d1046 to 8d7afad Compare August 9, 2019 15:35

xaverdh force-pushed the dunst-config branch from 8d7afad to 7cb4d95 Compare August 9, 2019 15:44

infinisil requested changes Aug 9, 2019

View reviewed changes

xaverdh force-pushed the dunst-config branch 4 times, most recently from 46d0a22 to 62f76b6 Compare August 9, 2019 18:35

nixos/dunst: init

9644076

xaverdh force-pushed the dunst-config branch from e27132d to 9644076 Compare August 11, 2019 18:21

lheckemann added this to the 20.03 milestone Sep 10, 2019

rnhmjoj mentioned this pull request Sep 21, 2019

nixos module for dunst #47398

Open

Lassulus reviewed Oct 2, 2019

View reviewed changes

disassembler modified the milestones: 20.03, 20.09 Feb 5, 2020

JohnAZoidberg reviewed Jun 19, 2020

View reviewed changes

ryantm added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 3, 2020

FRidh modified the milestones: 20.09, 21.03 Dec 20, 2020

RocketPuppy mentioned this pull request Feb 6, 2021

Add dunst service #112100

Closed

10 tasks

xaverdh closed this Feb 6, 2021

	assertion = ! ( any (ruleName: ruleName == name) reservedSections );
	assertion = ! elem name reservedSections;

	iconPath = builtins.concatStringsSep ":" cfg.iconDirs;
	iconPath = concatStringsSep ":" cfg.iconDirs;


		iconPath = concatStringsSep ":" cfg.iconDirs;

		dunst-args = "-config ${pkgs.writeText "dunstrc" dunstConfig} -icon_path ${iconPath} ${concatStringsSep " " cfg.extraCliOptions}";

Add dunst service #58209

Add dunst service #58209

Conversation

xaverdh commented Mar 24, 2019

Motivation for this change

Things done

infinisil commented Mar 25, 2019

xaverdh commented Mar 25, 2019 • edited Loading

xaverdh commented Mar 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xaverdh commented Jun 15, 2019 • edited Loading

aanderse commented Jul 16, 2019

xaverdh commented Jul 16, 2019

xaverdh commented Jul 16, 2019

aanderse commented Jul 17, 2019

xaverdh commented Jul 17, 2019

aanderse commented Jul 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xaverdh commented Aug 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnhmjoj commented Sep 10, 2019

Choose a reason for hiding this comment

dmrauh commented Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

JohnAZoidberg commented Jun 19, 2020

RocketPuppy commented Feb 6, 2021 • edited Loading

xaverdh commented Feb 6, 2021

xaverdh commented Mar 25, 2019 •

edited

Loading

xaverdh commented Mar 25, 2019 •

edited

Loading

xaverdh commented Jun 15, 2019 •

edited

Loading

dmrauh commented Jun 18, 2020 •

edited

Loading

RocketPuppy commented Feb 6, 2021 •

edited

Loading