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

Previous Next

Full log

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’.

Previous Next

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