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

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

[Ludovic Courtès]

* 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))
+
+\f
+;;;
+;;; 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
+  (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}."))
+
+(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
+                   (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)))
+   (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
-- 
2.34.0

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

[Andrew Tropin]

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

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))
> +
> +\f
> +;;;
> +;;; 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Ludovic Courtès]

Hi Andrew,

Andrew Tropin <andrew@trop.in> skribis:

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

[...]

>> +(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.

Oops, indeed; I’ll add it.

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

What would the service do when set to #f?

>> +  (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:

For the record, the whole project avoids abbreviations.  I think it’s an
important part of making things intelligible, especially to non-native
speakers.

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

Yes, that’s the usual tradeoff.  The choice made so far in Guix has been
to choose clarity over faithfulness to upstream’s name choices.

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

Oops yes, I’ll add an escape hatch.

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

I’ll look into it, and I think that’ll help me understand why this
file-like vs. string is so important to you.

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

Yes.  Again, that’s the choice we made in Guix: providing bindings for
config file formats.  It’s ambitious, but it’s worked well so far.  If
it worked for the Dovecot, surely it won’t be a problem here.  :-)

> 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 view ‘guix home import’ as a helper, like ‘guix import’.  I don’t
think it would make sense to have it automatically handle all the config
files that could possibly be handled by Guix Home services.

> 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)))))

I can see the appeal of alists, but the choice made in Guix is to use
records for configuration; that has advantages, such as type checking,
detection of incorrect field names, and the ability to use all the bells
and whistles of (guix records).

[...]

>> +  (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.

Oh, I see.  I’ll add a FIXME.  In practice, that problem would manifest
only if someone logs in at the console first, right?

Perhaps we could define a pseudo ‘xserver-xorg’ Shepherd service that
would be down when ‘DISPLAY’ is undefined, or something like that?

>> +                   (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.

Regarding the former, that’s not something we usually do for system
services.

As for the latter, I thought about it but I’m not sure what it would be
used for.  WDYT?

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

In that case it may be best to let users explicitly install it in their
profile maybe?

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

I understand your concerns, but I think they’re beyond the scope of this
review.  I also think that there’s ample experience with system services
showing that writing “nice” configuration bindings actually works in
practice.

Thanks,
Ludo’.

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

[Andrew Tropin]

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

On 2022-01-28 19:37, Ludovic Courtès wrote:

> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> 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.
>
> [...]
>
>>> +(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.
>
> Oops, indeed; I’ll add it.
>

Good, thank you.

>> 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.
>
> What would the service do when set to #f?
>

Only add a package to profile and configuration to home-files.

>>> +  (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:
>
> For the record, the whole project avoids abbreviations.  I think it’s an
> important part of making things intelligible, especially to non-native
> speakers.
>

It's really cool. 

>> 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.
>
> Yes, that’s the usual tradeoff.  The choice made so far in Guix has been
> to choose clarity over faithfulness to upstream’s name choices.
>

Maybe I'm wrong, but it's very likely that most of the users will be
checking out upstream documentation anyway during configuration of some
programs and those renamings will bring a lot of confusion and
especially, when the record fields names will be combined with names in
escape hatches.

>> 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.
>
> Oops yes, I’ll add an escape hatch.
>
>> 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.
>
> I’ll look into it, and I think that’ll help me understand why this
> file-like vs. string is so important to you.
>

I'll make another reply to the second version of the patch and highlight
this topic a little.

>> 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.
>
> Yes.  Again, that’s the choice we made in Guix: providing bindings for
> config file formats.  It’s ambitious, but it’s worked well so far.  If
> it worked for the Dovecot, surely it won’t be a problem here.  :-)
>
>> 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 view ‘guix home import’ as a helper, like ‘guix import’.  I don’t
> think it would make sense to have it automatically handle all the config
> files that could possibly be handled by Guix Home services.
>

I treat it the same, I forgot to add that guix home import is just an
example, any generation of configurations or other automated processing
becomes magnitudes harder.  Even including sophisticated "type checks".

>> 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)))))
>
> I can see the appeal of alists, but the choice made in Guix is to use
> records for configuration; that has advantages, such as type checking,
> detection of incorrect field names, and the ability to use all the bells
> and whistles of (guix records).

Type checks are possible with data structure driven approach as well and
in a fact it's much more flexible and powerful, however to be fair it
will require some work to prepare a good framework for that like
https://github.com/plumatic/schema
or
https://github.com/metosin/spec-tools/blob/master/docs/02_data_specs.md
for Clojure.

It's also possible to generate documentation for such specs.

Potentially, such approach is more powerful.

However, the big pros of guix records, that it's already have some
whistles and bells.

