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

From: Julien Lepiller <[email-masked]>
Date: Mon, 19 Oct 2020 10:11:32 -0400

Le 19 octobre 2020 09:31:32 GMT-04:00, zimoun <[email-masked]> a écrit :
>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. :-)

Thanks for the review!

>
>
>>>>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: :-)

The goal is to switcg to ocaml 4.11 by default, so after the switcg we need a way to express pcakages with ocaml 4.09 :)

>
>--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...

I understand this is confusing. The issue is that package-with-ocaml4.09 uses ocaml4.09-dune, but ocaml4.09-dune-configurator (as well as csexp and result) are dependencies of dune.

So I define them separately, and use the properties field, so I can use package-with-ocaml4.09, and use the variant I define, instead of the default.

>
>>  (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)))))
>
>[...]

Uh oh this is an issue I think :)

>
>> +(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?

Actually, the properties ensure that I actually use the one 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)))

Ah no, that should be fine, thanks.

>
>?
>
>
>> +(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.

Ah right, thanks!

>
>
>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?

No, the comment might have come from a previous version or just copied by mistake. There is really no test in this package (unless there's something weird going on with oasis scripts).

>
>> +       #: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