Make wasm_of_ocaml-compiler an optional dependency#878
Draft
hhugo wants to merge 9 commits into
Draft
Conversation
It is only needed to compile the client side to WebAssembly. Eliom itself builds without it: the client library is byte-mode and eliom_client.wat is installed as a runtime asset rather than compiled, so `dune build -p eliom` never invokes wasm_of_ocaml. Move it from depends to depopts so pure-JS users (and their CI) no longer have to pull the wasm toolchain (and binaryen). Verified: `dune build -p eliom` succeeds on a switch with wasm_of_ocaml-compiler removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that wasm_of_ocaml-compiler is an optional dependency, `opam install . --deps-only` no longer pulls it. Install it explicitly in the build job so the WebAssembly client target (and binaryen) stay exercised. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that wasm_of_ocaml-compiler is optional, a generated project must
not require it unless wasm is actually wanted. Three changes:
- Makefile.options: ENABLE_WASM now auto-detects the wasm_of_ocaml
compiler (`command -v wasm_of_ocaml`), defaulting to yes when present
and no otherwise, instead of a hardcoded yes.
- app.{exe,lib}/dune: drop client/<project>.bc.wasm.js from the
<project> alias deps. That dep made `make all` (which builds
`@install @<project>`) compile wasm unconditionally, regardless of
ENABLE_WASM. The alias is now a plain module check; the gated `js`
target still builds wasm when enabled. (modes js byte wasm) is kept so
that gated build works.
- app.lib/Makefile.app: route `byte`/`opt` through the gated `js` target
(app.exe already did) so wasm is still built when ENABLE_WASM=yes now
that it no longer comes from the alias.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a `distillery` job that generates a project from each template (app.exe, app.lib) and builds it, so the templates are actually exercised. It runs on OCaml 4 because the templates' default ocsipersist-sqlite backend pulls lwt_log, which requires OCaml < 5. wasm_of_ocaml is left uninstalled, so this also checks a generated project builds out-of-the-box on a JS-only install (its WASM build auto-disables) — the guarantee behind making wasm_of_ocaml-compiler optional. Fix two pre-existing staleness bugs the new job surfaced in PROJECT_NAME.opam (both templates), which otherwise prevent a generated project from building against current eliom: - eliom was capped at < 12.0.0, rejecting the current 12.0.1; bump to >= 12.0.0 & < 13.0.0. - ocsigen-ppx-rpc is used by the generated dune files but was not in the dependency list; add it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The build matrix installs wasm_of_ocaml-compiler but `dune build -p eliom` never emits wasm (the client lib is byte-mode; eliom_client.wat ships as a data file), so nothing actually compiled to WebAssembly in CI. Add a second phase to the distillery job that installs the wasm toolchain (binaryen + wasm_of_ocaml-compiler) and rebuilds each generated project; the templates auto-detect wasm_of_ocaml and compile the client to .bc.wasm.js, which is asserted to exist. This is the only job that exercises wasm_of_ocaml compilation end to end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pinning the js_of_ocaml repo installs every package in it, including wasm_of_ocaml-compiler, so the "JS only" phase was building wasm anyway (ENABLE_WASM auto-detected the compiler on PATH) and never exercised the opt-out path. Pin with -n so only eliom's real deps are installed by `opam install .` (wasm, being optional, stays out), and assert the wasm artifact is *not* produced in that phase so the regression can't recur. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
It is not a dependency of eliom in any sense: eliom links no wasm_of_ocaml library and never invokes the compiler at build time (eliom_client.wat is shipped as a runtime asset, not compiled), so eliom builds and installs identically whether or not it is present. It is only a tool downstream apps use to compile their client to WebAssembly. So it does not belong in depends nor depopts (a depopt would wrongly imply eliom's build varies with its presence); remove it. CI installs it explicitly where the wasm path is exercised. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sing ENABLE_WASM auto-detects wasm_of_ocaml, but a user can still force ENABLE_WASM=yes in Makefile.options without the compiler installed. Guard the wasm build in the `js` target so it fails with an actionable message (install wasm_of_ocaml-compiler, or set ENABLE_WASM=no) instead of a cryptic dune "program wasm_of_ocaml not found" error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The accepted failures on centos-10 / opensuse-* existed because eliom pulled wasm_of_ocaml, whose binaryen requirement those distributions couldn't meet. eliom no longer depends on wasm_of_ocaml at all, so the opam-repo CI won't build it there and the exception is moot. Remove it, which empties eliom.opam.template, so drop that file too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stop requiring
wasm_of_ocaml-compilerto use eliom, make the distillery templates' WebAssembly build opt-in on the toolchain, and add CI that actually builds generated projects (JS-only and wasm).1. wasm_of_ocaml-compiler is not a dependency
eliom links no
wasm_of_ocamllibrary and never invokes the compiler at build time —eliom_client.watis shipped as a runtime asset, not compiled, sodune build -p eliombuilds and installs identically with or without it. It is only a tool a downstream app uses to compile its client to WebAssembly. So it is removed from the opam metadata entirely (neitherdependsnordepopts, since a depopt would wrongly imply eliom's build varies with its presence).Consequently the
x-ci-accept-failuresentry forcentos-10/opensuse-*(added because those distros lacked a recent enough binaryen for wasm_of_ocaml) is now obsolete — the opam-repo CI no longer builds wasm_of_ocaml for eliom — and is removed (which empties and so deleteseliom.opam.template).Verified: with
wasm_of_ocaml-compilerremoved from the switch,dune build -p eliomstill succeeds.2. Distillery templates: WASM opt-in on the toolchain
A generated project must not require the wasm toolchain unless wasm is actually wanted:
Makefile.options:ENABLE_WASMauto-detectswasm_of_ocaml(command -v), defaulting toyeswhen present andnootherwise (was hardcodedyes).app.{exe,lib}/dune: dropclient/<project>.bc.wasm.jsfrom the<project>aliasdeps. That dep mademake allcompile wasm unconditionally regardless ofENABLE_WASM; the alias is now a plain module check, and the gatedjstarget still builds wasm when enabled.(modes js byte wasm)is kept so the gated build works.app.lib/Makefile.app: routebyte/optthrough the gatedjstarget (app.exe already did).Makefile.app: ifENABLE_WASM=yesis forced withoutwasm_of_ocamlinstalled, the build fails with an actionable hint instead of a cryptic dune "program not found" error.Also fix two pre-existing staleness bugs in
PROJECT_NAME.opam(both templates) that prevented a generated project from building against current eliom:< 12.0.0, rejecting the current12.0.1-> bump to>= 12.0.0 & < 13.0.0.ocsigen-ppx-rpcis used by the generated dune files but was not declared -> add it.3. CI
buildmatrix installswasm_of_ocaml-compilerexplicitly (it is no longer pulled as a dependency) so the eliom library still builds with it present.distilleryjob: generates a project from each template and builds it. Runs on OCaml 4 because the templates'ocsipersist-sqlitebackend pullslwt_log, which requires OCaml < 5. Two phases:-nsowasm_of_ocaml-compileris not installed; asserts no.bc.wasm.jsis produced (the opt-out path).wasm_of_ocaml-compiler, rebuilds, and asserts.bc.wasm.jsis produced. This is the only job that exerciseswasm_of_ocamlcompilation end to end.🤖 Generated with Claude Code