[bug#53466] home: Add redshift service.

* 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


base-commit: ee6bf00b2d89f6acab55b7a82896d99e39c1229b

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

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

(redshift
 (package redshift))

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

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

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

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

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

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

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

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

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

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

So final configuration will be something like:

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

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

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

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

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

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

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

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

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

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

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

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.

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.

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.

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

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