[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#54561] [PATCH 0/4] Add service declarations for Samba
From: |
Ludovic Courtès |
Subject: |
[bug#54561] [PATCH 0/4] Add service declarations for Samba |
Date: |
Fri, 08 Apr 2022 23:23:18 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
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’.
- [bug#54561] v2 [PATCH 2/5] doc: Add "Samba" chapter., (continued)
- [bug#54561] v2 [PATCH 2/5] doc: Add "Samba" chapter., Simon Streit, 2022/04/08
- [bug#54561] v2 [PATCH 1/5] services: Add samba service., Simon Streit, 2022/04/08
- [bug#54561] v2 [PATCH 3/5] doc: Add documentation for WSDD service., Simon Streit, 2022/04/08
- [bug#54561] v2 [PATCH 5/5] gnu: Add wsdd., Simon Streit, 2022/04/08
- [bug#54561] v2 [PATCH 4/5] services: Add wsdd service., Simon Streit, 2022/04/08
- [bug#54561] [PATCH 0/4] Add service declarations for Samba,
Ludovic Courtès <=