[PATCH] Fix XFCE shutdown/reboot via menu.

[PATCH] Fix XFCE shutdown/reboot via menu.

[Ricardo Wurmus]

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

Hi Guix,

in the XFCE panel there is an item called “Action Buttons” offering
screen locking, log out, switch user, shutdown and reboot.  Shutdown and
reboot do not work at the moment.

I investigated a little and came up with a configuration in which these
two features do work.

First I had to patch our xfce-session package.  Upower is checked for at
configure time and needed for shutdown and reboot.  Polkit’s “pkexec” is
required to run the shutdown helper.

The helper contains a bunch of hardcoded paths to /sbin/shutdown, which
are replaced in a build phase — not in a snippet because eventually we
should also fix the paths to pm-{suspend,hibernate}, which would be
provided by an input that is currently not present(?).

I also needed to change my system configuration to add “pkexec” to the
list of setuid programs:

  (setuid-programs (cons #~(string-append #$polkit "/bin/pkexec")
                           %setuid-programs))

Maybe this should just be added to %setuid-programs?

Is it okay to use /run/setuid-programs/pkexec in the shutdown helper or
should this rather be a reference to the polkit input?

Here’s the patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-xfce-session-Fix-shutdown-reboot-via-menu.patch --]
[-- Type: text/x-patch, Size: 2074 bytes --]

From 2d39ace79b79a1672488e09bdcf442ff893cbf98 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sat, 21 Nov 2015 17:47:00 +0100
Subject: [PATCH] WIP: xfce-session: Fix shutdown/reboot via menu.

---
 gnu/packages/xfce.scm | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/xfce.scm b/gnu/packages/xfce.scm
index a4987c1..4c223d7 100644
--- a/gnu/packages/xfce.scm
+++ b/gnu/packages/xfce.scm
@@ -35,6 +35,7 @@
   #:use-module (gnu packages image)
   #:use-module (gnu packages gnome)
   #:use-module (gnu packages pdf)
+  #:use-module (gnu packages polkit)
   #:use-module (gnu packages gstreamer)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages photo)
@@ -429,13 +430,29 @@ your system in categories, so you can quickly find and launch them.")
                (list (search-patch "xfce4-session-fix-xflock4.patch")))))
     (build-system gnu-build-system)
     (arguments
-     '(#:configure-flags
+     '(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'fix-paths-to-shutdown-tools
+           (lambda _
+             ;; FIXME: also replace paths to "pm-hibernate" and "pm-suspend"
+             (substitute* "xfsm-shutdown-helper/main.c"
+               (("/sbin/shutdown -h now")
+                "/run/current-system/profile/sbin/halt")
+               (("/sbin/shutdown -r now")
+                "/run/current-system/profile/sbin/restart"))
+             ;; TODO: should we use a store reference to polkit instead?
+             (substitute* "xfce4-session/xfsm-shutdown-fallback.c"
+               (("pkexec ") "/run/setuid-programs/pkexec "))
+             #t)))
+       #:configure-flags
        (list (string-append "--with-xsession-prefix=" %output))))
     (native-inputs
      `(("pkg-config" ,pkg-config)
        ("intltool" ,intltool)))
     (inputs
      `(("iceauth" ,iceauth)
+       ("upower" ,upower)
+       ("polkit" ,polkit)
        ("libsm" ,libsm)
        ("libwnck" ,libwnck-1)
        ("libxfce4ui" ,libxfce4ui)))
-- 
2.5.0


