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: module compilation rule env #6527

Merged
merged 1 commit into from Nov 22, 2022
Merged

fix: module compilation rule env #6527

merged 1 commit into from Nov 22, 2022

Conversation

rgrinberg
Copy link
Member

ths environment should be obtained from the directory of the module and
not the root.

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

ps-id: b1a39f83-6e31-4229-af7c-b428df4f5aa6

@rgrinberg rgrinberg added this to the 3.7.0 milestone Nov 20, 2022
@rgrinberg rgrinberg force-pushed the ps/rr/fix__module_compilation_rule_env branch from 0809c3d to 7609370 Compare November 20, 2022 02:56
@anmonteiro
Copy link
Collaborator

What are the implications of this change? Asking because it seems significant enough that you gated it to dune version >= 3.7

@rgrinberg
Copy link
Member Author

The main significance is that setting the environment and overriding binaries in (env ..) will now propagate to compilation actions. That's how we handle all actions, so I don't see why we should treat compilation differently.

As for why guarding it behind 3.7, I try to minimized unversioned changes to the rules.

nojb
nojb reviewed Nov 22, 2022
View reviewed changes
Copy link
Collaborator

@nojb nojb left a comment

Choose a reason for hiding this comment

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

Sorry, but the diff doesn't tell me much. Could you say a few words of what changes concretely with this change?

@nojb
Copy link
Collaborator

nojb commented Nov 22, 2022

Sorry, but the diff doesn't tell me much. Could you say a few words of what changes concretely with this change?

OK, I saw your explanation above, thanks.

nojb
nojb approved these changes Nov 22, 2022
View reviewed changes
Copy link
Collaborator

@nojb nojb left a comment

Choose a reason for hiding this comment

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

LGTM

@rgrinberg
fix: module compilation rule env
4fbc0a0 
ths environment should be obtained from the directory of the module and
not the root.

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

ps-id: b1a39f83-6e31-4229-af7c-b428df4f5aa6
@rgrinberg rgrinberg force-pushed the ps/rr/fix__module_compilation_rule_env branch from 7609370 to 4fbc0a0 Compare November 22, 2022 23:26
@rgrinberg rgrinberg merged commit e5d1604 into main Nov 22, 2022
jchavarri added a commit to jchavarri/dune that referenced this pull request Nov 24, 2022
@jchavarri
Merge branch 'main' into melange/error-out-melc-missing
1059861 
* main: (58 commits)
  test: formatting of alternative dune files (ocaml#6567)
  refactor: remove Modules.is_empty (ocaml#6564)
  refactor: module kinds (ocaml#6562)
  refactor(coq): resolve lack of coqc properly
  Cache file contents in action builder by name. (ocaml#6555)
  fix: re-enable dune on older macos sdk's (ocaml#6515)
  fix: do not hide lib interface module (ocaml#6549)
  test: remove pkg-config output for reproducibility (ocaml#6543)
  melange: add test for ocaml flags (ocaml#6548)
  fix: improve virtual library error messages
  test: virtual library and impl locations
  test: alias module regression (ocaml#6544)
  refactor(merlin): dump config sub command (ocaml#6547)
  refactor: simplify merlin (ocaml#6508)
  chore(nix): use nix-overlays for the slim devShell (ocaml#6546)
  fix: module compilation rule env (ocaml#6527)
  chore: update nix (ocaml#6536)
  fix: merlin rules with pp's (ocaml#6474)
  Call [Dune_util.Log.init] as soon as possible (ocaml#6542)
  refactor: speed up stdlib build (ocaml#6524)
  ...
@Alizter Alizter deleted the ps/rr/fix__module_compilation_rule_env branch November 26, 2022 14:46
emillon added a commit to emillon/opam-repository that referenced this pull request Feb 6, 2023
@emillon
[new release] xdg, stdune, ordering, ocamlc-loc, dyn, dune, dune-site…
4eab166 
…, 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)
@emillon emillon mentioned this pull request Feb 6, 2023
Closed
@emillon emillon mentioned this pull request Feb 17, 2023
Merged
emillon added a commit to emillon/opam-repository that referenced this pull request Feb 17, 2023
@emillon
[new release] xdg, stdune, ordering, ocamlc-loc, dyn, dune, dune-site…
c5fc79e 
…, 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nojb nojb nojb approved these changes

@emillon emillon Awaiting requested review from emillon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.7.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rgrinberg @anmonteiro @nojb