* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-17 13:52 [bug#52578] [PATCH] updating openldap and adding service definition Jean-Francois GUILLAUME
@ 2021-12-17 22:39 ` Maxime Devos
2021-12-18 10:53 ` zimoun
2021-12-17 22:46 ` Maxime Devos
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Maxime Devos @ 2021-12-17 22:39 UTC (permalink / raw)
To: 52578
Hi,
>+ "--disable-static"
>+ "--enable-shared"
>+ "--with-tls=openssl"
>+ "--disable-static"
A single "--disable-static" should be suficient.
> + ,@(if (%current-target-system)
> + '("--with-yielding_select=yes"
"ac_cv_func_memcmp_working=yes")
> + '()
> + )
is this speculation on what's necessary for cross-compilation, or has
it been determined these flags are necessary?
>+ #:make-flags '("STRIP=")
Why?
>+ #:parallel-build? #t
This is the default, no need to mention it.
> + ,@(if (%current-target-system)
> + '(
> + (add-before 'make-depend 'fix-cross-gcc
> + (lambda* (#:key target #:allow-other-keys)
> + (setenv "CC" (string-append target "-gcc"))
> + #t
> + )
> + )
> + )
> + '()
You can use ,(cc-for-target) here. Also, CC can be set in #:make-flags.
> + (synopsis "Implementation of the Lightweight Directory Access
Protocol")
> + (description "OpenLDAP is a free implementation of the
Lightweight Directory Access Protocol.")
That's a very terse description --- is it a server, a client
application, programming APIs for communicating with a server, or all
of these? Also, no need to mention it's free, everything in Guix is
free.
> +(define-public openldap-2.5.9
> + (package
> + (inherit openldap)
What's the reason for defining multiple versions of openldap?
Usually, it is only necessary to keep the latest version of a package
(with some rare exceptions).
>+(define-module (gnu services openldap)
A copyright + license header is missing, and this file needs to be
added to Makefile.am (or local.mk, I'm not sure about the details).
>+ #:use-module (gnu packages openldap)
>+ #:use-module (gnu services)
>+ #:use-module (gnu services shepherd)
>+ #:use-module (guix)
>+ #:use-module (guix records)
>+ #:use-module (ice-9 match)
>+ #: export (
This seems unlikely to compile, what's the space doing here?
Something I'm missing here, is some documentation. As it is, this
openldap service isn't documented anywhere, so nobody would figure out
it even exists, unless they search in the source code.
> + (shepherd-service [...])
As-is, this service would be run as root, which is very suboptimal from
a security perspective. Consider running it as a separate user & group,
and if feasible in a container (the latter is optional but would be
great).
> + (pid-file openldap-configuration-pid-file
> + (default "/var/run/openldap/slapd.pid"))
> + (log-file openldap-configuration-log-file
> + (default "/var/log/slapd.log"))
I don't see the point in making this customisable.
Why would anyone want to change the log locations or location of the
pid file? Unless there's some compelling reason otherwise, I'd prefer
to keep complexity down by not making this configurable.
> + (config-file openldap-configuration-config-file
> + (default (file-append openldap "/etc/openldap/slapd.conf"))
> + )
Allowing writing the configuration with configuration records would be
preferred (with an 'extra-content'-style escape hatch, because it would
probably be infeasible to support every single configuration option of
openldap, but some basic options like ‘which network port to bind to’
should be configurable in Scheme).
> + (requirement '(user-processes))
This service probably requires a network interface, so loopback might
be required. Also, why is user-processes included? I know many services
include it, but it doesn't appear to be documented anywhere when user-
processes must be added to 'requirement'.
>+ openldap-configuration
>+ openldap-configuration?
>+ openldap-shepherd-service
>+ openldap-service-type
>+ )
These parentheses are lonely, consider moving the parenthese to right
after openldap-service-type, to keep the style consistent in Guix.
Greetings,
Maxime.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-17 22:39 ` Maxime Devos
@ 2021-12-18 10:53 ` zimoun
0 siblings, 0 replies; 16+ messages in thread
From: zimoun @ 2021-12-18 10:53 UTC (permalink / raw)
To: Maxime Devos, 52578
Hi Maxime,
The package ’openldap’ already exists and some of your comments ask
about the existing recipe. :-)
I think Jean-François just copy/pasted the current recipe and expand it
for their own requirements: having the service they need, IIUC.
On Fri, 17 Dec 2021 at 22:39, Maxime Devos <maximedevos@telenet.be> wrote:
>> + ,@(if (%current-target-system)
>> + '("--with-yielding_select=yes"
> "ac_cv_func_memcmp_working=yes")
>> + '()
>> + )
>
> is this speculation on what's necessary for cross-compilation, or has
> it been determined these flags are necessary?
This bits had been added by Mathieu 1c8b1870a60de12f6c19d809522f5d8362215131.
>>+ #:make-flags '("STRIP=")
>
> Why?
This one too.
>> + ,@(if (%current-target-system)
>> + '(
>> + (add-before 'make-depend 'fix-cross-gcc
>> + (lambda* (#:key target #:allow-other-keys)
>> + (setenv "CC" (string-append target "-gcc"))
>> + #t
>> + )
>> + )
>> + )
>> + '()
>
> You can use ,(cc-for-target) here. Also, CC can be set in #:make-flags.
Again this one.
>> + (synopsis "Implementation of the Lightweight Directory Access
> Protocol")
>> + (description "OpenLDAP is a free implementation of the
> Lightweight Directory Access Protocol.")
>
> That's a very terse description --- is it a server, a client
> application, programming APIs for communicating with a server, or all
> of these? Also, no need to mention it's free, everything in Guix is
> free.
This description is from 2013, 2a75d4e664e802d3a3e2ed6455c63f32541559c8. ;-)
Your comments about the package itself make sense but I am not convinced
that they are related to the first Jean-François submission. :-)
Cheers,
simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-17 13:52 [bug#52578] [PATCH] updating openldap and adding service definition Jean-Francois GUILLAUME
2021-12-17 22:39 ` Maxime Devos
@ 2021-12-17 22:46 ` Maxime Devos
2021-12-18 10:22 ` zimoun
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Maxime Devos @ 2021-12-17 22:46 UTC (permalink / raw)
To: 52578
Hi,
> + ; this is needed because the make check does not work inside
guix
> + #:tests? #f
What do you mean with ‘does not work inside guix’?
A failing test, a missing test dependency, ...?
If it doesn't work, then it should be fixed if feasible
-- test suites exist for a reason!
And if it is a failing test, that would mean the test suite caught a
bug, so in that case, the test suite is succeeding in its purpose, not
failing.
Greetings,
Maxime
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-17 13:52 [bug#52578] [PATCH] updating openldap and adding service definition Jean-Francois GUILLAUME
2021-12-17 22:39 ` Maxime Devos
2021-12-17 22:46 ` Maxime Devos
@ 2021-12-18 10:22 ` zimoun
2021-12-18 11:09 ` Jean-Francois GUILLAUME
2021-12-18 10:37 ` Jean-Francois GUILLAUME
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: zimoun @ 2021-12-18 10:22 UTC (permalink / raw)
To: Jean-Francois GUILLAUME, 52578
Hi Jean-François,
Nice to see you here. :-)
Various comments for improving the submission.
On Fri, 17 Dec 2021 at 14:52, Jean-Francois GUILLAUME <Jean-Francois.Guillaume@univ-nantes.fr> wrote:
> * gnu/packages/openldap.scm (openldap): Update to 2.6.0, adding 2.5.7,
> 2.5.8, 2.5.9
> * gnu/services/openldap.scm (openldap): Adding slapd service
I would split: one commit for adding a big openldap and another for
adding the service. WDYT?
(I have not looked yet to the service.)
> (define-public openldap
> + (package
> + (name "openldap")
> + (version "2.6.0")
> + (source (origin
> + (method url-fetch)
> + (uri (list
> + (string-append
> "https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-"
> version ".tgz")
Why the mirror list had been removed?
> + (string-append
> "http://repository.linagora.org/OpenLDAP/openldap-release/openldap-"
> version ".tgz")
This is new, right?
> + (string-append
> "ftp://ftp.dti.ad.jp/pub/net/OpenLDAP/openldap-release/openldap-"
> version ".tgz")
As it is currently and already done in gnu/packages/openldap.scm, to
ease the reading, this long string could be slip as,
--8<---------------cut here---------------start------------->8---
(string-append
"ftp://ftp.dti.ad.jp/pub/net/OpenLDAP/"
"openldap-release/openldap-" version ".tgz")))
--8<---------------cut here---------------end--------------->8---
(See below for details if many variants are required.)
> + (inputs `(
> + ("argon2", argon2)
> + ("cyrus-sasl", cyrus-sasl)
> + ("libevent", libevent)
> + ("libgcrypt", libgcrypt)
> + ("libltdl", libltdl)
> + ("lz4", lz4)
> + ("openssl", openssl)
> + ("perl", perl)
> + ("snappy", snappy)
> + ("unixodbc", unixodbc)
> + ("wiredtiger", wiredtiger)
> + ("zlib", zlib)
> + ))
> + (native-inputs `(
> + ("bdb", bdb)
> + ("groff", groff)
> + ("libtool", libtool)
> + ("pkg-config", pkg-config)
> + ))
Currently, openldap@2.4.57 is built using (reformatted by me to ease the
comparison):
--8<---------------cut here---------------start------------->8---
(inputs (list bdb-5.3
cyrus-sasl
gnutls
libgcrypt
zlib))
(native-inputs (list libtool
groff
bdb-5.3))
--8<---------------cut here---------------end--------------->8---
Aside the new style vs the old style which is a detail, are these lists
expanded because the version bump or because more OpenLDAP is built
using more features?
> + (arguments `(
> + ; this is needed because the make check does not work inside guix
> + #:tests? #f
It was already off, but I do not understand the new comment. Well,
maybe this commentary is not necessary.
> + #:configure-flags '(
> + "--enable-debug"
> + "--enable-dynamic"
> + "--enable-syslog"
> + "--enable-ipv6"
> + "--enable-local"
> + "--enable-slapd"
> + "--enable-dynacl"
> + "--enable-aci"
> + "--enable-cleartext"
> + "--enable-crypt"
> + "--enable-spasswd"
> + "--enable-modules"
> + "--enable-rlookups"
> + "--enable-slapi"
> + "--enable-backends=mod"
> + "--enable-overlays=mod"
> + "--enable-argon2"
> + "--enable-balancer"
> + "--disable-static"
> + "--enable-shared"
> + "--with-tls=openssl"
> + "--disable-static"
This is a lot more. :-) Therefore, the question is: is it better
- to have only one BIG openldap package?
- or to have one minimal openldap and a bigger variant?
Well, “guix refresh -l openldap” answers for us. ;-)
I propose to keep openldap@2.4.57 minimal, as it currently is, and use
’inherit’ to build BIG ’openldap@2.6.0.’ and variants.
> + ,@(if (%current-target-system)
> + '("--with-yielding_select=yes"
> "ac_cv_func_memcmp_working=yes")
> + '()
> + )
> + )
> + #:make-flags '("STRIP=")
> + #:parallel-build? #t
This is not necessary because it is the default.
> + #:phases (modify-phases %standard-phases
> + (add-before 'build 'make-depend
> + (lambda* (#:key input #:allow-other-keys)
> + (invoke "make" "depend")
> + )
> + )
> + ,@(if (%current-target-system)
> + '(
> + (add-before 'make-depend 'fix-cross-gcc
> + (lambda* (#:key target #:allow-other-keys)
> + (setenv "CC" (string-append target "-gcc"))
> + #t
> + )
> + )
> + )
> + '()
> + )
> + )
> + ))
A minor comment, usually, we do:
--8<---------------cut here---------------start------------->8---
,@(if (%current-target-system)
'((add-before 'make-depend 'fix-cross-gcc
(lambda* (#:key target #:allow-other-keys)
(setenv "CC" (string-append target "-gcc"))
#t)))
'()))))
--8<---------------cut here---------------end--------------->8---
instead of all these closing parens, each on one line.
Using ’inherit’, this is even probably not required. :-)
> +(define-public openldap-2.5.9
> + (package
> + (inherit openldap)
> + (name "openldap")
> + (version "2.5.9")
> + (source (origin
> + (method url-fetch)
> + (uri (list
> + (string-append
> "https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-"
> version ".tgz")
> + (string-append
> "http://repository.linagora.org/OpenLDAP/openldap-release/openldap-"
> version ".tgz")
> + (string-append
> "ftp://ftp.dti.ad.jp/pub/net/OpenLDAP/openldap-release/openldap-"
> version ".tgz")
> + ))
> + (sha256 ( base32
> "17pvwrj27jybbmjqpv0p7kd2qa4i6jnp134lz7cxa0sqrbs153n0" ))
> + )
Do you need all these variants? If yes, it could be nice to have,
instead of copy/paste all, something like:
--8<---------------cut here---------------start------------->8---
(define (openldap-uris version)
(let ((openldap-release "OpenLDAP/openldap-release/")
(openldap-version.tgz
(string-append "openldap-" version ".tgz")))
(map (lambda (url)
(string-append url openldap-release openldap-version.tgz))
(list "https://www.openldap.org/software/download/"
"http://repository.linagora.org/"
"ftp://ftp.dti.ad.jp/pub/net/"))))
(define-public openldap-2.5.8
(package
(inherit openldap)
(name "openldap")
(version "2.5.8")
(source (origin
(method url-fetch)
(uri (openldap-uris version))
(sha256
(base32 "1p3jck2kh7rsz6mkrqaailaf9ky050hn72wph52dw0j2nb1s2vin")))))
[…]
--8<---------------cut here---------------end--------------->8---
(Untested though. :-)))
Cheers,
simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-18 10:22 ` zimoun
@ 2021-12-18 11:09 ` Jean-Francois GUILLAUME
0 siblings, 0 replies; 16+ messages in thread
From: Jean-Francois GUILLAUME @ 2021-12-18 11:09 UTC (permalink / raw)
To: zimoun; +Cc: 52578
Hi Simon,
> Nice to see you here. :-)
Thanks :)
> Various comments for improving the submission.
Angain, thank you. I'll glady take on these as i've other packages to
contribute.
> I would split: one commit for adding a big openldap and another for
> adding the service. WDYT?
>
> (I have not looked yet to the service.)
As you wish, i must admit i was kind of lazy and wanted to provide
everything in one go.
>
> Why the mirror list had been removed?
>
> [...]
>
> This is new, right?
>
It's still using a mirror list, i've tried to select a few on each
region of th e world on openldap's download page.
> As it is currently and already done in gnu/packages/openldap.scm, to
> ease the reading, this long string could be slip as,
>
> --8<---------------cut here---------------start------------->8---
> (string-append
> "ftp://ftp.dti.ad.jp/pub/net/OpenLDAP/"
> "openldap-release/openldap-" version ".tgz")))
> --8<---------------cut here---------------end--------------->8---
>
> (See below for details if many variants are required.)
>
Well, i found it more easy to read on one line but it's true that i use
a wide terminal. I can change it, no problems.
>
>> + (inputs `(
>> + ("argon2", argon2)
>> + ("cyrus-sasl", cyrus-sasl)
>> + ("libevent", libevent)
>> + ("libgcrypt", libgcrypt)
>> + ("libltdl", libltdl)
>> + ("lz4", lz4)
>> + ("openssl", openssl)
>> + ("perl", perl)
>> + ("snappy", snappy)
>> + ("unixodbc", unixodbc)
>> + ("wiredtiger", wiredtiger)
>> + ("zlib", zlib)
>> + ))
>> + (native-inputs `(
>> + ("bdb", bdb)
>> + ("groff", groff)
>> + ("libtool", libtool)
>> + ("pkg-config", pkg-config)
>> + ))
>
> Currently, openldap@2.4.57 is built using (reformatted by me to ease
> the
> comparison):
>
> --8<---------------cut here---------------start------------->8---
> (inputs (list bdb-5.3
> cyrus-sasl
> gnutls
> libgcrypt
> zlib))
> (native-inputs (list libtool
> groff
> bdb-5.3))
> --8<---------------cut here---------------end--------------->8---
>
> Aside the new style vs the old style which is a detail, are these lists
> expanded because the version bump or because more OpenLDAP is built
> using more features?
>
With his definition you can now run a fully featured openldap server. We
were missing quite a few features when using the 2.4.57 version (which
is nearly only the client tools).
>
>> + (arguments `(
>> + ; this is needed because the make check does not work inside
>> guix
>> + #:tests? #f
>
> It was already off, but I do not understand the new comment. Well,
> maybe this commentary is not necessary.
>
My bad, leftovers from our local repo. For some strange reasons, when
the tests are run by guix build they do not properly clean after each
steps and ends up failing. If you do the same inside a guix environment
they work properly. And i think some tests need some kinds of network
connection but that could be on another package.
>
>> + #:configure-flags '(
>> + "--enable-debug"
>> + "--enable-dynamic"
>> + "--enable-syslog"
>> + "--enable-ipv6"
>> + "--enable-local"
>> + "--enable-slapd"
>> + "--enable-dynacl"
>> + "--enable-aci"
>> + "--enable-cleartext"
>> + "--enable-crypt"
>> + "--enable-spasswd"
>> + "--enable-modules"
>> + "--enable-rlookups"
>> + "--enable-slapi"
>> + "--enable-backends=mod"
>> + "--enable-overlays=mod"
>> + "--enable-argon2"
>> + "--enable-balancer"
>> + "--disable-static"
>> + "--enable-shared"
>> + "--with-tls=openssl"
>> + "--disable-static"
>
> This is a lot more. :-) [...]
Indeed, need quite a lot to get a fully featured server.
> [...] Therefore, the question is: is it better
>
> - to have only one BIG openldap package?
> - or to have one minimal openldap and a bigger variant?
>
> Well, “guix refresh -l openldap” answers for us. ;-)
>
> I propose to keep openldap@2.4.57 minimal, as it currently is, and use
> ’inherit’ to build BIG ’openldap@2.6.0.’ and variants.
>
As you wish either work for me. I can also do a "-minimal" version with
only what is needed to get a client version and a "-full" version to get
a fully featured server.
>
>> + ,@(if (%current-target-system)
>> + '("--with-yielding_select=yes"
>> "ac_cv_func_memcmp_working=yes")
>> + '()
>> + )
>> + )
>> + #:make-flags '("STRIP=")
>> + #:parallel-build? #t
>
> This is not necessary because it is the default.
>
OK.
>
>> + #:phases (modify-phases %standard-phases
>> + (add-before 'build 'make-depend
>> + (lambda* (#:key input #:allow-other-keys)
>> + (invoke "make" "depend")
>> + )
>> + )
>> + ,@(if (%current-target-system)
>> + '(
>> + (add-before 'make-depend 'fix-cross-gcc
>> + (lambda* (#:key target #:allow-other-keys)
>> + (setenv "CC" (string-append target "-gcc"))
>> + #t
>> + )
>> + )
>> + )
>> + '()
>> + )
>> + )
>> + ))
>
> A minor comment, usually, we do:
>
> --8<---------------cut here---------------start------------->8---
> ,@(if (%current-target-system)
> '((add-before 'make-depend 'fix-cross-gcc
> (lambda* (#:key target #:allow-other-keys)
> (setenv "CC" (string-append target "-gcc"))
> #t)))
> '()))))
> --8<---------------cut here---------------end--------------->8---
>
> instead of all these closing parens, each on one line.
>
> Using ’inherit’, this is even probably not required. :-)
>
Leftovers from our local repo, we rely a bit to much on indentation to
help us have a better view of where blocks start and stop.
>
>> +(define-public openldap-2.5.9
>> + (package
>> + (inherit openldap)
>> + (name "openldap")
>> + (version "2.5.9")
>> + (source (origin
>> + (method url-fetch)
>> + (uri (list
>> + (string-append
>> "https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-"
>> version ".tgz")
>> + (string-append
>> "http://repository.linagora.org/OpenLDAP/openldap-release/openldap-"
>> version ".tgz")
>> + (string-append
>> "ftp://ftp.dti.ad.jp/pub/net/OpenLDAP/openldap-release/openldap-"
>> version ".tgz")
>> + ))
>> + (sha256 ( base32
>> "17pvwrj27jybbmjqpv0p7kd2qa4i6jnp134lz7cxa0sqrbs153n0" ))
>> + )
>
> Do you need all these variants? If yes, it could be nice to have,
> instead of copy/paste all, something like:
>
> --8<---------------cut here---------------start------------->8---
> (define (openldap-uris version)
> (let ((openldap-release "OpenLDAP/openldap-release/")
> (openldap-version.tgz
> (string-append "openldap-" version ".tgz")))
> (map (lambda (url)
> (string-append url openldap-release openldap-version.tgz))
> (list "https://www.openldap.org/software/download/"
> "http://repository.linagora.org/"
> "ftp://ftp.dti.ad.jp/pub/net/"))))
>
> (define-public openldap-2.5.8
> (package
> (inherit openldap)
> (name "openldap")
> (version "2.5.8")
> (source (origin
> (method url-fetch)
> (uri (openldap-uris version))
> (sha256
> (base32
> "1p3jck2kh7rsz6mkrqaailaf9ky050hn72wph52dw0j2nb1s2vin")))))
>
> […]
> --8<---------------cut here---------------end--------------->8---
>
> (Untested though. :-)))
>
This is mostly another case of copy-paste from our local repository went
wrong.
Initially i intended to provide only the latests versions for 2.6.x and
2.5.x and keeping 2.4.57 from compatibility reasons.
While doing the definitions, i was wondering how i could provide only
the hash and the version, guess i'll try your solution :)
Best,
---
Cordialement,
Jean-François GUILLAUME
Plateforme Bioinformatique BiRD
Tél. : +33 (0)2 28 08 00 57
www.pf-bird.univ-nantes.fr
Inserm UMR 1087/CNRS UMR 6291
IRS-UN - 8 quai Moncousu - BP 70721
44007 Nantes Cedex 1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-17 13:52 [bug#52578] [PATCH] updating openldap and adding service definition Jean-Francois GUILLAUME
` (2 preceding siblings ...)
2021-12-18 10:22 ` zimoun
@ 2021-12-18 10:37 ` Jean-Francois GUILLAUME
2021-12-18 10:49 ` Jean-Francois GUILLAUME
2022-03-25 15:34 ` [bug#52578] [PATCH v2 0/2] OpenLDAP service zimoun
2022-03-25 15:35 ` [bug#52578] [PATCH v2 1/2] DRAFT gnu: Add openldap-for-services zimoun
5 siblings, 1 reply; 16+ messages in thread
From: Jean-Francois GUILLAUME @ 2021-12-18 10:37 UTC (permalink / raw)
To: 52578
Hi,
> A single "--disable-static" should be suficient.
Indeed, copy-paste from our local repository went wrong.
> is this speculation on what's necessary for cross-compilation, or has
> it been determined these flags are necessary?
These were necessary with the old autoconf in <= 2.5 realeases. It's
mostly a leftover from the older definition already in guix.
> Why?
Stripping was sometime leading to crash of the build on my side.
> This is the default, no need to mention it.
True, leftover from when i needed the build to be monothread to see
where it failed.
> You can use ,(cc-for-target) here. Also, CC can be set in #:make-flags.
Ok, i will look into it.
> That's a very terse description --- is it a server, a client
> application, programming APIs for communicating with a server, or all
> of these? Also, no need to mention it's free, everything in Guix is
> free.
I'll be honest, it's a copy-paste from the already defined package. I'll
update it to be more meaningfull.
> What's the reason for defining multiple versions of openldap? Usually,
> it is only necessary to keep the latest version of a package (with some
> rare exceptions).
This is mostly another case of copy-paste from our local repository went
wrong.
> A copyright + license header is missing, and this file needs to be
> added to Makefile.am (or local.mk, I'm not sure about the details).
Ok, i will look into it.
> This seems unlikely to compile, what's the space doing here?
Well, we use this in our local guix infrastructure and it doesn't
complain, nor does our building of ldap server vms with guix system
build.
> Something I'm missing here, is some documentation. As it is, this
> openldap service isn't documented anywhere, so nobody would figure out
> it even exists, unless they search in the source code.
True, forgot about this, my bad. Could you please point me to an example
?
> As-is, this service would be run as root, which is very suboptimal from
> a security perspective. Consider running it as a separate user & group,
> and if feasible in a container (the latter is optional but would be
> great).
True, i'll try to get it work with it's own user and group.
> I don't see the point in making this customisable. Why would anyone
> want to change the log locations or location of the pid file? Unless
> there's some compelling reason otherwise, I'd prefer to keep complexity
> down by not making this configurable.
This allow us to run multiple instance of this service on the same
machine (granted you also change the storage directory slapd.conf).
> Allowing writing the configuration with configuration records would be
> preferred (with an 'extra-content'-style escape hatch, because it would
> probably be infeasible to support every single configuration option of
> openldap, but some basic options like ‘which network port to bind
> to’ should be configurable in Scheme).
Well this is beyond my current abilities.
> This service probably requires a network interface, so loopback might
> be required. Also, why is user-processes included? I know many services
> include it, but it doesn't appear to be documented anywhere when
> user-processes must be added to 'requirement'.
True. From my understanding, when you reach user-processes you're in the
late stage of booting your system and everything network-wise should be
available.
> These parentheses are lonely, consider moving the parenthese to right
> after openldap-service-type, to keep the style consistent in Guix.
Leftovers from our local repo, we rely a bit to much on indentation to
help us have a better view of where blocks start and stop.
> What do you mean with ‘does not work inside guix’?
For some strange reasons, when the tests are run by guix build they do
not properly clean after each steps and ends up failing. If you do the
same inside a guix environment test work properly. And i think some
tests need some kinds of network connection but that could be on another
package.
Sorry for the messy patch.
Best,
--
Cordialement,
Jean-François GUILLAUME
Plateforme Bioinformatique BiRD
Tél. : +33 (0)2 28 08 00 57
www.pf-bird.univ-nantes.fr
Inserm UMR 1087/CNRS UMR 6291
IRS-UN - 8 quai Moncousu - BP 70721
44007 Nantes Cedex 1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH] updating openldap and adding service definition
2021-12-18 10:37 ` Jean-Francois GUILLAUME
@ 2021-12-18 10:49 ` Jean-Francois GUILLAUME
0 siblings, 0 replies; 16+ messages in thread
From: Jean-Francois GUILLAUME @ 2021-12-18 10:49 UTC (permalink / raw)
To: 52578
Hi Maxime,
> A single "--disable-static" should be suficient.
Indeed, copy-paste from our local repository went wrong.
> is this speculation on what's necessary for cross-compilation, or has
> it been determined these flags are necessary?
These were necessary with the old autoconf in <= 2.5 realeases. It's
mostly a leftover from the older definition already in guix.
> Why?
Stripping was sometime leading to crash of the build on my side.
> This is the default, no need to mention it.
True, leftover from when i needed the build to be monothread to see
where it failed.
> You can use ,(cc-for-target) here. Also, CC can be set in #:make-flags.
Ok, i will look into it.
> That's a very terse description --- is it a server, a client
> application, programming APIs for communicating with a server, or all
> of these? Also, no need to mention it's free, everything in Guix is
> free.
I'll be honest, it's a copy-paste from the already defined package. I'll
update it to be more meaningfull.
> What's the reason for defining multiple versions of openldap? Usually,
> it is only necessary to keep the latest version of a package (with some
> rare exceptions).
This is mostly another case of copy-paste from our local repository went
wrong.
> A copyright + license header is missing, and this file needs to be
> added to Makefile.am (or local.mk, I'm not sure about the details).
Ok, i will look into it.
> This seems unlikely to compile, what's the space doing here?
Well, we use this in our local guix infrastructure and it doesn't
complain, nor does our building of ldap server vms with guix system
build.
> Something I'm missing here, is some documentation. As it is, this
> openldap service isn't documented anywhere, so nobody would figure out
> it even exists, unless they search in the source code.
True, forgot about this, my bad. Could you please point me to an example
?
> As-is, this service would be run as root, which is very suboptimal from
> a security perspective. Consider running it as a separate user & group,
> and if feasible in a container (the latter is optional but would be
> great).
True, i'll try to get it work with it's own user and group.
> I don't see the point in making this customisable. Why would anyone
> want to change the log locations or location of the pid file? Unless
> there's some compelling reason otherwise, I'd prefer to keep complexity
> down by not making this configurable.
This allow us to run multiple instance of this service on the same
machine (granted you also change the storage directory slapd.conf).
> Allowing writing the configuration with configuration records would be
> preferred (with an 'extra-content'-style escape hatch, because it would
> probably be infeasible to support every single configuration option of
> openldap, but some basic options like ‘which network port to bind
> to’ should be configurable in Scheme).
Well this is beyond my current abilities.
> This service probably requires a network interface, so loopback might
> be required. Also, why is user-processes included? I know many services
> include it, but it doesn't appear to be documented anywhere when
> user-processes must be added to 'requirement'.
True. From my understanding, when you reach user-processes you're in the
late stage of booting your system and everything network-wise should be
available.
> These parentheses are lonely, consider moving the parenthese to right
> after openldap-service-type, to keep the style consistent in Guix.
Leftovers from our local repo, we rely a bit to much on indentation to
help us have a better view of where blocks start and stop.
> What do you mean with ‘does not work inside guix’?
For some strange reasons, when the tests are run by guix build they do
not properly clean after each steps and ends up failing. If you do the
same inside a guix environment test work properly. And i think some
tests need some kinds of network connection but that could be on another
package.
Sorry for the messy patch.
Best,
---
Cordialement,
Jean-François GUILLAUME
Plateforme Bioinformatique BiRD
Tél. : +33 (0)2 28 08 00 57
www.pf-bird.univ-nantes.fr
Inserm UMR 1087/CNRS UMR 6291
IRS-UN - 8 quai Moncousu - BP 70721
44007 Nantes Cedex 1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 0/2] OpenLDAP service
2021-12-17 13:52 [bug#52578] [PATCH] updating openldap and adding service definition Jean-Francois GUILLAUME
` (3 preceding siblings ...)
2021-12-18 10:37 ` Jean-Francois GUILLAUME
@ 2022-03-25 15:34 ` zimoun
2022-03-28 8:38 ` Jean-Francois GUILLAUME
2022-03-25 15:35 ` [bug#52578] [PATCH v2 1/2] DRAFT gnu: Add openldap-for-services zimoun
5 siblings, 1 reply; 16+ messages in thread
From: zimoun @ 2022-03-25 15:34 UTC (permalink / raw)
To: 52578; +Cc: jean-francois.guillaume, zimoun
Hi,
Sorry for the delay.
Well, I am not convinced that the package 'openldap-for-services' is really
required and perhaps the tweak of openldap-2.6 is enough.
Moreover, do you need openssl instead of gnutls? I would be in favor to keep
gnutls as the base package and if you absolutely need openssl, write a
variant; along the proposed modify-inputs.
About the old versions of openldap, I am going to send you a recipe for your
own channel. I am not convinced that maintain such old variants makes sense
at the Guix level.
About the service, it still misses some documentation for the manual. And
'tests' would also be very welcome. :-)
Note that gnu/tests/ldap.scm already some tests. Maybe this file could be
updated with the new service.
WDYT?
Cheers,
simon
Jean-François Guillaume (1):
DRAFT services: Add openldap service.
zimoun (1):
DRAFT gnu: Add openldap-for-services.
gnu/packages/openldap.scm | 47 ++++++++++++++++++++++
gnu/services/openldap.scm | 84 +++++++++++++++++++++++++++++++++++++++
2 files changed, 131 insertions(+)
create mode 100644 gnu/services/openldap.scm
base-commit: f76898be6ded531e459f106549886afbdc426a78
--
2.34.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 0/2] OpenLDAP service
2022-03-25 15:34 ` [bug#52578] [PATCH v2 0/2] OpenLDAP service zimoun
@ 2022-03-28 8:38 ` Jean-Francois GUILLAUME
2024-05-16 21:08 ` Ludovic Courtès
0 siblings, 1 reply; 16+ messages in thread
From: Jean-Francois GUILLAUME @ 2022-03-28 8:38 UTC (permalink / raw)
To: zimoun; +Cc: 52578
Hello,
> Sorry for the delay.
No worries, I must admit that I didn't have much time on hand to correct
things on my side.
> Well, I am not convinced that the package 'openldap-for-services' is
> really
> required and perhaps the tweak of openldap-2.6 is enough.
It's only required if you have the need for a fully featured server like
we do (we use this definition in our openldap cluster infrastructure).
Given enough time, we will provides a stable repository and artifacts
for our definitions.
> Moreover, do you need openssl instead of gnutls?
Nope, I just took the définition of the RHEL package and moved it into
a guix format.
> About the old versions of openldap, I am going to send you a recipe for
> your
> own channel. I am not convinced that maintain such old variants makes
> sense
> at the Guix level.
I think we still have this old version because it's the version provided
in .deb anb .rpm distributions.
> About the service, it still misses some documentation for the manual.
Yep, I still need to take the time to check how to do it.
> And 'tests' would also be very welcome. :-)
> Note that gnu/tests/ldap.scm already some tests. Maybe this file could
> be
> updated with the new service.
From what I see, what is already present gnu/tests/ldap.scm should be
sufficient.
---
Cordialement,
Jean-François GUILLAUME
Plateforme Bioinformatique BiRD
Tél. : +33 (0)2 28 08 00 57
www.pf-bird.univ-nantes.fr
Inserm UMR 1087/CNRS UMR 6291
IRS-UN - 8 quai Moncousu - BP 70721
44007 Nantes Cedex 1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 0/2] OpenLDAP service
2022-03-28 8:38 ` Jean-Francois GUILLAUME
@ 2024-05-16 21:08 ` Ludovic Courtès
2024-05-17 12:04 ` Simon Tournier
2024-05-22 10:18 ` Jean-Francois GUILLAUME
0 siblings, 2 replies; 16+ messages in thread
From: Ludovic Courtès @ 2024-05-16 21:08 UTC (permalink / raw)
To: Jean-Francois GUILLAUME; +Cc: 52578, zimoun
Hello,
Damn, it’s been two years already since you submitted these OpenLDAP
patches. 😱
You probably had problems with the NSS plugins to get LDAP user/group
lookups working. I have good news: <https://issues.guix.gnu.org/70992>
probably fixes that.
(Besides, we should finally schedule some time to finish the reviewing
effort of these patches that Simon started.)
Ludo’.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 0/2] OpenLDAP service
2024-05-16 21:08 ` Ludovic Courtès
@ 2024-05-17 12:04 ` Simon Tournier
2024-05-22 10:18 ` Jean-Francois GUILLAUME
1 sibling, 0 replies; 16+ messages in thread
From: Simon Tournier @ 2024-05-17 12:04 UTC (permalink / raw)
To: Ludovic Courtès, Jean-Francois GUILLAUME; +Cc: 52578
Hi,
On jeu., 16 mai 2024 at 23:08, Ludovic Courtès <ludovic.courtes@inria.fr> wrote:
> (Besides, we should finally schedule some time to finish the reviewing
> effort of these patches that Simon started.)
Sorry, I have never felt confident about the service part. Yeah, it
definitively needs some love. :-)
Cheers,
simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 0/2] OpenLDAP service
2024-05-16 21:08 ` Ludovic Courtès
2024-05-17 12:04 ` Simon Tournier
@ 2024-05-22 10:18 ` Jean-Francois GUILLAUME
2024-05-23 7:12 ` Ludovic Courtès
1 sibling, 1 reply; 16+ messages in thread
From: Jean-Francois GUILLAUME @ 2024-05-22 10:18 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 52578, zimoun
Hello,
> Damn, it’s been two years already since you submitted these OpenLDAP
> patches. 😱
Well, damn the time fly fast...
> You probably had problems with the NSS plugins to get LDAP user/group
> lookups working. I have good news: <https://issues.guix.gnu.org/70992>
> probably fixes that.
We indeed had a problème with lookup, we did trace it back to the
libnss-ldap not in the correct path. We are doing a quick and dirty fix
for now using our rc-local service :
> mount -o remount,rw /gnu/store
> echo 'export LD_LIBRARY_PATH="/run/current-system/profile/lib"' >>
> /run/current-system/profile/etc/profile
> echo " " >> /run/current-system/profile/etc/profile
> mount -o remount,ro /gnu/store
Theses services indeed need some love, especially on the config file
part. At glicid we are building it by using split files :
> (define slapd-part-1a (call-with-input-file
> "../common/conf/slapd-part-01-a.conf" get-string-all))
> (define openldap-modules-path (string-append "modulepath " (with-store
> store (package-output store glicid:openldap)) "/libexec/openldap"))
> (define slapd-part-1b (call-with-input-file
> "../common/conf/slapd-part-01-b.conf" get-string-all))
> (define slapd-part-serverid (call-with-input-file
> "./conf/serverID.conf" get-string-all))
> (define slapd-part-2 (call-with-input-file
> "../common/conf/slapd-part-02.conf" get-string-all))
> (define slapd-part-syncrepl (call-with-input-file
> "./conf/syncrepl.conf" get-string-all))
> (define slapd-part-3 (call-with-input-file
> "../common/conf/slapd-part-03.conf" get-string-all))
> (define slapd-conf-file (plain-file "slapd-merged.conf"
> (string-append slapd-part-1a
>
> openldap-modules-path
> slapd-part-1b
> slapd-part-serverid
> slapd-part-2
> slapd-part-syncrepl
> slapd-part-3)))
But it definitively need some love to have a proper config file builder
(way above my current guix/guile expertise).
---
Cordialement,
Jean-François GUILLAUME
Ingénieur Systèmes, Réseaux, Virtualisation
Plateforme Bioinformatique BiRD, GLiCID, Nantes Université, CHU Nantes,
CNRS, Inserm, BioCore, US16, SFR Bonamy, F
tél : 02-28-08-00-57 (320057)
mail: Jean-Francois.Guillaume@univ-nantes.fr
Bâtiment 06, IRS UN - 8 quai Moncousu - BP 70721 - 44007 Nantes Cedex 1
https://www.pf-bird.univ-nantes.fr/
https://clam.glicid.fr/
https://www.univ-nantes.fr/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 0/2] OpenLDAP service
2024-05-22 10:18 ` Jean-Francois GUILLAUME
@ 2024-05-23 7:12 ` Ludovic Courtès
0 siblings, 0 replies; 16+ messages in thread
From: Ludovic Courtès @ 2024-05-23 7:12 UTC (permalink / raw)
To: Jean-Francois GUILLAUME; +Cc: 52578, zimoun
Hi Jean-Francois,
Jean-Francois GUILLAUME <Jean-Francois.Guillaume@univ-nantes.fr>
skribis:
>> You probably had problems with the NSS plugins to get LDAP user/group
>> lookups working. I have good news: <https://issues.guix.gnu.org/70992>
>> probably fixes that.
>
> We indeed had a problème with lookup, we did trace it back to the
> libnss-ldap not in the correct path. We are doing a quick and dirty
> fix for now using our rc-local service :
Ah well, you’ll no longer need this hack. :-)
> But it definitively need some love to have a proper config file
> builder (way above my current guix/guile expertise).
Yes, one of us should take a closer look.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 1/2] DRAFT gnu: Add openldap-for-services.
2021-12-17 13:52 [bug#52578] [PATCH] updating openldap and adding service definition Jean-Francois GUILLAUME
` (4 preceding siblings ...)
2022-03-25 15:34 ` [bug#52578] [PATCH v2 0/2] OpenLDAP service zimoun
@ 2022-03-25 15:35 ` zimoun
2022-03-25 15:35 ` [bug#52578] [PATCH v2 2/2] DRAFT services: Add openldap service zimoun
5 siblings, 1 reply; 16+ messages in thread
From: zimoun @ 2022-03-25 15:35 UTC (permalink / raw)
To: 52578; +Cc: jean-francois.guillaume, zimoun
* gnu/packages/openldap.scm (openldap-for-services): New variable.
Co-Authored-By: Jean-François Guillaume <jean-francois.guillaume@univ-nantes.fr>.
---
gnu/packages/openldap.scm | 47 +++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/gnu/packages/openldap.scm b/gnu/packages/openldap.scm
index c8a47e45d5..8374386807 100644
--- a/gnu/packages/openldap.scm
+++ b/gnu/packages/openldap.scm
@@ -9,6 +9,8 @@
;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2022 Marius Bakke <marius@gnu.org>
+;;; Copyright © 2022 Jean-François Guillaume <jean-francois.guillaume@univ-nantes.fr>
+;;; Copyright © 2022 Simon Tournier <zimoun.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -30,6 +32,7 @@ (define-module (gnu packages openldap)
#:use-module (gnu packages check)
#:use-module (gnu packages compression)
#:use-module (gnu packages cyrus-sasl)
+ #:use-module (gnu packages databases)
#:use-module (gnu packages dbm)
#:use-module (gnu packages documentation)
#:use-module (gnu packages gettext)
@@ -164,6 +167,50 @@ (define-public openldap-2.6
(lambda (port)
(format port "INPUT ( libldap.so )~%")))))))))))
+(define-public openldap-for-services
+ ;; TODO: Update in the next rebuild cycle
+ (let* ((openldap-minimal openldap-2.6))
+ (package
+ (inherit openldap-minimal)
+ (name (string-append (package-name openldap-minimal) "-for-services"))
+ (arguments
+ (substitute-keyword-arguments (package-arguments openldap-minimal)
+ ((#:configure-flags flags)
+ `(append (list
+ "--enable-aci"
+ "--enable-argon2"
+ "--enable-backends=mod"
+ "--enable-balancer"
+ "--enable-cleartext"
+ "--enable-crypt"
+ "--enable-debug"
+ "--enable-dynacl"
+ "--enable-modules"
+ "--enable-ipv6"
+ "--enable-local"
+ "--enable-overlays=mod"
+ "--enable-rlookups"
+ "--enable-shared"
+ "--enable-slapd"
+ "--enable-slapi"
+ "--enable-spasswd"
+ "--enable-syslog"
+ "--with-tls=openssl")
+ ,flags))))
+ (inputs (modify-inputs (package-inputs openldap-minimal)
+ (delete "gnutls")
+ (append argon2
+ libevent
+ libltdl
+ lz4
+ openssl
+ perl
+ snappy
+ unixodbc
+ wiredtiger)))
+ (native-inputs (modify-inputs (package-native-inputs openldap-minimal)
+ (append pkg-config))))))
+
(define-public nss-pam-ldapd
(package
(name "nss-pam-ldapd")
--
2.34.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [bug#52578] [PATCH v2 2/2] DRAFT services: Add openldap service.
2022-03-25 15:35 ` [bug#52578] [PATCH v2 1/2] DRAFT gnu: Add openldap-for-services zimoun
@ 2022-03-25 15:35 ` zimoun
0 siblings, 0 replies; 16+ messages in thread
From: zimoun @ 2022-03-25 15:35 UTC (permalink / raw)
To: 52578; +Cc: jean-francois.guillaume
From: Jean-François Guillaume <jean-francois.guillaume@univ-nantes.fr>
* gnu/services/openlda.scm (<openldap-configuration>): New record.
(openldap-shepherd-service): New procedure.
(%openldap-activation, openldap-service-type): New variable.
---
gnu/services/openldap.scm | 84 +++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 gnu/services/openldap.scm
diff --git a/gnu/services/openldap.scm b/gnu/services/openldap.scm
new file mode 100644
index 0000000000..dc5ae3fa8f
--- /dev/null
+++ b/gnu/services/openldap.scm
@@ -0,0 +1,84 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2022 Jean-François Guillaume <jean-francois.guillaume@univ-nantes.fr>
+;;;
+;;; 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 services openldap)
+ #:use-module (gnu packages openldap)
+ #:use-module (gnu services)
+ #:use-module (gnu services shepherd)
+ #:use-module (guix)
+ #:use-module (guix records)
+ #:use-module (ice-9 match)
+ #:export (openldap-configuration
+ openldap-configuration?
+ openldap-service-type
+
+ ))
+
+(define-record-type* <openldap-configuration>
+ openldap-configuration make-openldap-configuration
+ openldap-configuration?
+ (openldap openldap-configuration-openldap
+ (default openldap))
+ (uri openldap-configuration-uri
+ (default "ldapi:// ldap://"))
+ (logflags openldap-configuration-logflags
+ (default "0"))
+ (pid-file openldap-configuration-pid-file
+ (default "/var/run/openldap/slapd.pid"))
+ (config-file openldap-configuration-config-file
+ (default (file-append openldap "/etc/openldap/slapd.conf")))
+ (log-file openldap-configuration-log-file
+ (default "/var/log/slapd.log")))
+
+(define openldap-shepherd-service
+ (match-lambda
+ (($ <openldap-configuration> openldap uri logflags pid-file config-file log-file)
+ (list
+ (shepherd-service
+ (provision '(slapd) )
+ (documentation "Run OpenLDAP.")
+ (requirement '(user-processes))
+ (respawn? #t)
+ (start #~(make-forkexec-constructor
+ (list
+ #$(file-append openldap "/libexec/slapd")
+ "-h" #$uri
+ "-d" #$logflags
+ "-f" #$config-file)
+ #:pid-file #$pid-file
+ #:log-file #$log-file))
+ (stop #~(make-kill-destructor)))))))
+
+(define %openldap-activation
+ (with-imported-modules '((guix build utils))
+ #~(begin
+ (use-modules (guix build utils))
+ (mkdir-p "/var/run/openldap")
+ (mkdir-p "/var/lib/ldap")
+ #t)))
+
+(define openldap-service-type
+ (service-type (name 'slapd)
+ (extensions
+ (list
+ (service-extension shepherd-root-service-type
+ openldap-shepherd-service)
+ (service-extension activation-service-type
+ (const %openldap-activation))))
+ (description
+ "Run @uref{https://www.openldap.org, OpenLDAP}.")))
--
2.34.0
^ permalink raw reply related [flat|nested] 16+ messages in thread