[bug#54561] [PATCH 0/4] Add service declarations for Samba

["Ludovic Courtès" <ludo@gnu.org>]

Hi Simon,

Simon Streit <simon@netpanic.org> skribis:

> Please find attached an updated patch series.

It’s a huge amount of work that you did, and that’ll certainly be useful
to many!

> I've made slight changes as follows:
>
>  * The reference to further config options in the manual have been removed.
>  * Samba's (samba-activation config) procedure has been slightly modified,
>  * better cleaned up, regarding the mkdirs.  I've done more testing and it
>  * appears that samba will only run when /var/{lib,log,run}/samba exist,
>    including /var/lib/samba/private.  In this case it is chmod now to o700 to
>    be on the save side.  Debian's directory structure is world readable though.
>    In Arch it is o700.  If anyone objects, please make it world readable.  It
>    appears that Samba lives and breathes in these directories, so they better
>    be put there. 
>  * Regarding smb.conf -- while this service technically doesn't need it placed
>    at /etc/samba -- is convenient to have it placed there for other tools part
>    of the Samba family to read it, and so that others can quickly look into its
>    configuration.  I'll leave this for further debate whether it can stay there
>    or not.
>  * The packages samba and wsdd are included in profile-service-type so that they
>    are generally available in the system profile.

I didn’t look at everything in detail, but overall that LGTM.

There’s a couple of things that I think would be worth adjusting though:

>   services: Add samba service.
>   doc: Add "Samba" chapter.
>   doc: Add documentation for WSDD service.
>   services: Add wsdd service.
>   gnu: Add wsdd.

It seems patches are in the wrong order: I’d expect the wsdd package to
come before the wsdd service.

Regarding documentation: by convention, documentation for a service is
added in the same commit that adds the service, so that it’s
self-contained.  Could you squash them?

Last, it would be great if you could add a system test under
gnu/tests/samba.scm.  Essentially, that test would do what you probably
did manually already: spawning a VM running an OS with
‘samba-service-type’ and/or ‘wsdd-service-type’ and running an SMB
and/or WSD client to make sure the basics work.  You can get inspiration
from other system tests there, and see:

  https://guix.gnu.org/manual/devel/en/html_node/Running-the-Test-Suite.html

I have minor cosmetic comments that I’ll send separately.

Could you send a v3 addressing these issues?

Thanks!

Ludo’.