GNU bug report logs - #77266
[PATCH] gnu: Merge xorg configurations when extending.

Previous Next

To reply to this bug, email your comments to 77266 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: guix-patches <at> gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH] gnu: Merge xorg configurations when extending.
Date: Tue, 25 Mar 2025 21:23:54 -0700

Configuration for xorg is embedded in the various display-manager
configuration records, and extension support is factored out into the
`handle-xorg-configuration' macro.  However, the extension mechanism replaces
the existing xorg-configuration with the supplied one, making it impossible to
compose configuration from multiple sources.  This patch adds a procedure to
merge two xorg-configuration records, and calls it within
handle-xorg-configuration, allowing the config to be built piecemeal.

* gnu/services/xorg.scm (merge-xorg-configurations): New variable.
(handle-xorg-configuration): Merge xorg configs.

Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
---
 gnu/services/xorg.scm | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index bef05b9bb9..dbd1513cc8 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -209,6 +209,34 @@ (define-record-type* <xorg-configuration>
   (server-arguments xorg-configuration-server-arguments ;list of strings
                     (default %default-xorg-server-arguments)))
 
+(define (merge-xorg-configurations a b)
+  (let ((configs (list a b)))
+    (xorg-configuration
+     (modules
+      (delete-duplicates (append-map xorg-configuration-modules configs)))
+     (fonts
+      (delete-duplicates (append-map xorg-configuration-fonts configs)))
+     (drivers
+      (delete-duplicates (append-map xorg-configuration-drivers configs)))
+     (resolutions
+      (delete-duplicates (append-map xorg-configuration-resolutions configs)))
+     (extra-config (append-map xorg-configuration-extra-config configs))
+     ;; Prefer the more recently set layout.
+     (keyboard-layout (or (xorg-configuration-keyboard-layout b)
+                          (xorg-configuration-keyboard-layout a)
+                          #f))
+     (server
+      ;; Prefer the non-default server.
+      (if (eq? xorg-server (xorg-configuration-server a))
+          (xorg-configuration-server b)
+          (xorg-configuration-server a)))
+     (server-arguments
+      ;; Prefer the non-default arguments.
+      (if (eq? %default-xorg-server-arguments
+             (xorg-configuration-server-arguments a))
+          (xorg-configuration-server-arguments b)
+          (xorg-configuration-server-arguments a))))))
+
 (define (xorg-configuration->file config)
   "Compute an Xorg configuration file corresponding to CONFIG, an
 <xorg-configuration> record."
@@ -628,10 +656,7 @@ (define-syntax handle-xorg-configuration
     ((_ configuration-record service-type-definition)
      (service-type
        (inherit service-type-definition)
-       (compose (lambda (extensions)
-                  (match extensions
-                    (() #f)
-                    ((config . _) config))))
+       (compose (cut reduce merge-xorg-configurations #f <>))
        (extend (lambda (config xorg-configuration)
                  (if xorg-configuration
                      (configuration-record
-- 
2.48.1

Message #8 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Wed, 26 Mar 2025 18:16:01 +0100

Hi!

Ian Eure <ian <at> retrospec.tv> skribis:

> Configuration for xorg is embedded in the various display-manager
> configuration records, and extension support is factored out into the
> `handle-xorg-configuration' macro.  However, the extension mechanism replaces
> the existing xorg-configuration with the supplied one, making it impossible to
> compose configuration from multiple sources.  This patch adds a procedure to
> merge two xorg-configuration records, and calls it within
> handle-xorg-configuration, allowing the config to be built piecemeal.
>
> * gnu/services/xorg.scm (merge-xorg-configurations): New variable.
> (handle-xorg-configuration): Merge xorg configs.
>
> Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba

Maybe you can add a word in the relevant part of the manual to give an
idea of how configs are merged.

Apart from, it LGTM.  I can’t think of a scenario where it would break
existing config or where the current setup is preferable.

Thanks!

Ludo’.

Message #11 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Rutherther <rutherther <at> ditigal.xyz>
To: 77266 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Ian Eure <ian <at> retrospec.tv>
Subject: Re: [PATCH] gnu: Merge xorg configurations when extending.
Date: Wed, 26 Mar 2025 18:31:46 +0100

Hello, just wondering, wouldn't it make sense to inherit from the
previous a or b in case someone added a field to xorg-configuration and
forgot to change this service?

Or is there some kind of a smart way how we can in Guix ensure stuff
like that is properly checked? I can see someone forgetting about stuff
like that...

Rutherther

Message #14 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Rutherther <rutherther <at> ditigal.xyz>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 77266 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: Merge xorg configurations when extending.
Date: Wed, 26 Mar 2025 12:15:03 -0700

Hi Rutherther,

Rutherther <rutherther <at> ditigal.xyz> writes:

> Hello, just wondering, wouldn't it make sense to inherit from 
> the
> previous a or b in case someone added a field to 
> xorg-configuration and
> forgot to change this service?

I considered using inherit, but since every field has to be set to 
the combined values of both records, saw no value in doing so.  In 
the case you outline, both inheriting and not will produce an 
incorrect result -- inheriting will discard all but one 
user-specified’s value, while not inheriting discards all 
user-specified value and uses the default.  Both are wrong, but I 
think the latter is more likely to get noticed (therefore fixed).

Thanks,
 -- Ian

Message #17 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Wed, 26 Mar 2025 12:15:46 -0700

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

> Hi!
>
> Ian Eure <ian <at> retrospec.tv> skribis:
>
>> Configuration for xorg is embedded in the various 
>> display-manager
>> configuration records, and extension support is factored out 
>> into the
>> `handle-xorg-configuration' macro.  However, the extension 
>> mechanism replaces
>> the existing xorg-configuration with the supplied one, making 
>> it impossible to
>> compose configuration from multiple sources.  This patch adds a 
>> procedure to
>> merge two xorg-configuration records, and calls it within
>> handle-xorg-configuration, allowing the config to be built 
>> piecemeal.
>>
>> * gnu/services/xorg.scm (merge-xorg-configurations): New 
>> variable.
>> (handle-xorg-configuration): Merge xorg configs.
>>
>> Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
>
> Maybe you can add a word in the relevant part of the manual to 
> give an
> idea of how configs are merged.

Definitely, do you want a look at the wording, or should I write 
that up and merge?

Will send a v2 later today.

Thanks,
 -- Ian

Message #20 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v2] gnu: Merge xorg configurations when extending.
Date: Wed, 26 Mar 2025 16:59:32 -0700

Configuration for xorg is embedded in the various display-manager
configuration records, and extension support is factored out into the
`handle-xorg-configuration' macro.  However, the extension mechanism replaces
the existing xorg-configuration with the supplied one, making it impossible to
compose configuration from multiple sources.  This patch adds a procedure to
merge two xorg-configuration records, and calls it within
handle-xorg-configuration, allowing the config to be built piecemeal.

* gnu/services/xorg.scm (merge-xorg-configurations): New variable.
(handle-xorg-configuration): Merge xorg configs.
* doc/guix.texi (X Window): Document xorg-configuration composition.

Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
---
 doc/guix.texi         | 34 ++++++++++++++++++++++++++++++++--
 gnu/services/xorg.scm | 33 +++++++++++++++++++++++++++++----
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 730fb45798..1dedf81715 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -138,6 +138,7 @@ Copyright @copyright{} 2024 45mg@*
 Copyright @copyright{} 2025 Sören Tempel@*
 Copyright @copyright{} 2025 Rostislav Svoboda@*
 Copyright @copyright{} 2025 Zacchaeus@*
+Copyright @copyright{} 2025 Ian Eure@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -24865,8 +24866,37 @@ Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an @code{<xorg-configuration>} record.
 
 Since the Xorg configuration is embedded in the log-in manager's