>
> [...]
>
>>> +  (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.
>
> Oh, I see.  I’ll add a FIXME.  In practice, that problem would manifest
> only if someone logs in at the console first, right?

Mostly yes.

>
> Perhaps we could define a pseudo ‘xserver-xorg’ Shepherd service that
> would be down when ‘DISPLAY’ is undefined, or something like that?
>

It's only a part of the puzzle, we also need somehow to make this
service to share environment variables with other services, when X
window manager or Wayland compositor is finally started.

>>> +                   (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.
>
> Regarding the former, that’s not something we usually do for system
> services.

Imagine terminal or almost any other user space program, which doesn't
have a configuration in ~/.config and binary in the profile.  Many of
home services will require both to make underlying programs operate
correctly (also think about setting search paths and similar things).  I
can imagine some exceptions, but better to keep it consistent and do it
for all home services not to confuse people.

>
> As for the latter, I thought about it but I’m not sure what it would be
> used for.  WDYT?
>

It can be used for debugging, for man pages or when redshift don't use
shepherd service and started in different way (by wm for example).

>> 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.
>
> In that case it may be best to let users explicitly install it in their
> profile maybe?
>

I think for home services it's ok to always add a package to profile.

>> 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.
>
> I understand your concerns, but I think they’re beyond the scope of this
> review.  I also think that there’s ample experience with system services
> showing that writing “nice” configuration bindings actually works in
> practice.

I saw how well-written, but macros-based solutions in Clojure ecosystem
slowly died and substituted with data-based.  I understand that Guile
ecosystem has a slightly weaker toolkit for processing datastructures,
but by the end of the day I think we will be here sooner or later.
Using macros instead of datastructures feels for me like remaking the
same mistake again knowing the consequences.  Maybe I'm wrong.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Ludovic Courtès]

Hi Andrew,

We’re drifting away from the practical issue of adding a Redshift
service, but you raise interesting issues.

Andrew Tropin <andrew@trop.in> skribis:

[...]

>> Yes, that’s the usual tradeoff.  The choice made so far in Guix has been
>> to choose clarity over faithfulness to upstream’s name choices.
>>
>
> Maybe I'm wrong, but it's very likely that most of the users will be
> checking out upstream documentation anyway during configuration of some
> programs and those renamings will bring a lot of confusion and
> especially, when the record fields names will be combined with names in
> escape hatches.

I think there doesn’t have to be a single answer.  For Redshift and its
handful of options, I see little incentive to go look at ‘man redshift’;
it doesn’t add much to what we provide.

For more complex services, the answer might be different, although again
the Dovecot service shows that, even for this big a service, we can
provide comprehensive bindings and associated documentation.

[...]

>> I can see the appeal of alists, but the choice made in Guix is to use
>> records for configuration; that has advantages, such as type checking,
>> detection of incorrect field names, and the ability to use all the bells
>> and whistles of (guix records).
>
> Type checks are possible with data structure driven approach as well and
> in a fact it's much more flexible and powerful, however to be fair it
> will require some work to prepare a good framework for that like
> https://github.com/plumatic/schema
> or
> https://github.com/metosin/spec-tools/blob/master/docs/02_data_specs.md
> for Clojure.
>
> It's also possible to generate documentation for such specs.
>
> Potentially, such approach is more powerful.

I’m aware of Clojure specs, but I don’t find it convincing compared to
records, at least for our use case.

[...]

>>> 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.
>>
>> Regarding the former, that’s not something we usually do for system
>> services.
>
> Imagine terminal or almost any other user space program

I’m not imagining: we’re discussing a very concrete service here.  :-)

For Redshift, I don’t see the point of making the config available
globally.  For system services, there’s only a handful of exception (PAM
and OpenSSH come to mind, but see /etc).

Again, there doesn’t have to be a single answer.  I suspect many
services won’t need to make their config available under ~/.config, but
if some do, so be it.  I’d say that the default should be to not make
config available unless that’s required, just like what we do for system
services.

How does that sound?

>> As for the latter, I thought about it but I’m not sure what it would be
>> used for.  WDYT?
>>
>
> It can be used for debugging, for man pages or when redshift don't use
> shepherd service and started in different way (by wm for example).

The point of this Redshift service is to have it started automatically,
so to me the only reason to add ‘redshift’ to the user profile would be
to allow ‘man redshift’.

I don’t view it as super useful in this case, and would instead lean on
the side of not “polluting” the user’s profile, but I can very well
imagine that in other cases we’d prefer to extend the user’s profile.

>> I understand your concerns, but I think they’re beyond the scope of this
>> review.  I also think that there’s ample experience with system services
>> showing that writing “nice” configuration bindings actually works in
>> practice.
>
> I saw how well-written, but macros-based solutions in Clojure ecosystem
> slowly died and substituted with data-based.  I understand that Guile
> ecosystem has a slightly weaker toolkit for processing datastructures,
> but by the end of the day I think we will be here sooner or later.
> Using macros instead of datastructures feels for me like remaking the

Records are data structures, not macros.

> same mistake again knowing the consequences.  Maybe I'm wrong.

Maybe one of us is wrong, or maybe it’s more complex than this.  :-)

