Subject: Re: [bug#44032] [PATCH] gnu: ocaml: Update to 4.11.1

From: zimoun <[email-masked]>
Date: Mon, 19 Oct 2020 15:31:32 +0200

Hi!

On Fri, 16 Oct 2020 at 17:14, Julien Lepiller <[email-masked]> wrote:

> Note that with that, previously unreproducible packages are
> reproducible!

I have not checked yet this point. :-)

>>>From 2dc52a0077ffe1f0b416032fb1dfbf035f82c34e Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Thu, 15 Oct 2020 22:02:56 +0200
> Subject: [PATCH 1/7] gnu: ocaml: Update to 4.11.1.
>
> * gnu/packages/ocaml.scm (ocaml): Update to 4.11.1.
> ---
>  gnu/packages/ocaml.scm | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)

LTGM!

>>>From 2b5fa1b7f30e7ff0315a752e10e87930e72dbb8e Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Thu, 15 Oct 2020 22:04:36 +0200
> Subject: [PATCH 2/7] gnu: Add ocaml4.09-findlib.
>
> * gnu/packages/ocaml.scm (ocaml4.09-findlib): New variable.
> ---
>  gnu/packages/ocaml.scm | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index be2f486179..84daa8afca 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -1007,6 +1007,14 @@ compilers that can directly deal with packages.")
>       `(("m4" ,m4)
>         ("ocaml" ,ocaml-4.07)))))
>  
> +(define-public ocaml4.09-findlib
> +  (package
> +    (inherit ocaml-findlib)
> +    (name "ocaml4.09-findlib")
> +    (native-inputs
> +     `(("m4" ,m4)
> +       ("ocaml" ,ocaml-4.09)))))
> +
>  ;; note that some tests may hang for no obvious reason.
>  (define-public ocaml-ounit
>    (package
> -- 
> 2.28.0

LGTM!  Even if I was confused at first by: :-)

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix build ocaml4.09-findlib
/gnu/store/p49l1cr1wmf53szd5k4s2nx9n3p3qfc3-ocaml4.09-findlib-1.8.1

$ guix build ocaml-findlib
/gnu/store/n791rs3wvbm0fpmd0bqfhb3f4ysjwqia-ocaml-findlib-1.8.1

$ ./pre-inst-env guix build ocaml-findlib
/gnu/store/n791rs3wvbm0fpmd0bqfhb3f4ysjwqia-ocaml-findlib-1.8.1
--8<---------------cut here---------------end--------------->8---

>>>From a810e6647ba30aba02b58840101ee66b7fbcd792 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Thu, 15 Oct 2020 22:34:38 +0200
> Subject: [PATCH 3/7] guix: ocaml: Add package-with-ocaml4.09.
>
> * guix/build-system/ocaml.scm (package-with-ocaml4.09)
> (strip-ocaml4.09-variant): New variables.
> * gnu/packages/ocaml.scm (ocaml4.09-result, ocaml4.09-csexp)
> (ocaml4.09-dune-configurator, ocaml4.09-dune): New variables.
> ---
>  gnu/packages/ocaml.scm      | 46 ++++++++++++++++++++++++++++++++++++-
>  guix/build-system/ocaml.scm | 27 ++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 1 deletion(-)

[...]
  
> +(define-public ocaml4.09-dune-configurator
> +  (package
> +    (inherit dune-configurator)
> +    (name "ocaml4.09-dune-configurator")
> +    (arguments
> +     `(#:package "dune-configurator"
> +       #:tests? #f
> +       #:dune ,(package-with-ocaml4.09 dune-bootstrap)
> +       #:ocaml ,ocaml-4.09
> +       #:findlib ,ocaml4.09-findlib))
> +    (propagated-inputs
> +     `(("ocaml-csexp" ,(package-with-ocaml4.09 ocaml-csexp))))))

As I wrote in the other thread about updating Dune, it looks ackward to
me the use the 'package-with-ocaml4.09' inside and not wrap outside...

>  (define-public dune
>    (package
>      (inherit dune-bootstrap)
>      (propagated-inputs
>       `(("dune-configurator" ,dune-configurator)))
> -    (properties `((ocaml4.07-variant . ,(delay ocaml4.07-dune))))))
> +    (properties `((ocaml4.07-variant . ,(delay ocaml4.07-dune))
> +                  (ocaml4.09-variant . ,(delay ocaml4.09-dune))))))

...especially these 'delay' in properties.  But I got your
arguments. :-) 

> +(define-public ocaml4.09-dune
> +  (package
> +    (inherit (package-with-ocaml4.09 dune-bootstrap))
> +    (propagated-inputs
> +     `(("dune-configurator" ,dune-configurator)))))

