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.

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

sway: improve systemdIntegration with dbus #2385

Closed
wants to merge 1 commit into from

Conversation

genofire
Copy link

@genofire genofire commented Oct 8, 2021

used for pinentry-gnome3 e.g.

Description

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.


adopted from archlinux:
https://github.com/archlinux/svntogit-community/blob/packages/sway/trunk/50-systemd-user.conf

used for pinentry-gnome3 e.g.
Copy link
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

Sorry for missing this PR and thanks for the contribution!

@berbiche
Copy link
Member

berbiche commented Feb 1, 2022

The logs have expired and I cannot re-run the workflow.

All the tests have to be updated since the generated configuration has changed.

$ nix eval --raw --impure --expr 'builtins.concatStringsSep "\n" (builtins.attrNames (import ./tests {}).run)' | grep sway | xargs -I{} nix-shell -I nixpkgs=channel:nixos-21.11 --pure tests -A run.{}

I fixed the tests and took the liberty to add XDG_CURRENT_DESKTOP to the imported variables.
This is useful with xdg-desktop-portal and xdg-desktop-portal-{gtk,wlr}.

@berbiche
Copy link
Member

berbiche commented Feb 1, 2022

Well, I cannot push (or force-push) the fix to your branch for whatever reason.

Here's the patch (save it and apply it with git apply):

commit 5eb23e1dcefbbf1b292ce3a1049205d0ddb2f178
Author: Nicolas Berbiche <nicolas@normie.dev>
Date:   Mon Jan 31 23:06:22 2022 -0500

    sway: fix tests

