Import environment variables into systemd in NixOS modules

Mikilio commented 1 year ago

Description

Issue

Frankly speaking, using the nixos modules without modification is a XDG-"nightmare".

This is because almost all XDG/GIO calls happen through portals. The problem being that the portals are being started as a user session service by systemd. This means that important environment variables like PATH and XDG_DATA_DIRS are not accessible. Then again there are many applications that ignore default mime applications completely and look at Well-known Message Bus Instances instead.

To bring order into this situation I personally did 2 things:

Some packages install well-known bus type dbus services (usually located in .../share/dbus-1/) . To select a default I will symlink the corrects one the most dominant name (usually something like org.freedesktop.BasicFoo1).
I add the following line in my config after declaring variables and before starting any services:
```
exec-once = ${pkgs.dbus}/bin/dbus-update-activation-environment --systemd --all
```

Proposal

I would like to:

Add this information to documentation. Even the first point is necessary because Hyperland does not ship with any default tools, so every n00b will throw together their own distro and cry about XDG issues.
Add the line in the config by default (at least when using nix module). Home-manager already provides a way to declare environment variables for a session, so it should be fine to add the line at the top of the config.

Related Issues

NixOS/nixpkgs

PatrickShaw commented 1 year ago

Are you running home-manager?

To my knowledge, something similar is already handled for those running HM: https://github.com/hyprwm/Hyprland/blob/main/nix/hm-module.nix#L138

Mikilio commented 1 year ago

I think I remember seeing that. However it still still is not enough. Important variables for xdg are missing like XDG_DATA_DIRS and PATH. If there isn't any security issue I would just shorten the line by using the flag --all.

On Tue, Jul 25, 2023, 08:22 Patrick Shaw @.***> wrote:

Are you running home-manager?

To my knowledge, something similar is already handled for those running HM: https://github.com/hyprwm/Hyprland/blob/main/nix/hm-module.nix#L138

— Reply to this email directly, view it on GitHub https://github.com/hyprwm/Hyprland/issues/2800#issuecomment-1649195432, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUQFFF4Y53X72GHIW3B3DFDXR5Q4DANCNFSM6AAAAAA2V5XKLI . You are receiving this because you authored the thread.Message ID: @.***>

fufexan commented 1 year ago

Are you sure we want to import all environment variables when starting the session? I don't have info on this so I just kept the var list small so as not to potentially conflict with other things.

Mikilio commented 1 year ago

I wasn't too concerned about any side effects because greetd is started by a greetd user anyways. I agree, in general it might be better to import only the ones you need. However right now some seems to be missing. I actually based my config off of yours (@fufexan) and I still encountered the 25sec issue when regreet starts.

spikespaz commented 1 year ago

Are you sure we want to import all environment variables when starting the session? I don't have info on this so I just kept the var list small so as not to potentially conflict with other things.

I have been deliberating this with myself as well. I think maybe it would be the most painless way to go. Many programs started from systemd expect these variables to be present.

If we do import all environment variables, programs started from waybar click handlers would actually use my QT_QPA_PLATFORMTHEME. Right now they don't, and I haven't found an easy way to handle this without carefully wrapping each individual program (or having waybar be wrapped and pass along the environment).

Considering that most distros would have every variable imported, it wouldn't be unreasonable to do. However, I know that this isn't "the NixOS way".

I really don't know of reasons to not have every environment variable imported. It would certainly increase compatibility, in ways that most programs expect. Let me check what Sway does.

spikespaz commented 1 year ago

This seems to be it. I don't use Sway, so I don't know if anything else works as I expect or if it has the same problems I do on HL. https://github.com/nix-community/home-manager/blob/master/modules/services/window-managers/i3-sway/sway.nix#L311

DISPLAY WAYLAND_DISPLAY SWAYSOCK XDG_CURRENT_DESKTOP XDG_SESSION_TYPE NIXOS_OZONE_WL

Worth looking at KDE also.

Mikilio commented 1 year ago

This seems to be it. I don't use Sway, so I don't know if anything else works as I expect or if it has the same problems I do on HL. https://github.com/nix-community/home-manager/blob/master/modules/services/window-managers/i3-sway/sway.nix#L311

DISPLAY WAYLAND_DISPLAY SWAYSOCK XDG_CURRENT_DESKTOP XDG_SESSION_TYPE NIXOS_OZONE_WL Unfortunately the above link again is intended for a sway session started by systemd with a proper user. The sway session I launch is before any login and just to run regreet for greetd. While I don't know why, in this particular use-case those variables are not enough (it's actually one of the first things I tried).

Mikilio commented 1 year ago

I am sorry for the confusion in my last two comments for some reason I was thinking about this issue rharish101/ReGreet#34 which had a very similar workaround, as again environment variables where missing.

Mange commented 11 months ago

Another option is to allow us to specify a list of variables to import.

(I'm a total newbie at Nix so bear with me here.)

systemdIntegration.enable = lib.mkEnableOption "Hyprland wayland compositor";

systemdIntegration.variables = lib.mkOption {
  type = with lib.types; listOf (string);
  default = [ "DISPLAY" "HYPRLAND_INSTANCE_SIGNATURE" "WAYLAND_DISPLAY" "XDG_CURRENT_DESKTOP" ];
};

systemdIntegration.extraVariables = lib.mkOption {
  type = with lib.types; listOf (string);
  default = [ ];
};

xdg.configFile."hypr/hyprland.conf" = let
      # ...
    systemdVars = cfg.systemdIntegration.variables ++ cfg.systemdIntegration.extraVariables;
    updateEnvironment = "${pkgs.dbus}/bin/dbus-update-activation-environment --systemd ${lib.concatStrings systemdVars " "}";

    in lib.mkIf shouldGenerate {
      text = lib.optionalString cfg.systemdIntegration.enable ''
        exec-once = ${updateEnvironment} && systemctl --user start hyprland-session.target
      '' + lib.optionalString (combinedSettings != { })
        (toHyprconf combinedSettings 0)
        + lib.optionalString (cfg.extraConfig != "") cfg.extraConfig;
      # ...
    };

Then users can configure it like this:

wayland.windowManager.hyprland = {
  enable = true;
  systemdIntegration = {
    enable = true;
    extraVars = [ "PATH" "QT_QPA_PLATFORMTHEME" ];
  };
};

spikespaz commented 11 months ago

Another option is to allow us to specify a list of variables to import. ...

This is totally viable and definitely something that we should do. However, this can already be accomplished by another config.exec_start, if you need it now.

I think the goal of this issue is to get the defaults correct, not to offload that responsibility to the user (which is what we would effectively do if we allowed extraVars without getting the defaults correct).

Mikilio commented 1 month ago

Since then options seem to have been created and I can't reproduce the issues anymore, so closing for now.

hyprwm / Hyprland