[-- Attachment #3: Type: text/plain, Size: 12 bytes --]


~~ Ricardo

Re: [PATCH] Fix XFCE shutdown/reboot via menu.

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> in the XFCE panel there is an item called “Action Buttons” offering
> screen locking, log out, switch user, shutdown and reboot.  Shutdown and
> reboot do not work at the moment.
>
> I investigated a little and came up with a configuration in which these
> two features do work.

\o/

> First I had to patch our xfce-session package.  Upower is checked for at
> configure time and needed for shutdown and reboot.  Polkit’s “pkexec” is
> required to run the shutdown helper.
>
> The helper contains a bunch of hardcoded paths to /sbin/shutdown, which
> are replaced in a build phase — not in a snippet because eventually we
> should also fix the paths to pm-{suspend,hibernate}, which would be
> provided by an input that is currently not present(?).

The ‘run’ function in xfsm-shutdown-helper/main.c runs these programs
with:

      result = g_spawn_sync (NULL, argv, envp,
                             G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL |
                             G_SPAWN_STDERR_TO_DEV_NULL,
                             NULL, NULL, NULL, NULL, &status, &err);

I’m guessing G_SPAWN_SEARCH_PATH means that the program is searched for
in $PATH, in which case writing ‘halt’ would be enough (it would be
found in /run/setuid-programs.)

> I also needed to change my system configuration to add “pkexec” to the
> list of setuid programs:
>
>   (setuid-programs (cons #~(string-append #$polkit "/bin/pkexec")
>                            %setuid-programs))
>
> Maybe this should just be added to %setuid-programs?

I think polkit-service-type should extend setuid-service-type to add
that program.

> Is it okay to use /run/setuid-programs/pkexec in the shutdown helper or
> should this rather be a reference to the polkit input?

The code that spawns pkexec looks like this:

  command = g_strdup_printf ("pkexec " XFSM_SHUTDOWN_HELPER_CMD " --%s", action);
  ret = g_spawn_command_line_sync (command, NULL, NULL, &exit_status, error);

I think this can be left unchanged, as long as we provide pkexec in
$PATH (which is the case if it’s in /run/setuid-programs.)

Thanks for looking into it!

Ludo’.

Re: [PATCH] Fix XFCE shutdown/reboot via menu.

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> First I had to patch our xfce-session package.  Upower is checked for at
>> configure time and needed for shutdown and reboot.  Polkit’s “pkexec” is
>> required to run the shutdown helper.
>>
>> The helper contains a bunch of hardcoded paths to /sbin/shutdown, which
>> are replaced in a build phase — not in a snippet because eventually we
>> should also fix the paths to pm-{suspend,hibernate}, which would be
>> provided by an input that is currently not present(?).
>
> The ‘run’ function in xfsm-shutdown-helper/main.c runs these programs
> with:
>
>       result = g_spawn_sync (NULL, argv, envp,
>                              G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL |
>                              G_SPAWN_STDERR_TO_DEV_NULL,
>                              NULL, NULL, NULL, NULL, &status, &err);
>
> I’m guessing G_SPAWN_SEARCH_PATH means that the program is searched for
> in $PATH, in which case writing ‘halt’ would be enough (it would be
> found in /run/setuid-programs.)

I see.

>> I also needed to change my system configuration to add “pkexec” to the
>> list of setuid programs:
>>
>>   (setuid-programs (cons #~(string-append #$polkit "/bin/pkexec")
>>                            %setuid-programs))
>>
>> Maybe this should just be added to %setuid-programs?
>
> I think polkit-service-type should extend setuid-service-type to add
> that program.

‘polkit-service-type’ already extends ‘setuid-programs-service-type’
with ‘polkit-setuid-programs’:

  (define polkit-setuid-programs
    (match-lambda
      (($ <polkit-configuration> polkit)
       (list #~(string-append #$polkit
                              "/lib/polkit-1/polkit-agent-helper-1")))))

I guess we can just append ‘#~(string-append #$polkit "/bin/pkexec")’
here.

>> Is it okay to use /run/setuid-programs/pkexec in the shutdown helper or
>> should this rather be a reference to the polkit input?
>
> The code that spawns pkexec looks like this:
>
>   command = g_strdup_printf ("pkexec " XFSM_SHUTDOWN_HELPER_CMD " --%s", action);
>   ret = g_spawn_command_line_sync (command, NULL, NULL, &exit_status, error);
>
> I think this can be left unchanged, as long as we provide pkexec in
> $PATH (which is the case if it’s in /run/setuid-programs.)

Okay.  I’ll try the above changes and report back.

~~ Ricardo

Re: [PATCH] Fix XFCE shutdown/reboot via menu.

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>> I also needed to change my system configuration to add “pkexec” to the
>>> list of setuid programs:
>>>
>>>   (setuid-programs (cons #~(string-append #$polkit "/bin/pkexec")
>>>                            %setuid-programs))
>>>
>>> Maybe this should just be added to %setuid-programs?
>>
>> I think polkit-service-type should extend setuid-service-type to add
>> that program.
>
> ‘polkit-service-type’ already extends ‘setuid-programs-service-type’
> with ‘polkit-setuid-programs’:
>
>   (define polkit-setuid-programs
>     (match-lambda
>       (($ <polkit-configuration> polkit)
>        (list #~(string-append #$polkit
>                               "/lib/polkit-1/polkit-agent-helper-1")))))
>
> I guess we can just append ‘#~(string-append #$polkit "/bin/pkexec")’
> here.

Yes.

Ludo’.

Re: [PATCH] Fix XFCE shutdown/reboot via menu.

[Ricardo Wurmus]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

Here are two new patches.

The first adds missing inputs to xfce-session and replaces the paths to
the shutdown, pm-suspend, and pm-hibernate commands with the plain
names.

The second simply adds pkexec to the list of setuid programmes via
‘polkit-setuid-programs’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-xfce-session-Enable-shutdown-reboot-menu-entries.patch --]
[-- Type: text/x-patch, Size: 2127 bytes --]

From dd10a2c30a6ef57b6d0bd9cecb263b731d6e1483 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sat, 21 Nov 2015 17:47:00 +0100
Subject: [PATCH 1/2] gnu: xfce-session: Enable shutdown/reboot menu entries.

