[bug#52578] [PATCH] updating openldap and adding service definition

[bug#52578] [PATCH] updating openldap and adding service definition

[Jean-Francois GUILLAUME]

* 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
---
  gnu/packages/openldap.scm | 148 ++++++++++++++++++++++++++++++++++++++
  gnu/services/openldap.scm |  87 ++++++++++++++++++++++
  2 files changed, 235 insertions(+)
  create mode 100644 gnu/services/openldap.scm

diff --git a/gnu/packages/openldap.scm b/gnu/packages/openldap.scm
index b0ce899696..61f99dea7a 100644
--- a/gnu/packages/openldap.scm
+++ b/gnu/packages/openldap.scm
@@ -60,6 +60,154 @@
    #:use-module (guix build-system python))

  (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")
+        (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 
"0kqswk8pxgnlibh0kz6py3a2x3yh9pfk6pyr2nx9lgjpmh75h75p" ))
+      )
+    )
+    (build-system gnu-build-system)
+    (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)
+    ))
+    (arguments `(
+      ; this is needed because the make check does not work inside guix
+      #:tests? #f
+      #: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"
+        ,@(if (%current-target-system)
+          '("--with-yielding_select=yes" 
"ac_cv_func_memcmp_working=yes")
+          '()
+        )
+      )
+      #:make-flags '("STRIP=")
+      #:parallel-build? #t
+      #: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
+                )
+              )
+            )
+            '()
+        )
+      )
+    ))
+    (synopsis "Implementation of the Lightweight Directory Access 
Protocol")
+    (description "OpenLDAP is a free implementation of the Lightweight 
Directory Access Protocol.")
+    (license openldap2.8)
+    (home-page "https://www.openldap.org/")
+  )
+)
+
+(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" ))
+      )
+    )
+  )
+)
+
+(define-public openldap-2.5.8
+  (package
+    (inherit openldap)
+    (name "openldap")
+    (version "2.5.8")
+    (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 
"1p3jck2kh7rsz6mkrqaailaf9ky050hn72wph52dw0j2nb1s2vin" ))
+      )
+    )
+  )
+)
+
+(define-public openldap-2.5.7
+  (package
+    (inherit openldap)
+    (name "openldap")
+    (version "2.5.7")
+    (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 
"1ayr76sa5hjwldqzis5v71sbp88hd3hysc00gw1raqn33c05g5za" ))
+      )
+    )
+  )
+)
+
+(define-public openldap-2.4.57
    (package
     (name "openldap")
     (version "2.4.57")
diff --git a/gnu/services/openldap.scm b/gnu/services/openldap.scm
new file mode 100644
index 0000000000..0fd329d611
--- /dev/null
+++ b/gnu/services/openldap.scm
@@ -0,0 +1,87 @@
+(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-shepherd-service
+    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} 
community developped LDAP software.")
+  )
+)
--
2.30.2

[bug#52578] [PATCH] updating openldap and adding service definition

[Maxime Devos]

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.

[bug#52578] [PATCH] updating openldap and adding service definition

[Maxime Devos]

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

[bug#52578] [PATCH] updating openldap and adding service definition

[zimoun]

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

[bug#52578] [PATCH] updating openldap and adding service definition

[Jean-Francois GUILLAUME]

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

[bug#52578] [PATCH] updating openldap and adding service definition

[Jean-Francois GUILLAUME]

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

[bug#52578] [PATCH] updating openldap and adding service definition

[zimoun]

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

[bug#52578] [PATCH] updating openldap and adding service definition

[Jean-Francois GUILLAUME]

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

[bug#52578] [PATCH v2 0/2] OpenLDAP service

[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

[bug#52578] [PATCH v2 1/2] DRAFT gnu: Add openldap-for-services.

[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

[bug#52578] [PATCH v2 2/2] DRAFT services: Add openldap service.

[zimoun]

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

[bug#52578] [PATCH v2 0/2] OpenLDAP service

[Jean-Francois GUILLAUME]

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