the target looks defined in several modules like modules/services/window-managers/wayfire.nix. / river /hyperland/sway. shouldn't the tray.target be removed from there as well ?

I'd recommend defining tray.target independently from either one of
those modules, since virtually every graphical session that requires a
system tray would need such target.

fine by me. We could also merge the current approach as a first step.

@teto, you're right. I've added some commits removing them to be squashed if this is to be merged.

Regarding setting tray.target independently from either the xsession or a window manager, systemd.user.targets.tray could be a read-only/internal option from either xsession or wayland so that it would not, as my current implementation, allow the tray targets to get out of sync, since there'd be only one.

Created a trayTarget option (internal/read-only) under xsession, which is not only used by it, but also by wayland's config when the former is not enabled.

Maintainer CC for xsession and wayland:

@thiagokokada @rycee

Just added the removal of the systemd.user.targets.tray definitions from other modules to the git commit message.

• Unit = { + Description = "Home Manager System Tray"; + Requires = [ "graphical-session-pre.target" ]; + }; This is the implementation that is fine, but the option shouldn't implement a whole Unit unless there is a good reason to do so.
  I've set it just like the implementation because, as of now, it's a readOnly/internal option, so the user would not be allowed to change it. Do you see a use case in which the user would have to modify systemd.user.target.tray?

Oh ok, true, didn't realise that this was set to internal/readOnly.

I can find a few cases where this would be a good idea to change it, for example I know that grapgical-session-pre.target is a bad choice for Hyprland in a few other cases at least (not sure about the tray though).

I would recommend at least making the actual target an option just to make sure that we can cover other use cases, but the current implementation looks fine and we can add an option later too if you want.

I can find a few cases where this would be a good idea to change it, for example I know that grapgical-session-pre.target is a bad choice for Hyprland in a few other cases at least (not sure about the tray though).

I would recommend at least making the actual target an option just to make sure that we can cover other use cases, but the current implementation looks fine and we can add an option later too if you want.

I think we could create an option under the wayland module for enabling the systemd tray target, while leaving as is for the xsession one, since, on the latter, the tray target is always created.

I saw that the wayland module has a systemd.target option already. For a more smooth transition, I'd suggest something like the following:

# wayland module

options = {
  wayland.systemd.enableTrayTarget = mkEnableOption "tray.target" // { default = true; };
};

config = mkIf config.wayland.systemd.enableTrayTarget {
  systemd.user.targets.tray = config.xsession.trayTarget;
};

That would allow modules, for instance, hyprland's, to not have a tray.target:

# hyprland module

config = mkIf cfg.enable {
  assertions = [
    {
      assertion = !config.wayland.systemd.enableTrayTarget;
      message = ''
        hyprland cannot work correctly when `wayland.systemd.enableTrayTarget` is enabled.
      '';
    }
  ];

  wayland.systemd.enableTrayTarget = false;
};

What do you think?

What do you think?

I think the main issue is that the tray target is hardcoded to graphical-session-pre.target that is known to cause some issues in Hyprland and probably other Wayland DMs too, not that we need to enable/disable this target. graphical-session.target is more reliable in general since it ensures that everything in the graphical session is initialised (e.g.: WAYLAND_DISPLAY).

In my opinion we should:

• Either change the target to graphical-session.target (this would change the behavior but should work fine)
• Or allow the target to be configurable

It can be made configurable by removing the readOnly/internal attributes; however, as of now, wayland DM modules would have to access an xsession attribute to change their tray.target.

This goes back to what I've mention on the description of this PR:

I'd recommend defining tray.target independently from either one of
those modules, since virtually every graphical session that requires a
system tray would need such target.

We could either create a graphical-session module, defining the trayTarget on it, or move it to another module like systemd's.

It can be made configurable by removing the readOnly/internal attributes; however, as of now, wayland DM modules would have to access an xsession attribute to change their tray.target.

This goes back to what I've mention on the description of this PR:

I'd recommend defining tray.target independently from either one of
those modules, since virtually every graphical session that requires a
system tray would need such target.

We could either create a graphical-session module, defining the trayTarget on it, or move it to another module like systemd's.

@thiagokokada, are you approving the current state of the PR or the proposed changes mentioned above?

@thiagokokada, are you approving the current state of the PR or the proposed changes mentioned above?

The current state, it doesn't make sense to approve for something that doesn't exist.

I think we can merge it in the current state and change it to accommodate customization in the future.

Got it.