Subject: Re: [bug#33265] [WIP RFC v4] services: Add file system monitoring service.

From: [email-masked] (Ludovic Courtès)
Date: Sat, 10 Nov 2018 23:19:53 +0100

Hello!

Danny Milosavljevic <[email-masked]> skribis:

> * gnu/services/monitoring.scm (file-system-monitoring-configuration): New
> variable.
> (file-system-monitoring-entry): New variable.
> (file-system-monitoring-service-type): New variable.
> * gnu/tests/monitoring.scm (%test-file-system-monitoring): New variable.
> * doc/guix.texi (File System Monitoring Service): New subsubsection.

Nice!  Should we call it ‘fswatch-monitoring-service’, since there may
be other tools to achieve similar results?

> +@cindex file system monitoring
> +@subsubheading File System Monitoring Service
> +
> +The @code{(gnu services monitoring)} module provides a shepherd service to

s/shepherd/Shepherd/

> +@defvr {Scheme Variable} file-system-monitoring-service-type
> +The service type for @command{fswatch}, which provides the file system
> +monitoring capability.

Perhaps add a @uref to the fswatch home page.

> +@example
> +(service file-system-monitoring-service-type
> + (file-system-monitoring-configuration
> +   (monitored-files '("/foo/bar"))
> +   (handlers '((lambda args
> +                 (display "UH OH\n"))))))

It would seem more natural to me for ‘handlers’ to be singular,
‘handler’.  Also I’d use a gexp there (though in this particular example
it doesn’t make any difference):

  (file-system-monitoring-configuration
    (monitored-files '("/foo/bar"))
    (handler #~(lambda (file whatever)
                 (format #t "~a modified!~%" file))))

> +           (start #~(let ((handlers
> +                           (list #$@(map file-system-monitoring-entry-handler
> +                                         monitored-files))))
> +                      (lambda ()
> +                        (sleep 1)
> +                        (for-each (lambda (handler)
> +                                    (handler))
> +                                  handlers)
> +                        (fork+exec-command
> +                         `(#$(file-append package "/bin/fswatch")
> +                           "--one-event"
> +                           "-l" "1" ; latency: 1 s
> +                           ; "-d"
> +                           "--"
> +                           #$@(if monitored-files
> +                                  (map file-system-monitoring-entry-file-name
> +                                       monitored-files)
> +                                  '("/does_not_exist")))))))

The ‘sleep’ call looks suspicious.  :-)

IIUC, the service stops (and is respawned) every time an even occurs, is
that right?

Can’t we instead remove ‘--one-event’ and pass fswatch a script to run?
In that case, we’d pass it something along these lines:

  #$(scheme-file "event-handler.scm"
                 #~(for-each (lambda (handler) …) …))

WDYT?

> +(define file-system-monitoring-service-type
> +  (service-type (name 'monitor-file-system)
> +                (extensions
> +                 (list (service-extension shepherd-root-service-type
> +                                          file-system-monitoring-shepherd-services)))
> +                (compose concatenate)
> +                (extend (lambda (config monitored-entries)
> +                          (let ((monitored-files
> +                                 (map file-system-monitoring-entry-file-name
> +                                      monitored-entries))
> +                                (handlers
> +                                 (map file-system-monitoring-entry-handler
> +                                      monitored-entries)))

So here if we changes ‘handlers’ to ‘handler’, we could do:

  (apply compose (map file-system-monitoring-entry-handler entries))

Thanks,
Ludo’.