mbox series

[bug#64209,v2,00/14] Reorder commits and better descriptions

Message ID cover.1691500834.git.benjamin@uvy.fr
Headers show
Series Reorder commits and better descriptions | expand

Message

Benjamin Aug. 8, 2023, 1:22 p.m. UTC 
Hi

I’am updating, my first series of patches.

I bumped ocaml-lsp-server to latest patch

> There seems to be an issue with the ordering of the patches. I can not
> build ocaml-dune-rpc, as it requires ocaml-ordering which is only
> available a couple of commits later.

I reordered commits so that each package can be built for each commit

> Could you explain why the 'remove-vendor phase is needed? It seems like
> if code is vendored it should maybe also be removed in the `dune`
> package?

I did this because I saw that opam build is doing this way :
https://github.com/ocaml/dune/blob/main/dyn.opam#L21-L22

If those vendor files are not remove, there was some errors when
building ocaml-stdune

```
File "otherlibs/stdune/path.mli", line 1:
Error: The files otherlibs/stdune/.stdune.objs/byte/stdune__Path_intf.cmi
       and /gnu/store/wig75gdqm284z6y89dcd75fmxzim307a-ocaml5.0-pp-1.1.2/lib/ocaml/site-lib/pp/pp.cmi
       make inconsistent assumptions over interface Pp
error: in phase 'build': uncaught exception:
%exception #<&invoke-error program: "dune" arguments: ("build" "@install" "-p" "stdune") exit-status: 1 term-signal: #f stop-signal: #f>
phase `build' failed after 1.8 seconds
command "dune" "build" "@install" "-p" "stdune" failed with status 1
builder for `/gnu/store/cb2n85al5bqn6q3pcaiil01p9wvfbpvw-ocaml5.0-stdune-3.6.1.drv' failed with exit code 1
build of /gnu/store/cb2n85al5bqn6q3pcaiil01p9wvfbpvw-ocaml5.0-stdune-3.6.1.drv failed
```

Because it has been repeated for each dune sublibraries, it probably
make sense to share this part of the code.
Though I don't know if we can directly do that for dune as those
vendoring are not removed here https://github.com/ocaml/dune/blob/main/dune.opam

> We need a more descriptive description.

> I guess the descriptions come from OPAM. Unfortunately Guix has stricter
> requirements than OPAM...:)

You are right, I just took descriptions from opam and did not edit them
I  updated some of the description and synopsis. Tell if it is enough.
As I don't exactly know what packages are doing, I just picked up descriptions
from source files comments

Tell me if you have other comments on those patches

Best

Benjamin

Benjamin (14):
  gnu: Fix ocaml-dune-build-info
  gnu: Add ocaml-pp.
  gnu: Add ocamlformat-rpc-lib.
  gnu: Add ocaml-ppx-yojson-conv-lib.
  gnu: Add ocaml-ordering.
  gnu: Add ocaml-dyn.
  gnu: Add ocaml-stdune.
  gnu: Add ocaml-chrome-trace.
  gnu: Add ocaml-xdg.
  gnu: Add ocamlc-loc.
  gnu: Add ocaml-fiber.
  gnu: Add ocaml-dune-rpc.
  gnu: Add ocaml-merlin-lib-4.9.
  gnu: Add ocaml-lsp-server.

 gnu/packages/ocaml.scm | 295 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 294 insertions(+), 1 deletion(-)


base-commit: b20e5bcafd9654a767ffb78eefd7494f73ca67c8

Comments

pukkamustard Oct. 14, 2023, 9:37 a.m. UTC | #1 
Hi,

Thanks for the update. I haven't been able to check it thoroughly yet.

Unfortunately the synopsis/descriptions still need a bit of polish. For
example (non-exhaustive):

- ocaml-pp: The synopsis should indicate that it is an OCaml
  library. For markup (in this case hyerplinks) you must use the Texinfo
  markup (see https://guix.gnu.org/manual/en/guix.html#Synopses-and-Descriptions).
- ocamlformat-rpc-lib: Final sentence is missing a punctuation.
- ocaml-ppx-yojson-conv-lib: See ocaml-ppx-fields-conv for an example of
  a better description/synopsis.
- ocaml-dyn: Missing punctiation mark.
- ocaml-stdune: Not sure about the description.
- ocaml-xdg: Description should be more than just a link.

Also maybe we first re-solve #63947 (https://issues.guix.gnu.org/63947;
friendly ping to Julien) instead of re-fixining it in this series.

Cheers,
pukkamustard