diff mbox series

[bug#44032] gnu: ocaml: Update to 4.11.1

Message ID 20201016171436.5f642654@tachikoma
State New
Headers show
Series [bug#44032] gnu: ocaml: Update to 4.11.1 | expand

Checks

Context Check Description
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Julien Lepiller Oct. 16, 2020, 3:14 p.m. UTC
Hi Guix!

this small patch series update ocaml to 4.11.1. The first patch adds
ocaml 4.11.1, the second adds findlib for 4.09, the third add
package-with-ocaml4.09 (along with ocaml4.09-dune, which I can't put in
a separate patch without breaking something). Then, three patches fix
the build of three packages (with these patches, they build fine with
4.09 and 4.11).  The last patch performs the final switch to 4.11.
Easier than I expected! :)

Note that with that, previously unreproducible packages are
reproducible!

Comments

Brett Gilio Oct. 16, 2020, 7:21 p.m. UTC | #1
Julien Lepiller <julien@lepiller.eu> writes:

> Hi Guix!
>
> this small patch series update ocaml to 4.11.1. The first patch adds
> ocaml 4.11.1, the second adds findlib for 4.09, the third add
> package-with-ocaml4.09 (along with ocaml4.09-dune, which I can't put in
> a separate patch without breaking something). Then, three patches fix
> the build of three packages (with these patches, they build fine with
> 4.09 and 4.11).  The last patch performs the final switch to 4.11.
> Easier than I expected! :)
>
> Note that with that, previously unreproducible packages are
> reproducible!
>

Great work Julien! We are also working on this on the Debian side as
well. I am looking forward to seeing 4.11.1 hit Guix master. Thanks a
lot for your and other's hard work on this.
zimoun Oct. 19, 2020, 1:31 p.m. UTC | #2
Hi!

On Fri, 16 Oct 2020 at 17:14, Julien Lepiller <julien@lepiller.eu> 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 <julien@lepiller.eu>
> 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 <julien@lepiller.eu>
> 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 <julien@lepiller.eu>
> 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 <julien@lepiller.eu>
> 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 <julien@lepiller.eu>
> 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 <julien@lepiller.eu>
> 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 <julien@lepiller.eu>
> 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
Julien Lepiller Oct. 19, 2020, 2:11 p.m. UTC | #3
Le 19 octobre 2020 09:31:32 GMT-04:00, zimoun <zimon.toutoune@gmail.com> a écrit :
>Hi!
>
>On Fri, 16 Oct 2020 at 17:14, Julien Lepiller <julien@lepiller.eu>
>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 <julien@lepiller.eu>
>> 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 <julien@lepiller.eu>
>> 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 <julien@lepiller.eu>
>> 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 <julien@lepiller.eu>
>> 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 <julien@lepiller.eu>
>> 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 <julien@lepiller.eu>
>> 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 <julien@lepiller.eu>
>> 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
zimoun Oct. 19, 2020, 2:24 p.m. UTC | #4
On Mon, 19 Oct 2020 at 16:11, Julien Lepiller <julien@lepiller.eu> wrote:

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

I am not sure.  You are recreating a package based on 'ocaml-result'
and not using the package 'ocaml4.09-result'.  Or I miss something
about the symbols.


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

Moreover, my point is: you are using

 once:
    (properties `((ocaml4.09-variant . ,(delay ocaml4.09-result))))
and once:
     `(("ocaml-result" ,(package-with-ocaml4.09 ocaml-result))))))

and it seems easier to only use one form.  Other said, maybe you do
not need the new 'ocaml4.09-result' and '(package-with-ocaml4.09
ocaml-result)' is enough.


> >> -     `(#: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).

Maybe reword the comment: "no test provided by the package"or
something like that.


All the best,
simon
Julien Lepiller Oct. 19, 2020, 3:14 p.m. UTC | #5
Le 19 octobre 2020 10:24:23 GMT-04:00, zimoun <zimon.toutoune@gmail.com> a écrit :
>On Mon, 19 Oct 2020 at 16:11, Julien Lepiller <julien@lepiller.eu>
>wrote:
>
>> >> +(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 :)
>
>I am not sure.  You are recreating a package based on 'ocaml-result'
>and not using the package 'ocaml4.09-result'.  Or I miss something
>about the symbols.

package-with-ocaml4.09 first reads the properties field of its arguments, and if it has an ocaml4.09-variant,it uses it. Otherwise it creates a new package that uses ocaml 4.09, ocaml4.09-findlib and ocaml4.09-dune.

If you remove the property in ocaml-result, you'll see that building (package-with-ocaml4.09 result) ends up in a loop: it wants ocaml4.09-dune, but is itself a dependency of ocaml4.09-dune.

>
>
>> >> +(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!
>
>Moreover, my point is: you are using
>
> once:
>    (properties `((ocaml4.09-variant . ,(delay ocaml4.09-result))))
>and once:
>     `(("ocaml-result" ,(package-with-ocaml4.09 ocaml-result))))))
>
>and it seems easier to only use one form.  Other said, maybe you do
>not need the new 'ocaml4.09-result' and '(package-with-ocaml4.09
>ocaml-result)' is enough.

OK, I'll use the variant I define everywhere if it makes things easier to understand later.

>
>
>> >> -     `(#: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).
>
>Maybe reword the comment: "no test provided by the package"or
>something like that.
>
>
>All the best,
>simon
diff mbox series

Patch

From 0c2ba2bf29d4a72fe05710a84ca2ca548801702a Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
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
-- 
2.28.0