Bug#764607: systemd: systemctl does not re-create display-manager.service symlink

[Didier Roche]

Le 22/11/2014 21:28, Andrei POPESCU a écrit :
>
> 2b. Each display manager must add a Conflicts=[all other DMs]

You don't really need to maintain a long list of Conflicts (which will 
never be kept up to date). I suggested last wek to the gdm maintainers 
that we start using the Alias. That enables to have only one DM enable 
at a time, having systemctl creating the symlink and conflicting when 
needed. Finally, the postinst maintainer script is easier. Then, you 
don't really need WantedBy=graphical.target I guess.


I didn't hear from the gdm3-pkg team yet, let me paste my suggestions:

---------------
We discussed a little bit today with Martin on providing other display 
managers like xdm with systemd services.

I looked at the existing postinst of lightdm and I think we can leverage 
systemd Alias to keep the exact same functionality, but removing the 
internal systemd knowledge from the postinst scripts. The end result on 
disk would be exactly the same than the existing implementation, we just 
remove the manual handling of symlinks.

The idea is to add:
[Install]
Alias=display-manager.service

to the service unit, and then, replacing the existing postinst symlinks 
dance with only systemctl commands. Here is the commit for lightdm: 
http://bazaar.launchpad.net/~didrocks/lightdm/systemd-alias/revision/2099, 
resulting in a simpler postinst: 
http://bazaar.launchpad.net/~didrocks/lightdm/systemd-alias/view/head:/debian/lightdm.postinst#L72 


Martin agrees that this approach is more decouple from systemd internals 
and I'm here to reach your feedback about it. As the result on disk is 
the same in the end, we don't have to migrate all DMs at the same time 
if you agree with that idea.

I've prepared in case you are in favor of this change a gdm3 debdiif 
(attached), I saw that upstream was shipping for gdm the Alias section, 
so modified the patch + postinst for it.

---------

Does it make sense to you?
-------------- next part --------------
diff -Nru gdm3-3.14.1/debian/changelog gdm3-3.14.1/debian/changelog
--- gdm3-3.14.1/debian/changelog	2014-11-09 18:16:03.000000000 +0100
+++ gdm3-3.14.1/debian/changelog	2014-11-20 09:44:14.000000000 +0100
@@ -1,3 +1,10 @@
+gdm3 (3.14.1-4) UNRELEASED; urgency=medium
+
+  * debian/patches/92_systemd_unit.patch, debian/gdm3.postinst:
+    - Using Alias and systemctl to handle systemd unit alternatives.
+
+ -- Didier Roche <didrocks at ubuntu.com>  Thu, 20 Nov 2014 09:40:25 +0100
+
 gdm3 (3.14.1-3) unstable; urgency=medium
 
   * 18_all_displays_transient.patch: fix autologin for the initial 
diff -Nru gdm3-3.14.1/debian/gdm3.postinst gdm3-3.14.1/debian/gdm3.postinst
--- gdm3-3.14.1/debian/gdm3.postinst	2014-04-27 15:07:16.000000000 +0200
+++ gdm3-3.14.1/debian/gdm3.postinst	2014-11-20 09:45:01.000000000 +0100
@@ -40,21 +40,18 @@
   fi
 fi
 
-DEFAULT_SERVICE=/etc/systemd/system/display-manager.service
+DEFAULT_SERVICE=display-manager.service
+SERVICE=$(basename $(cat "$DEFAULT_DISPLAY_MANAGER_FILE")).service
 # set default-display-manager systemd service link according to our config
-if [ "$1" = configure ] && [ -d /etc/systemd/system/ ]; then
-  if [ -e "$DEFAULT_DISPLAY_MANAGER_FILE" ]; then
-    SERVICE=/lib/systemd/system/$(basename $(cat "$DEFAULT_DISPLAY_MANAGER_FILE")).service
-    if [ -h "$DEFAULT_SERVICE" ] && [ $(readlink "$DEFAULT_SERVICE") = /dev/null ]; then
-      echo "Display manager service is masked" >&2
-    elif [ -e "$SERVICE" ]; then
-      ln -sf "$SERVICE" "$DEFAULT_SERVICE"
-    else
-      echo "WARNING: $SERVICE is the selected default display manager but does not exist" >&2
-      rm -f "$DEFAULT_SERVICE"
-    fi
+if [ "$1" = configure ] && [ -x /bin/systemctl ]; then
+  if [ $(systemctl is-enabled "$DEFAULT_SERVICE") = masked ]; then
+    echo "Display manager service is masked" >&2
   else
-    rm -f "$DEFAULT_SERVICE"
+    [ -d /run/systemd/system ] && systemctl daemon-reload
+    systemctl enable --force $SERVICE 2>/dev/null || true
+    if [ $? != 0 ]; then
+      echo "WARNING: $SERVICE is the selected default display manager but does not have a systemd service" >&2
+    fi
   fi
 fi
 
diff -Nru gdm3-3.14.1/debian/patches/92_systemd_unit.patch gdm3-3.14.1/debian/patches/92_systemd_unit.patch
--- gdm3-3.14.1/debian/patches/92_systemd_unit.patch	2014-04-27 14:44:32.000000000 +0200
+++ gdm3-3.14.1/debian/patches/92_systemd_unit.patch	2014-11-20 09:43:34.000000000 +0100
@@ -1,8 +1,8 @@
-Index: gdm3-3.12.1/data/gdm.service.in
+Index: gdm3-3.14.1/data/gdm.service.in
 ===================================================================
---- gdm3-3.12.1.orig/data/gdm.service.in	2014-04-27 14:40:14.210580120 +0200
-+++ gdm3-3.12.1/data/gdm.service.in	2014-04-27 14:43:22.350149176 +0200
-@@ -4,12 +4,15 @@ Conflicts=getty at tty@GDM_INITIAL_VT at .serv
+--- gdm3-3.14.1.orig/data/gdm.service.in
++++ gdm3-3.14.1/data/gdm.service.in
+@@ -4,10 +4,16 @@ Conflicts=getty at tty@GDM_INITIAL_VT at .serv
  After=systemd-user-sessions.service getty at tty@GDM_INITIAL_VT at .service plymouth-quit.service
  
  [Service]
@@ -20,6 +20,4 @@
 +#BusName=org.gnome.DisplayManager
  StandardOutput=syslog
  StandardError=inherit
--
--[Install]
--Alias=display-manager.service
+