[bug#34412,bug#45498,00/12] Add ocaml-merlin.

Le Mon, 28 Dec 2020 13:40:17 +0100,
pukkamustard <pukkamustard@posteo.net> a écrit :

> Hi Guix,
> 
> This adds ocaml-merlin, a development tool for OCaml (and
> dependencies).

Thanks for the patches!

> 
> Some notes and questions:
> 
> - ocaml4.07-merlin is already available (version 3.2.2). This version
> has not been updated (but changed to inherit from ocaml-merlin).
> 
> - There are many ocaml4.07-* packages in Guix. The reason for this is
> that some packages need to be built with OCaml 4.07 but some newer
> versions of libraries do no longer support OCaml 4.07 (or the old
> version of dune), requiring older versions of the same packages. See
> also commit message 9ada1555.

Have you tried updating dune to the latest version that builds with
ocaml 4.07?

> 
>   This is quite cumbersome as two trees of OCaml packages need to be
>   maintained.

Actually 3, there are ocaml 4.07, 4.09 and 4.11 currently. I'm working
on a bootstrap for ocaml 4.07 (ocaml builds from a binary version of
itself), so I don't think it'll disappear soon, but at least we can try
and make ocaml4.07-* packages disappear :)

> 
>   A nice solution would be to upgrade packages requiring OCaml 4.07
> to also build with the default OCaml (4.11). Packages that are not
> dependencies of other packages that require OCaml 4.07 seem to be:
> `bap`, `ocaml-earley` and `pplacer`. Maybe this should be opened as
> an issue?

pretty sure they build with at least ocaml-4.09 now, I'll see what I
can do :)

> 
> - Some OCaml libraries have dependencies in `inputs` that seem to be
> required from packages using the libraries.  E.g. `ocaml-yojson`
> depends on `ocaml-biniou`. Packages depending on `ocaml-yojson` need
> to manually add `ocaml-biniou` to their inputs. Would it work/make
> sense to add `ocaml-biniou` to `propagated-inputs` of `ocaml-yojson`?

Yeah, could you provide an additional patch for that?

> 
> - Test for ocaml-merlin (version 3.4.2) work. This resolves issue
> #34412 (alltough ocaml4.07-merlin still can not run tests).
> 
> Thanks!
> -pukkamustard

Thank you! I've sent some remarks to your patches, but generally LGTM :)

> 
> pukkamustard (12):
>   gnu: Remove ocaml-js-build-tools.
>   gnu: Add ocaml-sexplib.
>   gnu: Add ocaml-base.
>   gnu: Add ocaml-parsexp.
>   gnu: Add ocaml-sexplib.
>   gnu: Add ocaml-ounit2.
>   gnu: ocaml-tyxml: Update to 4.4.0.
>   gnu: ocaml-markup: Update to 1.0.0.
>   gnu: Add ocaml-odoc.
>   gnu: Add ocaml-version.
>   gnu: Add ocaml-mdx.
>   gnu: Add ocaml-merlin.
> 
>  gnu/packages/ocaml.scm | 527
> ++++++++++++++++++++++++++++------------- 1 file changed, 359
> insertions(+), 168 deletions(-)
>

Le Mon, 28 Dec 2020 19:10:48 +0100,
pukkamustard <pukkamustard@posteo.net> a écrit :

> Thank you for the feedback! v2 of patch series coming shortly...
> 
> I was not aware of the `ocaml4.07-variant` property. It makes 
> things a
> lot nicer!
> 
> I've added it for `ocaml-markup` and `ocaml-odoc` and also
> `ocaml-sexplib0`, `ocaml-parsexp`, `ocaml-base` and 
> `ocaml-sexplib`.

Thank you, it will make things easier to maintain I think :)

> 
> This makes `ocaml4.07-*` vanish from lot of places in favor of
> `(package-with-ocaml4.07 *)`.
> 
> > Have you tried updating dune to the latest version that builds 
> > with
> > ocaml 4.07?  
> 
> Not yet. For ocaml-markup 1.0.0 the problem is that the dune file
> version is set to 2.7. Any dune below 2.7.x will probably not 
> work.

