unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#35493] [PATCH 0/3] Allow multiple SLiM services.
@ 2019-04-29 18:19 Diego Nicola Barbato
  2019-04-29 18:20 ` [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display Diego Nicola Barbato
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-04-29 18:19 UTC (permalink / raw)
  To: 35493

Hello Guix,

This patch series makes it possible to run multiple instances of SLiM
(on different ttys) by adding multiple SLiM services to system services.
I mentioned this idea on guix-devel [0].

Regards,

Diego

[0]: https://lists.gnu.org/archive/html/guix-devel/2019-04/msg00535.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-04-29 18:19 [bug#35493] [PATCH 0/3] Allow multiple SLiM services Diego Nicola Barbato
@ 2019-04-29 18:20 ` Diego Nicola Barbato
  2019-05-06  8:35   ` Ludovic Courtès
  2019-04-29 18:20 ` [bug#35493] [PATCH 2/3] services: slim: Allow SLiM to be started on multiple ttys Diego Nicola Barbato
  2019-04-29 18:20 ` [bug#35493] [PATCH 3/3] doc: Document 'display' and 'vt' fields of 'slim-configuration' Diego Nicola Barbato
  2 siblings, 1 reply; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-04-29 18:20 UTC (permalink / raw)
  To: 35493

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-gnu-slim-Allow-customisation-of-Xorg-display.patch --]
[-- Type: text/x-patch, Size: 5002 bytes --]

From 7e7205e0f8d70a89a7ed4fd4844feb9b6a8d8567 Mon Sep 17 00:00:00 2001
From: Diego Nicola Barbato <dnbarbato@posteo.de>
Date: Sun, 28 Apr 2019 14:48:47 +0200
Subject: [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.

Previously SLiM could only start on display :0.0.  This change makes it
possible to run multiple instances of SLiM at the same time.

* gnu/packages/patches/slim-display.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/display-managers.scm (slim): Use it.
---
 gnu/local.mk                            |  1 +
 gnu/packages/display-managers.scm       |  3 +-
 gnu/packages/patches/slim-display.patch | 75 +++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/slim-display.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index f79bfd8b93..0b952f6f9f 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1267,6 +1267,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/slim-sigusr1.patch			\
   %D%/packages/patches/slim-reset.patch				\
   %D%/packages/patches/slim-login.patch				\
+  %D%/packages/patches/slim-display.patch			\
   %D%/packages/patches/snappy-add-O2-flag-in-CmakeLists.txt.patch	\
   %D%/packages/patches/sooperlooper-build-with-wx-30.patch 	\
   %D%/packages/patches/soundconverter-remove-gconf-dependency.patch	\
diff --git a/gnu/packages/display-managers.scm b/gnu/packages/display-managers.scm
index afe5cc2b7b..f653bd60c5 100644
--- a/gnu/packages/display-managers.scm
+++ b/gnu/packages/display-managers.scm
@@ -322,7 +322,8 @@ GTK+, lets you select a desktop session and log in to it.")
                                       "slim-reset.patch"
                                       "slim-login.patch"
                                       "slim-session.patch"
-                                      "slim-sigusr1.patch"))))
+                                      "slim-sigusr1.patch"
+                                      "slim-display.patch"))))
     (build-system cmake-build-system)
     (inputs `(("linux-pam" ,linux-pam)
 	      ("libpng" ,libpng)
diff --git a/gnu/packages/patches/slim-display.patch b/gnu/packages/patches/slim-display.patch
new file mode 100644
index 0000000000..f68604a94b
--- /dev/null
+++ b/gnu/packages/patches/slim-display.patch
@@ -0,0 +1,75 @@
+Add "display_name" configuration option and use its value instead of
+the hard coded one.
+
+Patch by Diego N. Barbato
+
+--- a/app.cpp	1970-01-01 01:00:00.000000000 +0100
++++ b/app.cpp	2019-04-27 13:48:23.479133531 +0200
+@@ -190,7 +190,13 @@
+ }
+ 
+ void App::Run() {
+-	DisplayName = DISPLAY;
++	/* Read configuration */
++	cfg = new Cfg;
++	char *cfgfile = getenv("SLIM_CFGFILE");
++	if (!cfgfile) cfgfile = CFGFILE;
++	cfg->readConf(cfgfile);
++
++	DisplayName = cfg->getOption("display_name").c_str();
+ 
+ #ifdef XNEST_DEBUG
+ 	char* p = getenv("DISPLAY");
+@@ -200,11 +206,7 @@
+ 	}
+ #endif
+ 
+-	/* Read configuration and theme */
+-	cfg = new Cfg;
+-	char *cfgfile = getenv("SLIM_CFGFILE");
+-	if (!cfgfile) cfgfile = CFGFILE;
+-	cfg->readConf(cfgfile);
++	/* Read theme */
+ 	string themebase = "";
+ 	string themefile = "";
+ 	string themedir = "";
+@@ -911,9 +913,7 @@
+ 	static const int MAX_XSERVER_ARGS = 256;
+ 	static char* server[MAX_XSERVER_ARGS+2] = { NULL };
+ 	server[0] = (char *)cfg->getOption("default_xserver").c_str();
+-	string argOption = cfg->getOption("xserver_arguments");
+-	/* Add mandatory -xauth option */
+-	argOption = argOption + " -auth " + cfg->getOption("authfile");
++	string argOption = cfg->getOption("display_name") + " " + cfg->getOption("xserver_arguments") + " -auth " + cfg->getOption("authfile");
+ 	char* args = new char[argOption.length()+2]; /* NULL plus vt */
+ 	strcpy(args, argOption.c_str());
+ 
+@@ -1233,7 +1233,7 @@
+ 	authfile = cfg->getOption("authfile");
+ 	remove(authfile.c_str());
+ 	putenv(StrConcat("XAUTHORITY=", authfile.c_str()));
+-	Util::add_mcookie(mcookie, ":0", cfg->getOption("xauth_path"),
++	Util::add_mcookie(mcookie, cfg->getOption("display_name").c_str(), cfg->getOption("xauth_path"),
+ 	  authfile);
+ }
+ 
+--- a/cfg.cpp	1970-01-01 01:00:00.000000000 +0100
++++ b/cfg.cpp	2019-04-27 13:49:40.511773743 +0200
+@@ -31,6 +31,7 @@
+ 	/* Configuration options */
+ 	options.insert(option("default_path","/bin:/usr/bin:/usr/local/bin"));
+ 	options.insert(option("default_xserver","/usr/bin/X"));
++	options.insert(option("display_name",":0.0"));
+ 	options.insert(option("xserver_arguments",""));
+ 	options.insert(option("numlock",""));
+ 	options.insert(option("daemon",""));
+--- a/switchuser.cpp	1970-01-01 01:00:00.000000000 +0100
++++ b/switchuser.cpp	2019-04-27 13:50:19.380096651 +0200
+@@ -54,6 +54,6 @@
+ 	string home = string(Pw->pw_dir);
+ 	string authfile = home + "/.Xauthority";
+ 	remove(authfile.c_str());
+-	Util::add_mcookie(mcookie, ":0", cfg->getOption("xauth_path"),
++	Util::add_mcookie(mcookie, displayName.c_str(), cfg->getOption("xauth_path"),
+ 	  authfile);
+ }
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 2/3] services: slim: Allow SLiM to be started on multiple ttys.
  2019-04-29 18:19 [bug#35493] [PATCH 0/3] Allow multiple SLiM services Diego Nicola Barbato
  2019-04-29 18:20 ` [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display Diego Nicola Barbato
@ 2019-04-29 18:20 ` Diego Nicola Barbato
  2019-04-29 18:20 ` [bug#35493] [PATCH 3/3] doc: Document 'display' and 'vt' fields of 'slim-configuration' Diego Nicola Barbato
  2 siblings, 0 replies; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-04-29 18:20 UTC (permalink / raw)
  To: 35493

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0002-services-slim-Allow-SLiM-to-be-started-on-multiple-t.patch --]
[-- Type: text/x-patch, Size: 6078 bytes --]

From fddb8de350bcf5c89932e032b3fe2ad4dc7c91dd Mon Sep 17 00:00:00 2001
From: Diego Nicola Barbato <dnbarbato@posteo.de>
Date: Sun, 28 Apr 2019 15:52:50 +0200
Subject: [PATCH 2/3] services: slim: Allow SLiM to be started on multiple
 ttys.

This change makes it possible to add multiple SLiM services to an operating
system configuration by setting the new 'display' and 'vt' fields in their
configurations to different values.  Each SLiM service will get its own
authfile, logfile, lockfile, and shepherd service, which will start SLiM on a
different tty.

* gnu/services/xorg.scm: Export slim-configuration-display and
  slim-configuration-vt.
  (<slim-configuration>)[display, vt]: New fields.
  (slim-shepherd-service): Refactor let.
  [slim.cfg]: Use new fields for setting display_name, xserver_arguments,
  authfile, lockfile, and logfile.
  [shepherd-service][provision]: Name the shepherd service according to the
  value of 'vt'.
  [shepherd-service][start]: Delete the right lockfile.
---
 gnu/services/xorg.scm | 85 +++++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 36 deletions(-)

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 44dcec4ec9..65e9d48915 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -83,6 +83,8 @@
             slim-configuration-shepherd
             slim-configuration-auto-login-session
             slim-configuration-xorg
+            slim-configuration-display
+            slim-configuration-vt
             slim-configuration-sessreg
 
             slim-service-type
@@ -488,6 +490,10 @@ desktop session from the system or user profile will be used."
                       (default #f))
   (xorg-configuration slim-configuration-xorg
                       (default (xorg-configuration)))
+  (display slim-configuration-display
+           (default ":0"))
+  (vt slim-configuration-vt
+      (default "vt7"))
   (sessreg slim-configuration-sessreg
            (default sessreg)))
 
@@ -499,20 +505,26 @@ desktop session from the system or user profile will be used."
          (slim-configuration-allow-empty-passwords? config))))
 
 (define (slim-shepherd-service config)
-  (define slim.cfg
-    (let ((xinitrc (xinitrc #:fallback-session
-                            (slim-configuration-auto-login-session config)))
-          (xauth   (slim-configuration-xauth config))
-          (startx  (xorg-start-command (slim-configuration-xorg config)))
-          (shepherd   (slim-configuration-shepherd config))
-          (theme-name (slim-configuration-theme-name config))
-          (sessreg (slim-configuration-sessreg config)))
+  (let* ((xinitrc (xinitrc #:fallback-session
+                           (slim-configuration-auto-login-session config)))
+         (xauth   (slim-configuration-xauth config))
+         (startx  (xorg-start-command (slim-configuration-xorg config)))
+         (display (slim-configuration-display config))
+         (vt (slim-configuration-vt config))
+         (shepherd   (slim-configuration-shepherd config))
+         (theme-name (slim-configuration-theme-name config))
+         (sessreg (slim-configuration-sessreg config))
+         (lockfile (string-append "/var/run/slim-" vt ".lock")))
+    (define slim.cfg
       (mixed-text-file "slim.cfg"  "
 default_path /run/current-system/profile/bin
 default_xserver " startx "
-xserver_arguments :0 vt7
+display_name " display "
+xserver_arguments " vt "
 xauth_path " xauth "/bin/xauth
-authfile /var/run/slim.auth
+authfile /var/run/slim-" vt ".auth
+lockfile " lockfile "
+logfile /var/log/slim-" vt ".log
 
 # The login command.  '%session' is replaced by the chosen session name, one
 # of the names specified in the 'sessions' setting: 'wmaker', 'xfce', etc.
@@ -530,32 +542,33 @@ reboot_cmd " shepherd "/sbin/reboot\n"
     "")
 (if theme-name
     (string-append "current_theme " theme-name "\n")
-    ""))))
-
-  (define theme
-    (slim-configuration-theme config))
-
-  (list (shepherd-service
-         (documentation "Xorg display server")
-         (provision '(xorg-server))
-         (requirement '(user-processes host-name udev))
-         (start
-          #~(lambda ()
-              ;; A stale lock file can prevent SLiM from starting, so remove it to
-              ;; be on the safe side.
-              (false-if-exception (delete-file "/var/run/slim.lock"))
-
-              (fork+exec-command
-               (list (string-append #$(slim-configuration-slim config)
-                                    "/bin/slim")
-                     "-nodaemon")
-               #:environment-variables
-               (list (string-append "SLIM_CFGFILE=" #$slim.cfg)
-                     #$@(if theme
-                            (list #~(string-append "SLIM_THEMESDIR=" #$theme))
-                            #~())))))
-         (stop #~(make-kill-destructor))
-         (respawn? #t))))
+    "")))
+
+    (define theme
+      (slim-configuration-theme config))
+
+    (list (shepherd-service
+           (documentation "Xorg display server")
+           (provision (list (symbol-append 'xorg-server-
+                                           (string->symbol vt))))
+           (requirement '(user-processes host-name udev))
+           (start
+            #~(lambda ()
+                ;; A stale lock file can prevent SLiM from starting, so remove it to
+                ;; be on the safe side.
+                (false-if-exception (delete-file lockfile))
+
+                (fork+exec-command
+                 (list (string-append #$(slim-configuration-slim config)
+                                      "/bin/slim")
+                       "-nodaemon")
+                 #:environment-variables
+                 (list (string-append "SLIM_CFGFILE=" #$slim.cfg)
+                       #$@(if theme
+                              (list #~(string-append "SLIM_THEMESDIR=" #$theme))
+                              #~())))))
+           (stop #~(make-kill-destructor))
+           (respawn? #t)))))
 
 (define slim-service-type
   (service-type (name 'slim)
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 3/3] doc: Document 'display' and 'vt' fields of 'slim-configuration'.
  2019-04-29 18:19 [bug#35493] [PATCH 0/3] Allow multiple SLiM services Diego Nicola Barbato
  2019-04-29 18:20 ` [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display Diego Nicola Barbato
  2019-04-29 18:20 ` [bug#35493] [PATCH 2/3] services: slim: Allow SLiM to be started on multiple ttys Diego Nicola Barbato
@ 2019-04-29 18:20 ` Diego Nicola Barbato
  2 siblings, 0 replies; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-04-29 18:20 UTC (permalink / raw)
  To: 35493

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0003-doc-Document-display-and-vt-fields-of-slim-configura.patch --]
[-- Type: text/x-patch, Size: 2578 bytes --]

From a17223fd626d0039259053ff412fbe56c9cdc78c Mon Sep 17 00:00:00 2001
From: Diego Nicola Barbato <dnbarbato@posteo.de>
Date: Sun, 28 Apr 2019 22:18:28 +0200
Subject: [PATCH 3/3] doc: Document 'display' and 'vt' fields of
 'slim-configuration'.

* doc/guix.texi (X Window): Mention 'display' and 'vt' fields of
  'slim-configuration' and explain how to use them to add multiple SLiM
  services to system services.
---
 doc/guix.texi | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 4f43db3443..6ac76c18f6 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -13450,6 +13450,33 @@ This is the type for the SLiM graphical login manager for X11.
 Like GDM, SLiM looks for session types described by @file{.desktop} files and
 allows users to choose a session from the log-in screen using @kbd{F1}.  It
 also honors @file{~/.xsession} files.
+
+Unlike GDM, SLiM does not spawn the user session on a different VT after
+logging in, which means that you can only start one graphical session.  If you
+want to be able to run multiple graphical sessions at the same time you have
+to add multiple SLiM services to your system services.  The following example
+shows how to replace the default GDM service with two SLiM services on tty7
+and tty8.
+
+@lisp
+(use-modules (gnu services)
+             (gnu services desktop)
+             (gnu services xorg)
+             (srfi srfi-1))  ;for 'remove'
+
+(operating-system
+  ;; ...
+  (services (cons* (service slim-service-type (slim-configuration
+                                               (display ":0")
+                                               (vt "vt7")))
+                   (service slim-service-type (slim-configuration
+                                               (display ":1")
+                                               (vt "vt8")))
+                   (remove (lambda (service)
+                             (eq? (service-kind service) gdm-service-type))
+                           %desktop-services))))
+@end lisp
+
 @end defvr
 
 @deftp {Data Type} slim-configuration
@@ -13487,6 +13514,12 @@ false, you will be unable to log in.
 @item @code{xorg-configuration} (default @code{(xorg-configuration)})
 Configuration of the Xorg graphical server.
 
+@item @code{display} (default @code{":0"})
+The display on which to start the Xorg graphical server.
+
+@item @code{vt} (default @code{"vt7"})
+The VT on which to start the Xorg graphical server.
+
 @item @code{xauth} (default: @code{xauth})
 The XAuth package to use.
 
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-04-29 18:20 ` [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display Diego Nicola Barbato
@ 2019-05-06  8:35   ` Ludovic Courtès
  2019-05-06 11:31     ` Diego Nicola Barbato
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2019-05-06  8:35 UTC (permalink / raw)
  To: Diego Nicola Barbato; +Cc: 35493

Hi Diego,

Diego Nicola Barbato <dnbarbato@posteo.de> skribis:

>>From 7e7205e0f8d70a89a7ed4fd4844feb9b6a8d8567 Mon Sep 17 00:00:00 2001
> From: Diego Nicola Barbato <dnbarbato@posteo.de>
> Date: Sun, 28 Apr 2019 14:48:47 +0200
> Subject: [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
>
> Previously SLiM could only start on display :0.0.  This change makes it
> possible to run multiple instances of SLiM at the same time.
>
> * gnu/packages/patches/slim-display.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/display-managers.scm (slim): Use it.

[...]

> +++ b/gnu/packages/patches/slim-display.patch
> @@ -0,0 +1,75 @@
> +Add "display_name" configuration option and use its value instead of
> +the hard coded one.
> +
> +Patch by Diego N. Barbato

The patch LGTM.  Just one question: did you submit it upstream (is there
still an upstream?), and if so, could we add the URL of upstream’s
tracking software or an indication about the submission?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-05-06  8:35   ` Ludovic Courtès
@ 2019-05-06 11:31     ` Diego Nicola Barbato
  2019-05-07 13:55       ` Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-05-06 11:31 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 35493

Hi Ludo,

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

> Hi Diego,
>
> Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
>
>>>From 7e7205e0f8d70a89a7ed4fd4844feb9b6a8d8567 Mon Sep 17 00:00:00 2001
>> From: Diego Nicola Barbato <dnbarbato@posteo.de>
>> Date: Sun, 28 Apr 2019 14:48:47 +0200
>> Subject: [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
>>
>> Previously SLiM could only start on display :0.0.  This change makes it
>> possible to run multiple instances of SLiM at the same time.
>>
>> * gnu/packages/patches/slim-display.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add it.
>> * gnu/packages/display-managers.scm (slim): Use it.
>
> [...]
>
>> +++ b/gnu/packages/patches/slim-display.patch
>> @@ -0,0 +1,75 @@
>> +Add "display_name" configuration option and use its value instead of
>> +the hard coded one.
>> +
>> +Patch by Diego N. Barbato
>
> The patch LGTM.  Just one question: did you submit it upstream (is there
> still an upstream?), and if so, could we add the URL of upstream’s
> tracking software or an indication about the submission?

I did not submit it upstream (the most likely candidate seems to be this
[0] even though it is not mentioned e.g. on the ArchWiki), since the
general consensus seems to be that the project has been abandoned.

Regards,

Diego

[0]: https://github.com/iwamatsu/slim

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-05-06 11:31     ` Diego Nicola Barbato
@ 2019-05-07 13:55       ` Ludovic Courtès
  2019-05-07 21:54         ` Diego Nicola Barbato
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2019-05-07 13:55 UTC (permalink / raw)
  To: Diego Nicola Barbato; +Cc: 35493

Hi Diego,

Diego Nicola Barbato <dnbarbato@posteo.de> skribis:

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

[...]

>> The patch LGTM.  Just one question: did you submit it upstream (is there
>> still an upstream?), and if so, could we add the URL of upstream’s
>> tracking software or an indication about the submission?
>
> I did not submit it upstream (the most likely candidate seems to be this
> [0] even though it is not mentioned e.g. on the ArchWiki), since the
> general consensus seems to be that the project has been abandoned.

That makes sense.

I’ve applied the 3 patches and confirmed that
gnu/system/examples/vm-image.tmpl, which uses SLiM, still works
correctly.

However, I’ve noticed that this changes the name of the default Shepherd
service from ‘xorg-server’ to ‘xorg-server-vt7’.

What about generating an ‘xorg-server’ Shepherd service that would
depend on all the ‘xorg-server-vtN’ services?  Hmm that wouldn’t be
entirely equivalent: ‘herd start xorg-server’ would start them all, but
‘herd stop xorg-server’ wouldn’t stop any of them.

Perhaps a simple fix is, when there’s only one server launched, to have
the Shepherd service provide both ‘xorg-server-vtN’ and ‘xorg-server’.
Or to special-case vt7 such that the service for vt7 provides both
‘xorg-server’ and ‘xorg-server-vt7’.

Thoughts?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-05-07 13:55       ` Ludovic Courtès
@ 2019-05-07 21:54         ` Diego Nicola Barbato
  2019-05-08 10:29           ` Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-05-07 21:54 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 35493

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

Hi Ludo,

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

> Hi Diego,
>
> Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>> The patch LGTM.  Just one question: did you submit it upstream (is there
>>> still an upstream?), and if so, could we add the URL of upstream’s
>>> tracking software or an indication about the submission?
>>
>> I did not submit it upstream (the most likely candidate seems to be this
>> [0] even though it is not mentioned e.g. on the ArchWiki), since the
>> general consensus seems to be that the project has been abandoned.
>
> That makes sense.
>
> I’ve applied the 3 patches and confirmed that
> gnu/system/examples/vm-image.tmpl, which uses SLiM, still works
> correctly.
>
> However, I’ve noticed that this changes the name of the default Shepherd
> service from ‘xorg-server’ to ‘xorg-server-vt7’.
>
> What about generating an ‘xorg-server’ Shepherd service that would
> depend on all the ‘xorg-server-vtN’ services?  Hmm that wouldn’t be
> entirely equivalent: ‘herd start xorg-server’ would start them all, but
> ‘herd stop xorg-server’ wouldn’t stop any of them.
>
> Perhaps a simple fix is, when there’s only one server launched, to have
> the Shepherd service provide both ‘xorg-server-vtN’ and ‘xorg-server’.
> Or to special-case vt7 such that the service for vt7 provides both
> ‘xorg-server’ and ‘xorg-server-vt7’.
>
> Thoughts?

How about changing the default value of vt to #f and having the Shepherd
service provide ‘xorg-server-vtN’ only if vt is set to "vtN" and
‘xorg-server’ otherwise.  That way the name of the default Shepherd
service would not change.

I have attached a patch that does just that.  If we apply it we will
also have to update the documentation to reflect the change of vt's
default value.

Regards,

Diego


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-services-slim-Make-default-Shepherd-service-provide-.patch --]
[-- Type: text/x-patch, Size: 2497 bytes --]

From 14d0106f53ee17aea8f176745bf6dd5d8a748965 Mon Sep 17 00:00:00 2001
From: Diego Nicola Barbato <dnbarbato@posteo.de>
Date: Tue, 7 May 2019 22:22:48 +0200
Subject: [PATCH] services: slim: Make default Shepherd service provide
 'xorg-server'.

Commit 6426a8fb6c changed the name of the default Shepherd service from
'xorg-server' to 'xorg-server-vt7'.  This patch changes it back.

* gnu/services/xorg.scm (define-module): Use module (ice-9 format).
  (<slim-configuration>)[vt]: Change default value to #f.
  (slim-shepherd-service)[shepherd-service][provision]: Provide
  'xorg-server-vtN' if vt is set to "vtN" and 'xorg-server' if vt is #f.
---
 gnu/services/xorg.scm | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 65e9d48915..81e38665c9 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -52,6 +52,7 @@
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 format)
   #:export (xorg-configuration
             xorg-configuration?
             xorg-configuration-modules
@@ -493,7 +494,7 @@ desktop session from the system or user profile will be used."
   (display slim-configuration-display
            (default ":0"))
   (vt slim-configuration-vt
-      (default "vt7"))
+      (default #f))
   (sessreg slim-configuration-sessreg
            (default sessreg)))
 
@@ -510,7 +511,8 @@ desktop session from the system or user profile will be used."
          (xauth   (slim-configuration-xauth config))
          (startx  (xorg-start-command (slim-configuration-xorg config)))
          (display (slim-configuration-display config))
-         (vt (slim-configuration-vt config))
+         (vt? (slim-configuration-vt config))
+         (vt (or vt? "vt7"))
          (shepherd   (slim-configuration-shepherd config))
          (theme-name (slim-configuration-theme-name config))
          (sessreg (slim-configuration-sessreg config))
@@ -549,8 +551,8 @@ reboot_cmd " shepherd "/sbin/reboot\n"
 
     (list (shepherd-service
            (documentation "Xorg display server")
-           (provision (list (symbol-append 'xorg-server-
-                                           (string->symbol vt))))
+           (provision (list (string->symbol
+                             (format #f "xorg-server~@[-~a~]" vt?))))
            (requirement '(user-processes host-name udev))
            (start
             #~(lambda ()
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-05-07 21:54         ` Diego Nicola Barbato
@ 2019-05-08 10:29           ` Ludovic Courtès
  2019-05-08 20:12             ` Diego Nicola Barbato
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2019-05-08 10:29 UTC (permalink / raw)
  To: Diego Nicola Barbato; +Cc: 35493

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

Hi,

Diego Nicola Barbato <dnbarbato@posteo.de> skribis:

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

[...]

>> Perhaps a simple fix is, when there’s only one server launched, to have
>> the Shepherd service provide both ‘xorg-server-vtN’ and ‘xorg-server’.
>> Or to special-case vt7 such that the service for vt7 provides both
>> ‘xorg-server’ and ‘xorg-server-vt7’.
>>
>> Thoughts?
>
> How about changing the default value of vt to #f and having the Shepherd
> service provide ‘xorg-server-vtN’ only if vt is set to "vtN" and
> ‘xorg-server’ otherwise.  That way the name of the default Shepherd
> service would not change.

I think I would prefer special-casing vt7, as shown below.  That way the
‘vt’ field would always be a string, which looks more consistent to me.

Thoughts?

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 785 bytes --]

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 65e9d48915..f7c8684738 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -549,8 +549,11 @@ reboot_cmd " shepherd "/sbin/reboot\n"
 
     (list (shepherd-service
            (documentation "Xorg display server")
-           (provision (list (symbol-append 'xorg-server-
-                                           (string->symbol vt))))
+           (provision (cons (symbol-append 'xorg-server-
+                                           (string->symbol vt))
+                            (if (string=? vt "vt7")
+                                '(xorg-server)
+                                '())))
            (requirement '(user-processes host-name udev))
            (start
             #~(lambda ()

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-05-08 10:29           ` Ludovic Courtès
@ 2019-05-08 20:12             ` Diego Nicola Barbato
  2019-05-09 10:19               ` bug#35493: " Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Diego Nicola Barbato @ 2019-05-08 20:12 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 35493

Hello,

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

> Hi,
>
> Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>> Perhaps a simple fix is, when there’s only one server launched, to have
>>> the Shepherd service provide both ‘xorg-server-vtN’ and ‘xorg-server’.
>>> Or to special-case vt7 such that the service for vt7 provides both
>>> ‘xorg-server’ and ‘xorg-server-vt7’.
>>>
>>> Thoughts?
>>
>> How about changing the default value of vt to #f and having the Shepherd
>> service provide ‘xorg-server-vtN’ only if vt is set to "vtN" and
>> ‘xorg-server’ otherwise.  That way the name of the default Shepherd
>> service would not change.
>
> I think I would prefer special-casing vt7, as shown below.  That way the
> ‘vt’ field would always be a string, which looks more consistent to me.
>
> Thoughts?

I have no strong feelings either way.  If it is OK that 'herd status'
displays 'xorg-server-vt7' instead of 'xorg-server' we can go for the
special-casing approach.

Regards,

Diego

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#35493: [PATCH 1/3] gnu: slim: Allow customisation of Xorg display.
  2019-05-08 20:12             ` Diego Nicola Barbato
@ 2019-05-09 10:19               ` Ludovic Courtès
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2019-05-09 10:19 UTC (permalink / raw)
  To: Diego Nicola Barbato; +Cc: 35493-done

Hi,

Diego Nicola Barbato <dnbarbato@posteo.de> skribis:

> I have no strong feelings either way.  If it is OK that 'herd status'
> displays 'xorg-server-vt7' instead of 'xorg-server' we can go for the
> special-casing approach.

OK, done in 6ad799d7c5c14b1bbf1e3db7198b312965d3b786.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-09 10:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 18:19 [bug#35493] [PATCH 0/3] Allow multiple SLiM services Diego Nicola Barbato
2019-04-29 18:20 ` [bug#35493] [PATCH 1/3] gnu: slim: Allow customisation of Xorg display Diego Nicola Barbato
2019-05-06  8:35   ` Ludovic Courtès
2019-05-06 11:31     ` Diego Nicola Barbato
2019-05-07 13:55       ` Ludovic Courtès
2019-05-07 21:54         ` Diego Nicola Barbato
2019-05-08 10:29           ` Ludovic Courtès
2019-05-08 20:12             ` Diego Nicola Barbato
2019-05-09 10:19               ` bug#35493: " Ludovic Courtès
2019-04-29 18:20 ` [bug#35493] [PATCH 2/3] services: slim: Allow SLiM to be started on multiple ttys Diego Nicola Barbato
2019-04-29 18:20 ` [bug#35493] [PATCH 3/3] doc: Document 'display' and 'vt' fields of 'slim-configuration' Diego Nicola Barbato

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).