-configuration---e.g., @code{gdm-configuration}---this procedure provides a
-shorthand to set the Xorg configuration.
+configuration---e.g., @code{gdm-configuration}---this procedure provides
+a shorthand to set the Xorg configuration.
+
+Note that @code{set-xorg-configuration} can only be used once, and must
+provide a complete configuration.  If you know the service-type of the
+log-in manager, you can compose a configuration from smaller pieces:
+
+@lisp
+(define %xorg-intel-service
+  (simple-service
+  'xorg-intel
+  gdm-service-type
+  (xorg-configuration
+   (modules (list xf86-video-intel))
+   (drivers '("intel")))))
+
+(define %xorg-keyboard-service
+  (simple-service
+   'xorg-keyboard
+   gdm-service-type
+   (xorg-configuration (keyboard-layout keyboard-layouut))))
+
+(operating-system
+  (services
+   (cons*
+    (service gdm-service-type)
+    %xorg-intel-service
+    %xorg-keyboard-service
+    %base-services))
+  ...)
+@end lisp
 @end deffn
 
 @deffn {Procedure} xorg-start-command [config]
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index bef05b9bb9..c2e34bc10c 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -209,6 +209,34 @@ (define-record-type* <xorg-configuration>
   (server-arguments xorg-configuration-server-arguments ;list of strings
                     (default %default-xorg-server-arguments)))
 
+(define (merge-xorg-configurations a b)
+  (let ((configs (list b a)))           ; Prefer later configurations.
+    (xorg-configuration
+     (modules
+      (delete-duplicates (append-map xorg-configuration-modules configs)))
+     (fonts
+      (delete-duplicates (append-map xorg-configuration-fonts configs)))
+     (drivers
+      (delete-duplicates (append-map xorg-configuration-drivers configs)))
+     (resolutions
+      (delete-duplicates (append-map xorg-configuration-resolutions configs)))
+     (extra-config (append-map xorg-configuration-extra-config configs))
+     ;; Prefer the more recently set layout.
+     (keyboard-layout (or (xorg-configuration-keyboard-layout b)
+                          (xorg-configuration-keyboard-layout a)
+                          #f))
+     (server
+      ;; Prefer the non-default server.
+      (if (eq? xorg-server (xorg-configuration-server a))
+          (xorg-configuration-server b)
+          (xorg-configuration-server a)))
+     (server-arguments
+      ;; Prefer the non-default arguments.
+      (if (eq? %default-xorg-server-arguments
+             (xorg-configuration-server-arguments a))
+          (xorg-configuration-server-arguments b)
+          (xorg-configuration-server-arguments a))))))
+
 (define (xorg-configuration->file config)
   "Compute an Xorg configuration file corresponding to CONFIG, an
 <xorg-configuration> record."
@@ -628,10 +656,7 @@ (define-syntax handle-xorg-configuration
     ((_ configuration-record service-type-definition)
      (service-type
        (inherit service-type-definition)
-       (compose (lambda (extensions)
-                  (match extensions
-                    (() #f)
-                    ((config . _) config))))
+       (compose (cut reduce merge-xorg-configurations #f <>))
        (extend (lambda (config xorg-configuration)
                  (if xorg-configuration
                      (configuration-record
-- 
2.48.1

Message #23 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org, Rutherther <rutherther <at> ditigal.xyz>, Ludovic
 Courtès <ludo <at> gnu.org>
Subject: Re: [bug#77266] [PATCH v2] gnu: Merge xorg configurations when
 extending.
Date: Mon, 14 Apr 2025 15:20:49 -0700

Any other feedback on this?  Does the manual wording look good?

 -- Ian

Message #26 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> chbouib.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: bug#77266: [PATCH] gnu: Merge xorg configurations when extending.
Date: Tue, 15 Apr 2025 13:20:23 +0200

Hello Ian,

Ian Eure <ian <at> retrospec.tv> writes:

> Configuration for xorg is embedded in the various display-manager
> configuration records, and extension support is factored out into the
> `handle-xorg-configuration' macro.  However, the extension mechanism replaces
> the existing xorg-configuration with the supplied one, making it impossible to
> compose configuration from multiple sources.  This patch adds a procedure to
> merge two xorg-configuration records, and calls it within
> handle-xorg-configuration, allowing the config to be built piecemeal.
>
> * gnu/services/xorg.scm (merge-xorg-configurations): New variable.
> (handle-xorg-configuration): Merge xorg configs.
> * doc/guix.texi (X Window): Document xorg-configuration composition.
>
> Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
> +@lisp
> +(define %xorg-intel-service
> +  (simple-service
> +  'xorg-intel
> +  gdm-service-type
> +  (xorg-configuration
> +   (modules (list xf86-video-intel))
> +   (drivers '("intel")))))
> +
> +(define %xorg-keyboard-service
> +  (simple-service
> +   'xorg-keyboard
> +   gdm-service-type
> +   (xorg-configuration (keyboard-layout keyboard-layouut))))

Could you fix the indentation and add short comments above the
definitions?

> +(operating-system
> +  (services
> +   (cons*
> +    (service gdm-service-type)
> +    %xorg-intel-service

Rather:

  (append (list (service gdm-service-type)
                %xorg-intel-service
                …)
          @dots{})

> +(define (merge-xorg-configurations a b)
> +  (let ((configs (list b a)))           ; Prefer later configurations.
> +    (xorg-configuration
> +     (modules
> +      (delete-duplicates (append-map xorg-configuration-modules configs)))
> +     (fonts
> +      (delete-duplicates (append-map xorg-configuration-fonts configs)))
> +     (drivers
> +      (delete-duplicates (append-map xorg-configuration-drivers configs)))
> +     (resolutions
> +      (delete-duplicates (append-map xorg-configuration-resolutions configs)))

Something I had overlooked: this assumes that elements of this list can
be compared with ‘equal?’.  Typically, these are <file-append> records
that refer to <package> records, and those cannot be usefully compared
with ‘equal?’ (it’s effectively equivalent to ‘eq?’ in the case of
<package> records, but more expensive).

So, what about removing ‘delete-duplicates’ from here, and instead…

>  (define (xorg-configuration->file config)
>    "Compute an Xorg configuration file corresponding to CONFIG, an

… adding ‘delete-duplicates’ calls in here, on the build side.

WDYT?

Thanks,
Ludo’.

Message #29 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> chbouib.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Tue, 15 Apr 2025 18:08:10 -0700

Ludovic Courtès <ludo <at> chbouib.org> writes:

> Hello Ian,
>
> Ian Eure <ian <at> retrospec.tv> writes:
>
>> +(define %xorg-keyboard-service
>> +  (simple-service
>> +   'xorg-keyboard
>> +   gdm-service-type
>> +   (xorg-configuration (keyboard-layout keyboard-layouut))))
>
> Could you fix the indentation and add short comments above the
> definitions?

Definitely.

>> +(operating-system
>> +  (services
>> +   (cons*
>> +    (service gdm-service-type)
>> +    %xorg-intel-service
>
> Rather:
>
>   (append (list (service gdm-service-type)
>                 %xorg-intel-service
>                 …)
>           @dots{})

Can make these changes, but I’m curious why (append (list ...) 
...) is preferred over cons*.  I default to the latter, because I 
like the removal of the extra level of indent.

> Something I had overlooked: this assumes that elements of this 
> list can
> be compared with ‘equal?’.  Typically, these are <file-append> 
> records
> that refer to <package> records, and those cannot be usefully 
> compared
> with ‘equal?’ (it’s effectively equivalent to ‘eq?’ in the case 
> of
> <package> records, but more expensive).
>
> So, what about removing ‘delete-duplicates’ from here, and 
> instead…
>
>>  (define (xorg-configuration->file config)
>>    "Compute an Xorg configuration file corresponding to CONFIG, 
>>    an
>
> … adding ‘delete-duplicates’ calls in here, on the build side.

Good catch, makes sense to me.  Will send a v3.

Thanks,
 -- Ian

Message #32 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> chbouib.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Wed, 16 Apr 2025 16:07:38 +0200

Hi,

Ian Eure <ian <at> retrospec.tv> writes:

>>   (append (list (service gdm-service-type)
>>                 %xorg-intel-service
>>                 …)
>>           @dots{})
>
> Can make these changes, but I’m curious why (append (list ...) ...) is
> preferred over cons*.  I default to the latter, because I like the
> removal of the extra level of indent.

In most of the examples in the documentation, we went for ‘append’
because it less obscure to a newcomer than ‘cons*’ (or ‘cons’, even).

That’s also what the installer generates, and ‘guix home import’ I
believe.

Ludo’.

Message #35 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Rutherther <rutherther <at> ditigal.xyz>
To: Ian Eure <ian <at> retrospec.tv>, 77266 <at> debbugs.gnu.org, Ludovic
 Courtès <ludo <at> gnu.org>
Subject: Re: [bug#77266] [PATCH v2] gnu: Merge xorg configurations when
 extending.
Date: Thu, 17 Apr 2025 09:10:21 +0200

Hi,

Ian Eure <ian <at> retrospec.tv> writes:

> Any other feedback on this?  Does the manual wording look good?

I am wondering about the "and must provide a complete configuration."
part in documentation. Is that really so after this patch? You can still
extend with other services, no? So it doesn't seem right to me it would
be necessary to use only set-xorg-configuration, it can be combined with
custom service that will append parts of the config.
At least if I am not missing anything here.

Additionally, I am wondering why do we have that limitation of just one
usage of set-xorg-configuration. I suppose the name 'set-xorg-configuration'
implies you set it, not append it, etc., but that's not really true
after changes from this patch. The limitation comes from the name of the service
that is created. So why not allow a new key/optional argument to set name of
the service, so that it can be used multiple times, and default it to
'set-xorg-configuration? On the other hand this is probably not so
important, I personally don't really see any gain in this
set-xorg-configuration when user's can just extend the appropriate
service instead, so it doesn't seem to me that big of a deal to change it.

Best regards,
Rutherther

Message #38 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Rutherther <rutherther <at> ditigal.xyz>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v2] gnu: Merge xorg configurations when
 extending.
Date: Sat, 19 Apr 2025 12:07:42 -0700

Hi Rutherther,

Rutherther <rutherther <at> ditigal.xyz> writes:

> Hi,
>
> Ian Eure <ian <at> retrospec.tv> writes:
>
>> Any other feedback on this?  Does the manual wording look good?
>
> I am wondering about the "and must provide a complete 
> configuration."
> part in documentation. Is that really so after this patch? You 
> can still
> extend with other services, no? So it doesn't seem right to me 
> it would
> be necessary to use only set-xorg-configuration, it can be 
> combined with
> custom service that will append parts of the config.
> At least if I am not missing anything here.
>
> Additionally, I am wondering why do we have that limitation of 
> just one
> usage of set-xorg-configuration. I suppose the name 
> 'set-xorg-configuration'
> implies you set it, not append it, etc., but that's not really 
> true
> after changes from this patch. The limitation comes from the 
> name of the service
> that is created.

You can mix `set-xorg-configuration' and service extensions, but 
cannot call `set-xorg-configuration' more than once.  As your 
note, it has a fixed service name, and having more than one 
service with the same name causes an error.

> So why not allow a new key/optional argument to set name of
> the service, so that it can be used multiple times, and default 
> it to
> 'set-xorg-configuration? On the other hand this is probably not 
> so
> important, I personally don't really see any gain in this
> set-xorg-configuration when user's can just extend the 
> appropriate
> service instead, so it doesn't seem to me that big of a deal to 
> change it.

I agree with the latter half of your message: allowing a service 
name argument makes `set-xorg-configuration' basically the same as 
writing a `simple-service' definition.  I don’t think it’s worth 
doing.

I’ll clarify the docs and send a v3.

A thing I dislike about all this stuff is how the display managers 
carry the xorg configuration, vs. having an xorg service which the 
DMs depend on.  I suppose it’s the way it is because the DMs spawn 
the X server process, but it feels like it should be possible to 
disentangle things at least a bit more than they are now.  The 
current setup also seems to preclude usecases like running gdm 
locally, while using sddm as an XDMCP greeter for other systems.

Thanks,

 -- Ian

Message #41 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v3 1/2] gnu: Merge xorg configurations when extending.
Date: Sat, 19 Apr 2025 23:21:20 -0700

Configuration for xorg is embedded in the various display-manager
configuration records, and extension support is factored out into the
`handle-xorg-configuration' macro.  However, the extension mechanism replaces
the existing xorg-configuration with the supplied one, making it impossible to
compose configuration from multiple sources.  This patch adds a procedure to
merge two xorg-configuration records, and calls it within
handle-xorg-configuration, allowing the config to be built piecemeal.

* gnu/services/xorg.scm (merge-xorg-configurations): New variable.
(handle-xorg-configuration): Merge xorg configs.
* doc/guix.texi (X Window): Document xorg-configuration composition.

Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
---
 doc/guix.texi         | 49 +++++++++++++++++++++++++++++++++++++++++--
 gnu/services/xorg.scm | 42 ++++++++++++++++++++++++++++++-------
 2 files changed, 82 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index eebc1a1590..02150eb497 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -138,6 +138,7 @@ Copyright @copyright{} 2024 45mg@*
 Copyright @copyright{} 2025 Sören Tempel@*
 Copyright @copyright{} 2025 Rostislav Svoboda@*
 Copyright @copyright{} 2025 Zacchaeus@*
+Copyright @copyright{} 2025 Ian Eure@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -24916,8 +24917,52 @@ Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an @code{<xorg-configuration>} record.
 
 Since the Xorg configuration is embedded in the log-in manager's
-configuration---e.g., @code{gdm-configuration}---this procedure provides a
-shorthand to set the Xorg configuration.
+configuration---e.g., @code{gdm-configuration}---this procedure provides
+a shorthand to set the Xorg configuration.
+
+In addition to @code{set-xorg-configuration}, you may extend your log-in
+manager’s service-type to provide reusable, partial configurations,
+which are concatenated to create the final Xorg configuration:
+
+@lisp
+;; A service which configures Intel video drivers.
+(define %xorg-intel-service
+  (simple-service
+   'xorg-intel
+   gdm-service-type
+   (xorg-configuration
+    (modules (list xf86-video-intel))
+    (drivers '("intel"))
+    (extra-config
+    "
+    Section \"Device\"
+        Identifier \"Intel GPU\"
+        Driver \"intel\"
+        Option \"TearFree\" \"true\"
+    EndSection"))))
+
+;; A service which configures the keyboard.
+(define %xorg-keyboard-service
+  (simple-service
+   'xorg-keyboard
+   gdm-service-type
+   (xorg-configuration
+    (keyboard-layout
+     (keyboard-layout "us" #:options '("ctrl:nocaps"))))))
+
+(operating-system
+  (services
+   (append
+       (list (service gdm-service-type)
+          %xorg-intel-service
+          %xorg-keyboard-service)
+    %base-services))
+  @dots{})
+@end lisp
+
+Service extension and @code{set-xorg-configuration} can each be used
+seperately, or in conjunction.
+
 @end deffn
 
 @deffn {Procedure} xorg-start-command [config]
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index bef05b9bb9..cb933693b7 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -209,6 +209,34 @@ (define-record-type* <xorg-configuration>
   (server-arguments xorg-configuration-server-arguments ;list of strings
                     (default %default-xorg-server-arguments)))
 
+(define (merge-xorg-configurations a b)
+  (let ((configs (list b a)))           ; Prefer later configurations.
+    (xorg-configuration
+     (modules
+      (append-map xorg-configuration-modules configs))
+     (fonts
+      (append-map xorg-configuration-fonts configs))
+     (drivers
+      (append-map xorg-configuration-drivers configs))
+     (resolutions
+      (append-map xorg-configuration-resolutions configs))
+     (extra-config (append-map xorg-configuration-extra-config configs))
+     ;; Prefer the more recently set layout.
+     (keyboard-layout (or (xorg-configuration-keyboard-layout b)
+                          (xorg-configuration-keyboard-layout a)
+                          #f))
+     (server
+      ;; Prefer the non-default server.
+      (if (eq? xorg-server (xorg-configuration-server a))
+          (xorg-configuration-server b)
+          (xorg-configuration-server a)))
+     (server-arguments
+      ;; Prefer the non-default arguments.
+      (if (eq? %default-xorg-server-arguments
+             (xorg-configuration-server-arguments a))
+          (xorg-configuration-server-arguments b)
+          (xorg-configuration-server-arguments a))))))
+
 (define (xorg-configuration->file config)
   "Compute an Xorg configuration file corresponding to CONFIG, an
 <xorg-configuration> record."
@@ -334,9 +362,12 @@ (define (expand modules)
                            port)
                   (newline port)))
 
-              (for-each (lambda (config)
-                          (display config port))
-                        '#$(xorg-configuration-extra-config config))))))
+              (for-each
+               (lambda (config)
+                 (display config port)
+                 (newline port))
+               (delete-duplicates
+                '#$(xorg-configuration-extra-config config)))))))
 
     (computed-file "xserver.conf" build)))
 
@@ -628,10 +659,7 @@ (define-syntax handle-xorg-configuration
     ((_ configuration-record service-type-definition)
      (service-type
        (inherit service-type-definition)
-       (compose (lambda (extensions)
-                  (match extensions
-                    (() #f)
-                    ((config . _) config))))
+       (compose (cut reduce merge-xorg-configurations #f <>))
        (extend (lambda (config xorg-configuration)
                  (if xorg-configuration
                      (configuration-record
-- 
2.49.0

Message #44 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v3 0/2] gnu: Merge xorg configurations when extending.
Date: Sat, 19 Apr 2025 23:21:19 -0700

I reworded the documentation, and improved the example.

While I was in here, I also fixed the TODO on `set-xorg-configuration'.
Rather than assuming gdm on x86 and sddm on everything else, it now looks up
the service-type containing xorg configuration with
`desktop-services-for-system'.

Ian Eure (2):
  gnu: Merge xorg configurations when extending.
  gnu: set-xorg-configuration: Be smarter finding the display-manager.

 doc/guix.texi            | 49 +++++++++++++++++++++++++++--
 gnu/services/desktop.scm |  1 +
 gnu/services/xorg.scm    | 67 +++++++++++++++++++++++++++++++---------
 3 files changed, 101 insertions(+), 16 deletions(-)

-- 
2.49.0

Message #47 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v3 2/2] gnu: set-xorg-configuration: Be smarter finding the
 display-manager.
Date: Sat, 19 Apr 2025 23:21:21 -0700

* gnu/services/desktop.scm (desktop-services-for-system): Export.
* gnu/services/desktop.scm (set-xorg-configuration): Look for a
display-manager service in (desktop-services-for-system).
* gnu/services/desktop.scm (set-xorg-configuration): Remove TODO comment.

Change-Id: Iceda2d13d16435517cd92bcb49e6937ed096f392
---
 gnu/services/desktop.scm |  1 +
 gnu/services/xorg.scm    | 25 ++++++++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index fe034cfa8f..f7fb3c5bf0 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -199,6 +199,7 @@ (define-module (gnu services desktop)
             seatd-configuration
             seatd-service-type
 
+            desktop-services-for-system
             %desktop-services))
 
 ;;; Commentary:
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index cb933693b7..4aec265098 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -16,6 +16,7 @@
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Zheng Junjie <873216071 <at> qq.com>
 ;;; Copyright © 2024 Tomas Volf <~@wolfsden.cz>
+;;; Copyright © 2025 Ian Eure <ian <at> retrospec.tv>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -43,6 +44,7 @@ (define-module (gnu services xorg)
   #:use-module (gnu system privilege)
   #:use-module (gnu services base)
   #:use-module (gnu services dbus)
+  #:use-module (gnu services desktop)
   #:use-module (gnu packages base)
   #:use-module (gnu packages guile)
   #:use-module (gnu packages xorg)
@@ -139,6 +141,7 @@ (define-module (gnu services xorg)
             gdm-configuration
             gdm-service-type
 
+            xorg-configuration-service-type
             handle-xorg-configuration
             set-xorg-configuration))
 
@@ -1391,16 +1394,24 @@ (define gdm-service-type
                    "Run the GNOME Desktop Manager (GDM), a program that allows
 you to log in in a graphical session, whether or not you use GNOME."))))
 
-;; Since GDM depends on Rust and Rust is not available on all platforms,
-;; use SDDM as the fall-back display manager.
-;; TODO: Switch the condition to take into account if Rust is supported and
-;; match the configuration in desktop-services-for-system.
+(define (xorg-configuration-service? s)
+  "Returns #t if @var{s} is a service carrying xorg configuration."
+  (let ((val (service-value s)))
+    (and (record? val)
+         (memq 'xorg-configuration
+                 (record-type-fields (record-type-descriptor val))))))
+
+(define (xorg-configuration-service-type services)
+  "Return the service type of the first service in @var{services}
+which carries xorg configuration. "
+  (let ((lmst (find xorg-configuration-service? services)))
+    (when lmst (service-kind lmst))))
+
 (define* (set-xorg-configuration config
                                  #:optional
                                  (login-manager-service-type
-                                  (if (target-x86-64?)
-                                      gdm-service-type
-                                      sddm-service-type)))
+                                  (xorg-configuration-service-type
+                                   (desktop-services-for-system))))
   "Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an <xorg-configuration> record."
   (simple-service 'set-xorg-configuration
-- 
2.49.0

Message #50 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v3 1/2] gnu: Merge xorg configurations when
 extending.
Date: Tue, 22 Apr 2025 00:12:42 +0200

Hi,

Ian Eure <ian <at> retrospec.tv> writes:

> Configuration for xorg is embedded in the various display-manager
> configuration records, and extension support is factored out into the
> `handle-xorg-configuration' macro.  However, the extension mechanism replaces
> the existing xorg-configuration with the supplied one, making it impossible to
> compose configuration from multiple sources.  This patch adds a procedure to
> merge two xorg-configuration records, and calls it within
> handle-xorg-configuration, allowing the config to be built piecemeal.
>
> * gnu/services/xorg.scm (merge-xorg-configurations): New variable.
> (handle-xorg-configuration): Merge xorg configs.
> * doc/guix.texi (X Window): Document xorg-configuration composition.
>
> Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
> +   (append
> +       (list (service gdm-service-type)
> +          %xorg-intel-service
> +          %xorg-keyboard-service)

Nitpick: indentation is off.  :-)

>  (define (xorg-configuration->file config)
>    "Compute an Xorg configuration file corresponding to CONFIG, an
>  <xorg-configuration> record."
> @@ -334,9 +362,12 @@ (define (expand modules)
>                             port)
>                    (newline port)))
>  
> -              (for-each (lambda (config)
> -                          (display config port))
> -                        '#$(xorg-configuration-extra-config config))))))
> +              (for-each
> +               (lambda (config)
> +                 (display config port)
> +                 (newline port))
> +               (delete-duplicates
> +                '#$(xorg-configuration-extra-config config)))))))

I think ‘delete-duplicates’ should be used for drivers, modules, and
fonts, but not for ‘extra-config’ (which is merely a string).

Does that make sense?

Thanks,
Ludo’.

Message #53 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v3 1/2] gnu: Merge xorg configurations when
 extending.
Date: Mon, 21 Apr 2025 17:15:31 -0700

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

> Hi,
>
> Ian Eure <ian <at> retrospec.tv> writes:
>
>> Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
>> +   (append
>> +       (list (service gdm-service-type)
>> +          %xorg-intel-service
>> +          %xorg-keyboard-service)
>
> Nitpick: indentation is off.  :-)

Do you have / know of a setup for editing Guile inside the 
documentation?  I’ve been editing in a *scheme* scratch buffer in 
Emacs and pasting back into the docs, but this clearly doesn’t 
work very well.


> I think ‘delete-duplicates’ should be used for drivers, modules, 
> and
> fonts, but not for ‘extra-config’ (which is merely a string).
>
> Does that make sense?

Makes perfect sense, updated patch coming soon.

Thanks,
 -- Ian

Message #56 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v4 1/2] gnu: Merge xorg configurations when extending.
Date: Mon, 21 Apr 2025 17:18:43 -0700

Configuration for xorg is embedded in the various display-manager
configuration records, and extension support is factored out into the
`handle-xorg-configuration' macro.  However, the extension mechanism replaces
the existing xorg-configuration with the supplied one, making it impossible to
compose configuration from multiple sources.  This patch adds a procedure to
merge two xorg-configuration records, and calls it within
handle-xorg-configuration, allowing the config to be built piecemeal.

* gnu/services/xorg.scm (merge-xorg-configurations): New variable.
(handle-xorg-configuration): Merge xorg configs.
* doc/guix.texi (X Window): Document xorg-configuration composition.

Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
---
 doc/guix.texi         | 48 +++++++++++++++++++++++++++++++++++++++++--
 gnu/services/xorg.scm | 42 ++++++++++++++++++++++++++++++-------
 2 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index eebc1a1590..15e3192939 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -138,6 +138,7 @@ Copyright @copyright{} 2024 45mg@*
 Copyright @copyright{} 2025 Sören Tempel@*
 Copyright @copyright{} 2025 Rostislav Svoboda@*
 Copyright @copyright{} 2025 Zacchaeus@*
+Copyright @copyright{} 2025 Ian Eure@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -24916,8 +24917,51 @@ Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an @code{<xorg-configuration>} record.
 
 Since the Xorg configuration is embedded in the log-in manager's
-configuration---e.g., @code{gdm-configuration}---this procedure provides a
-shorthand to set the Xorg configuration.
+configuration---e.g., @code{gdm-configuration}---this procedure provides
+a shorthand to set the Xorg configuration.
+
+In addition to @code{set-xorg-configuration}, you may extend your log-in
+manager’s service-type to provide reusable, partial configurations,
+which are concatenated to create the final Xorg configuration:
+
+@lisp
+;; A service which configures Intel video drivers.
+(define %xorg-intel-service
+  (simple-service
+   'xorg-intel
+   gdm-service-type
+   (xorg-configuration
+    (modules (list xf86-video-intel))
+    (drivers '("intel"))
+    (extra-config "
+    Section \"Device\"
+        Identifier \"Intel GPU\"
+        Driver \"intel\"
+        Option \"TearFree\" \"true\"
+    EndSection"))))
+
+;; A service which configures the keyboard.
+(define %xorg-keyboard-service
+  (simple-service
+   'xorg-keyboard
+   gdm-service-type
+   (xorg-configuration
+    (keyboard-layout
+     (keyboard-layout "us" #:options '("ctrl:nocaps"))))))
+
+(operating-system
+  (services
+   (append
+    (list (service gdm-service-type)
+          %xorg-intel-service
+          %xorg-keyboard-service)
+    %base-services))
+  @dots{})
+@end lisp
+
+Service extension and @code{set-xorg-configuration} can each be used
+seperately, or in conjunction.
+
 @end deffn
 
 @deffn {Procedure} xorg-start-command [config]
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index bef05b9bb9..c313b7bc59 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -209,6 +209,34 @@ (define-record-type* <xorg-configuration>
   (server-arguments xorg-configuration-server-arguments ;list of strings
                     (default %default-xorg-server-arguments)))
 
+(define (merge-xorg-configurations a b)
+  (let ((configs (list b a)))           ; Prefer later configurations.
+    (xorg-configuration
+     (modules
+      (delete-duplicates (append-map xorg-configuration-modules configs)))
+     (fonts
+      (delete-duplicates (append-map xorg-configuration-fonts configs)))
+     (drivers
+      (delete-duplicates (append-map xorg-configuration-drivers configs)))
+     (resolutions
+      (delete-duplicates (append-map xorg-configuration-resolutions configs)))
+     (extra-config (append-map xorg-configuration-extra-config configs))
+     ;; Prefer the more recently set layout.
+     (keyboard-layout (or (xorg-configuration-keyboard-layout b)
+                          (xorg-configuration-keyboard-layout a)
+                          #f))
+     (server
+      ;; Prefer the non-default server.
+      (if (eq? xorg-server (xorg-configuration-server a))
+          (xorg-configuration-server b)
+          (xorg-configuration-server a)))
+     (server-arguments
+      ;; Prefer the non-default arguments.
+      (if (eq? %default-xorg-server-arguments
+             (xorg-configuration-server-arguments a))
+          (xorg-configuration-server-arguments b)
+          (xorg-configuration-server-arguments a))))))
+
 (define (xorg-configuration->file config)
   "Compute an Xorg configuration file corresponding to CONFIG, an
 <xorg-configuration> record."
@@ -334,9 +362,12 @@ (define (expand modules)
                            port)
                   (newline port)))
 
-              (for-each (lambda (config)
-                          (display config port))
-                        '#$(xorg-configuration-extra-config config))))))
+              (for-each
+               (lambda (config)
+                 (display config port)
+                 (newline port))
+               (delete-duplicates
+                '#$(xorg-configuration-extra-config config)))))))
 
     (computed-file "xserver.conf" build)))
 
@@ -628,10 +659,7 @@ (define-syntax handle-xorg-configuration
     ((_ configuration-record service-type-definition)
      (service-type
        (inherit service-type-definition)
-       (compose (lambda (extensions)
-                  (match extensions
-                    (() #f)
-                    ((config . _) config))))
+       (compose (cut reduce merge-xorg-configurations #f <>))
        (extend (lambda (config xorg-configuration)
                  (if xorg-configuration
                      (configuration-record
-- 
2.49.0

Message #59 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v4 2/2] gnu: set-xorg-configuration: Be smarter finding the
 display-manager.
Date: Mon, 21 Apr 2025 17:18:44 -0700

* gnu/services/desktop.scm (desktop-services-for-system): Export.
* gnu/services/desktop.scm (set-xorg-configuration): Look for a
display-manager service in (desktop-services-for-system).
* gnu/services/desktop.scm (set-xorg-configuration): Remove TODO comment.

Change-Id: Iceda2d13d16435517cd92bcb49e6937ed096f392
---
 gnu/services/desktop.scm |  1 +
 gnu/services/xorg.scm    | 25 ++++++++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index fe034cfa8f..f7fb3c5bf0 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -199,6 +199,7 @@ (define-module (gnu services desktop)
             seatd-configuration
             seatd-service-type
 
+            desktop-services-for-system
             %desktop-services))
 
 ;;; Commentary:
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index c313b7bc59..8fff8fd491 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -16,6 +16,7 @@
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Zheng Junjie <873216071 <at> qq.com>
 ;;; Copyright © 2024 Tomas Volf <~@wolfsden.cz>
+;;; Copyright © 2025 Ian Eure <ian <at> retrospec.tv>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -43,6 +44,7 @@ (define-module (gnu services xorg)
   #:use-module (gnu system privilege)
   #:use-module (gnu services base)
   #:use-module (gnu services dbus)
+  #:use-module (gnu services desktop)
   #:use-module (gnu packages base)
   #:use-module (gnu packages guile)
   #:use-module (gnu packages xorg)
@@ -139,6 +141,7 @@ (define-module (gnu services xorg)
             gdm-configuration
             gdm-service-type
 
+            xorg-configuration-service-type
             handle-xorg-configuration
             set-xorg-configuration))
 
@@ -1391,16 +1394,24 @@ (define gdm-service-type
                    "Run the GNOME Desktop Manager (GDM), a program that allows
 you to log in in a graphical session, whether or not you use GNOME."))))
 
-;; Since GDM depends on Rust and Rust is not available on all platforms,
-;; use SDDM as the fall-back display manager.
-;; TODO: Switch the condition to take into account if Rust is supported and
-;; match the configuration in desktop-services-for-system.
+(define (xorg-configuration-service? s)
+  "Returns #t if @var{s} is a service carrying xorg configuration."
+  (let ((val (service-value s)))
+    (and (record? val)
+         (memq 'xorg-configuration
+                 (record-type-fields (record-type-descriptor val))))))
+
+(define (xorg-configuration-service-type services)
+  "Return the service type of the first service in @var{services}
+which carries xorg configuration. "
+  (let ((lmst (find xorg-configuration-service? services)))
+    (when lmst (service-kind lmst))))
+
 (define* (set-xorg-configuration config
                                  #:optional
                                  (login-manager-service-type
-                                  (if (target-x86-64?)
-                                      gdm-service-type
-                                      sddm-service-type)))
+                                  (xorg-configuration-service-type
+                                   (desktop-services-for-system))))
   "Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an <xorg-configuration> record."
   (simple-service 'set-xorg-configuration
-- 
2.49.0

Message #62 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v3 1/2] gnu: Merge xorg configurations when
 extending.
Date: Tue, 22 Apr 2025 15:18:36 +0200

Ian Eure <ian <at> retrospec.tv> writes:

> Do you have / know of a setup for editing Guile inside the
> documentation?  I’ve been editing in a *scheme* scratch buffer in
> Emacs and pasting back into the docs, but this clearly doesn’t work
> very well.

That’s what I do as well.  Low-tech, but it’s also useful to test the
example beforehand.

Ludo’.

Message #65 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> chbouib.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Thu, 24 Apr 2025 16:58:36 -0700

Hi Ludo’,

Ludovic Courtès <ludo <at> chbouib.org> writes:

> Hi,
>
> Ian Eure <ian <at> retrospec.tv> writes:
>
>>>   (append (list (service gdm-service-type)
>>>                 %xorg-intel-service
>>>                 …)
>>>           @dots{})
>>
>> Can make these changes, but I’m curious why (append (list ...) 
>> ...) is
>> preferred over cons*.  I default to the latter, because I like 
>> the
>> removal of the extra level of indent.
>
> In most of the examples in the documentation, we went for 
> ‘append’
> because it less obscure to a newcomer than ‘cons*’ (or ‘cons’, 
> even).
>
> That’s also what the installer generates, and ‘guix home import’ 
> I
> believe.

Makes sense.  I sent a v4 with these changes and some doc 
rewording.

Thanks,

 -- Ian

Message #68 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> chbouib.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Sun, 11 May 2025 09:41:58 -0700

Hi all,

Any other feedback on this, or does it look ready to push?

Thanks,
 -- Ian

Message #71 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v4 2/2] gnu: set-xorg-configuration: Be
 smarter finding the display-manager.
Date: Mon, 12 May 2025 09:05:29 +0200

Ian Eure <ian <at> retrospec.tv> writes:

> * gnu/services/desktop.scm (desktop-services-for-system): Export.
> * gnu/services/desktop.scm (set-xorg-configuration): Look for a
> display-manager service in (desktop-services-for-system).
> * gnu/services/desktop.scm (set-xorg-configuration): Remove TODO comment.
>
> Change-Id: Iceda2d13d16435517cd92bcb49e6937ed096f392
> ---
>  gnu/services/desktop.scm |  1 +
>  gnu/services/xorg.scm    | 25 ++++++++++++++++++-------
>  2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
> index fe034cfa8f..f7fb3c5bf0 100644
> --- a/gnu/services/desktop.scm
> +++ b/gnu/services/desktop.scm
> @@ -199,6 +199,7 @@ (define-module (gnu services desktop)
>              seatd-configuration
>              seatd-service-type
>  
> +            desktop-services-for-system

This is unnecessary since ‘%desktop-services’ behaves in the same way.

> +            xorg-configuration-service-type

I would not export it (the name is confusing and it’s not used outside).

> +(define (xorg-configuration-service? s)
> +  "Returns #t if @var{s} is a service carrying xorg configuration."
> +  (let ((val (service-value s)))
> +    (and (record? val)
> +         (memq 'xorg-configuration
> +                 (record-type-fields (record-type-descriptor val))))))

Please don’t use introspection on records: it may sound appealing, but
it would make refactoring next-to-impossible since it becomes very hard
to know how records are used.

Instead just do:

  (and (or (sddm-configuration? value) …)
       value)

> +(define (xorg-configuration-service-type services)
> +  "Return the service type of the first service in @var{services}
> +which carries xorg configuration. "
> +  (let ((lmst (find xorg-configuration-service? services)))
> +    (when lmst (service-kind lmst))))

Rather:

  (let ((xorg (find …)))
    (and=> xorg service-kind))

… otherwise the procedure can return *unspecified*, leading to a type
error down the road.

But hmm, sorry for overlooking it but I’m not convinced by this patch.
I would tend to keep just patch 1/2.

WDYT?

Thanks, and apologies for the delay,
Ludo’.

Message #74 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v4 1/2] gnu: Merge xorg configurations when
 extending.
Date: Mon, 12 May 2025 08:59:24 +0200

Hi Ian,

Ian Eure <ian <at> retrospec.tv> writes:

> Configuration for xorg is embedded in the various display-manager
> configuration records, and extension support is factored out into the
> `handle-xorg-configuration' macro.  However, the extension mechanism replaces
> the existing xorg-configuration with the supplied one, making it impossible to
> compose configuration from multiple sources.  This patch adds a procedure to
> merge two xorg-configuration records, and calls it within
> handle-xorg-configuration, allowing the config to be built piecemeal.
>
> * gnu/services/xorg.scm (merge-xorg-configurations): New variable.
> (handle-xorg-configuration): Merge xorg configs.
> * doc/guix.texi (X Window): Document xorg-configuration composition.
>
> Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba

[...]

> +(define (merge-xorg-configurations a b)
> +  (let ((configs (list b a)))           ; Prefer later configurations.
> +    (xorg-configuration
> +     (modules
> +      (delete-duplicates (append-map xorg-configuration-modules configs)))
> +     (fonts
> +      (delete-duplicates (append-map xorg-configuration-fonts configs)))
> +     (drivers
> +      (delete-duplicates (append-map xorg-configuration-drivers configs)))
> +     (resolutions
> +      (delete-duplicates (append-map xorg-configuration-resolutions configs)))

As I mentioned before, since ‘equal?’ cannot be meaningfully applied
here on file-like objects in general, all these ‘delete-duplicates’
calls should be removed.

>  (define (xorg-configuration->file config)
>    "Compute an Xorg configuration file corresponding to CONFIG, an
>  <xorg-configuration> record."
> @@ -334,9 +362,12 @@ (define (expand modules)
>                             port)
>                    (newline port)))
>  
> -              (for-each (lambda (config)
> -                          (display config port))
> -                        '#$(xorg-configuration-extra-config config))))))
> +              (for-each
> +               (lambda (config)
> +                 (display config port)
> +                 (newline port))
> +               (delete-duplicates
> +                '#$(xorg-configuration-extra-config config)))))))

As I wrote before, in ‘xorg-configuration->file’, ‘delete-duplicates’
should be used for drivers, modules, and fonts, but not for
‘extra-config’ (which is merely a string).

Could you update it accordingly?

Thanks,
Ludo’.

Message #77 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v4 2/2] gnu: set-xorg-configuration: Be
 smarter finding the display-manager.
Date: Mon, 12 May 2025 06:54:05 -0700

Hi Ludo’,

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

> Ian Eure <ian <at> retrospec.tv> writes:
>
>> * gnu/services/desktop.scm (desktop-services-for-system): 
>> Export.
>> * gnu/services/desktop.scm (set-xorg-configuration): Look for a
>> display-manager service in (desktop-services-for-system).
>> * gnu/services/desktop.scm (set-xorg-configuration): Remove 
>> TODO comment.
>>
>> Change-Id: Iceda2d13d16435517cd92bcb49e6937ed096f392
>> ---
>>  gnu/services/desktop.scm |  1 +
>>  gnu/services/xorg.scm    | 25 ++++++++++++++++++-------
>>  2 files changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/gnu/services/desktop.scm 
>> b/gnu/services/desktop.scm
>> index fe034cfa8f..f7fb3c5bf0 100644
>> --- a/gnu/services/desktop.scm
>> +++ b/gnu/services/desktop.scm
>> @@ -199,6 +199,7 @@ (define-module (gnu services desktop)
>>              seatd-configuration
>>              seatd-service-type
>>  
>> +            desktop-services-for-system
>
> This is unnecessary since ‘%desktop-services’ behaves in the 
> same way.
>
>> +            xorg-configuration-service-type
>
> I would not export it (the name is confusing and it’s not used 
> outside).

I agree, I’ll change this.


>> +(define (xorg-configuration-service? s)
>> +  "Returns #t if @var{s} is a service carrying xorg 
>> configuration."
>> +  (let ((val (service-value s)))
>> +    (and (record? val)
>> +         (memq 'xorg-configuration
>> +                 (record-type-fields (record-type-descriptor 
>> val))))))
>
> Please don’t use introspection on records: it may sound 
> appealing, but
> it would make refactoring next-to-impossible since it becomes 
> very hard
> to know how records are used.
>
> Instead just do:
>
>   (and (or (sddm-configuration? value) …)
>        value)

Hardcoding the configuration types here creates a maintenance 
burden, where adding a DM service requires updating a list of DM 
services, which is likely to be a source of bugs.  And since the 
`xorg-configuration' field is added programmatically by 
`handle-xorg-configuration', it seems reasonable to rely on here, 
as it’s unlikely to change in a refactor.  WDYT?

Is there another way to programmatically determine whether a given 
service carries an xorg-configuration without introspection?


>> +(define (xorg-configuration-service-type services)
>> +  "Return the service type of the first service in 
>> @var{services}
>> +which carries xorg configuration. "
>> +  (let ((lmst (find xorg-configuration-service? services)))
>> +    (when lmst (service-kind lmst))))
>
> Rather:
>
>   (let ((xorg (find …)))
>     (and=> xorg service-kind))
>
> … otherwise the procedure can return *unspecified*, leading to a 
> type
> error down the road.

Will do.

> But hmm, sorry for overlooking it but I’m not convinced by this 
> patch.
> I would tend to keep just patch 1/2.

I’m okay with dropping this patch from the series, but would 
prefer to include it.  The existing logic in 
`set-xorg-configuration' is crude and has a TODO to improve it. 
While this patch doesn’t solve 100% of the problem (this is 
impossible without signigiantly reworking every DM), it does 
improve the situation and provide some helpful facilities for the 
future.

Thanks,
 -- Ian

Message #80 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v4 1/2] gnu: Merge xorg configurations when
 extending.
Date: Mon, 12 May 2025 07:09:25 -0700

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

> Hi Ian,
>
> Ian Eure <ian <at> retrospec.tv> writes:
>
>> +(define (merge-xorg-configurations a b)
>> +  (let ((configs (list b a)))           ; Prefer later 
>> configurations.
>> +    (xorg-configuration
>> +     (modules
>> +      (delete-duplicates (append-map 
>> xorg-configuration-modules configs)))
>> +     (fonts
>> +      (delete-duplicates (append-map xorg-configuration-fonts 
>> configs)))
>> +     (drivers
>> +      (delete-duplicates (append-map 
>> xorg-configuration-drivers configs)))
>> +     (resolutions
>> +      (delete-duplicates (append-map 
>> xorg-configuration-resolutions configs)))
>
> As I mentioned before, since ‘equal?’ cannot be meaningfully 
> applied
> here on file-like objects in general, all these 
> ‘delete-duplicates’
> calls should be removed.

Hmm, I thought your previous comment was about the `extra-config' 
field specifically, and I did update that one.  I agree that’s a 
problem, but I’m not sure about the other ones, ex. `drivers' is 
documented as containing a "list of strings."  I can see an 
argument for consistent behavior of deduplication here, and having 
it all in one place.


>>  (define (xorg-configuration->file config)
>>    "Compute an Xorg configuration file corresponding to CONFIG, 
>>    an
>>  <xorg-configuration> record."
>> @@ -334,9 +362,12 @@ (define (expand modules)
>>                             port)
>>                    (newline port)))
>>  
>> -              (for-each (lambda (config)
>> -                          (display config port))
>> -                        '#$(xorg-configuration-extra-config 
>> config))))))
>> +              (for-each
>> +               (lambda (config)
>> +                 (display config port)
>> +                 (newline port))
>> +               (delete-duplicates
>> +                '#$(xorg-configuration-extra-config 
>> config)))))))
>
> As I wrote before, in ‘xorg-configuration->file’, 
> ‘delete-duplicates’
> should be used for drivers, modules, and fonts, but not for
> ‘extra-config’ (which is merely a string).
>
> Could you update it accordingly?

Can’t `extra-config' be a file-like?

 -- Ian

Message #83 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH v4 1/2] gnu: Merge xorg configurations when
 extending.
Date: Sun, 18 May 2025 11:30:59 +0200

Hello,

Ian Eure <ian <at> retrospec.tv> writes:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Hi Ian,
>>
>> Ian Eure <ian <at> retrospec.tv> writes:
>>
>>> +(define (merge-xorg-configurations a b)
>>> +  (let ((configs (list b a)))           ; Prefer later
>>> configurations.
>>> +    (xorg-configuration
>>> +     (modules
>>> +      (delete-duplicates (append-map xorg-configuration-modules
>>> configs)))
>>> +     (fonts
>>> +      (delete-duplicates (append-map xorg-configuration-fonts
>>> configs)))
>>> +     (drivers
>>> +      (delete-duplicates (append-map xorg-configuration-drivers
>>> configs)))
>>> +     (resolutions
>>> +      (delete-duplicates (append-map
>>> xorg-configuration-resolutions configs)))
>>
>> As I mentioned before, since ‘equal?’ cannot be meaningfully applied
>> here on file-like objects in general, all these ‘delete-duplicates’
>> calls should be removed.
>
> Hmm, I thought your previous comment was about the `extra-config'
> field specifically, and I did update that one.

My point is that ‘delete-duplicates’ cannot be applied on file-like
objects, as is the case for ‘modules’, ‘fonts’, and ‘drivers’ in the
excerpt above.  That’s why I said in this case ‘delete-duplicates’
should be moved to the “build side”.

> Can’t `extra-config' be a file-like?

The doc says:

     ‘extra-config’ (default: ‘'()’)
          This is a list of strings or objects appended to the
          configuration file.  It is used to pass extra text to be added
          verbatim to the configuration file.

Ludo’.

Message #86 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v5 1/2] gnu: Merge xorg configurations when extending.
Date: Sun, 18 May 2025 17:07:10 -0700

Configuration for xorg is embedded in the various display-manager
configuration records, and extension support is factored out into the
`handle-xorg-configuration' macro.  However, the extension mechanism replaces
the existing xorg-configuration with the supplied one, making it impossible to
compose configuration from multiple sources.  This patch adds a procedure to
merge two xorg-configuration records, and calls it within
handle-xorg-configuration, allowing the config to be built piecemeal.

* gnu/services/xorg.scm (merge-xorg-configurations): New variable.
(handle-xorg-configuration): Merge xorg configs.
* doc/guix.texi (X Window): Document xorg-configuration composition.

Change-Id: I20e9db911eef5d4efe98fdf382f3084e4defc1ba
---
 doc/guix.texi         | 48 +++++++++++++++++++++++++++++++++++--
 gnu/services/xorg.scm | 55 +++++++++++++++++++++++++++++++++----------
 2 files changed, 88 insertions(+), 15 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 4f9cd56aa5b..dd184b27e78 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -140,6 +140,7 @@ Copyright @copyright{} 2025 Rostislav Svoboda@*
 Copyright @copyright{} 2025 Zacchaeus@*
 Copyright @copyright{} 2025 Sergio Pastor Pérez@*
 Copyright @copyright{} 2024 Evgeny Pisemsky@*
+Copyright @copyright{} 2025 Ian Eure@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -25039,8 +25040,51 @@ Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an @code{<xorg-configuration>} record.
 
 Since the Xorg configuration is embedded in the log-in manager's
-configuration---e.g., @code{gdm-configuration}---this procedure provides a
-shorthand to set the Xorg configuration.
+configuration---e.g., @code{gdm-configuration}---this procedure provides
+a shorthand to set the Xorg configuration.
+
+In addition to @code{set-xorg-configuration}, you may extend your log-in
+manager’s service-type to provide reusable, partial configurations,
+which are concatenated to create the final Xorg configuration:
+
+@lisp
+;; A service which configures Intel video drivers.
+(define %xorg-intel-service
+  (simple-service
+   'xorg-intel
+   gdm-service-type
+   (xorg-configuration
+    (modules (list xf86-video-intel))
+    (drivers '("intel"))
+    (extra-config "
+    Section \"Device\"
+        Identifier \"Intel GPU\"
+        Driver \"intel\"
+        Option \"TearFree\" \"true\"
+    EndSection"))))
+
+;; A service which configures the keyboard.
+(define %xorg-keyboard-service
+  (simple-service
+   'xorg-keyboard
+   gdm-service-type
+   (xorg-configuration
+    (keyboard-layout
+     (keyboard-layout "us" #:options '("ctrl:nocaps"))))))
+
+(operating-system
+  (services
+   (append
+    (list (service gdm-service-type)
+          %xorg-intel-service
+          %xorg-keyboard-service)
+    %base-services))
+  @dots{})
+@end lisp
+
+Service extension and @code{set-xorg-configuration} can each be used
+seperately, or in conjunction.
+
 @end deffn
 
 @deffn {Procedure} xorg-start-command [config]
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index bef05b9bb9b..c2ee68b1ab2 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -209,14 +209,43 @@ (define-record-type* <xorg-configuration>
   (server-arguments xorg-configuration-server-arguments ;list of strings
                     (default %default-xorg-server-arguments)))
 
+(define (merge-xorg-configurations a b)
+  (let ((configs (list b a)))           ; Prefer later configurations.
+    (xorg-configuration
+     (modules
+      (append-map xorg-configuration-modules configs))
+     (fonts
+      (append-map xorg-configuration-fonts configs))
+     (drivers
+      (append-map xorg-configuration-drivers configs))
+     (resolutions
+      (append-map xorg-configuration-resolutions configs))
+     (extra-config (append-map xorg-configuration-extra-config configs))
+     ;; Prefer the more recently set layout.
+     (keyboard-layout (or (xorg-configuration-keyboard-layout b)
+                          (xorg-configuration-keyboard-layout a)
+                          #f))
+     (server
+      ;; Prefer the non-default server.
+      (if (eq? xorg-server (xorg-configuration-server a))
+          (xorg-configuration-server b)
+          (xorg-configuration-server a)))
+     (server-arguments
+      ;; Prefer the non-default arguments.
+      (if (eq? %default-xorg-server-arguments
+             (xorg-configuration-server-arguments a))
+          (xorg-configuration-server-arguments b)
+          (xorg-configuration-server-arguments a))))))
+
 (define (xorg-configuration->file config)
   "Compute an Xorg configuration file corresponding to CONFIG, an
 <xorg-configuration> record."
   (let ((xorg-server (xorg-configuration-server config)))
     (define all-modules
       ;; 'xorg-server' provides 'fbdevhw.so' etc.
-      (append (xorg-configuration-modules config)
-              (list xorg-server)))
+      (delete-duplicates
+       (append (xorg-configuration-modules config)
+               (list xorg-server))))
 
     (define build
       #~(begin
@@ -227,7 +256,7 @@ (define build
           (call-with-output-file #$output
             (lambda (port)
               (define drivers
-                '#$(xorg-configuration-drivers config))
+                (delete-duplicates '#$(xorg-configuration-drivers config)))
 
               (define (device-section driver)
                 (string-append "
@@ -247,7 +276,7 @@ (define (screen-section driver resolutions)
                       ((x y)
                        (string-append "\"" (number->string x)
                                       "x" (number->string y) "\"")))
-                    resolutions)) "
+                    (delete-duplicates resolutions))) "
   EndSubSection
 EndSection"))
 
@@ -294,7 +323,7 @@ (define (expand modules)
               (display "Section \"Files\"\n" port)
               (for-each (lambda (font)
                           (format port "  FontPath \"~a\"~%" font))
-                        '#$(xorg-configuration-fonts config))
+                        (delete-duplicates '#$(xorg-configuration-fonts config)))
               (for-each (lambda (module)
                           (format port
                                   "  ModulePath \"~a\"~%"
@@ -315,7 +344,7 @@ (define (expand modules)
               (newline port)
               (display (string-join
                         (map (cut screen-section <>
-                                  '#$(xorg-configuration-resolutions config))
+                                  (delete-duplicates '#$(xorg-configuration-resolutions config)))
                              drivers)
                         "\n")
                        port)
@@ -334,9 +363,12 @@ (define (expand modules)
                            port)
                   (newline port)))
 
-              (for-each (lambda (config)
-                          (display config port))
-                        '#$(xorg-configuration-extra-config config))))))
+              (for-each
+               (lambda (config)
+                 (display config port)
+                 (newline port))
+               (delete-duplicates
+                '#$(xorg-configuration-extra-config config)))))))
 
     (computed-file "xserver.conf" build)))
 
@@ -628,10 +660,7 @@ (define-syntax handle-xorg-configuration
     ((_ configuration-record service-type-definition)
      (service-type
        (inherit service-type-definition)
-       (compose (lambda (extensions)
-                  (match extensions
-                    (() #f)
-                    ((config . _) config))))
+       (compose (cut reduce merge-xorg-configurations #f <>))
        (extend (lambda (config xorg-configuration)
                  (if xorg-configuration
                      (configuration-record
-- 
2.49.0

Message #89 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v5 0/2] gnu: Merge xorg configurations when extending.
Date: Sun, 18 May 2025 17:07:09 -0700

Moved all deduplication into `xorg-configuration->file', and rebased on master
and resolved a conflict.

Ian Eure (2):
  gnu: Merge xorg configurations when extending.
  gnu: set-xorg-configuration: Be smarter finding the display-manager.

 doc/guix.texi            | 48 +++++++++++++++++++++++-
 gnu/services/desktop.scm |  1 +
 gnu/services/xorg.scm    | 80 ++++++++++++++++++++++++++++++----------
 3 files changed, 107 insertions(+), 22 deletions(-)

-- 
2.49.0

Message #92 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> retrospec.tv>
Subject: [PATCH v5 2/2] gnu: set-xorg-configuration: Be smarter finding the
 display-manager.
Date: Sun, 18 May 2025 17:07:11 -0700

* gnu/services/desktop.scm (desktop-services-for-system): Export.
* gnu/services/desktop.scm (set-xorg-configuration): Look for a
display-manager service in (desktop-services-for-system).
* gnu/services/desktop.scm (set-xorg-configuration): Remove TODO comment.

Change-Id: Iceda2d13d16435517cd92bcb49e6937ed096f392
---
 gnu/services/desktop.scm |  1 +
 gnu/services/xorg.scm    | 25 ++++++++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 12560a6249d..fd4a452aa56 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -205,6 +205,7 @@ (define-module (gnu services desktop)
             seatd-configuration
             seatd-service-type
 
+            desktop-services-for-system
             %desktop-services))
 
 ;;; Commentary:
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index c2ee68b1ab2..9b82882116e 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -16,6 +16,7 @@
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Zheng Junjie <873216071 <at> qq.com>
 ;;; Copyright © 2024 Tomas Volf <~@wolfsden.cz>
+;;; Copyright © 2025 Ian Eure <ian <at> retrospec.tv>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -43,6 +44,7 @@ (define-module (gnu services xorg)
   #:use-module (gnu system privilege)
   #:use-module (gnu services base)
   #:use-module (gnu services dbus)
+  #:use-module (gnu services desktop)
   #:use-module (gnu packages base)
   #:use-module (gnu packages guile)
   #:use-module (gnu packages xorg)
@@ -139,6 +141,7 @@ (define-module (gnu services xorg)
             gdm-configuration
             gdm-service-type
 
+            xorg-configuration-service-type
             handle-xorg-configuration
             set-xorg-configuration))
 
@@ -1392,16 +1395,24 @@ (define gdm-service-type
                    "Run the GNOME Desktop Manager (GDM), a program that allows
 you to log in in a graphical session, whether or not you use GNOME."))))
 
-;; Since GDM depends on Rust and Rust is not available on all platforms,
-;; use SDDM as the fall-back display manager.
-;; TODO: Switch the condition to take into account if Rust is supported and
-;; match the configuration in desktop-services-for-system.
+(define (xorg-configuration-service? s)
+  "Returns #t if @var{s} is a service carrying xorg configuration."
+  (let ((val (service-value s)))
+    (and (record? val)
+         (memq 'xorg-configuration
+                 (record-type-fields (record-type-descriptor val))))))
+
+(define (xorg-configuration-service-type services)
+  "Return the service type of the first service in @var{services}
+which carries xorg configuration. "
+  (let ((lmst (find xorg-configuration-service? services)))
+    (when lmst (service-kind lmst))))
+
 (define* (set-xorg-configuration config
                                  #:optional
                                  (login-manager-service-type
-                                  (if (target-x86-64?)
-                                      gdm-service-type
-                                      sddm-service-type)))
+                                  (xorg-configuration-service-type
+                                   (desktop-services-for-system))))
   "Tell the log-in manager (of type @var{login-manager-service-type}) to use
 @var{config}, an <xorg-configuration> record."
   (simple-service 'set-xorg-configuration
-- 
2.49.0

Message #95 received at 77266 <at> debbugs.gnu.org (full text, mbox):

From: Ian Eure <ian <at> retrospec.tv>
To: 77266 <at> debbugs.gnu.org
Subject: Re: [bug#77266] [PATCH] gnu: Merge xorg configurations when extending.
Date: Sun, 25 May 2025 09:57:45 -0700

Migrated this to a Codeberg PR: 
https://codeberg.org/guix/guix/pulls/21

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.