Propagate linkall to js_of_ocaml for separate compilation by hhugo · Pull Request #6832 · ocaml/dune

hhugo · 2023-01-05T15:53:53Z

I'm currently working on ocsigen/js_of_ocaml#1378 which make js_of_ocaml link only link required compilation units (similar to what ocamlc/ocamlopt do).
The goal is reduce the size of the generated js files and fix a semantic difference between whole program and separate compilation (Side effects in a library are always performed with separate compilation).

with ocsigen/js_of_ocaml#1378, js_of_ocaml link accept a new flag --linkall. This PR tries to propagate that information where it's currently ignored.

The first commit is mostly a cleanup.

it replaces Action_builder.map/bind with the Action_builder.O let syntax
it extract the type Link_time_code_gen.t to its own module to avoid circular deps.
make it clear where we ignore -linkall information

The second just actually propagate the linkall flags

I'm not sure how to proceed from there

because the released version of jsoo doesn't support --linkall, the dune PR cannot be merged in the current state.
If I release jsoo with
Separate compilation: Link only what we need ocsigen/js_of_ocaml#1378, then inline_tests will silently break.

One solution would be the check the jsoo version to decide whether to include the flag or not. Is it acceptable ? does this exist elsewhere in the codebase ?

src/dune_rules/exe.ml

Alizter · 2023-01-05T15:57:06Z

For Coq, we query the coqc installation early on in order to determine whether to pass native compilation flags. I suppose you could query jsoo to determine if --linkall will be supported and add them in accordingly. The relevant code is in coq_config.ml. OCaml is queried early on in every Dune build, so Coq is only an example of a deferred version query.

hhugo · 2023-01-06T10:28:33Z

For Coq, we query the coqc installation early on in order to determine whether to pass native compilation flags. I suppose you could query jsoo to determine if --linkall will be supported and add them in accordingly. The relevant code is in coq_config.ml. OCaml is queried early on in every Dune build, so Coq is only an example of a deferred version query.

Thanks, I've tried to do something similar.

src/dune_rules/jsoo_rules.ml

hhugo · 2023-01-07T12:40:25Z

ocsigen/js_of_ocaml#1378 has just been merged, keeping the default behavior unchanged for now to prevent silent breakage.

hhugo · 2023-01-11T09:37:31Z

I've rebased and squashed some commits.

hhugo · 2023-01-12T16:34:49Z

Gentle ping

src/dune_rules/dune_rules.ml

src/dune_rules/jsoo_rules.ml

src/dune_rules/exe.mli

src/dune_rules/exe_rules.ml

hhugo · 2023-01-17T08:56:20Z

I've rebased and squashed commits again.
ping @rgrinberg

Signed-off-by: Hugo Heuzard <hugo.heuzard@gmail.com>

@rgrinberg

…, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0~alpha1) CHANGES: - Fix parsing of OCaml errors that contain code excerpts with `...` in them. (ocaml/dune#7008, @rgrinberg) - Pre-emptively clear screen in watch mode (ocaml/dune#6987, fixes ocaml/dune#6884, @rgrinberg) - Fix cross compilation configuration when a context with targets is itself a host of another context (ocaml/dune#6958, fixes ocaml/dune#6843, @rgrinberg) - Fix parsing of the `<=` operator in *blang* expressions of `dune` files. Previously, the operator would be interpreted as `,`. (ocaml/dune#6928, @tatchi) - Fix `--trace-file` output. Dune now emits a single *complete* event for every executed process. Unterminated *async* events are no longer written. (ocaml/dune#6892, @rgrinberg) - Fix preprocessing with `staged_pps` (ocaml/dune#6748, fixes ocaml/dune#6644, @rgrinberg) - Use colored output with MDX when Dune colors are enabled. (ocaml/dune#6462, @MisterDA) - Make `dune describe workspace` return consistent dependencies for executables and for libraries. By default, compile-time dependencies towards PPX-rewriters are from now not taken into account (but runtime dependencies always are). Compile-time dependencies towards PPX-rewriters can be taken into account by providing the `--with-pps` flag. (ocaml/dune#6727, fixes ocaml/dune#6486, @esope) - Print missing newline after `$ dune exec`. (ocaml/dune#6821, fixes ocaml/dune#6700, @rgrinberg, @Alizter) - Fix binary corruption when installing or promoting in parallel (ocaml/dune#6669, fixes ocaml/dune#6668, @edwintorok) - Use colored output with GCC and Clang when compiling C stubs. The flag `-fdiagnostics-color=always` is added to the `:standard` set of flags. (ocaml/dune#4083, @MisterDA) - Fix the parsing of decimal and hexadecimal escape literals in `dune`, `dune-package`, and other dune s-expression based files (ocaml/dune#6710, @shym) - Report an error if `dune init ...` would create a "dune" file in a location which already contains a "dune" directory (ocaml/dune#6705, @gridbugs) - Fix the parsing of alerts. They will now show up in diagnostics correctly. (ocaml/dune#6678, @rginberg) - Fix the compilation of modules generated at link time when `implicit_transitive_deps` is enabled (ocaml/dune#6642, @rgrinberg) - Allow `$ dune utop` to load libraries defined in data only directories defined using `(subdir ..)` (ocaml/dune#6631, @rgrinberg) - Format dune files when they are named `dune-file`. This occurs when we enable the alternative file names project option. (ocaml/dune#6566, @rgrinberg) - Move `$ dune ocaml-merlin -dump-config=$dir` to `$ dune ocaml merlin dump-config $dir`. (ocaml/dune#6547, @rgrinberg) - Allow compilation rules to be impacted by `(env ..)` stanzas that modify the environment or set binaries. (ocaml/dune#6527, @rgrinberg) - Coq native mode is now automatically detected by Dune starting with Coq lang 0.7. `(mode native)` has been deprecated in favour of detection from the configuration of Coq. (ocaml/dune#6409, @Alizter) - Print "Leaving Directory" whenever "Entering Directory" is printed. (ocaml/dune#6149, fixes ocaml/dune#138, @cpitclaudel, @rgrinberg) - Allow `$ dune ocaml dump-dot-merlin` to run in watch mode. Also this command shouldn't print "Entering Directory" mesages. (ocaml/dune#6497, @rgrinberg) - `dune clean` should no longer fail under Windows due to the inability to remove the `.lock` file. Also, bring the implementation of the global lock under Windows closer to that of Unix. (ocaml/dune#6523, @nojb) - Remove "Entering Directory" messages for `$ dune install`. (ocaml/dune#6513, @rgrinberg) - Stop passing `-q` flag in `dune coq top`, which allows for `.coqrc` to be loaded. (ocaml/dune#6848, fixes ocaml/dune#6847, @Alizter) - Fix missing dependencies when detecting the kind of C compiler we're using (ocaml/dune#6610, fixes ocaml/dune#6415, @emillon) - Allow `(include_subdirs qualified)` for OCaml projects. (ocaml/dune#6594, fixes ocaml/dune#1084, @rgrinberg) - Accurately determine merlin configuration for all sources selected with `copy#` and `copy_files#`. The old heuristic of looking for a module in parent directories is removed (ocaml/dune#6594, @rgrinberg) - Fix inline tests with *js_of_ocaml* and whole program compilation mode enabled (ocaml/dune#6645, @hhugo) - Fix *js_of_ocaml* separate compilation rules when `--enable=effects` ,`--enable=use-js-string` or `--toplevel` is used. (ocaml/dune#6714, ocaml/dune#6828, ocaml/dune#6920, @hhugo) - Fix *js_of_ocaml* separate compilation in presence of linkall (ocaml/dune#6832, ocaml/dune#6916, @hhugo) - Remove spurious build dir created when running `dune init proj ...` (ocaml/dune#6707, fixes ocaml/dune#5429, @gridbugs) - Allow `--sandbox` to affect `ocamldep` invocations. Previously, they were wrongly marked as incompatible (ocaml/dune#6749, @rgrinberg) - Validate the command line arguments for `$ dune ocaml top-module`. This command requires one positional argument (ocaml/dune#6796, fixes ocaml/dune#6793, @rgrinberg) - Add a `dune cache size` command for displaying the size of the cache (ocaml/dune#6638, @Alizter) - Add 4.14.0 MSVC to CI (ocaml/dune#6917, @jonahbeckford) - Fix dependency cycle when installing files to the bin section with `glob_files` (ocaml/dune#6764, fixes ocaml/dune#6708, @gridbugs) - Handle "Too many links" errors when using Dune cache on Windows (ocaml/dune#6993, @nojb)

@gridbugs

…, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0) CHANGES: - Allow running `$ dune exec` in watch mode (with the `-w` flag). In watch mode, `$ dune exec` the executed binary whenever it is recompiled. (ocaml/dune#6966, @gridbugs) - `coqdep` is now called once per theory, instead of one time per Coq file. This should significantly speed up some builds, as `coqdep` startup time is often heavy (ocaml/dune#7048, @Alizter, @ejgallego) - Add `map_workspace_root` dune-project stanza to allow disabling of mapping of workspace root to `/workspace_root`. (ocaml/dune#6988, fixes ocaml/dune#6929, @richardlford) - Fix handling of support files generated by odoc. (ocaml/dune#6913, @jonludlam) - Fix parsing of OCaml errors that contain code excerpts with `...` in them. (ocaml/dune#7008, @rgrinberg) - Pre-emptively clear screen in watch mode (ocaml/dune#6987, fixes ocaml/dune#6884, @rgrinberg) - Fix cross compilation configuration when a context with targets is itself a host of another context (ocaml/dune#6958, fixes ocaml/dune#6843, @rgrinberg) - Fix parsing of the `<=` operator in *blang* expressions of `dune` files. Previously, the operator would be interpreted as `<`. (ocaml/dune#6928, @tatchi) - Fix `--trace-file` output. Dune now emits a single *complete* event for every executed process. Unterminated *async* events are no longer written. (ocaml/dune#6892, @rgrinberg) - Fix preprocessing with `staged_pps` (ocaml/dune#6748, fixes ocaml/dune#6644, @rgrinberg) - Use colored output with MDX when Dune colors are enabled. (ocaml/dune#6462, @MisterDA) - Make `dune describe workspace` return consistent dependencies for executables and for libraries. By default, compile-time dependencies towards PPX-rewriters are from now not taken into account (but runtime dependencies always are). Compile-time dependencies towards PPX-rewriters can be taken into account by providing the `--with-pps` flag. (ocaml/dune#6727, fixes ocaml/dune#6486, @esope) - Print missing newline after `$ dune exec`. (ocaml/dune#6821, fixes ocaml/dune#6700, @rgrinberg, @Alizter) - Fix binary corruption when installing or promoting in parallel (ocaml/dune#6669, fixes ocaml/dune#6668, @edwintorok) - Use colored output with GCC and Clang when compiling C stubs. The flag `-fdiagnostics-color=always` is added to the `:standard` set of flags. (ocaml/dune#4083, @MisterDA) - Fix the parsing of decimal and hexadecimal escape literals in `dune`, `dune-package`, and other dune s-expression based files (ocaml/dune#6710, @shym) - Report an error if `dune init ...` would create a "dune" file in a location which already contains a "dune" directory (ocaml/dune#6705, @gridbugs) - Fix the parsing of alerts. They will now show up in diagnostics correctly. (ocaml/dune#6678, @rginberg) - Fix the compilation of modules generated at link time when `implicit_transitive_deps` is enabled (ocaml/dune#6642, @rgrinberg) - Allow `$ dune utop` to load libraries defined in data only directories defined using `(subdir ..)` (ocaml/dune#6631, @rgrinberg) - Format dune files when they are named `dune-file`. This occurs when we enable the alternative file names project option. (ocaml/dune#6566, @rgrinberg) - Move `$ dune ocaml-merlin -dump-config=$dir` to `$ dune ocaml merlin dump-config $dir`. (ocaml/dune#6547, @rgrinberg) - Allow compilation rules to be impacted by `(env ..)` stanzas that modify the environment or set binaries. (ocaml/dune#6527, @rgrinberg) - Coq native mode is now automatically detected by Dune starting with Coq lang 0.7. `(mode native)` has been deprecated in favour of detection from the configuration of Coq. (ocaml/dune#6409, @Alizter) - Print "Leaving Directory" whenever "Entering Directory" is printed. (ocaml/dune#6419, fixes ocaml/dune#138, @cpitclaudel, @rgrinberg) - Allow `$ dune ocaml dump-dot-merlin` to run in watch mode. Also this command shouldn't print "Entering Directory" mesages. (ocaml/dune#6497, @rgrinberg) - `dune clean` should no longer fail under Windows due to the inability to remove the `.lock` file. Also, bring the implementation of the global lock under Windows closer to that of Unix. (ocaml/dune#6523, @nojb) - Remove "Entering Directory" messages for `$ dune install`. (ocaml/dune#6513, @rgrinberg) - Stop passing `-q` flag in `dune coq top`, which allows for `.coqrc` to be loaded. (ocaml/dune#6848, fixes ocaml/dune#6847, @Alizter) - Fix missing dependencies when detecting the kind of C compiler we're using (ocaml/dune#6610, fixes ocaml/dune#6415, @emillon) - Allow `(include_subdirs qualified)` for OCaml projects. (ocaml/dune#6594, fixes ocaml/dune#1084, @rgrinberg) - Accurately determine merlin configuration for all sources selected with `copy#` and `copy_files#`. The old heuristic of looking for a module in parent directories is removed (ocaml/dune#6594, @rgrinberg) - Fix inline tests with *js_of_ocaml* and whole program compilation mode enabled (ocaml/dune#6645, @hhugo) - Fix *js_of_ocaml* separate compilation rules when `--enable=effects` ,`--enable=use-js-string` or `--toplevel` is used. (ocaml/dune#6714, ocaml/dune#6828, ocaml/dune#6920, @hhugo) - Fix *js_of_ocaml* separate compilation in presence of linkall (ocaml/dune#6832, ocaml/dune#6916, @hhugo) - Remove spurious build dir created when running `dune init proj ...` (ocaml/dune#6707, fixes ocaml/dune#5429, @gridbugs) - Allow `--sandbox` to affect `ocamldep` invocations. Previously, they were wrongly marked as incompatible (ocaml/dune#6749, @rgrinberg) - Validate the command line arguments for `$ dune ocaml top-module`. This command requires one positional argument (ocaml/dune#6796, fixes ocaml/dune#6793, @rgrinberg) - Add a `dune cache size` command for displaying the size of the cache (ocaml/dune#6638, @Alizter) - Fix dependency cycle when installing files to the bin section with `glob_files` (ocaml/dune#6764, fixes ocaml/dune#6708, @gridbugs) - Handle "Too many links" errors when using Dune cache on Windows (ocaml/dune#6993, @nojb) - Allow the `cinaps` stanza to set a custom alias. By default, if the alias is not set then the cinaps actions will be attached to both `@cinaps` and `@runtest` (ocaml/dune#6991, @rgrinberg) - Add `(using ctypes 0.3)`. When used, paths in `(ctypes)` are interpreted relative to where the stanza is defined. (ocaml/dune#6883, fixes ocaml/dune#5325, @emillon) - Auto-detect `dune-workspace` files as `dune` files in Emacs (ocaml/dune#7061, @ilankri) - Add native support for polling mode on Windows (ocaml/dune#7010, @yams-yams, @nojb)

hhugo · 2023-03-14T12:01:52Z

FYI, Jsoo 5.1 has just been released. Users will exercise the new behavior implemented in this PR as soon as they upgrade jsoo.

hhugo commented Jan 5, 2023

View reviewed changes

src/dune_rules/exe.ml Outdated Show resolved Hide resolved

hhugo mentioned this pull request Jan 5, 2023

Separate compilation: Link only what we need ocsigen/js_of_ocaml#1378

Merged

1 task

hhugo force-pushed the jsoo_linkall branch from fcd2db7 to 1c07c58 Compare January 6, 2023 09:54

hhugo force-pushed the jsoo_linkall branch from 49d034a to eec5483 Compare January 6, 2023 13:26

Alizter reviewed Jan 6, 2023

View reviewed changes

src/dune_rules/jsoo_rules.ml Outdated Show resolved Hide resolved

hhugo force-pushed the jsoo_linkall branch from ac856c9 to 24bd30e Compare January 6, 2023 14:42

hhugo added this to the 3.7.0 milestone Jan 6, 2023

hhugo requested review from Alizter and rgrinberg January 6, 2023 17:06

rgrinberg added the jsoo label Jan 6, 2023

hhugo mentioned this pull request Jan 7, 2023

False positive your program contains effect handlers warning? ocsigen/js_of_ocaml#1375

Closed

hhugo mentioned this pull request Jan 10, 2023

chore: move jsoo rules to jsoo dir #6861

Merged

hhugo force-pushed the jsoo_linkall branch from 24bd30e to 4a5cb08 Compare January 11, 2023 09:37

rgrinberg reviewed Jan 12, 2023

View reviewed changes

hhugo commented Jan 12, 2023

View reviewed changes

src/dune_rules/exe_rules.ml Outdated Show resolved Hide resolved

hhugo requested a review from rgrinberg January 13, 2023 00:51

hhugo force-pushed the jsoo_linkall branch from 22e33d7 to c3984e2 Compare January 17, 2023 08:53

hhugo force-pushed the jsoo_linkall branch from c3984e2 to 2ae2d36 Compare January 18, 2023 10:29

hhugo added 2 commits January 19, 2023 18:00

refactor(jsoo): don't ignore linkall (prepare)

bbaceea

Signed-off-by: Hugo Heuzard <hugo.heuzard@gmail.com>

fix(jsoo): don't ignore linkall

35b9f13

Signed-off-by: Hugo Heuzard <hugo.heuzard@gmail.com>

rgrinberg force-pushed the jsoo_linkall branch from 2ae2d36 to 35b9f13 Compare January 20, 2023 00:01

rgrinberg merged commit c357cee into ocaml:main Jan 20, 2023

hhugo deleted the jsoo_linkall branch January 20, 2023 05:42

hhugo mentioned this pull request Jan 23, 2023

Fix jsoo linkall detection #6916

Merged

emillon mentioned this pull request Feb 6, 2023

[new release] xdg, stdune, ordering, ocamlc-loc, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0~alpha1) ocaml/opam-repository#23248

Closed

emillon mentioned this pull request Feb 17, 2023

[new release] xdg, stdune, ordering, ocamlc-loc, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.7.0) ocaml/opam-repository#23349

Merged

sim642 mentioned this pull request Jul 24, 2024

[BUG] Runtile failure with -linkall since 5.1.0 ocsigen/js_of_ocaml#1641

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate linkall to js_of_ocaml for separate compilation#6832

Propagate linkall to js_of_ocaml for separate compilation#6832
rgrinberg merged 2 commits intoocaml:mainfrom
hhugo:jsoo_linkall

hhugo commented Jan 5, 2023

Uh oh!

Uh oh!

Alizter commented Jan 5, 2023 •

edited

Loading

Uh oh!

hhugo commented Jan 6, 2023

Uh oh!

Uh oh!

hhugo commented Jan 7, 2023

Uh oh!

hhugo commented Jan 11, 2023

Uh oh!

hhugo commented Jan 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hhugo commented Jan 17, 2023

Uh oh!

hhugo commented Mar 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

hhugo commented Jan 5, 2023

Uh oh!

Uh oh!

Alizter commented Jan 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hhugo commented Jan 6, 2023

Uh oh!

Uh oh!

hhugo commented Jan 7, 2023

Uh oh!

hhugo commented Jan 11, 2023

Uh oh!

hhugo commented Jan 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hhugo commented Jan 17, 2023

Uh oh!

hhugo commented Mar 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Alizter commented Jan 5, 2023 •

edited

Loading