nixos/portals: point $XDG_DESKTOP_PORTAL_DIR at the current system

[Atemu]

Previously, it'd be pointed at a fixed Nix store location. Doing any changes to
the available portals would require logging out and in again.

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.11 Release Notes (or backporting 22.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[pasqui23]

LGTM

[kira-bruneau]

Thanks for the fix! 🙂 It looks like my PR to change this to use sessionVariables introduced this problem.

I wonder if maybe we should just set the variable directly on the systemd service as an override, rather than trying to set it globally:

diff --git a/nixos/modules/config/xdg/portal.nix b/nixos/modules/config/xdg/portal.nix
index e28ff74e5d80..c6ce43c03c3e 100644
--- a/nixos/modules/config/xdg/portal.nix
+++ b/nixos/modules/config/xdg/portal.nix
@@ -84,7 +84,16 @@ in
       ];
 
       services.dbus.packages = packages;
-      systemd.packages = packages;
+
+      systemd = {
+        inherit packages;
+        user.services.xdg-desktop-portal = {
+          restartIfChanged = true;
+          environment = {
+            XDG_DESKTOP_PORTAL_DIR = "${joinedPortals}/share/xdg-desktop-portal/portals";
+          };
+        };
+      };
 
       environment = {
         # fixes screen sharing on plasmawayland on non-chromium apps by linking
@@ -92,10 +101,8 @@ in
         # see https://github.com/NixOS/nixpkgs/issues/145174
         systemPackages = [ joinedPortals ];
         pathsToLink = [ "/share/applications" ];
-
-        sessionVariables = {
+        variables = {
           GTK_USE_PORTAL = mkIf cfg.gtkUsePortal "1";
-          XDG_DESKTOP_PORTAL_DIR = "${joinedPortals}/share/xdg-desktop-portal/portals";
         };
       };
     };

That way it's more self-contained, and still updates on rebuilds.

@Atemu With your fix, do the portal changes get applied without restarting the service? It looks like the portals are loaded in main through load_installed_portals and wouldn't be reloaded if the path changes while it's running.

@jtojnar I'm pretty sure XDG_DATA_PORTAL_DIRS is only used by xdg-desktop-portal.service, do you know if there would be any impact by not exporting this globally?

[Atemu]

Restarting the service is a good idea but I think we should still keep the environment variable around globally to account for people's custom setups.

[jtojnar]

I also think the environment variable is only used for dispatch by x-d-p.

I like the self-contained approach more. The possibility of breaking custom setups is unfortunate but reliance on a global variable is breaking custom configs as well, requiring hacks like this:

https://github.com/lilyinstarlight/foosteros/blob/78110a51c9066020cb21b662fbd48768ea5a7501/modules/nixos/programs/sway.nix#L13

I have seen several people on Matrix confused by x-d-p not receiving the variable.

[Atemu]

If users truly need the variable, they can actually set it themselves because xdp are now in the global share. I'll try to get it into the user service but I don't know how well that will turn out since we don't actually generate it ourselves.

[jtojnar]

For existing services, the NixOS module will just use systemd “drop-in” overrides, see https://www.freedesktop.org/software/systemd/man/systemd.unit.html.

[Atemu]

I integrated @kira-bruneau's feedback. However, it does not seem like the portal services are getting restarted, despite restartIfChanged. Is there something else you need to set for user services for the activation script to restart them?

[Atemu]

Ah, problem is not that the xdg-desktop-portal service isn't being restarted, it's that the individual portals' services aren't started or stopped.

[kira-bruneau]

I integrated @kira-bruneau's feedback. However, it does not seem like the portal services are getting restarted, despite restartIfChanged. Is there something else you need to set for user services for the activation script to restart them?

Oh weird, I would've thought restartIfChanged is all you'd need based on the docs (I haven't tried using it before). I'll take some time to try to debug this.

Ah, problem is not that the xdg-desktop-portal service isn't being restarted, it's that the individual portals' services aren't started or stopped.

Oh they're not? I would've assumed that adding/removing those packages would've started/stopped the services. I definitely don't know enough about how NixOS manages it's services though. We'd still want to restart xdg-desktop-portal anyway since it needs to register the portals by what's set in XDG_DESKTOP_PORTAL_DIR. Strange that it doesn't work 😕

[Atemu]

I think the big problem is that they're user services rather than system services.

They're defined via unit files by the packages themselves and NixOS only reloads them on activation.

[Atemu]

I think we should go with my initial plan for now.

[jtojnar]

They're defined via unit files by the packages themselves and NixOS only reloads them on activation.

When would you expect them to be reloaded other than on activation?

I think we should go with my initial plan for now.

Would not that also not reload the services, in addition to worsening the global contamination?

[Atemu]

When would you expect them to be reloaded other than on activation?

Perhaps I wasn't clear enough, what I meant is that they're only reloaded, not started or stopped. Stopped services stay stopped and running ones continue to run.

Would not that also not reload the services

Neither does the other solution.

in addition to worsening the global contamination?

There's no worsening happening here. The amount of global variables stays the same. What's changed is that one of them is slightly more flexible now.

[jtojnar]

I meant compared to 94f69f0