diff mbox series

[bug#77266] gnu: Merge xorg configurations when extending.

Message ID 20250326042354.14033-1-ian@retrospec.tv
State New
Headers
Series [bug#77266] gnu: Merge xorg configurations when extending. |

Commit Message

Ian Eure March 26, 2025, 4:23 a.m. UTC 
  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(-)

Comments

Ludovic Courtès March 26, 2025, 5:16 p.m. UTC | #1 
Hi!

Ian Eure <ian@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’.
Rutherther March 26, 2025, 5:31 p.m. UTC | #2 
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
Ian Eure March 26, 2025, 7:15 p.m. UTC | #3 
Hi Rutherther,

Rutherther <rutherther@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
Ian Eure March 26, 2025, 7:15 p.m. UTC | #4 
Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> Ian Eure <ian@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
diff mbox series

Patch

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