Is this a hard requirement? I think we used to be able to use older
dune versions despite this.

> 
> For other packages it might work...
> 
> >  
> >>
> >>   This is quite cumbersome as two trees of OCaml packages need 
> >>   to be
> >>   maintained.  
> >
> > Actually 3, there are ocaml 4.07, 4.09 and 4.11 currently. I'm 
> > working
> > on a bootstrap for ocaml 4.07 (ocaml builds from a binary 
> > version of
> > itself), so I don't think it'll disappear soon, but at least we 
> > can try
> > and make ocaml4.07-* packages disappear :)
> >  
> 
> \o/ Looking forward to the bootstrap!

Yay, it should become reality in the next few months. Currently we
target ocaml 4.07 because it's the last version that doesn't require
merlin; after that we should be able to use ocaml4.07-merlin to build
the parser for more recent versions.

> With the `ocaml4.*-variant` property managing the 3 trees does not 
> seem
> so bad after all.
> 
> Currently there are a lot of packages only for 4.07. It would be 
> nicer
> to have packages for default compiler and when needed variants for 
> older
> compilers.

This is for historical reasons: we had these packages with ocaml 4.07,
and only for those packages that still require ocaml 4.07. More recent
versions of these packages have a different dependency tree, which
makes it hard to maintain both trees at the same time, especially as
there is no version that supports any two of our ocaml compilers at the
same time.

> I plan to do some OCaml hacking in the next months and will send 
> in
> patches toward this, if that is ok.

That would be very appreciated!

> >> - Some OCaml libraries have dependencies in `inputs` that seem 
> >> to be
> >> required from packages using the libraries.  E.g. 
> >> `ocaml-yojson`
> >> depends on `ocaml-biniou`. Packages depending on `ocaml-yojson` 
> >> need
> >> to manually add `ocaml-biniou` to their inputs. Would it 
> >> work/make
> >> sense to add `ocaml-biniou` to `propagated-inputs` of 
> >> `ocaml-yojson`?  
> >
> > Yeah, could you provide an additional patch for that?  
> 
> Done. Patch 13 in v2.

Could you instead put this as patch 12, and add merlin as patch 13?
That way you don't have to add the dependencies in merlin and remove
them in the following patch.

Also, patch 12 adds ocaml-dot-merlin-reader and ocaml-merlin. could you
separate it in two separate patches? I'm also wondering if we should
call this package "merlin" instead of ocaml-merlin, since it provides a
binary of that name?

> 
> -pukkamustard
>

>> > Have you tried updating dune to the latest version that 
>> > builds
>> > with
>> > ocaml 4.07?
>>
>> Not yet. For ocaml-markup 1.0.0 the problem is that the dune 
>> file
>> version is set to 2.7. Any dune below 2.7.x will probably not
>> work.
>
> Is this a hard requirement? I think we used to be able to use 
> older
> dune versions despite this.

Not sure. Will keep this for future investigation.

>> >> - Some OCaml libraries have dependencies in `inputs` that 
>> >> seem
>> >> to be
>> >> required from packages using the libraries.  E.g.
>> >> `ocaml-yojson`
>> >> depends on `ocaml-biniou`. Packages depending on 
>> >> `ocaml-yojson`
>> >> need
>> >> to manually add `ocaml-biniou` to their inputs. Would it
>> >> work/make
>> >> sense to add `ocaml-biniou` to `propagated-inputs` of
>> >> `ocaml-yojson`?
>> >
>> > Yeah, could you provide an additional patch for that?
>>
>> Done. Patch 13 in v2.
>
> Could you instead put this as patch 12, and add merlin as patch 
> 13?
> That way you don't have to add the dependencies in merlin and 
> remove
> them in the following patch.
>
> Also, patch 12 adds ocaml-dot-merlin-reader and ocaml-merlin. 
> could you
> separate it in two separate patches?

Done. v3 coming shortly...

> I'm also wondering if we should
> call this package "merlin" instead of ocaml-merlin, since it 
> provides a
> binary of that name?

Hm, both seems fine to me. I've stuck to "ocaml-merlin" for now. 
Feel
free to change if you think "merlin" is more suitable.

-pukkamustard