[...]

> +(define-public ocaml4.09-csexp
> +  (package
> +    (inherit ocaml-csexp)
> +    (name "ocaml4.09-csexp")
> +    (arguments
> +     `(#:ocaml ,ocaml-4.09
> +       #:findlib ,ocaml4.09-findlib
> +       ,@(substitute-keyword-arguments (package-arguments ocaml-csexp)
> +           ((#:dune _) (package-with-ocaml4.09 dune-bootstrap)))))
> +    (propagated-inputs
> +     `(("ocaml-result" ,(package-with-ocaml4.09 ocaml-result))))))

You do not use the one you define below.  Why?  Other said, do you
really need 'ocaml4.09.result' defined below?

> +
>  (define-public ocaml-migrate-parsetree
>    (package
>      (name "ocaml-migrate-parsetree")
> @@ -1494,12 +1528,22 @@ powerful.")
>      (arguments
>       `(#:test-target "."
>         #:dune ,dune-bootstrap))
> +    (properties `((ocaml4.09-variant . ,(delay ocaml4.09-result))))

Does it fail if instead

    (properties `((ocaml4.09-variant . ,(package-with-ocaml4.09 ocaml-result)))

?

> +(define-public ocaml4.09-result
> +  (package
> +    (inherit ocaml-result)
> +    (arguments
> +     `(#:test-target "."
> +       #:dune ,(package-with-ocaml4.09 dune-bootstrap)
> +       #:ocaml ,ocaml-4.09
> +       #:findlib ,ocaml4.09-findlib))))

Since the name is not changed after inheritance, this package is
ambiguous.  And possibly do not compile.

Otherwise, LGTM.

>>>From 447de03a51a39fa7a7d8c5216c8ba23e632d0b88 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 16 Oct 2020 17:01:40 +0200
> Subject: [PATCH 4/7] gnu: laby: Update to 0.7.0.
>
> * gnu/packages/games.scm (laby): Update to 0.7.0.
> ---
>  gnu/packages/games.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I am not sure to get why this one needs update instead of replacing
'ocaml' by 'ocaml4.09'.  And the update in other patch set.  Well,
bikeshed. :-)

LGTM.

>>>From cb1c4f34d558b3b0b3cfc2d21d77c3d703f6013c Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 16 Oct 2020 17:02:56 +0200
> Subject: [PATCH 5/7] gnu: ocaml-ppx-tools-versioned: Use release profile.
>
> * gnu/packages/ocaml.scm (ocaml-ppx-tools-versioned): Use release
> profile.
> ---
>  gnu/packages/ocaml.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM!

> From 6b08d42d9ebaa3ee6da4a9f8aa9cc6d70cf19231 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 16 Oct 2020 17:06:53 +0200
> Subject: [PATCH 6/7] gnu: ocamlify: Perform bootstrap build.
>
> * gnu/packages/ocaml.scm (ocamlify): Perform bootstrap build.
> ---
>  gnu/packages/ocaml.scm | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 15ca326376..660aeef25a 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -2433,21 +2433,28 @@ radix-64 representation.  It is specified in RFC 4648.")
>          (base32 "1f0fghvlbfryf5h3j4as7vcqrgfjb4c8abl5y0y5h069vs4kp5ii"))))
>      (build-system ocaml-build-system)
>      (arguments
> -     `(#:phases
> +     `(#:tests? #f; no tests

Why?  Because the tests are run during the build?  If yes, does it make
sense to move the comment there?

> +       #:phases

[...]

>               #t))
> -         (delete 'check)                ; tests are run during the build

[...]

>      (home-page "https://forge.ocamlcore.org/projects/ocamlify")
>      (synopsis "Include files in OCaml code")
>      (description "OCamlify creates OCaml source code by including

Otherwise, LGTM!

>>>From 0c2ba2bf29d4a72fe05710a84ca2ca548801702a Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Fri, 16 Oct 2020 17:07:45 +0200
> Subject: [PATCH 7/7] gnu: ocaml: Switch to 4.11 by default.
>
> * gnu/packages/ocaml.scm (ocaml): Switch to 4.11 by default.
> ---
>  gnu/packages/ocaml.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 660aeef25a..e486a09fbb 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -243,7 +243,7 @@ functional, imperative and object-oriented styles of programming.")
>                            "--prefix" out
>                            "--mandir" mandir))))))))))
>  
> -(define-public ocaml ocaml-4.09)
> +(define-public ocaml ocaml-4.11)
>  
>  (define-public ocamlbuild
>    (package

Cool!  LGTM!

All the best,
simon