all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Jean-Francois GUILLAUME <Jean-Francois.Guillaume@univ-nantes.fr>
To: zimoun <zimon.toutoune@gmail.com>
Cc: 52578@debbugs.gnu.org
Subject: [bug#52578] [PATCH] updating openldap and adding service definition
Date: Sat, 18 Dec 2021 12:09:49 +0100	[thread overview]
Message-ID: <cd837d5f6f77189145409fb1f7f46da6@imap.univ-nantes.prive> (raw)
In-Reply-To: <86ee6aqkky.fsf@gmail.com>

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	other threads:[~2021-12-18 17:22 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
2021-12-18 11:09   ` Jean-Francois GUILLAUME [this message]
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=cd837d5f6f77189145409fb1f7f46da6@imap.univ-nantes.prive \
    --to=jean-francois.guillaume@univ-nantes.fr \
    --cc=52578@debbugs.gnu.org \
    --cc=zimon.toutoune@gmail.com \
    /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.