As it turns out, I find Guix’s configuration records rather nice to
use—much nicer than, for example, Gnus’ loose configuration trees.

Thanks for your feedback,
Ludo’.

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

[Andrew Tropin]

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

On 2022-02-01 10:15, Ludovic Courtès wrote:

> Hi Andrew,
>
> We’re drifting away from the practical issue of adding a Redshift
> service, but you raise interesting issues.

That's true, but a few first home services will set the tone for the
rest, so it's probably a good idea to look ahead right now, then later.

>
> Andrew Tropin <andrew@trop.in> skribis:
>
> [...]
>
>>> Yes, that’s the usual tradeoff.  The choice made so far in Guix has been
>>> to choose clarity over faithfulness to upstream’s name choices.
>>>
>>
>> Maybe I'm wrong, but it's very likely that most of the users will be
>> checking out upstream documentation anyway during configuration of some
>> programs and those renamings will bring a lot of confusion and
>> especially, when the record fields names will be combined with names in
>> escape hatches.
>
> I think there doesn’t have to be a single answer.  For Redshift and its
> handful of options, I see little incentive to go look at ‘man redshift’;
> it doesn’t add much to what we provide.
>
> For more complex services, the answer might be different, although again
> the Dovecot service shows that, even for this big a service, we can
> provide comprehensive bindings and associated documentation.
>
> [...]
>
>>> I can see the appeal of alists, but the choice made in Guix is to use
>>> records for configuration; that has advantages, such as type checking,
>>> detection of incorrect field names, and the ability to use all the bells
>>> and whistles of (guix records).
>>
>> Type checks are possible with data structure driven approach as well and
>> in a fact it's much more flexible and powerful, however to be fair it
>> will require some work to prepare a good framework for that like
>> https://github.com/plumatic/schema
>> or
>> https://github.com/metosin/spec-tools/blob/master/docs/02_data_specs.md
>> for Clojure.
>>
>> It's also possible to generate documentation for such specs.
>>
>> Potentially, such approach is more powerful.
>
> I’m aware of Clojure specs, but I don’t find it convincing compared to
> records, at least for our use case.
>

Ok.

>>>> 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.
>>>
>>> Regarding the former, that’s not something we usually do for system
>>> services.
>>
>> Imagine terminal or almost any other user space program
>
> I’m not imagining: we’re discussing a very concrete service here.  :-)
>
> For Redshift, I don’t see the point of making the config available
> globally.  For system services, there’s only a handful of exception (PAM
> and OpenSSH come to mind, but see /etc).
>
> Again, there doesn’t have to be a single answer.  I suspect many
> services won’t need to make their config available under ~/.config, but
> if some do, so be it.  I’d say that the default should be to not make
> config available unless that’s required, just like what we do for system
> services.
>
> How does that sound?

It sounds logical for redshift, but inconsistent in general: some home
services install package to profile, some not, some create config in
XDG_CONFIG_DIR, some not.

I still think that almost all services must provide both package and
configs.

>>> As for the latter, I thought about it but I’m not sure what it would be
>>> used for.  WDYT?
>>>
>>
>> It can be used for debugging, for man pages or when redshift don't use
>> shepherd service and started in different way (by wm for example).
>
> The point of this Redshift service is to have it started automatically,
> so to me the only reason to add ‘redshift’ to the user profile would be
> to allow ‘man redshift’.
>
> I don’t view it as super useful in this case, and would instead lean on
> the side of not “polluting” the user’s profile, but I can very well
> imagine that in other cases we’d prefer to extend the user’s profile.
>
>>> I understand your concerns, but I think they’re beyond the scope of this
>>> review.  I also think that there’s ample experience with system services
>>> showing that writing “nice” configuration bindings actually works in
>>> practice.
>>
>> I saw how well-written, but macros-based solutions in Clojure ecosystem
>> slowly died and substituted with data-based.  I understand that Guile
>> ecosystem has a slightly weaker toolkit for processing datastructures,
>> but by the end of the day I think we will be here sooner or later.
>> Using macros instead of datastructures feels for me like remaking the
>
> Records are data structures, not macros.
>

But, IIRC, define-configuration, define-record-type are.

>> same mistake again knowing the consequences.  Maybe I'm wrong.
>
> Maybe one of us is wrong, or maybe it’s more complex than this.  :-)

Also, maybe my non-guile experience doesn't apply here.

>
> As it turns out, I find Guix’s configuration records rather nice to
> use—much nicer than, for example, Gnus’ loose configuration trees.

Unfortunately, I'm not familiar with Gnus internals and can't say if
it's related, similar or not.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Andrew Tropin]

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

On 2022-02-01 10:15, Ludovic Courtès wrote:

> Hi Andrew,
>
> We’re drifting away from the practical issue of adding a Redshift
> service, but you raise interesting issues.

I have a few more to raise, but they are unrelated to redshift and even
weakly related to configuration styles :)

Overall, I'm ok with this redshift service and if you are sure that
tradeoffs accepted by guix system services are correct even in the light
of issues I mentioned and we don't need to try a slightly different and
more data-structure oriented way, I think we can proceed with this
service.

Just give it one more week, after a next revision of the patch with
fixed escape hatch is published, just in case someone else has good
arguments, but didn't speak yet.

I think I'll move (gnu home-services ...) I personally use and
incompatible with system services approach to (rde home services) not to
confuse people and will maintain them myself.  But I'm ok if someone
would like to rewrite to the usual style and upstream them, I will help
with the review.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Ludovic Courtès]

Hi Andrew,

I hadn’t noticed this message of yours earlier…

Andrew Tropin <andrew@trop.in> skribis:

> I think I'll move (gnu home-services ...) I personally use and
> incompatible with system services approach to (rde home services) not to
> confuse people and will maintain them myself.  But I'm ok if someone
> would like to rewrite to the usual style and upstream them, I will help
> with the review.

Alright noted.  I would obviously prefer if you would instead contribute
those services to Guix Home proper—that was the whole point of getting
it merge in Guix in the first place, right?

I feel that perhaps, contrary to what I thought, we hadn’t reached a
shared understanding of what it means to maintain things collectively
withing Guix.  It’s a bit of a sad situation, but I’m sure there’ll
still be good things happening in Guix Home, and in rde.

Thanks,
Ludo’.

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

[Andrew Tropin]

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

On 2022-02-08 10:22, Ludovic Courtès wrote:

> Hi Andrew,
>
> I hadn’t noticed this message of yours earlier…
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> I think I'll move (gnu home-services ...) I personally use and
>> incompatible with system services approach to (rde home services) not to
>> confuse people and will maintain them myself.  But I'm ok if someone
>> would like to rewrite to the usual style and upstream them, I will help
>> with the review.
>
> Alright noted.  I would obviously prefer if you would instead contribute
> those services to Guix Home proper—that was the whole point of getting
> it merge in Guix in the first place, right?

Originally, I planned to upstream them, but the implementation of rde
home services is quite different from guix system services, that means
it would be necessary to almost completely rewrite them to match the
style of system services and move them to Guix proper.  In addition to
updating rde parts depending on those services it's a humongous amount
of work.  I can't do it right now for various reasons, so I will just
move them from (gnu home-services) to prevent future compatibilty
problems and the overall amount the usage by people who are looking for
guix home services.

In the meantime people looking for some missing home services can take
an inspiration from rde home service and send patches to guix-patches
with proper implementation, I can help with code review and provide some
insights, suggestions and knowledge about possible pitfalls.

Maybe in the future rde will be able to migrate to guix home services.

> I feel that perhaps, contrary to what I thought, we hadn’t reached a
> shared understanding of what it means to maintain things collectively
> withing Guix.  It’s a bit of a sad situation, but I’m sure there’ll
> still be good things happening in Guix Home, and in rde.

I try to upstream everything I can, to make everyone in Guix community
be able benefit from it and also to make a code benefit from collective
maintenance in Guix repo.  BTW, sending you a lot of kudos for improving
guix home and symlink-manager code.

Resending this email to keep public record of it. (It was declined by
debbugs cause issue was archived).

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Ludovic Courtès]

* 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                 |  70 +++++++++++++++
 gnu/home/services/desktop.scm | 158 ++++++++++++++++++++++++++++++++++
 gnu/local.mk                  |   1 +
 3 files changed, 229 insertions(+)
 create mode 100644 gnu/home/services/desktop.scm

Hello!

Changes compared to v1 account for Andrew’s suggestions:

  • add ‘redshift’ field to specify the package to use;

  • add ‘extra-content’ field as an escape hatch.

We could debate about the latter; from a pragmatic standpoint,
I think it gives all the flexibility one would need in practice.

Thoughts?

Ludo’.

diff --git a/doc/guix.texi b/doc/guix.texi
index 94f8e5e481..67a5517911 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -37461,6 +37461,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
 