diff --git a/modules/services/window-managers/i3-sway/sway.nix b/modules/services/window-managers/i3-sway/sway.nix
index 58e7b427..d8a7195a 100644
--- a/modules/services/window-managers/i3-sway/sway.nix
+++ b/modules/services/window-managers/i3-sway/sway.nix
@@ -312,10 +312,10 @@ let
     ''
   else
     "") + (concatStringsSep "\n" ((optional cfg.systemdIntegration ''
-      exec "systemctl --user import-environment DISPLAY WAYLAND_DISPLAY SWAYSOCK"
-      exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment --systemd DISPLAY WAYLAND_DISPLAY SWAYSOCK"
-      exec "systemctl --user start sway-session.target"
-    '') ++ (optional (!cfg.xwayland) "xwayland disable")
+      exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+      exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+      exec "systemctl --user start sway-session.target"'')
+      ++ (optional (!cfg.xwayland) "xwayland disable")
       ++ [ cfg.extraConfig ])));
 
   defaultSwayPackage = pkgs.sway.override {
diff --git a/tests/modules/services/window-managers/sway/sway-bar-focused-colors.conf b/tests/modules/services/window-managers/sway/sway-bar-focused-colors.conf
index d55c9200..d8d92d97 100644
--- a/tests/modules/services/window-managers/sway/sway-bar-focused-colors.conf
+++ b/tests/modules/services/window-managers/sway/sway-bar-focused-colors.conf
@@ -110,4 +110,6 @@ bar {
   
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-bindkeys-to-code.conf b/tests/modules/services/window-managers/sway/sway-bindkeys-to-code.conf
index 97c72710..3fe780d5 100644
--- a/tests/modules/services/window-managers/sway/sway-bindkeys-to-code.conf
+++ b/tests/modules/services/window-managers/sway/sway-bindkeys-to-code.conf
@@ -110,4 +110,6 @@ bar {
   
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-default.conf b/tests/modules/services/window-managers/sway/sway-default.conf
index 6ef61a7a..e8fe38e8 100644
--- a/tests/modules/services/window-managers/sway/sway-default.conf
+++ b/tests/modules/services/window-managers/sway/sway-default.conf
@@ -110,4 +110,6 @@ bar {
   
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-followmouse-expected.conf b/tests/modules/services/window-managers/sway/sway-followmouse-expected.conf
index 7fa4da07..a8b5e6c7 100644
--- a/tests/modules/services/window-managers/sway/sway-followmouse-expected.conf
+++ b/tests/modules/services/window-managers/sway/sway-followmouse-expected.conf
@@ -83,4 +83,6 @@ bindsym k resize shrink height 10 px
 bindsym l resize grow width 10 px
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-followmouse-legacy-expected.conf b/tests/modules/services/window-managers/sway/sway-followmouse-legacy-expected.conf
index c470a7c4..8996ae2e 100644
--- a/tests/modules/services/window-managers/sway/sway-followmouse-legacy-expected.conf
+++ b/tests/modules/services/window-managers/sway/sway-followmouse-legacy-expected.conf
@@ -83,4 +83,6 @@ bindsym k resize shrink height 10 px
 bindsym l resize grow width 10 px
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-modules.conf b/tests/modules/services/window-managers/sway/sway-modules.conf
index c0339d71..b029f2f7 100644
--- a/tests/modules/services/window-managers/sway/sway-modules.conf
+++ b/tests/modules/services/window-managers/sway/sway-modules.conf
@@ -122,4 +122,6 @@ bar {
   
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-null-package.conf b/tests/modules/services/window-managers/sway/sway-null-package.conf
index 6ef61a7a..e8fe38e8 100644
--- a/tests/modules/services/window-managers/sway/sway-null-package.conf
+++ b/tests/modules/services/window-managers/sway/sway-null-package.conf
@@ -110,4 +110,6 @@ bar {
   
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-workspace-default-expected.conf b/tests/modules/services/window-managers/sway/sway-workspace-default-expected.conf
index b5c92fa6..3b740479 100644
--- a/tests/modules/services/window-managers/sway/sway-workspace-default-expected.conf
+++ b/tests/modules/services/window-managers/sway/sway-workspace-default-expected.conf
@@ -109,4 +109,6 @@ bar {
   
 }
 
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"
diff --git a/tests/modules/services/window-managers/sway/sway-workspace-output-expected.conf b/tests/modules/services/window-managers/sway/sway-workspace-output-expected.conf
index bacb27f8..a209e8ce 100644
--- a/tests/modules/services/window-managers/sway/sway-workspace-output-expected.conf
+++ b/tests/modules/services/window-managers/sway/sway-workspace-output-expected.conf
@@ -114,4 +114,6 @@ workspace "1" output eDP
 workspace "ABC" output DP
 workspace "3: Test" output HDMI
 workspace "!"§$%&/(){}[]=?\*#<>-_.:,;²³" output DVI
-exec "systemctl --user import-environment; systemctl --user start sway-session.target"
+exec "systemctl --user import-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "hash dbus-update-activation-environment 2>/dev/null && dbus-update-activation-environment XDG_CURRENT_DESKTOP DISPLAY WAYLAND_DISPLAY SWAYSOCK"
+exec "systemctl --user start sway-session.target"

@stale
Copy link

stale bot commented May 2, 2022

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label May 2, 2022
oxalica added a commit to oxalica/home-manager that referenced this pull request Jun 17, 2022
- Importing all environment variables is considered deprecated for
  `systemdctl import-environment`. The list of variables are picked
  from:
  https://github.com/swaywm/sway/wiki/Systemd-integration#managing-user-applications-with-systemd

  The `XDG_CURRENT_DESKTOP` is said to be required for portals, see:
  nix-community#2385 (comment)

- DBus activation environment should also be updated. Otherwise, DBus
  activated programs, without a coresponding systemd service, cannot get a
  correct environment and would fail, eg. `mako`.
oxalica added a commit to oxalica/home-manager that referenced this pull request Jun 29, 2022
- Importing all environment variables is considered deprecated for
  `systemdctl import-environment`. The list of variables are picked
  from:
  https://github.com/swaywm/sway/wiki/Systemd-integration#managing-user-applications-with-systemd

  The `XDG_CURRENT_DESKTOP` is said to be required for portals, see:
  nix-community#2385 (comment)

- DBus activation environment should also be updated. Otherwise, DBus
  activated programs, without a coresponding systemd service, cannot get a
  correct environment and would fail, eg. `mako`.
sumnerevans pushed a commit that referenced this pull request Jul 7, 2022
- Importing all environment variables is considered deprecated for
  `systemdctl import-environment`. The list of variables are picked
  from:
  https://github.com/swaywm/sway/wiki/Systemd-integration#managing-user-applications-with-systemd

  The `XDG_CURRENT_DESKTOP` is said to be required for portals, see:
  #2385 (comment)

- DBus activation environment should also be updated. Otherwise, DBus
  activated programs, without a coresponding systemd service, cannot get a
  correct environment and would fail, eg. `mako`.
@genofire genofire deleted the patch-1 branch July 9, 2022 02:08
jevy pushed a commit to jevy/home-manager that referenced this pull request Jul 23, 2022
…#3031)

- Importing all environment variables is considered deprecated for
  `systemdctl import-environment`. The list of variables are picked
  from:
  https://github.com/swaywm/sway/wiki/Systemd-integration#managing-user-applications-with-systemd

  The `XDG_CURRENT_DESKTOP` is said to be required for portals, see:
  nix-community#2385 (comment)

- DBus activation environment should also be updated. Otherwise, DBus
  activated programs, without a coresponding systemd service, cannot get a
  correct environment and would fail, eg. `mako`.
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
…#3031)

- Importing all environment variables is considered deprecated for
  `systemdctl import-environment`. The list of variables are picked
  from:
  https://github.com/swaywm/sway/wiki/Systemd-integration#managing-user-applications-with-systemd

  The `XDG_CURRENT_DESKTOP` is said to be required for portals, see:
  nix-community#2385 (comment)

- DBus activation environment should also be updated. Otherwise, DBus
  activated programs, without a coresponding systemd service, cannot get a
  correct environment and would fail, eg. `mako`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants