Bug#854691: xscreensaver: errors in xscreensaver.service systemd unit file

[Laurent Bonnaud]

Package: xscreensaver
Version: 5.36-1
Severity: normal


Dear Maintainer,

here is the problem:

# systemd-analyze verify /usr/lib/systemd/user/xscreensaver.service
[/usr/lib/systemd/user/xscreensaver.service:5] Executable path is not absolute, ignoring: xscreensaver
xscreensaver.service: Service lacks both ExecStart= and ExecStop= setting. Refusing.
xscreensaver.service: Failed to create xscreensaver.service/start: Unit xscreensaver.service is not loaded properly: Invalid argument.


-- System Information:
Debian Release: 9.0
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'unstable'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 4.9.0-1-rt-amd64 (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages xscreensaver depends on:
ii libatk1.0-0 2.22.0-1
ii libc6 2.24-9
ii libcairo2 1.14.8-1
ii libfontconfig1 2.11.0-6.7
ii libfreetype6 2.6.3-3+b1
ii libgdk-pixbuf2.0-0 2.36.4-1
ii libglade2-0 1:2.6.4-2
ii libglib2.0-0 2.51.0-2
ii libgtk2.0-0 2.24.31-2
ii libice6 2:1.0.9-1+b1
ii libpam0g 1.1.8-3.5
ii libpango-1.0-0 1.40.3-3
ii libpangocairo-1.0-0 1.40.3-3
ii libpangoft2-1.0-0 1.40.3-3
ii libsm6 2:1.2.2-1+b1
ii libx11-6 2:1.6.4-3
ii libxext6 2:1.3.3-1
ii libxi6 2:1.7.9-1
ii libxinerama1 2:1.1.3-1+b1
ii libxml2 2.9.4+dfsg1-2.2
ii libxmu6 2:1.1.2-2
ii libxpm4 1:3.5.12-1
ii libxrandr2 2:1.5.1-1
ii libxrender1 1:0.9.10-1
ii libxt6 1:1.1.5-1
ii libxxf86vm1 1:1.1.4-1
ii xscreensaver-data 5.36-1
--
Laurent.

[Jeremy Visser]

I've attached a patch which fixes the issue.

[Daniel Kahn Gillmor]

On Thu 2017-08-31 12:07:10 +1000, Jeremy Visser wrote:
> I've attached a patch which fixes the issue.

Jeremy's patch (below) looks reasonable to me. Even better would be to
have it include other useful metadata in the [Unit] section, like:

Documentation=man:xscreensaver(1)
Documentation=man:xscreensaver-command(1)
Documentation=man:xscreensaver-demo(1)
PartOf=graphical-session.target

xscreensaver(1) currently has a suggestion for this file. It should
probably also be adjusted to remove the example, and explain
specifically that all the user needs to do is:

systemctl --user enable xscreensaver

Arguably, this user unit should be enabled by default. Is there any
reason to ship it disabled?

--dkg

> From 76072fa4cba00e6a6009324d6b9e0cf1d3fdc82f Mon Sep 17 00:00:00 2001
> From: Jeremy Visser <jeremy...@google.com>
> Date: Thu, 31 Aug 2017 11:53:00 +1000
> Subject: [PATCH] Fix broken systemd unit
>
> The ExecStart= line must be an absolute path as per systemd.service(5),
> but is currently a relative path. This change points it to
> /usr/bin/xscreensaver.
> ---
> debian/xscreensaver.service | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/debian/xscreensaver.service b/debian/xscreensaver.service
> index 5e8f13d..c5298f2 100644
> --- a/debian/xscreensaver.service
> +++ b/debian/xscreensaver.service
> @@ -2,7 +2,7 @@
> Description=XScreenSaver
>
> [Service]
> -ExecStart=xscreensaver
> +ExecStart=/usr/bin/xscreensaver
>
> [Install]
> WantedBy=default.target
> --
> 2.14.1.581.gf28d330327-goog

[Tormod Volden]

On Fri, Oct 27, 2017 at 4:23 AM, Daniel Kahn Gillmor wrote:
> On Thu 2017-08-31 12:07:10 +1000, Jeremy Visser wrote:
>> I've attached a patch which fixes the issue.

Thanks for the patch, and for forwarding it here (don't know where it was sent).

>
> Jeremy's patch (below) looks reasonable to me. Even better would be to
> have it include other useful metadata in the [Unit] section, like:
>
> Documentation=man:xscreensaver(1)
> Documentation=man:xscreensaver-command(1)
> Documentation=man:xscreensaver-demo(1)
> PartOf=graphical-session.target
>

Thanks.

> xscreensaver(1) currently has a suggestion for this file. It should
> probably also be adjusted to remove the example, and explain
> specifically that all the user needs to do is:
>
> systemctl --user enable xscreensaver
>
> Arguably, this user unit should be enabled by default. Is there any
> reason to ship it disabled?

I am not so into systemd "units", but possibly relevant to your
question is that it is fully possible that the xscreensaver package is
installed on a system, but that individual users don't want to have it
running. Whether it should start by default for a user depends on the
desktop environment.

Regards,
Tormod

[Daniel Kahn Gillmor]

On Mon 2017-12-04 00:14:05 +0100, Tormod Volden wrote:
> I am not so into systemd "units", but possibly relevant to your
> question is that it is fully possible that the xscreensaver package is
> installed on a system, but that individual users don't want to have it
> running. Whether it should start by default for a user depends on the
> desktop environment.

I think if xscreensaver is installed, it should be enabled by default.
desktop environments that don't want it enabled can explicitly disable
it for users which launch them, as can users who for whatever reason
want it disabled.

Most systems won't have xscreensaver installed at all unless the user
has requested it, right?

--dkg

[Antoine Beaupré]

On 2017-12-03 18:40:03, Daniel Kahn Gillmor wrote:
> On Mon 2017-12-04 00:14:05 +0100, Tormod Volden wrote:
>> I am not so into systemd "units", but possibly relevant to your
>> question is that it is fully possible that the xscreensaver package is
>> installed on a system, but that individual users don't want to have it
>> running. Whether it should start by default for a user depends on the
>> desktop environment.
>
> I think if xscreensaver is installed, it should be enabled by default.
> desktop environments that don't want it enabled can explicitly disable
> it for users which launch them, as can users who for whatever reason
> want it disabled.

I think it was explicitely disabled because of this bug:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=977804

> Most systems won't have xscreensaver installed at all unless the user
> has requested it, right?

I think that's not the point - in the above, the systemd activation
conflicted with the xsession one.

Also: it didn't work, for me. In Debian buster, xscreensaver tried to
start before Xorg was available, and failed to contact the DISPLAY
(server).

In bullseye, however, this unit works:

[Unit]
Description=XScreenSaver

Documentation=man:xscreensaver(1)
Documentation=man:xscreensaver-command(1)
Documentation=man:xscreensaver-demo(1)
PartOf=graphical-session.target

[Service]
ExecStart=/usr/bin/xscreensaver
Restart=on-failure

[Install]
WantedBy=default.target


I suspect that this is the part that doesn't work in buster:

PartOf=graphical-session.target

Also notice this trick:

Restart=on-failure

Which should prevent some security issues from unlocking the screen by
crashing it. It's not quite complete: it should also *lock* the screen
on crash, but it's a start.

--
It is not enough to fight for the land; it is even more important to
enjoy it. While you can. While it’s still here.
- Edward Abbey

[John Lines]

On bookworm, with xscreensaver 6.06+dfsg1-3 I had an error trying to
do

systemctl --user enable xscreensaver
Failed to enable unit: Cannot alias xscreensaver.service as
org.jwz.xscreensaver.

so did:
cp /usr/lib/systemd/user/xscreensaver.service ~/.config/systemd/user/

(note that the man page says to use
/usr/share/xscreensaver/xscreensaver.service which does not exist as
the source)

Then edited the user copy to edit out
Alias=org.jwz.xscreensaver

and now
$ systemctl --user enable xscreensaver

seems to work

John