On Wed, 06 Jul 2016 at 18:02:57 +0100, Simon McVittie wrote:
> Here are patches; I based them on what gdm3 does. In my testing on an
> embedded Debian derivative, they seem to interoperate nicely with xdm.

Sjoerd Simons pointed out some things that could be improved, leading
to the attached additional patches.

Sjoerd also suggested that nodm.service should probably be ordered
After=plymouth-quit.service, but I've had a hard time articulating
why, so I'll leave arguing that point to him :-)

Regards,
    S
>From cff720f93bc21707da04c634ea0ac99736a2b999 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvit...@collabora.co.uk>
Date: Thu, 7 Jul 2016 16:15:05 +0100
Subject: [PATCH 5/7] nodm.service: run one fewer subprocess

This is non-POSIX, but systemd services are Linux-specific, so it
seems reasonable to rely on coreutils test here.

Signed-off-by: Simon McVittie <simon.mcvit...@collabora.co.uk>
---
 nodm.service.in | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/nodm.service.in b/nodm.service.in
index fdc4785..71b81c9 100644
--- a/nodm.service.in
+++ b/nodm.service.in
@@ -8,7 +8,6 @@ After=systemd-user-sessions.service
 
 [Service]
 EnvironmentFile=-/etc/default/nodm
-ExecStartPre=/usr/bin/test ${NODM_ENABLED} != no
-ExecStartPre=/usr/bin/test ${NODM_ENABLED} != false
+ExecStartPre=/usr/bin/test ${NODM_ENABLED} != no -a ${NODM_ENABLED} != false
 ExecStart=@sbindir@/nodm $NODM_OPTIONS
 Restart=always
-- 
2.8.1

>From 7675897515fe72d2f46e459c9aaf1e4820155813 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvit...@collabora.co.uk>
Date: Thu, 7 Jul 2016 16:16:38 +0100
Subject: [PATCH 6/7] debian/nodm.init: don't order after obsolete hal service

Signed-off-by: Simon McVittie <simon.mcvit...@collabora.co.uk>
---
 debian/changelog | 1 +
 debian/nodm.init | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 2e2c9bd..e051a60 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -16,6 +16,7 @@ nodm (0.12-2) UNRELEASED; urgency=medium
   * nodm.service: add a native systemd service (Closes: #796203)
   * Participate in the /etc/X11/default-display-manager mechanism
   * debian/nodm.pam: don't log warnings if ConsoleKit isn't installed
+  * debian/nodm.init: don't order after obsolete hal service
 
  -- Mike Gabriel <sunwea...@debian.org>  Thu, 24 Mar 2016 14:43:36 +0100
 
diff --git a/debian/nodm.init b/debian/nodm.init
index 48cc28a..c95fd4d 100644
--- a/debian/nodm.init
+++ b/debian/nodm.init
@@ -1,7 +1,7 @@
 #!/bin/sh
 ### BEGIN INIT INFO
 # Provides:       nodm
-# Should-Start:   console-screen kbd hal bluetooth
+# Should-Start:   console-screen kbd bluetooth
 # Required-Start: $remote_fs
 # Required-Stop:
 # Default-Start:  2 3 4 5
-- 
2.8.1

>From d3566eb5b879aaec7c9c43d6ee949404334d7e5d Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvit...@collabora.co.uk>
Date: Thu, 7 Jul 2016 16:24:49 +0100
Subject: [PATCH 7/7] debian/nodm.init, nodm.service: don't order after
 console-screen or kbd

During review, Sjoerd Simons asked why the new systemd service was
ordered after console-screen or kbd. This was done for parity with
the old init script, which had the same ordering constraints.

However, it isn't clear from nodm's history why these ordering
constraints were first introduced. The one for console-screen was
added in commit 70c5c81a "Package change to nodm" with no further
explanation, and the one for kbd was added when console-screen was
superseded by kbd. nodm is a rc2..rc5 service, while console-screen
and kbd were rcS services, so I'm not sure that this was actually
ever effective: everything in rcS is started before we reach
rc2..rc5 anyway.

In the systemd unit I added an ordering constraint for
console-setup.service, the replacement for /etc/init.d/kbd, which
does get delayed until multi-user.target (the equivalent of rc2).
However, it's unclear how setting the console font and keyboard
settings would affect nodm, which runs X11 and hence is unaffected
by those settings.

Signed-off-by: Simon McVittie <simon.mcvit...@collabora.co.uk>
---
 debian/changelog | 3 +++
 debian/nodm.init | 2 +-
 nodm.service.in  | 3 ---
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index e051a60..e190020 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -17,6 +17,9 @@ nodm (0.12-2) UNRELEASED; urgency=medium
   * Participate in the /etc/X11/default-display-manager mechanism
   * debian/nodm.pam: don't log warnings if ConsoleKit isn't installed
   * debian/nodm.init: don't order after obsolete hal service
+  * debian/nodm.init, nodm.service: don't order after console-screen or
+    kbd, which were rcS services so would always have started before
+    nodm anyway
 
  -- Mike Gabriel <sunwea...@debian.org>  Thu, 24 Mar 2016 14:43:36 +0100
 
diff --git a/debian/nodm.init b/debian/nodm.init
index c95fd4d..6ab9738 100644
--- a/debian/nodm.init
+++ b/debian/nodm.init
@@ -1,7 +1,7 @@
 #!/bin/sh
 ### BEGIN INIT INFO
 # Provides:       nodm
-# Should-Start:   console-screen kbd bluetooth
+# Should-Start:   bluetooth
 # Required-Start: $remote_fs
 # Required-Stop:
 # Default-Start:  2 3 4 5
diff --git a/nodm.service.in b/nodm.service.in
index 71b81c9..638f919 100644
--- a/nodm.service.in
+++ b/nodm.service.in
@@ -1,9 +1,6 @@
 [Unit]
 Description=Display manager for automatic session logins
 Documentation=man:nodm(8)
-After=console-screen.service
-After=console-setup.service
-After=kbd.service
 After=systemd-user-sessions.service
 
 [Service]
-- 
2.8.1

Reply via email to