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.

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

fix: include expand aliases in digest #8990

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Oct 24, 2023

Action digests should depend on whether the aliases have been expanded in the sandbox.

Technically, this value should only matter if the action is sandboxed and there are alias dependencies, but it's likely not worth the effort to make the digest this precise.

cc @jchavarri who discovered this issue

@rgrinberg rgrinberg requested a review from snowleopard October 24, 2023 02:01
@rgrinberg rgrinberg force-pushed the ps/rr/fix__include_expand_aliases_in_digest branch from 4344443 to 9519b9b Compare October 24, 2023 06:21
@snowleopard
Copy link
Collaborator

snowleopard commented Oct 26, 2023

The change seems OK but I don't understand the problem that is being solved. Is there an GitHub issue with an explanation that you could link from the description?

@rgrinberg rgrinberg force-pushed the ps/rr/fix__include_expand_aliases_in_digest branch from 84a8eb3 to bfd56f3 Compare October 26, 2023 22:06
Signed-off-by: Rudi Grinberg <[email protected]>

<!-- ps-id: ac86e5a8-fd65-4280-85fc-3531704c90f9 -->
@rgrinberg rgrinberg force-pushed the ps/rr/fix__include_expand_aliases_in_digest branch from bfd56f3 to e0d3408 Compare October 26, 2023 22:18
Copy link
Collaborator

@snowleopard snowleopard left a comment

Choose a reason for hiding this comment

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

Looks good!

@rgrinberg rgrinberg merged commit bc98327 into main Oct 30, 2023
@rgrinberg rgrinberg deleted the ps/rr/fix__include_expand_aliases_in_digest branch October 30, 2023 20:22
emillon added a commit to emillon/opam-repository that referenced this pull request Nov 28, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants