guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jean-Francois GUILLAUME
Subject: [bug#52578] [PATCH] updating openldap and adding service definition
Date: Sat, 18 Dec 2021 12:09:49 +0100
User-agent: Roundcube Webmail/1.1.2

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






reply via email to

[Prev in Thread] Current Thread [Next in Thread]