From: zimoun <zimon.toutoune@gmail.com>
To: Jean-Francois GUILLAUME <Jean-Francois.Guillaume@univ-nantes.fr>,
52578@debbugs.gnu.org
Subject: [bug#52578] [PATCH] updating openldap and adding service definition
Date: Sat, 18 Dec 2021 11:22:05 +0100 [thread overview]
Message-ID: <86ee6aqkky.fsf@gmail.com> (raw)
In-Reply-To: <e4b25a81bb9401c74aa5db6c47185efe@imap.univ-nantes.prive>
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
next prev parent reply other threads:[~2021-12-18 10:28 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
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
2021-12-18 10:22 ` zimoun [this message]
2021-12-18 11:09 ` Jean-Francois GUILLAUME
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-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
2024-05-23 7:12 ` Ludovic Courtès
2022-03-25 15:35 ` [bug#52578] [PATCH v2 1/2] DRAFT gnu: Add openldap-for-services zimoun
2022-03-25 15:35 ` [bug#52578] [PATCH v2 2/2] DRAFT services: Add openldap service zimoun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86ee6aqkky.fsf@gmail.com \
--to=zimon.toutoune@gmail.com \
--cc=52578@debbugs.gnu.org \
--cc=Jean-Francois.Guillaume@univ-nantes.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.