[bug#53466] [PATCH] home: Add redshift service.

[Andrew Tropin]

On 2022-01-23 12:11, Ludovic Courtès wrote:

> * gnu/home/services/desktop.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * doc/guix.texi (Desktop Home Services): New node.
> ---
>  doc/guix.texi                 |  62 ++++++++++++++++
>  gnu/home/services/desktop.scm | 135 ++++++++++++++++++++++++++++++++++
>  gnu/local.mk                  |   1 +
>  3 files changed, 198 insertions(+)
>  create mode 100644 gnu/home/services/desktop.scm
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 912a8e3c5a..07414ec13d 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -37186,6 +37186,7 @@ services)}.
>  * Shells: Shells Home Services.          POSIX shells, Bash, Zsh.
>  * Mcron: Mcron Home Service.             Scheduled User's Job Execution.
>  * Shepherd: Shepherd Home Service.       Managing User's Daemons.
> +* Desktop: Desktop Home Services.        Services for graphical environments.
>  @end menu
>  @c In addition to that Home Services can provide
>  
> @@ -37567,6 +37568,67 @@ mechanism instead (@pxref{Shepherd Services}).
>  @end table
>  @end deftp
>  
> +@node Desktop Home Services
> +@subsection Desktop Home Services
> +
> +The @code{(gnu home services desktop)} module provides services that you
> +may find useful on ``desktop'' systems running a graphical user
> +environment such as Xorg.
> +
> +@defvr {Scheme Variable} home-redshift-service-type
> +This is the service type for @uref{https://github.com/jonls/redshift,
> +Redshift}, a program that adjusts the display color temperature
> +according to the time of day.  Its associated value must be a
> +@code{home-redshift-configuration} record, as shown below.
> +
> +A typical configuration, where we manually specify the latitude and
> +longitude, might look like this:
> +
> +@lisp
> +(service home-redshift-service-type
> +         (home-redshift-configuration
> +          (location-provider 'manual)
> +          (latitude 35.81)    ;northern hemisphere
> +          (longitude -0.80))) ;west of Greenwich
> +@end lisp
> +@end defvr
> +
> +@deftp {Data Type} home-redshift-configuration
> +Available @code{home-redshift-configuration} fields are:
> +
> +@table @asis
> +@item @code{location-provider} (default: @code{geoclue2}) (type: symbol)
> +Geolocation provider---@code{'manual} or @code{'geoclue2}.  In the
> +former case, you must also specify the @code{latitude} and
> +@code{longitude} fields so Redshift can determine daytime at your place.
> +In the latter case, the Geoclue system service must be running; it will
> +be queried for location information.
> +
> +@item @code{adjustment-method} (default: @code{randr}) (type: symbol)
> +Color adjustment method.
> +
> +@item @code{daytime-temperature} (default: @code{6500}) (type: integer)
> +Daytime color temperature (kelvins).
> +
> +@item @code{nighttime-temperature} (default: @code{4500}) (type: integer)
> +Nighttime color temperature (kelvins).
> +
> +@item @code{daytime-brightness} (default: @code{disabled}) (type: 
> maybe-inexact-number)
> +Daytime screen brightness, between 0.1 and 1.0.
> +
> +@item @code{nighttime-brightness} (default: @code{disabled}) (type: 
> maybe-inexact-number)
> +Nighttime screen brightness, between 0.1 and 1.0.
> +
> +@item @code{latitude} (default: @code{disabled}) (type: maybe-inexact-number)
> +Latitude, when @code{location-provider} is @code{'manual}.
> +
> +@item @code{longitude} (default: @code{disabled}) (type: 
> maybe-inexact-number)
> +Longitude, when @code{location-provider} is @code{'manual}.
> +
> +@end table
> +
> +@end deftp
> +
>  @node Invoking guix home
>  @section Invoking @code{guix home}
>  
> diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
> new file mode 100644
> index 0000000000..36e02e671f
> --- /dev/null
> +++ b/gnu/home/services/desktop.scm
> @@ -0,0 +1,135 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (gnu home services desktop)
> +  #:use-module (gnu home services)
> +  #:use-module (gnu home services shepherd)
> +  #:use-module (gnu services configuration)
> +  #:autoload   (gnu packages xdisorg) (redshift)
> +  #:use-module (guix records)
> +  #:use-module (guix gexp)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (ice-9 match)
> +  #:export (home-redshift-configuration
> +            home-redshift-configuration?
> +
> +            home-redshift-service-type))
> +
> +
> +;;;
> +;;; Redshift.
> +;;;
> +
> +(define (serialize-integer field value)
> +  (string-append (match field
> +                   ('daytime-temperature "temp-day")
> +                   ('nighttime-temperature "temp-night")
> +                   ('daytime-brightness "brightness-day")
> +                   ('nighttime-brightness "brightness-night")
> +                   ('latitude "lat")
> +                   ('longitude "lon")
> +                   (_ (symbol->string field)))
> +                 "=" (number->string value) "\n"))
> +
> +(define (serialize-symbol field value)
> +  (string-append (symbol->string field)
> +                 "=" (symbol->string value) "\n"))
> +
> +(define serialize-inexact-number serialize-integer)
> +
> +(define (inexact-number? n)
> +  (and (number? n) (inexact? n)))
> +(define-maybe inexact-number)
> +
> +(define-configuration home-redshift-configuration

(redshift
 (package redshift))

would be useful, especially for people who would like to use a patched
redshift supporting wayland or some other extended version of a package.

Another good candidate for a separate field is shepherd? or
shepherd-service? to make it possible to remove an integration with
shepherd if it's not needed.

> +  (location-provider
> +   (symbol 'geoclue2)
> +   "Geolocation provider---@code{'manual} or @code{'geoclue2}.
> +
> +In the former case, you must also specify the @code{latitude} and
> +@code{longitude} fields so Redshift can determine daytime at your place.  In
> +the latter case, the Geoclue system service must be running; it will be
> +queried for location information.")
> +  (adjustment-method
> +   (symbol 'randr)
> +   "Color adjustment method.")
> +
> +  ;; Default values from redshift(1).
> +  (daytime-temperature
> +   (integer 6500)
> +   "Daytime color temperature (kelvins).")
> +  (nighttime-temperature
> +   (integer 4500)
> +   "Nighttime color temperature (kelvins).")
> +
> +  (daytime-brightness
> +   (maybe-inexact-number 'disabled)
> +   "Daytime screen brightness, between 0.1 and 1.0.")
> +  (nighttime-brightness
> +   (maybe-inexact-number 'disabled)
> +   "Nighttime screen brightness, between 0.1 and 1.0.")
> +
> +  (latitude
> +   (maybe-inexact-number 'disabled)
> +   "Latitude, when @code{location-provider} is @code{'manual}.")
> +  (longitude
> +   (maybe-inexact-number 'disabled)
> +   "Longitude, when @code{location-provider} is @code{'manual}."))

While I like the naming of the fields more than original option names I
still not a big fan of this approach for various reasons:

1. Users of this home service would need to deal with one more level of
abstraction and keep in mind latitude -> manual.lat,
nighttime-brightness -> redshift.brightness-night, etc mappings.  Maybe
for completely new users it's not even necessary to think about
internals, but for person reading man pages or non-guix specific
articles it would be a headache.  It's not that bad for very simple
programs like redshift, but becomes much more significant for software
supporting much more options like sway or git.

2. With the current configuration implementation 8 options are missing
and not possible to set, also it's not possible to reuse already
existing configuration.  Escape hatch with extra-content field solves
this problem only partially and extra-content can NOT be combined with
the rest of fields representing redshift options.  So it should be a
named a "file", not extra-content, and when used will remove the effect
of all other fields.  extra-options is possible but will blow the mind,
we have mappings and all that stuff, also need a custom serialization
logic to merge sections.

3. We copy the documentation and part of implementation for software we
are wrapping and now we have to maintain it ourselves.  Probably,
redshift is quite stable and both documentation and options doesn't
change frequently, but for the bigger projects it will lead to outdated
docs and missing options quite fast or will put a huge maintanance
burden.

4. If we decide one day to continue development of guix home import
command it would be a little nightmare to write importers from existing
configuration to guix services configurations.

I would prefer to have one config field for all the fields above:

(redshift-conf
 '((redshift
    ((temp-day . 5700)
     (temp-night . 3600)
     (gamma . 0.8)
     ;; any other number of option some one would like to set
     #~"dawn-time=6:00\ndusk-time=18:00"
     ;; or a nasty slurp-file-gexp, which reads the existing
     ;; configuration or part of it if we migrate step by step
     (adjustment-method . randr)
     (location-provider . manual)))
   (manual
    ((lat . 55.7)
     (lon . 12.6)))))

So final configuration will be something like:

(define-configuration home-redshift-configuration
  (redshift
   (package redshift)
   "The redshift package to use.")
  (shepherd-service?
   (boolean #t)
   "Add a redshift service to user's shepherd?")
  (redshift-conf
   (conf-config '())
   "The configuration, which will go to ~/.config/redshift.conf,
Here is an example:
...
To get info about all options see @code{man 1 redshift}"))

Doing so we lose some explorability provided by geiser, because we don't
have separate record fields for each option any more, but get much more
maintainable and simplier configuration implementation.

We don't lose type checking: we can use redshift-config instead of
conf-config and implement proper type checking if we want, a little
harder than with macros on top of records, but not impossible and
probably much more flexible (we have access not only to the value of the
current option, but to the whole configuration).

> +
> +(define (serialize-redshift-configuration config)
> +  (define location-fields
> +    '(latitude longitude))
> +
> +  (define (location-field? field)
> +    (memq (configuration-field-name field) location-fields))
> +
> +  #~(string-append
> +     "[redshift]\n"
> +     #$(serialize-configuration config
> +                                (remove location-field?
> +                                        home-redshift-configuration-fields))
> +     "\n[manual]\n"
> +     #$(serialize-configuration config
> +                                (filter location-field?
> +                                        
> home-redshift-configuration-fields))))
> +
> +(define (redshift-shepherd-service config)
> +  (define config-file
> +    (computed-file "redshift.conf"
> +                   #~(call-with-output-file #$output
> +                       (lambda (port)
> +                         (display #$(serialize-redshift-configuration config)
> +                                  port)))))
> +
> +  (list (shepherd-service
> +         (documentation "Redshift program.")
> +         (provision '(redshift))
> +         (start #~(make-forkexec-constructor

There is a possibility that shepherd is launched before X or wayland
session started and redshift won't be able to access necessary
environment variables.  I have a few hacky solutions for other
applications, but need to come up with a better and more generic way to
handle it.

> +                   (list #$(file-append redshift "/bin/redshift")
> +                         "-c" #$config-file)))
> +         (stop #~(make-kill-destructor)))))
> +
> +(define home-redshift-service-type
> +  (service-type
> +   (name 'home-redshift)
> +   (extensions (list (service-extension home-shepherd-service-type
> +                                        redshift-shepherd-service)))

It would be good to extend home-files-service-type with config-file
generated above and home-profile-service-type with the value of
`redshift` field.  This way user will be able to launch redshift himself
or using other mechanism like wm startup file, it maybe necessary for
testing/debugging purpose or to be sure that redshift has DISPLAY or
other variables available or maybe some other use cases.

> +   (default-value (home-redshift-configuration))
> +   (description
> +    "Run Redshift, a program that adjust the color temperature of display
> +according to time of day.")))
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 03f6d90b9e..cf72926f5d 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -78,6 +78,7 @@ GNU_SYSTEM_MODULES =                                \
>    %D%/ci.scm                                 \
>    %D%/home.scm                                       \
>    %D%/home/services.scm                      \
> +  %D%/home/services/desktop.scm                      \
>    %D%/home/services/symlink-manager.scm              \
>    %D%/home/services/fontutils.scm            \
>    %D%/home/services/shells.scm                       \
>
> base-commit: ee6bf00b2d89f6acab55b7a82896d99e39c1229b

Yes, it's possible to use the approach proposed by this patch for
implementing configuration for such simple program, but I still have
a lot of concerns about applying it to more complex software.

Related discussions:
https://issues.guix.gnu.org/52698
https://yhetil.org/guix-devel/87h79qx5db.fsf@trop.in/

-- 
Best regards,
Andrew Tropin

signature.asc
Description: PGP signature