* gnu/packages/xfce.scm (xfce-session)[inputs]: Add upower and polkit.
[source]: Replace paths to "shutdown" with "halt" and "restart".
---
 gnu/packages/xfce.scm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/xfce.scm b/gnu/packages/xfce.scm
index a4987c1..e213fb6 100644
--- a/gnu/packages/xfce.scm
+++ b/gnu/packages/xfce.scm
@@ -35,6 +35,7 @@
   #:use-module (gnu packages image)
   #:use-module (gnu packages gnome)
   #:use-module (gnu packages pdf)
+  #:use-module (gnu packages polkit)
   #:use-module (gnu packages gstreamer)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages photo)
@@ -426,7 +427,16 @@ your system in categories, so you can quickly find and launch them.")
                 "01kvbd09c06j20n155hracsgrq06rlmfgdywffjsvlwpn19m9j38"))
               (patches
                ;; See: https://bugzilla.xfce.org/show_bug.cgi?id=12282
-               (list (search-patch "xfce4-session-fix-xflock4.patch")))))
+               (list (search-patch "xfce4-session-fix-xflock4.patch")))
+              (modules '((guix build utils)))
+              (snippet
+               '(begin
+                  (substitute* "xfsm-shutdown-helper/main.c"
+                    (("/sbin/shutdown -h now")  "halt")
+                    (("/sbin/shutdown -r now")  "restart")
+                    (("/usr/sbin/pm-suspend")   "pm-suspend")
+                    (("/usr/sbin/pm-hibernate") "pm-hibernate"))
+                  #t))))
     (build-system gnu-build-system)
     (arguments
      '(#:configure-flags
@@ -436,6 +446,8 @@ your system in categories, so you can quickly find and launch them.")
        ("intltool" ,intltool)))
     (inputs
      `(("iceauth" ,iceauth)
+       ("upower" ,upower)
+       ("polkit" ,polkit)
        ("libsm" ,libsm)
        ("libwnck" ,libwnck-1)
        ("libxfce4ui" ,libxfce4ui)))
-- 
2.5.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-services-Add-pkexec-to-setuid-programs.patch --]
[-- Type: text/x-patch, Size: 967 bytes --]

From e3050bed45d11d9dfc500c4f0d1b6132013f6b76 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sat, 21 Nov 2015 22:40:22 +0100
Subject: [PATCH 2/2] services: Add pkexec to setuid programs.

* gnu/services/desktop.scm (polkit-setuid-programs): Add pkexec to
  list of setuid programs.
---
 gnu/services/desktop.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 0b4ed56..694a8ed 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -381,7 +381,8 @@ site} for more information."
   (match-lambda
     (($ <polkit-configuration> polkit)
      (list #~(string-append #$polkit
-                            "/lib/polkit-1/polkit-agent-helper-1")))))
+                            "/lib/polkit-1/polkit-agent-helper-1")
+           #~(string-append #$polkit "/bin/pkexec")))))
 
 (define polkit-service-type
   (service-type (name 'polkit)
-- 
2.5.0


[-- Attachment #4: Type: text/plain, Size: 386 bytes --]


While these patches enable the “Suspend”, “Shut Down”, and “Restart”
fields of the account actions widget, they don’t actually cause the
computer to suspend, shut down, or reboot.  The X session is terminated,
but we end up at the SLIM login screen again.

I’ll try to investigate some more, but I don’t think there’s anything
wrong with these patches.

~~ Ricardo

Re: [PATCH] Fix XFCE shutdown/reboot via menu.

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> From dd10a2c30a6ef57b6d0bd9cecb263b731d6e1483 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Sat, 21 Nov 2015 17:47:00 +0100
> Subject: [PATCH 1/2] gnu: xfce-session: Enable shutdown/reboot menu entries.
>
> * gnu/packages/xfce.scm (xfce-session)[inputs]: Add upower and polkit.
> [source]: Replace paths to "shutdown" with "halt" and "restart".

LGTM.

> From e3050bed45d11d9dfc500c4f0d1b6132013f6b76 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Sat, 21 Nov 2015 22:40:22 +0100
> Subject: [PATCH 2/2] services: Add pkexec to setuid programs.
>
> * gnu/services/desktop.scm (polkit-setuid-programs): Add pkexec to
>   list of setuid programs.

LGTM.

> While these patches enable the “Suspend”, “Shut Down”, and “Restart”
> fields of the account actions widget, they don’t actually cause the
> computer to suspend, shut down, or reboot.  The X session is terminated,
> but we end up at the SLIM login screen again.

This is weird.  Could you maybe attach ‘strace -f’ to xfce-session,
press “Shut Down”, and check in the strace log what happened when
executing ‘halt’ or ‘pkexec’ or whatever?  Or maybe polkit leaves a hint
on syslog?

Thanks!

Ludo’.