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.

Sign up for GitHub

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/hypridle: only run the service on Hyprland #355416

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nixos/hypridle: only run the service on Hyprland #355416

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions nixos/modules/services/wayland/hypridle.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,31 @@ let
in
{
options.services.hypridle = {
enable = lib.mkEnableOption "hypridle, Hyprland's idle daemon";
enable = lib.mkEnableOption null // {
description = ''
Whether to enable Hypridle, Hyprland's idle daemon.

::: {.note}
For this service to work properly on Hyprland, you
need to have `programs.hyprland.withUWSM` enabled.
Comment on lines +18 to +19
Copy link
Member

Choose a reason for hiding this comment

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

We should assert that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because @fufexan wants it to be compatible with other compositors.

Copy link
Member

Choose a reason for hiding this comment

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

As it stands right now, it cannot be compatible with other compositors because it depends on the hyprland-uwsm-specific units.

If you wanted to use it with another compositor, you should bring your own hypridle unit.

Copy link

Choose a reason for hiding this comment

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

It could be compatible if you just add more targets in WantedBy and After.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or simply add systemd.user.services.swayidle.wantedBy = ["your-compositors-session.target"];. This should be an admonition in the enable option's description.

Copy link
Member

Choose a reason for hiding this comment

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

We could turn the targets that hypridle is wantedBy and before into an option. withUWSM would set it to the UWSM-generated session unit. We could then simply assert that this option's value isn't an empty list.

:::
'';
};
package = lib.mkPackageOption pkgs "hypridle" { };
};

config = lib.mkIf cfg.enable {
environment.systemPackages = [ cfg.package ];

systemd = {
packages = [ cfg.package ];
user.services.hypridle.wantedBy = [ "graphical-session.target" ];
user.services.hypridle.path = [
systemd.packages = [ cfg.package ];

systemd.user.services.hypridle = {
# Service should be only be started on Hyprland
# this target is started by UWSM
wantedBy = [ "[email protected]" ];
Comment on lines +32 to +34
Copy link
Member

Choose a reason for hiding this comment

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

This will only start the service. It will not wait for to have started and never stop it either.

Copy link

@Vladimir-csp Vladimir-csp Nov 21, 2024 •
edited by Atemu
Loading

Choose a reason for hiding this comment

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

Suggested change
wantedBy = [ "[email protected]" ];
wantedBy = [ "[email protected]" ];
# order start after compositor (and WAYLAND_DISPLAY) is available
# order stop before compositor
after = [ "[email protected]" ];
# squeeze before graphical session.target
before = [ "graphical-session.target" ];
# stop with the session
partOf = [ "graphical-session.target" ];

Copy link
Member

@Atemu Atemu Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

These two targets do not exist on NixOS. We've explicitly limited this module to only work with the programs.hyprland.withUWSM option which only creates the [email protected]. (I'd prefer a stronger API than the Hyprland package's binary name here but that's something our uwsm module would need to offer.)

We also want to order hypridle before the wayland session so that it's only considered up when hypridle has actually started. (Wants= does not imply any order.)

Copy link

Choose a reason for hiding this comment

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

edited

Copy link
Member

Choose a reason for hiding this comment

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

In my config I made it before and partOf [email protected] because I think of hypridle as belonging to the "Hyprland session" rather than the graphical-session directly. This would also be okay IMV.

I do find it a bit weird to depend on the session target rather than the wm service though as that's our actual dependency.

Copy link

Choose a reason for hiding this comment

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

Units of service and target are bound, so there is no difference for dependencies. Targets are more frequently used as dependencies.

$ grep -r ^WantedBy= /lib/systemd/system | grep -cF .target
124
$ grep -r ^WantedBy= /lib/systemd/system | grep -cF .service
90
$ grep -r ^WantedBy= /lib/systemd/user | grep -cF .target
24
$ grep -r ^WantedBy= /lib/systemd/user | grep -cF .service
17

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I don't follow. Could you reword?

Copy link

Choose a reason for hiding this comment

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

[email protected] is bound to [email protected], so they are interchangeable as dependencies (but can differ when ordering).

Targets are more frequently used for dependencies than services.

Copy link
Contributor Author

@JohnRTitor JohnRTitor Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Depending on graphical-session.target, is fine in my opinion.

When a WM managed by UWSM is terminated, graphical-session also stops. So I understand the partOf bit.

What I don't understand is why are we supposed to start it "before" graphical-session? Starting it after isn't correct?

I'd prefer a stronger API than the Hyprland package's binary name here

Agreed.

EDIT: I did read the above discussion about before, but still confused

Copy link
Member

Choose a reason for hiding this comment

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

What I don't understand is why are we supposed to start it "before" graphical-session? Starting it after isn't correct?

Because we only want the session to be considered up and running once hypridle is up and running.


# Essential package needed for the service to run properly
path = [
config.programs.hyprland.package
config.programs.hyprlock.package
pkgs.procps
Expand Down