Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: allow internal dune actions to be sandboxed #9041

Merged
merged 1 commit into from
Oct 31, 2023
Merged

fix: allow internal dune actions to be sandboxed #9041

merged 1 commit into from
Oct 31, 2023

Conversation

rgrinberg
Copy link
Member

Internal actions that that don't need to be sandboxed would prevent the
entire rule from being sandboxed. Now we just sandbox them even if it's
not necessary.

Signed-off-by: Rudi Grinberg [email protected]

@rgrinberg rgrinberg requested a review from snowleopard October 30, 2023 00:36
@rgrinberg rgrinberg force-pushed the ps/rr/fix__allow_internal_dune_actions_to_be_sandboxed branch 3 times, most recently from 70f4a46 to 8e6c4f4 Compare October 30, 2023 20:31
@rgrinberg rgrinberg linked an issue Oct 30, 2023 that may be closed by this pull request
rfc(engine): sandbox only when needed rather than throwing an error #8854
Closed
@rgrinberg rgrinberg added this to the 3.12.0 milestone Oct 31, 2023
@rgrinberg rgrinberg force-pushed the ps/rr/fix__allow_internal_dune_actions_to_be_sandboxed branch from 8e6c4f4 to 1eecf2f Compare October 31, 2023 05:19
snowleopard
snowleopard approved these changes Oct 31, 2023
View reviewed changes
Copy link
Collaborator

@snowleopard snowleopard left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, the changes look good to me but let's also delete Action.is_useful_to_sandbox.

@rgrinberg
fix: allow internal dune actions to be sandboxed
a1272dd 
Internal actions that that don't need to be sandboxed would prevent the
entire rule from being sandboxed. Now we just sandbox them even if it's
not necessary.

Signed-off-by: Rudi Grinberg <[email protected]>

<!-- ps-id: efdac4e7-d2b5-449e-9aad-468b2662128e -->
@rgrinberg rgrinberg force-pushed the ps/rr/fix__allow_internal_dune_actions_to_be_sandboxed branch from 1eecf2f to a1272dd Compare October 31, 2023 19:58
@rgrinberg rgrinberg merged commit 93a7699 into main Oct 31, 2023
@rgrinberg rgrinberg deleted the ps/rr/fix__allow_internal_dune_actions_to_be_sandboxed branch October 31, 2023 20:14
@emillon emillon mentioned this pull request Nov 28, 2023
Closed
emillon added a commit to emillon/opam-repository that referenced this pull request Nov 28, 2023
@emillon
[new release] dune (15 packages) (3.12.0)
d595c99 
CHANGES:

- Introduce `$ dune ocaml doc` to open and browse documentation. (ocaml/dune#7262, fixes
  ocaml/dune#6831, @EmileTrotignon)

- `dune cache trim` now accepts binary byte units: `KiB`, `MiB`, etc. (ocaml/dune#8618,
  @Alizter)

- No longer force colors for OCaml 4.03 and 4.04 (ocaml/dune#8778, @rgrinberg)

- Introduce new experimental odoc rules (ocaml/dune#8803, @jonjudlam)

- Introduce the `runtest_alias` field to the `cram` stanza. This allows
  removing default `runtest` alias from tests. (@rgrinberg, ocaml/dune#8887)

- Do not ignore libraries named `bigarray` when they are defined in conjunction
  with OCaml 5.0 (ocaml/dune#8902, fixes ocaml/dune#8901, @rgrinberg)

- Dependencies in the copying sandbox are now writeable (ocaml/dune#8920, @rgrinberg)

- Absent packages shouldn't prevent all rules from being loaded (ocaml/dune#8948, fixes
  ocaml/dune#8630, @rgrinberg)

- Correctly determine the stanza of menhir modules when `(include_subdirs
  qualified)` is enabled (@rgrinberg, ocaml/dune#8949, fixes ocaml/dune#7610)

- Display cache location in Dune log (ocaml/dune#8974, @nojb)

- Re-run actions whenever `(expand_aliases_in_sandbox)` changes (ocaml/dune#8990,
  @rgrinberg)

- Rules that only use internal dune actions (`write-file`, `echo`, etc.) can
  now be sandboxed. (ocaml/dune#9041, fixes ocaml/dune#8854, @rgrinberg)

- Do not re-run rules when their location changes (ocaml/dune#9052, @rgrinberg)

- Correctly ignore `bigarray` on recent version of OCaml (ocaml/dune#9076, @rgrinberg)

- Add `test_` prefix to default test name in `dune init project` (ocaml/dune#9257, fixes
  ocaml/dune#9131, @9sako6)

- Add `coqdoc_flags` field to `coq` field of `env` stanza allowing the setting
  of workspace-wide defaults for `coqdoc_flags`. (ocaml/dune#9280, fixes ocaml/dune#9139, @Alizter)

- [coq rules] Be more tolerant when coqc --print-version / --config don't work
  properly, and fallback to a reasonable default. This fixes problems when
  building Coq projects with `(stdlib no)` and likely other cases. (ocaml/dune#8966, fix
  ocaml/dune#8958, @Alizter, reported by Lasse Blaauwbroek)

- Dune will now run at a lower framerate of 15 fps rather than 60 when
  `INSIDE_EMACS`. (ocaml/dune#8812, @Alizter)

- dune-build-info: when `version=""` is found in a `META` file, we now return
  `None` as a version string (ocaml/dune#9177, @emillon)

- Dune can now be built and installed on Haiku (ocaml/dune#8795, fix ocaml/dune#8551, @Alizter)

- Mark installed directories in `dune-package` files. This fixes `(package)`
  dependencies against packages that contain such directories. (ocaml/dune#8953, fixes
  ocaml/dune#8915, @emillon)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snowleopard snowleopard snowleopard approved these changes

Assignees
No one assigned
Labels
upstream-sync
Projects
None yet
Milestone
3.12.0
Development

Successfully merging this pull request may close these issues.

rfc(engine): sandbox only when needed rather than throwing an error
2 participants
@rgrinberg @snowleopard