Skip to content

jsoo: honor build_runtime_flags when the shared standalone runtime is used#15455

Merged
Alizter merged 3 commits into
ocaml:mainfrom
hhugo:jsoo-shared-runtime-flags
Jul 20, 2026
Merged

jsoo: honor build_runtime_flags when the shared standalone runtime is used#15455
Alizter merged 3 commits into
ocaml:mainfrom
hhugo:jsoo-shared-runtime-flags

Conversation

@hhugo

@hhugo hhugo commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Since #13621 (workspace-shared standalone runtimes, released in 3.23.0), an executable in separate compilation mode that has no javascript_files/wasm_files of its own is always given the shared runtime — and the shared runtime is built by setup_shared_runtime_rule with In_context.default.flags. The stanza's (js_of_ocaml (build_runtime_flags ...)) are silently dropped: nothing is emitted at build time, and e.g. a --file %{dep:data.txt} pseudo-filesystem embed simply doesn't happen, breaking Sys_js.read_file at run time.

Minimal reproduction (first commit adds it as a cram test, recording the buggy output):

$ cat dune
(executable
 (name main)
 (modes js)
 (js_of_ocaml
  (compilation_mode separate)
  (build_runtime_flags (:standard --file %{dep:data.txt}))))
$ dune build ./main.bc.js && grep -c EMBEDDED_PAYLOAD _build/default/main.bc.js
0        # expected 1; the build-runtime command contains no --file

Adding a dummy (javascript_files extra.js) to the very same stanza — which disqualifies it from sharing — makes the flags take effect again, isolating the sharing path as the cause.

The fix (second commit) makes sharing eligibility also require that the stanza's build_runtime_flags spec is the default (:standard); otherwise the executable falls back to the per-stanza standalone runtime, where its flags apply as before 3.23. Executables with default flags keep sharing (the test's second half checks a single shared runtime is still produced).

Known limitation, unchanged by this PR: build_runtime_flags customized through an (env ...) stanza are still not reflected in the shared runtime, which is built with the flags of the workspace root. Fixing that would require either including expanded flags in the runtime key or comparing env nodes; both seemed better discussed separately.

Found in the wild: js_of_ocaml's own examples embed data files via --file and silently broke (blank canvas); see ocsigen/js_of_ocaml#2388 which now also ships the files over http as a workaround.

hhugo added 3 commits July 9, 2026 17:20
In separate compilation, an executable with no javascript_files of its
own gets the workspace-shared standalone runtime, which is built with
the default flags: the stanza's build_runtime_flags are silently
ignored. The test records the current (buggy) behaviour: the --file
payload is not embedded.
Since the introduction of workspace-shared standalone runtimes, the
eligibility check for sharing only looked at javascript_files/
wasm_files. An executable customizing (js_of_ocaml (build_runtime_flags
...)) was still given the shared runtime, which setup_shared_runtime_rule
builds with the default flags, so its flags were silently dropped
(e.g. --file embeds never happened).

Only share the runtime when the stanza's build_runtime_flags spec is
the default (:standard); otherwise fall back to the per-stanza
standalone runtime, where the flags apply.

Note that build_runtime_flags customized through an (env ...) stanza
are still not reflected in the shared runtime, which is always built
with the flags of the workspace root.
@hhugo

hhugo commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@vouillon, do you have time to look at this ? Test and fix were fully generated by AI assistant.

hhugo added a commit to ocsigen/js_of_ocaml that referenced this pull request Jul 9, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hhugo added a commit to ocsigen/js_of_ocaml that referenced this pull request Jul 9, 2026
Give each example using --file a dummy javascript_files entry: its
presence opts the executable out of dune's shared standalone runtime,
whose construction drops per-executable build_runtime_flags since dune
3.23 (ocaml/dune#15455). The pseudo-filesystem embeds work again, so
the JS examples can be opened directly from file:// as before; revert
the README caveat accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hhugo added a commit to ocsigen/js_of_ocaml that referenced this pull request Jul 9, 2026
Give each example using --file a dummy javascript_files entry: its
presence opts the executable out of dune's shared standalone runtime,
whose construction drops per-executable build_runtime_flags since dune
3.23 (ocaml/dune#15455). The pseudo-filesystem embeds work again, so
the JS examples can be opened directly from file:// as before; revert
the README caveat accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hhugo added a commit to ocsigen/js_of_ocaml that referenced this pull request Jul 10, 2026
Give each example using --file a dummy javascript_files entry: its
presence opts the executable out of dune's shared standalone runtime,
whose construction drops per-executable build_runtime_flags since dune
3.23 (ocaml/dune#15455). The pseudo-filesystem embeds work again, so
the JS examples can be opened directly from file:// as before; revert
the README caveat accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hhugo

hhugo commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@vouillon gentle ping

@hhugo hhugo mentioned this pull request Jul 20, 2026
56 tasks
@Alizter

Alizter commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

@hhugo The doc commit is missing DCO.

@Alizter

Alizter commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

I'd like to proceed with the release, so I will waive the DCO for the changelog entry commit here and add one myself in the backport.

@Alizter
Alizter merged commit e4ad0ad into ocaml:main Jul 20, 2026
36 checks passed
Alizter added a commit that referenced this pull request Jul 20, 2026
Backport #15455 onto `3.24.1-rc`.

The backport is signed off explicitly for DCO, and moves the changelog
entry into `doc/changes/fixed/` for the point release.
@Alizter Alizter added this to the 3.25.0 milestone Jul 21, 2026
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.

3 participants