Skip to content

add sandboxing to ocamldep, lint, dialect, pp#5807

Merged
rgrinberg merged 1 commit intomainfrom
ps/rr/refactor__add_sandboxing_option_to_ocamldep
Jun 15, 2022
Merged

add sandboxing to ocamldep, lint, dialect, pp#5807
rgrinberg merged 1 commit intomainfrom
ps/rr/refactor__add_sandboxing_option_to_ocamldep

Conversation

@rgrinberg
Copy link
Copy Markdown
Member

No description provided.

@rgrinberg rgrinberg requested review from emillon and nojb June 1, 2022 18:28
@rgrinberg rgrinberg force-pushed the ps/rr/refactor__add_sandboxing_option_to_ocamldep branch 2 times, most recently from b2abf7a to c913c93 Compare June 1, 2022 18:53
@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Jun 13, 2022

I did some benchmarking on js-monorepo and timed dune build @check:

PR:

  • Time (mean ± σ): 147.438 s ± 7.015 s [User: 788.445 s, System: 121.689 s]

3.2.0:

  • Time (mean ± σ): 158.137 s ± 10.761 s [User: 827.817 s, System: 126.613 s]

I can try to have more precise figures (those are not based on many runs, with parallelism) but yes it looks like it's noticeable.

@rgrinberg
Copy link
Copy Markdown
Member Author

Thanks Etienne. I'll drop the sandboxing on ocamldep as it's a fairly safe command.

I'd like to keep sandboxing on preprocessing even if it there is a small penalty. It's an issue of correctness for user rules so it's worth making sure these are actually solid.

@rgrinberg rgrinberg force-pushed the ps/rr/refactor__add_sandboxing_option_to_ocamldep branch from c913c93 to aa222ce Compare June 13, 2022 13:41
@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Jun 13, 2022

Am I reading the results correctly? It looks like it was faster with this PR?

@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Jun 13, 2022

I tried to retime this but TBH the numbers are all over the place with my measuring setup so I don't think we rely on them too much.

@rgrinberg
Copy link
Copy Markdown
Member Author

Let me do some tests as well then.

@nojb
Copy link
Copy Markdown
Collaborator

nojb commented Jun 13, 2022

Just now I did some informal tests under Windows doing a build from scratch of the LexiFi codebase, but the difference seems to be noise:

This PR: 4m39 4m29 4m30
3.1.1: 4m35 4m34 4m25

@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Jun 14, 2022

Would it perhaps be more noticeable if we restrict to a single job? -j1?

@rgrinberg rgrinberg force-pushed the ps/rr/refactor__add_sandboxing_option_to_ocamldep branch from aa222ce to 47e0687 Compare June 14, 2022 17:45
@rgrinberg
Copy link
Copy Markdown
Member Author

Okay, I did some benchmarks and there's essentially no overhead.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

ps-id: 6E2A6828-EB8A-4727-B2BA-344D665D340C
@rgrinberg rgrinberg force-pushed the ps/rr/refactor__add_sandboxing_option_to_ocamldep branch from 47e0687 to 98ce9cd Compare June 15, 2022 15:58
@rgrinberg rgrinberg merged commit 98ce9cd into main Jun 15, 2022
@rgrinberg rgrinberg deleted the ps/rr/refactor__add_sandboxing_option_to_ocamldep branch June 15, 2022 15:58
kit-ty-kate pushed a commit to ocaml/opam-repository that referenced this pull request Jun 17, 2022
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.3.0)

CHANGES:

- Sandbox preprocessing, lint, and dialect rules by default. All these rules
  now require precise dependency specifications (ocaml/dune#5807, @rgrinberg)

- Allow list expansion in the `pps` specification for preprocessing (ocaml/dune#5820,
  @Firobe)

- Add warnings 67-69 to dune's default set of warnings. These are warnings of
  the form "unused X.." (ocaml/dune#5844, @rgrinbreg)

- Introduce project "composition" for coq theories. Coq theories in separate
  projects can now refer to each other when in the same workspace (ocaml/dune#5784,
  @Alitzer, @rgrinberg)

- Fix hint message for ``data_only_dirs`` that wrongly mentions the unknown
  constructor ``data_only`` (ocaml/dune#5803, @lambdaxdotx)

- Fix creating sandbox directory trees by getting rid of buggy memoization
  (@5794, @rgrinberg, @snowleopard)

- Handle directory dependencies in sandboxed rules. Previously, the parents of
  these directory dependencies weren't created. (ocaml/dune#5754, @rgrinberg)

- Set the exit code to 130 when dune is terminated with a signal (ocaml/dune#5769, fixes
  ocaml/dune#5757)

- Support new locations of unix, str, dynlink in OCaml >= 5.0 (ocaml/dune#5582, @dra27)

- The ``coq.theory`` stanza now produces rules for running ``coqdoc``. Given a
  theory named ``mytheory``, the directory targets ``mytheory.html/`` and
  ``mytheory.tex/`` or additionally the aliases `@doc` and `@doc-latex` will
  build the HTML and LaTeX documentation repsectively. (ocaml/dune#5695, fixes ocaml/dune#3760,
  @Alizter)

- Coq theories marked as `(boot)` cannot depend on other theories
  (ocaml/dune#5867, @ejgallego)

- Ignore `bigarray` in `(libraries)` with OCaml >= 5.0. (ocaml/dune#5526, fixes ocaml/dune#5494,
  @moyodiallo)

- Start with :standard when building the ctypes generated foreign stubs so that
  we include important compiler flags, such as -fPIC (ocaml/dune#5816, fixes ocaml/dune#5809).
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.

4 participants