nixos/programs/gnome-session: refactor and update comments

modules/nixos/programs/gnome-session/default.nix

@@ -120,20 +120,19 @@ let
       let
         inherit (utils.systemdUtils.lib)
           pathToUnit serviceToUnit targetToUnit timerToUnit socketToUnit;
-        componentsUnits =
+
-          lib.concatMapAttrs
+        mkSystemdUnits = name: component: {
-            (name: component:
+          "${component.id}.service" = serviceToUnit component.id component.serviceUnit;
-              {
+          "${component.id}.target" = targetToUnit component.id component.targetUnit;
-                "${component.id}.service" = serviceToUnit component.id component.serviceUnit;
+        } // lib.optionalAttrs (component.socketUnit != null) {
-                "${component.id}.target" = targetToUnit component.id component.targetUnit;
+          "${component.id}.socket" = socketToUnit component.id component.socketUnit;
-              } // lib.optionalAttrs (component.socketUnit != null) {
+        } // lib.optionalAttrs (component.timerUnit != null) {
-                "${component.id}.socket" = socketToUnit component.id component.socketUnit;
+          "${component.id}.timer" = timerToUnit component.id component.timerUnit;
-              } // lib.optionalAttrs (component.timerUnit != null) {
+        } // lib.optionalAttrs (component.pathUnit != null) {
-                "${component.id}.timer" = timerToUnit component.id component.timerUnit;
+          "${component.id}.path" = pathToUnit component.id component.pathUnit;
-              } // lib.optionalAttrs (component.pathUnit != null) {
+        };
-                "${component.id}.path" = pathToUnit component.id component.pathUnit;
+
-              })
+        componentsUnits = lib.concatMapAttrs mkSystemdUnits session.components;
-            session.components;
       in
       componentsUnits // {
         "gnome-session@${name}.target" = targetToUnit "gnome-session@${name}" session.targetUnit;

modules/nixos/programs/gnome-session/submodules/component-type.nix

@@ -177,6 +177,10 @@ in
       desktopName = lib.mkDefault "${session.fullName} - ${config.description}";
       noDisplay = lib.mkForce true;
       onlyShowIn = session.desktopNames;
+
+      # For more information, you'll have to take a look into the
+      # gnome-session/README from its source code. Not even documented in the
+      # manual page but whatever. This is on the user to know.
       extraConfig = {
         X-GNOME-AutoRestart = lib.mkDefault "false";
         X-GNOME-Autostart-Notify = lib.mkDefault "true";
@@ -197,10 +201,10 @@ in
       especially if one of them failed.
 
       * Even if we have a way to limit starting desktop components with
-      `systemd-xdg-autostart-condition`, using `Service.ExecCondition=`
+      `systemd-xdg-autostart-condition`, using `Service.ExecCondition=` would
-      would severely limit possible reuse of desktop components with other
+      severely limit possible reuse of desktop components with other
-      NixOS-module-generated gnome-session sessions so we're not bothering
+      NixOS-module-generated sessiond sessions so we're not bothering with
-      with those.
+      those.
 
       TODO: Is `Type=notify` a good default?
       * `Service.Type=` is obviously not included since not all desktop
@@ -209,10 +213,6 @@ in
       as an explicit option set by the user instead of setting `Type=notify` as
       a default.
 
-      TODO: A good balance for this value, probably?
-      * `Service.OOMScoreAdjust=` have different values for different
-      components so it isn't included.
-
       * Most sandboxing options. Aside from the fact we're dealing with a
       systemd user unit, much of them are unnecessary and rarely needed (if
       ever like `Service.PrivateTmp=`?) so we didn't set such defaults here.
@@ -232,6 +232,12 @@ in
         Slice = lib.mkDefault "session.slice";
         Restart = lib.mkDefault "on-failure";
         TimeoutStopSec = lib.mkDefault 5;
+
+        # We'll assume most of the components are reasonably required so we'll
+        # set a reasonable middle-in-the-ground value for this one. The user
+        # should have the responsibility passing judgement for what is best for
+        # this.
+        OOMScoreAdjust = lib.mkDefault -500;
       };
 
       startLimitBurst = lib.mkDefault 3;
@@ -249,16 +255,28 @@ in
     };
 
     /*
-      Take note the session target unit already has `Wants=$COMPONENT.target`
+      Take note, we'll assume the session target unit will be the one to set
-      so no need to set dependency ordering directives here.
+      the dependency-related directives (i.e., `After=`, `Before=`, `Requires=`)
+      so no need to set any in here.
+
+      And another thing, we didn't set a default value for dependency-related
+      directives to one of the gnome-session-specific target unit. It is more
+      likely for a user to design their own desktop session with full control
+      so it would be better for these options to be empty for less confusion.
     */
     targetUnit = {
+      # This should be the dependency-related directive to be configured. The
+      # rest is for the user to judge.
       wants = [ "${config.id}.service" ];
+
       description = lib.mkDefault config.description;
       documentation = [
         "man:gnome-session(1)"
         "man:systemd.special(7)"
       ];
+
+      # Similar to the service unit, this is very much required as noted from
+      # the `gnome-session(1)` manual page.
       unitConfig.CollectMode = lib.mkForce "inactive-or-failed";
     };
   };