@@ -37848,6 +37849,75 @@ 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{redshift} (default: @code{redshift}) (type: file-like)
+Redshift package to use.
+
+@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}.
+
+@item @code{extra-content} (default: @code{""}) (type: raw-configuration-string)
+Extra content appended as-is to the Redshift configuration file.  Run
+@command{man redshift} for more information about the configuration file
+format.
+
+@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..010668550a
--- /dev/null
+++ b/gnu/home/services/desktop.scm
@@ -0,0 +1,158 @@
+;;; 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))
+
+\f
+;;;
+;;; 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 (serialize-raw-configuration-string field value)
+  value)
+(define raw-configuration-string? string?)
+
+(define-configuration home-redshift-configuration
+  (redshift
+   (file-like redshift)
+   "Redshift package to use.")
+
+  (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}.")
+
+  (extra-content
+   (raw-configuration-string "")
+   "Extra content appended as-is to the Redshift configuration file.  Run
+@command{man redshift} for more information about the configuration file
+format."))
+
+(define (serialize-redshift-configuration config)
+  (define location-fields
+    '(latitude longitude))
+
+  (define (location-field? field)
+    (memq (configuration-field-name field) location-fields))
+
+  (define (secondary-field? field)
+    (or (location-field? field)
+        (memq (configuration-field-name field)
+              '(redshift extra-content))))
+
+  #~(string-append
+     "[redshift]\n"
+     #$(serialize-configuration config
+                                (remove secondary-field?
+                                        home-redshift-configuration-fields))
+     "\n[manual]\n"
+     #$(serialize-configuration config
+                                (filter location-field?
+                                        home-redshift-configuration-fields))
+
+     #$(home-redshift-configuration-extra-content config)))
+
+(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))
+         ;; FIXME: This fails to start if Home is first activated from a
+         ;; non-X11 session.
+         (start #~(make-forkexec-constructor
+                   (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)))
+   (default-value (home-redshift-configuration))
+   (description
+    "Run Redshift, a program that adjusts the color temperature of display
+according to time of day.")))
diff --git a/gnu/local.mk b/gnu/local.mk
index 27e7877361..80cb760132 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -79,6 +79,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: 27c1d58d901dcf48929bcb6f76d861fc21575dbf
-- 
2.34.0

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

[Maxime Devos]

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

Ludovic Courtès schreef op zo 30-01-2022 om 16:11 [+0100]:
> +@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.

It would be nice to document which color adjustment methods exist,
as done for 'location-provider'.

Greetings,
Maxime

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

[Ludovic Courtès]

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op zo 30-01-2022 om 16:11 [+0100]:
>> +@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.
>
> It would be nice to document which color adjustment methods exist,
> as done for 'location-provider'.

Good idea, will do.

Thanks,
Ludo’.

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

[Andrew Tropin]

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

On 2022-01-30 16: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                 |  70 +++++++++++++++
>  gnu/home/services/desktop.scm | 158 ++++++++++++++++++++++++++++++++++
>  gnu/local.mk                  |   1 +
>  3 files changed, 229 insertions(+)
>  create mode 100644 gnu/home/services/desktop.scm
>
> Hello!
>
> Changes compared to v1 account for Andrew’s suggestions:
>
>   • add ‘redshift’ field to specify the package to use;
>
>   • add ‘extra-content’ field as an escape hatch.
>
> We could debate about the latter; from a pragmatic standpoint,
> I think it gives all the flexibility one would need in practice.
>
> Thoughts?

Probably, I already mentioned, but combining renamed option names from
the configuration record and option names in the escape hatch is
inconsistent, confusing and error-prone.

>
> Ludo’.
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 94f8e5e481..67a5517911 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -37461,6 +37461,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
>  
> @@ -37848,6 +37849,75 @@ 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{redshift} (default: @code{redshift}) (type: file-like)
> +Redshift package to use.
> +
> +@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}.
> +
> +@item @code{extra-content} (default: @code{""}) (type: raw-configuration-string)
> +Extra content appended as-is to the Redshift configuration file.  Run
> +@command{man redshift} for more information about the configuration file
> +format.
> +
> +@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..010668550a
> --- /dev/null
> +++ b/gnu/home/services/desktop.scm
> @@ -0,0 +1,158 @@
> +;;; 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))
> +
> +\f
> +;;;
> +;;; 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 (serialize-raw-configuration-string field value)
> +  value)
> +(define raw-configuration-string? string?)
> +
> +(define-configuration home-redshift-configuration
> +  (redshift
> +   (file-like redshift)
> +   "Redshift package to use.")
> +
> +  (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}.")
> +
> +  (extra-content
> +   (raw-configuration-string "")
> +   "Extra content appended as-is to the Redshift configuration file.  Run
> +@command{man redshift} for more information about the configuration file
> +format."))
> +
> +(define (serialize-redshift-configuration config)
> +  (define location-fields
> +    '(latitude longitude))
> +
> +  (define (location-field? field)
> +    (memq (configuration-field-name field) location-fields))
> +
> +  (define (secondary-field? field)
> +    (or (location-field? field)
> +        (memq (configuration-field-name field)
> +              '(redshift extra-content))))
> +
> +  #~(string-append
> +     "[redshift]\n"
> +     #$(serialize-configuration config
> +                                (remove secondary-field?
> +                                        home-redshift-configuration-fields))
> +     "\n[manual]\n"
> +     #$(serialize-configuration config
> +                                (filter location-field?
> +                                        home-redshift-configuration-fields))
> +

It's very unclear where this extra-content goes and user can't know it
until he check out the implementation or build the config (currently
it's also almost impossible to find it on file system after build).
Using such type of escape hatch is no joy at all.

Seems this one is missplaced and should be go before [manual] section,
otherwise it won't be possible to set values of redshift section.  And
doing so will lead to very ugly:

(extra-content "\
dawn-time=5:30
dusk-time=18:30
[geoclue2]
some-other-option=value
# Do I know that I'm in the middle of config file?")

It will be especially ugly or even erroneous, when the target config has
a format, which uses identation.

I didn't try it for redshift, but in many ini parser it's forbidden to
repeat sections with the same name.
> 
> +     #$(home-redshift-configuration-extra-content config)))

A little offtopic:

I know a number of system services, where the extra-content goes in
unexpected locations and overall behavior of escape hatch is unexpected
and incosistent with other escape hatches.  Some of the services has a
number of escape hatches in almost every nested record with different
names and behaviors.

I'm relatively fresh Guix user and this part really confused me at first
even having experience with NixOS module system before and it's very
likely that many people just don't report it, because it really hard to
get the roots of it.

> +
> +(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))
> +         ;; FIXME: This fails to start if Home is first activated from a
> +         ;; non-X11 session.
> +         (start #~(make-forkexec-constructor
> +                   (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)))
> +   (default-value (home-redshift-configuration))
> +   (description
> +    "Run Redshift, a program that adjusts the color temperature of display
> +according to time of day.")))
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 27e7877361..80cb760132 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -79,6 +79,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: 27c1d58d901dcf48929bcb6f76d861fc21575dbf

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Ludovic Courtès]

Hi,

Andrew Tropin <andrew@trop.in> skribis:

> On 2022-01-30 16: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.

[...]

> Probably, I already mentioned, but combining renamed option names from
> the configuration record and option names in the escape hatch is
> inconsistent, confusing and error-prone.

I think I replied already.

> It's very unclear where this extra-content goes and user can't know it
> until he check out the implementation or build the config (currently
> it's also almost impossible to find it on file system after build).

(He or she.)  I documented it in a way that I thought was clear:

  @item @code{extra-content} (default: @code{""}) (type: raw-configuration-string)
  Extra content appended as-is to the Redshift configuration file.  […]

Notice “appended”.  How would you phrase it?

> Using such type of escape hatch is no joy at all.

Escape hatches are meant to be used as a last resort; they’re a hack.
Normally, people would have everything they need with the provided
bindings.  So yes, using them is no fun, but that’s not a surprise IMO.

> Seems this one is missplaced and should be go before [manual] section,
> otherwise it won't be possible to set values of redshift section.  And
> doing so will lead to very ugly:
>
> (extra-content "\
> dawn-time=5:30
> dusk-time=18:30
> [geoclue2]
> some-other-option=value
> # Do I know that I'm in the middle of config file?")
>
> It will be especially ugly or even erroneous, when the target config has
> a format, which uses identation.
>
> I didn't try it for redshift, but in many ini parser it's forbidden to
> repeat sections with the same name.

Ah, bummer.

Another way to see it is that I should augment the bindings, maybe
that’s what you’re getting at?  :-)

I can do that.

>> +     #$(home-redshift-configuration-extra-content config)))
>
> A little offtopic:
>
> I know a number of system services, where the extra-content goes in
> unexpected locations and overall behavior of escape hatch is unexpected
> and incosistent with other escape hatches.  Some of the services has a
> number of escape hatches in almost every nested record with different
> names and behaviors.
>
> I'm relatively fresh Guix user and this part really confused me at first
> even having experience with NixOS module system before and it's very
> likely that many people just don't report it, because it really hard to
> get the roots of it.

How does NixOS handle escape hatches today?  Back when I was using it is
that it wasn’t any more consistent than what we have today, which is at
least a consolation.

Thanks,
Ludo’.

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

[Andrew Tropin]

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

On 2022-02-01 09:43, Ludovic Courtès wrote:

> Hi,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> On 2022-01-30 16: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.
>
> [...]
>
>> Probably, I already mentioned, but combining renamed option names from
>> the configuration record and option names in the escape hatch is
>> inconsistent, confusing and error-prone.
>
> I think I replied already.
>
>> It's very unclear where this extra-content goes and user can't know it
>> until he check out the implementation or build the config (currently
>> it's also almost impossible to find it on file system after build).
>
> (He or she.)  I documented it in a way that I thought was clear:
>
>   @item @code{extra-content} (default: @code{""}) (type: raw-configuration-string)
>   Extra content appended as-is to the Redshift configuration file.  […]
>
> Notice “appended”.  How would you phrase it?

(:

>> Using such type of escape hatch is no joy at all.
>
> Escape hatches are meant to be used as a last resort; they’re a hack.
> Normally, people would have everything they need with the provided
> bindings.  So yes, using them is no fun, but that’s not a surprise IMO.
>
>> Seems this one is missplaced and should be go before [manual] section,
>> otherwise it won't be possible to set values of redshift section.  And
>> doing so will lead to very ugly:
>>
>> (extra-content "\
>> dawn-time=5:30
>> dusk-time=18:30
>> [geoclue2]
>> some-other-option=value
>> # Do I know that I'm in the middle of config file?")
>>
>> It will be especially ugly or even erroneous, when the target config has
>> a format, which uses identation.
>>
>> I didn't try it for redshift, but in many ini parser it's forbidden to
>> repeat sections with the same name.
>
> Ah, bummer.
>
> Another way to see it is that I should augment the bindings, maybe
> that’s what you’re getting at?  :-)

Sorry, not sure if I can translate your question correctly, so I won't
try to answer it, but will rephrase my statement to crarify what I mean:

The current implementation of escape hatch doesn't work (I can't set
missing options in redshift section, because extra-content added after
manual section begins, and I'm not sure if redshift's ini parser allows
to define a section with the same name a few times, Git for example
allows to do so, but it's more an exception).  There are a few ways to fix it:

1. Move manual section on top of redshift section, so the extra-content
will be added just after redshift section
2. Move extra-content before manual section to accoplish the same.
3. Use file escape hatch (as it done in some system services), which
will invalidate all the options set before and just place the content of
file field as a value of configuration.
4. Provide the bindings for other options (probably what you were asking).

>
> I can do that.
>
>>> +     #$(home-redshift-configuration-extra-content config)))
>>
>> A little offtopic:
>>
>> I know a number of system services, where the extra-content goes in
>> unexpected locations and overall behavior of escape hatch is unexpected
>> and incosistent with other escape hatches.  Some of the services has a
>> number of escape hatches in almost every nested record with different
>> names and behaviors.
>>
>> I'm relatively fresh Guix user and this part really confused me at first
>> even having experience with NixOS module system before and it's very
>> likely that many people just don't report it, because it really hard to
>> get the roots of it.
>
> How does NixOS handle escape hatches today?  Back when I was using it is
> that it wasn’t any more consistent than what we have today, which is at
> least a consolation.

I don't remember, how it's done for nixos "system modules", but in
home-manager it's relatively consistent, in most cases it just a nested
maps (attrsets, a little more high-level analogue of alists):
https://rycee.gitlab.io/home-manager/options.html#opt-programs.git.extraConfig
https://rycee.gitlab.io/home-manager/options.html#opt-accounts.email.accounts._name_.msmtp.extraConfig
https://rycee.gitlab.io/home-manager/options.html#opt-gtk.gtk3.extraConfig

Sometimes, it allows to add a multi-line string to the end of the file:
https://rycee.gitlab.io/home-manager/options.html#opt-programs.abook.extraConfig

but it's getting deprecated, when proper serializer for target config
format is implemented:
https://github.com/nix-community/home-manager/blob/master/modules/programs/git.nix#L342

As you can see, they provide a list of frequently used options as
top-level "fields", but all of them, including extraConfig just extends
(get merged) to primary "config" (iniContent for git for example) and
after that, this primary "config" attributeset get serialized to the
target config file.  While they have this "escape hatch", it works the
same way as all other fields with the only difference: it doesn't have
schema (just untyped attributeset).  This is quite close to what we do
in rde.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

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

[Ludovic Courtès]

Hi Andrew,

Andrew Tropin <andrew@trop.in> skribis:

> The current implementation of escape hatch doesn't work (I can't set
> missing options in redshift section, because extra-content added after
> manual section begins, and I'm not sure if redshift's ini parser allows
> to define a section with the same name a few times, Git for example
> allows to do so, but it's more an exception).  There are a few ways to fix it:
>
> 1. Move manual section on top of redshift section, so the extra-content
> will be added just after redshift section
> 2. Move extra-content before manual section to accoplish the same.
> 3. Use file escape hatch (as it done in some system services), which
> will invalidate all the options set before and just place the content of
> file field as a value of configuration.
> 4. Provide the bindings for other options (probably what you were asking).

Ah got it.  I went for #2 and #4 (there are still missing options, but
we can add them when people actually need them.)

Pushed as 39e8025d3b40a0079f75e0ce9a91b6dad6766773.

>> How does NixOS handle escape hatches today?  Back when I was using it is
>> that it wasn’t any more consistent than what we have today, which is at
>> least a consolation.
>
> I don't remember, how it's done for nixos "system modules", but in
> home-manager it's relatively consistent, in most cases it just a nested
> maps (attrsets, a little more high-level analogue of alists):
> https://rycee.gitlab.io/home-manager/options.html#opt-programs.git.extraConfig
> https://rycee.gitlab.io/home-manager/options.html#opt-accounts.email.accounts._name_.msmtp.extraConfig
> https://rycee.gitlab.io/home-manager/options.html#opt-gtk.gtk3.extraConfig
>
> Sometimes, it allows to add a multi-line string to the end of the file:
> https://rycee.gitlab.io/home-manager/options.html#opt-programs.abook.extraConfig
>
> but it's getting deprecated, when proper serializer for target config
> format is implemented:
> https://github.com/nix-community/home-manager/blob/master/modules/programs/git.nix#L342
>
> As you can see, they provide a list of frequently used options as
> top-level "fields", but all of them, including extraConfig just extends
> (get merged) to primary "config" (iniContent for git for example) and
> after that, this primary "config" attributeset get serialized to the
> target config file.  While they have this "escape hatch", it works the
> same way as all other fields with the only difference: it doesn't have
> schema (just untyped attributeset).  This is quite close to what we do
> in rde.

OK, I see.  Nix is all about attribute sets, which are dictionaries,
like alists, and just as sloppy: you can add attributes but they may or
may not be ignored, you never know.  Here they “take advantage” of that,
IIUC, by serializing attributes that happen to be here.

Nix (the language) doesn’t have the ability to define disjoint types
like what we’re doing with configuration record types, and to me that’s
a weakness that directly affects usability.

Thanks,
Ludo’.

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

[Andrew Tropin]

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

On 2022-02-07 00:13, Ludovic Courtès wrote:

> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> The current implementation of escape hatch doesn't work (I can't set
>> missing options in redshift section, because extra-content added after
>> manual section begins, and I'm not sure if redshift's ini parser allows
>> to define a section with the same name a few times, Git for example
>> allows to do so, but it's more an exception).  There are a few ways to fix it:
>>
>> 1. Move manual section on top of redshift section, so the extra-content
>> will be added just after redshift section
>> 2. Move extra-content before manual section to accoplish the same.
>> 3. Use file escape hatch (as it done in some system services), which
>> will invalidate all the options set before and just place the content of
>> file field as a value of configuration.
>> 4. Provide the bindings for other options (probably what you were asking).
>
> Ah got it.  I went for #2 and #4 (there are still missing options, but
> we can add them when people actually need them.)
>
> Pushed as 39e8025d3b40a0079f75e0ce9a91b6dad6766773.
>

Good, thank you for working on this!)

>>> How does NixOS handle escape hatches today?  Back when I was using it is
>>> that it wasn’t any more consistent than what we have today, which is at
>>> least a consolation.
>>
>> I don't remember, how it's done for nixos "system modules", but in
>> home-manager it's relatively consistent, in most cases it just a nested
>> maps (attrsets, a little more high-level analogue of alists):
>> https://rycee.gitlab.io/home-manager/options.html#opt-programs.git.extraConfig
>> https://rycee.gitlab.io/home-manager/options.html#opt-accounts.email.accounts._name_.msmtp.extraConfig
>> https://rycee.gitlab.io/home-manager/options.html#opt-gtk.gtk3.extraConfig
>>
>> Sometimes, it allows to add a multi-line string to the end of the file:
>> https://rycee.gitlab.io/home-manager/options.html#opt-programs.abook.extraConfig
>>
>> but it's getting deprecated, when proper serializer for target config
>> format is implemented:
>> https://github.com/nix-community/home-manager/blob/master/modules/programs/git.nix#L342
>>
>> As you can see, they provide a list of frequently used options as
>> top-level "fields", but all of them, including extraConfig just extends
>> (get merged) to primary "config" (iniContent for git for example) and
>> after that, this primary "config" attributeset get serialized to the
>> target config file.  While they have this "escape hatch", it works the
>> same way as all other fields with the only difference: it doesn't have
>> schema (just untyped attributeset).  This is quite close to what we do
>> in rde.
>
> OK, I see.  Nix is all about attribute sets, which are dictionaries,
> like alists, and just as sloppy: you can add attributes but they may or
> may not be ignored, you never know.  Here they “take advantage” of that,
> IIUC, by serializing attributes that happen to be here.
>
> Nix (the language) doesn’t have the ability to define disjoint types
> like what we’re doing with configuration record types, and to me that’s
> a weakness that directly affects usability.
>
> Thanks,
> Ludo’.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]