Skip to content

Conversation

jchavarri
Copy link
Member

This change broke generation of the cmijs when melange-playground is installed, because there are no cmjs in ../jscomp/others/.belt.objs/melange/*.cm{i,j}.

There is no clean way to fix it, as it's not possible to use %{lib:melange:js... with globs like *.cm{i,j}:

  • if one uses (glob_files %{lib:melange:belt}/melange/*.cm{i,j}) the error is:
Error: File unavailable: /home/me/code/melange/_opam/lib/melange/belt
This is not a regular file (S_DIR)
  • if one uses (glob_files %{lib:melange:belt/melange/*.cm{i,j}}) the error is:
File "bin/dune", line 31, characters 17-18:
31 |    (glob_files %{lib:melange:belt/melange/*.cm{i,j}}))
                      ^
Error: This character is not allowed inside %{...} forms

This PR brings back the original hack, which adds a single cmi as dep, and then uses find, dirname and xargs to build the cmijs.

Copy link
Member

@anmonteiro anmonteiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might still be able to pack all of them together

@jchavarri
Copy link
Member Author

@anmonteiro thanks for the improvement. Tested with (wip) playground and everything seems to work fine, and the tests are passing too, so I think we can merge this one.

* main:
  Update CONTRIBUTING.md to include an installation step for melange.opam and a link to the Pull Request instructions (#651)
@jchavarri jchavarri merged commit 77bbb68 into main Jul 6, 2023
@jchavarri jchavarri deleted the playground/bring-back-cmijs-hack branch July 6, 2023 10:04
jchavarri added a commit that referenced this pull request Jul 6, 2023
* main:
  playground: bring back cmijs hack (#648)
jchavarri added a commit that referenced this pull request Jul 10, 2023
* main: (22 commits)
  playground: fix warning errors
  playground: add test for bad error msg
  playground: use js_of_ocaml library (#654)
  chore: update nix flakes (#652)
  playground: bring back cmijs hack (#648)
  Update CONTRIBUTING.md to include an installation step for melange.opam and a link to the Pull Request instructions (#651)
  playground: specify ppx dep (#646)
  playground: package + es6 + direct reason compilation (#632)
  feat: add `-rectypes` (#644)
  fix changelog
  fix: spurious warning 61 in externals
  test: show spurious warning 61 in externals
  test: show how to disable ppx alerts (#642)
  fix: disable warning 61 (`unboxable-type-in-prim-decl`) for deriving abstract externals
  test: show spurious warning on single field records with [@@deriving abstract]
  chore: format runtime / belt / node libraries (#640)
  chore: test on OCaml 5.1 (#639)
  chore: require dune 3.8, format, cleanup (#638)
  refactor: Don't use `(stdlib ...)` for the `Js.*` runtime (#637)
  chore: snapshot JS test files (#636)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants