Christopher Baines <mail@cbaines.net> writes:

> Ideally this wouldn't be in Miscellaneous Services as I don't think
> that is very helpful for users discovering what services Guix offers.
> I think Networking Services might be better, what do you think? It
> includes things like Tor, which are about using networks, and not
> about core networking. 

Done.

>> +@deffn {Scheme Variable} rsync-service-type
>> +This is the type for the @uref{https://rsync.samba.org} rsync daemon,
>> +@command{rsync-configuration} record as in this example:
>
> @uref{https://rsync.samba.org, rsync} looks nicer to me here.

Done.

>> +@example
>> +(service rsync-service-type
>> +         (rsync-configuration))
>> +@end example
>
> You may as well give the simpler (service rsync-service-type) version
> here now that the service-type has a default value.

Done.

>> +@item @code{port-number} (default: @code{873})
>> +TCP port on which @command{rsync} listens for incoming connections.
>> If +port is less than @code{1024} @command{rsync} will be started as
>> the +@code{root} user and group.
>
> I think this needs to read "needs to be" rather than "will be".

Done.

>> +@item @code{uid} (default: @var{"rsyncd"})
>> +User name or user ID that file transfers to and from that module
>> should take +place as when the daemon was run as @code{root}.
>> +
>> +@item @code{gid} (default: @var{"rsyncd"})
>> +Group name or group ID that will be  used  when  accessing the
>> module. 
>
> I'm not sure what "module" is here.

I took this description from *Man 5 rsyncd.conf* MODULE PARAMETERS.

  After the global parameters you should define a number of modules,
  each module exports a directory tree as a symbolic name. Modules are
  exported by specifying a module name in square brackets [module]
  followed by the param‐ eters for that module.  The module name cannot
  contain a slash or a closing square bracket.  If the name contains
  whitespace, each internal sequence of whitespace will be changed into
  a single space, while leading or trailing whitespace will be
  discarded.  Also, the name cannot be "global" as that exact name
  indicates that global parameters follow (see above).

>> +(define (rsync-activation config)
>> +  "Return the activation GEXP for CONFIG."
>> +  #~(begin
>> +      (use-modules (guix build utils))
>
> Using with-imported-modules with this gexp here would be good, to
> ensure it has the (guix build utils) module.

Done.

>> +(define (rsync-config-file config)
>> +  "Return the rsync configuration file corresponding to CONFIG."
>> +  (let ((port-number   (rsync-configuration-port-number config))
>> +        (pid-file      (rsync-configuration-pid-file config))
>> +        (lock-file     (rsync-configuration-lock-file config))
>> +        (log-file      (rsync-configuration-log-file config))
>> +        (use-chroot?   (rsync-configuration-use-chroot? config))
>> +        (share-path    (rsync-configuration-share-path config))
>> +        (share-comment (rsync-configuration-share-comment config))
>> +        (read-only?    (rsync-configuration-read-only? config))
>> +        (timeout       (rsync-configuration-timeout config))
>> +        (user          (rsync-configuration-user config))
>> +        (group         (rsync-configuration-group config))
>> +        (uid           (rsync-configuration-uid config))
>> +        (gid           (rsync-configuration-gid config)))
>
> Using match, or match-lambda might neaten this up a bit. There are a
> few examples of using this in Guix.

Done.

> I'm pretty happy with this now. I think updating the documentation to
> match the recent changes in the service would be good to do before
> merging this but that is all. Good job :)

Thank you for support!  I learned much new things.

Attached a patch.  Tested successfully.