dune exec: support pform syntax (rebased) by Leonidas-from-XIV · Pull Request #8474 · ocaml/dune

Leonidas-from-XIV · 2023-08-23T15:53:35Z

Rebased version of #6035 to deal with #6966. Meant to be pushed on top of #6035.

Currently cleaning up to be less of a mess after the rebase.

bin/exec.ml

Leonidas-from-XIV · 2023-08-28T14:46:15Z

@rgrinberg I've added a test that pforms outside of the program name are expanded and built and the expanding works fine, but I am not entirely sure why dune builds called as there is no code to actually call the get_path_and_build_if_necessary on args. It seems like there is some reason called is getting built but I don't know how to avoid this.

rgrinberg · 2023-08-29T12:06:44Z

It seems like there is some reason called is getting built but I don't know how to avoid this.

Why do you want to avoid this? Seems like it's correct to build it.

The reason why it's getting build is that the Expander will add the correct dependencies on every form that it will expand.

bin/target.mli

bin/target.ml

rgrinberg · 2023-08-29T12:54:13Z

I pushed a commit that tries to cleanup some stuff. The previous code made implementing this feature a lot tougher than it needed to be. Also, I'm pretty sure there are some watch mode bugs that were fixed as well. We should be re-expanding the prog/args on every iteration in case changing the rules would modify them.

Leonidas-from-XIV · 2023-08-29T14:59:59Z

Why do you want to avoid this? Seems like it's correct to build it.

Of course it is, but I did not understand that the expander is doing that. I thought I would need to call get_path_and_build_if_necessary to build it so I was surprised the test kept passing anyway despite me not actually triggering builds. As such I can probably just remove the function altogether and just use expand_arg instead.

rgrinberg · 2023-08-29T15:27:04Z

Of course it is, but I did not understand that the expander is doing that

Okay, I see. For reference, we have the Action_builder that is used for collecting dependencies. So anytime you see a function returning an Action_builder.t, it usually contains the dependencies that are attached to the value inside. For example, whenever we return Action.t Action_builder.t, you should read that as "produce an action and return all the dependencies it needs to execute it".

I thought I would need to call get_path_and_build_if_necessary to build it so I was surprised the test kept passing anyway despite me not actually triggering builds. As such I can probably just remove the function altogether and just use expand_arg instead.

I think that function is still needed for the prog because it's the one that has the logic for looking up the path of the binary while respecting the .exe stuff.

Leonidas-from-XIV · 2023-08-31T13:01:51Z

@rgrinberg I've addressed the comments, can you take a look if it is in a shape to merge?

doc/changes/6035.md

This supports things like `dune exec time %{bin:e}`. The syntax is consistent with what support in `dune build` and backwards compatible in cases where no arguments start with `%`. The resolution mechanism is slightly different for the program and the rest of the arguments: - the program is always considered a possible dependency, either in pform syntax (`%{bin:e}` or in string syntax (`./path/to/e`, `_build/default/path/to/e`). - arguments are only interpreted as dependencies if they are in pform syntax. Closes ocaml#2691 Signed-off-by: Etienne Millon <me@emillon.org> Signed-off-by: Marek Kubica <marek@tarides.com>

@Alizter

CHANGES: - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will raise an error. (ocaml/dune#7674, @Alizter) - `enabled_if` now supports `arch_sixtyfour` variable (ocaml/dune#8023, fixes ocaml/dune#7997, @Alizter)- Use `posix_spawn` instead of `fork` on MacOS. This gives us a performance boost and allows us to re-enable thread. (ocaml/dune#8090, @rgrinberg) - Experimental: Added a `$ dune monitor` command that can connect to a running `dune build` in watch mode and display the errors and progress. (ocaml/dune#8152, @Alizter) - No longer emit linkopts(javascript) in META files (ocaml/dune#8168, @hhugo) - The `progress` RPC procedure now has an extra field for the `In_progress` constructor for the number of failed jobs. (ocaml/dune#8212, @Alizter) - Add a `--preview` flag to `dune fmt` which causes it to print out the changes it would make without applying them (ocaml/dune#8289, @gridbugs) - Introduce `(source_trees ..)` to the install stanza to allow installing entire source trees. (ocaml/dune#8349, @rgrinberg) - Deprecate install destination paths beginning with ".." to prevent packages escaping their designated installation directories. (ocaml/dune#8350, @gridbugs) - Stop signing source files with substitutions. Sign only binaries instead (ocaml/dune#8361, fixes ocaml/dune#8360, @anmonteiro) - Add `--stop-on-first-error` option to `dune build` which will terminate the build when the first error is encountered. (ocaml/dune#8400, @pmwhite and @Alizter)- Dune now displays the number of errors when waiting for changes in watch mode. (ocaml/dune#8408, fixes ocaml/dune#6889, @Alizter) - Add `with_prefix` keyword for changing the prefix of the destination of installed files matched by globs. (ocaml/dune#8416, @gridbugs) - Added experimental `--display tui` option for Dune that opens an interactive Terminal User Interface (TUI) when Dune is running. Press '?' to open up a help screen when running for more information. (ocaml/dune#8429, @Alizter and @rgrinberg) - Add a `warnings` field to `dune-project` files as a unified mechanism to enable or disable dune warnings (@rgrinberg, 8448) - `dune exec`: support syntax like `%{bin:program}`. This can appear anywhere in the command line, so things like `dune exec time %{bin:program}` now work. (ocaml/dune#6035, ocaml/dune#8474, fixes ocaml/dune#2691, @emillon, @Leonidas-from-XIV) - RPC message styles are now serialised meaning that RPC diagnostics keep their Ansi styling. (ocaml/dune#8516, fixes ocaml/dune#6921, @Alizter)- Ignore internal promote rules when `--ignore-promoted-rules` is set (ocaml/dune#8518, fix ocaml/dune#8417, @rgrinberg) - Truncate output from actions that produce too much output (@tov, ocaml/dune#8351) - Allow libraries to shadow OCaml builtin libraries. Previously, builtin libraries would always take precedence. (@rgrinberg, ocaml/dune#8558) - Remove warning against `.dune` files generated by pre dune 2.0 (ocaml/dune#8611, @rgrinberg) - `dune utop` no longer links `utop` in "custom" mode, which should make this command considerably faster. (ocaml/dune#8631, fixes ocaml/dune#6894, @nojb) - Ensure that package names in `dune-project` are valid opam package names. (ocaml/dune#8331, @emillon) - dune init: parse `--public` as a public name (ocaml/dune#8603, fixes ocaml/dune#7108, @emillon) - Remove versions 0.1 and 0.2 of the experimental ctypes extension. (ocaml/dune#8293, @emillon)

@Alizter

CHANGES: - `enabled_if` now supports `arch_sixtyfour` variable (ocaml/dune#8023, fixes ocaml/dune#7997, @Alizter) - Use `posix_spawn` instead of `fork` on MacOS. This gives us a performance boost and allows us to re-enable thread. (ocaml/dune#8090, @rgrinberg) - Experimental: Added a `$ dune monitor` command that can connect to a running `dune build` in watch mode and display the errors and progress. (ocaml/dune#8152, @Alizter) - The `progress` RPC procedure now has an extra field for the `In_progress` constructor for the number of failed jobs. (ocaml/dune#8212, @Alizter) - Add a `--preview` flag to `dune fmt` which causes it to print out the changes it would make without applying them (ocaml/dune#8289, @gridbugs) - Introduce `(source_trees ..)` to the install stanza to allow installing entire source trees. (ocaml/dune#8349, @rgrinberg) - Add `--stop-on-first-error` option to `dune build` which will terminate the build when the first error is encountered. (ocaml/dune#8400, @pmwhite and @Alizter) - Dune now displays the number of errors when waiting for changes in watch mode. (ocaml/dune#8408, fixes ocaml/dune#6889, @Alizter) - Add `with_prefix` keyword for changing the prefix of the destination of installed files matched by globs. (ocaml/dune#8416, @gridbugs) - Added experimental `--display tui` option for Dune that opens an interactive Terminal User Interface (TUI) when Dune is running. Press '?' to open up a help screen when running for more information. (ocaml/dune#8429, @Alizter and @rgrinberg) - Add a `warnings` field to `dune-project` files as a unified mechanism to enable or disable dune warnings (@rgrinberg, 8448) - `dune exec`: support syntax like `%{bin:program}`. This can appear anywhere in the command line, so things like `dune exec time %{bin:program}` now work. (ocaml/dune#6035, ocaml/dune#8474, fixes ocaml/dune#2691, @emillon, @Leonidas-from-XIV) - Make copy sandbox support directory targets. (ocaml/dune#8705, fixes ocaml/dune#7724, @emillon) - Add a new alias `@doc-json` to build odoc documentation in JSON format. This output can be consumed by external tools. (ocaml/dune#8178, @emillon) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will raise an error. (ocaml/dune#7674, @Alizter) - No longer emit linkopts(javascript) in META files (ocaml/dune#8168, @hhugo) - Deprecate install destination paths beginning with ".." to prevent packages escaping their designated installation directories. (ocaml/dune#8350, @gridbugs) - RPC message styles are now serialised meaning that RPC diagnostics keep their Ansi styling. (ocaml/dune#8516, fixes ocaml/dune#6921, @Alizter) - Truncate output from actions that produce too much output (@tov, ocaml/dune#8351) - Allow libraries to shadow OCaml builtin libraries. Previously, builtin libraries would always take precedence. (@rgrinberg, ocaml/dune#8558) - Remove warning against `.dune` files generated by pre dune 2.0 (ocaml/dune#8611, @rgrinberg) - `dune utop` no longer links `utop` in "custom" mode, which should make this command considerably faster. (ocaml/dune#8631, fixes ocaml/dune#6894, @nojb) - Ensure that package names in `dune-project` are valid opam package names. (ocaml/dune#8331, @emillon) - init: check that module names are valid (ocaml/dune#8644, fixes ocaml/dune#8252, @emillon) - dune init: parse `--public` as a public name (ocaml/dune#8603, fixes ocaml/dune#7108, @emillon) - Stop signing source files with substitutions. Sign only binaries instead (ocaml/dune#8361, fixes ocaml/dune#8360, @anmonteiro) - Remove versions 0.1 and 0.2 of the experimental ctypes extension. (ocaml/dune#8293, @emillon)

@Alizter

CHANGES: - `enabled_if` now supports `arch_sixtyfour` variable (ocaml/dune#8023, fixes ocaml/dune#7997, @Alizter) - Use `posix_spawn` instead of `fork` on MacOS. This gives us a performance boost and allows us to re-enable thread. (ocaml/dune#8090, @rgrinberg) - Experimental: Added a `$ dune monitor` command that can connect to a running `dune build` in watch mode and display the errors and progress. (ocaml/dune#8152, @Alizter) - The `progress` RPC procedure now has an extra field for the `In_progress` constructor for the number of failed jobs. (ocaml/dune#8212, @Alizter) - Add a `--preview` flag to `dune fmt` which causes it to print out the changes it would make without applying them (ocaml/dune#8289, @gridbugs) - Introduce `(source_trees ..)` to the install stanza to allow installing entire source trees. (ocaml/dune#8349, @rgrinberg) - Add `--stop-on-first-error` option to `dune build` which will terminate the build when the first error is encountered. (ocaml/dune#8400, @pmwhite and @Alizter) - Dune now displays the number of errors when waiting for changes in watch mode. (ocaml/dune#8408, fixes ocaml/dune#6889, @Alizter) - Add `with_prefix` keyword for changing the prefix of the destination of installed files matched by globs. (ocaml/dune#8416, @gridbugs) - Added experimental `--display tui` option for Dune that opens an interactive Terminal User Interface (TUI) when Dune is running. Press '?' to open up a help screen when running for more information. (ocaml/dune#8429, @Alizter and @rgrinberg) - Add a `warnings` field to `dune-project` files as a unified mechanism to enable or disable dune warnings (@rgrinberg, 8448) - `dune exec`: support syntax like `%{bin:program}`. This can appear anywhere in the command line, so things like `dune exec time %{bin:program}` now work. (ocaml/dune#6035, ocaml/dune#8474, fixes ocaml/dune#2691, @emillon, @Leonidas-from-XIV) - Make copy sandbox support directory targets. (ocaml/dune#8705, fixes ocaml/dune#7724, @emillon) - Add a new alias `@doc-json` to build odoc documentation in JSON format. This output can be consumed by external tools. (ocaml/dune#8178, @emillon) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will raise an error. (ocaml/dune#7674, @Alizter) - No longer emit linkopts(javascript) in META files (ocaml/dune#8168, @hhugo) - Deprecate install destination paths beginning with ".." to prevent packages escaping their designated installation directories. (ocaml/dune#8350, @gridbugs) - RPC message styles are now serialised meaning that RPC diagnostics keep their Ansi styling. (ocaml/dune#8516, fixes ocaml/dune#6921, @Alizter) - Truncate output from actions that produce too much output (@tov, ocaml/dune#8351) - Allow libraries to shadow OCaml builtin libraries. Previously, builtin libraries would always take precedence. (@rgrinberg, ocaml/dune#8558) - Remove warning against `.dune` files generated by pre dune 2.0 (ocaml/dune#8611, @rgrinberg) - `dune utop` no longer links `utop` in "custom" mode, which should make this command considerably faster. (ocaml/dune#8631, fixes ocaml/dune#6894, @nojb) - Ensure that package names in `dune-project` are valid opam package names. (ocaml/dune#8331, @emillon) - init: check that module names are valid (ocaml/dune#8644, fixes ocaml/dune#8252, @emillon) - dune init: parse `--public` as a public name (ocaml/dune#8603, fixes ocaml/dune#7108, @emillon) - Stop signing source files with substitutions. Sign only binaries instead (ocaml/dune#8361, fixes ocaml/dune#8360, @anmonteiro) - Remove versions 0.1 and 0.2 of the experimental ctypes extension. (ocaml/dune#8293, @emillon)

rgrinberg reviewed Aug 23, 2023

View reviewed changes

bin/exec.ml Outdated Show resolved Hide resolved

Leonidas-from-XIV force-pushed the exec-bin branch from 249640a to e4962e2 Compare August 24, 2023 11:07

Leonidas-from-XIV marked this pull request as ready for review August 28, 2023 14:42

rgrinberg reviewed Aug 29, 2023

View reviewed changes

bin/target.mli Outdated Show resolved Hide resolved

rgrinberg reviewed Aug 29, 2023

View reviewed changes

bin/target.ml Outdated Show resolved Hide resolved

Leonidas-from-XIV force-pushed the exec-bin branch 2 times, most recently from e1dabca to cc44fd6 Compare August 31, 2023 12:48

Leonidas-from-XIV requested a review from rgrinberg August 31, 2023 13:01

rgrinberg approved these changes Aug 31, 2023

View reviewed changes

rgrinberg added this to the 3.11.0 milestone Aug 31, 2023

rgrinberg reviewed Aug 31, 2023

View reviewed changes

doc/changes/6035.md Outdated Show resolved Hide resolved

rgrinberg mentioned this pull request Aug 31, 2023

dune exec: support pform syntax #6035

Closed

Leonidas-from-XIV force-pushed the exec-bin branch from dbe23dd to 8bd2622 Compare September 1, 2023 08:30

Leonidas-from-XIV merged commit 5c00335 into ocaml:main Sep 1, 2023

Leonidas-from-XIV deleted the exec-bin branch September 1, 2023 09:10

emillon mentioned this pull request Sep 14, 2023

[new release] dune (15 packages) (3.11.0~alpha1) ocaml/opam-repository#24435

Closed

emillon mentioned this pull request Sep 22, 2023

[new release] dune (15 packages) (3.11.0) ocaml/opam-repository#24510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dune exec: support pform syntax (rebased)#8474

dune exec: support pform syntax (rebased)#8474
Leonidas-from-XIV merged 1 commit intoocaml:mainfrom
Leonidas-from-XIV:exec-bin

Leonidas-from-XIV commented Aug 23, 2023 •

edited

Loading

Uh oh!

Uh oh!

Leonidas-from-XIV commented Aug 28, 2023

Uh oh!

rgrinberg commented Aug 29, 2023

Uh oh!

Uh oh!

Uh oh!

rgrinberg commented Aug 29, 2023

Uh oh!

Leonidas-from-XIV commented Aug 29, 2023

Uh oh!

rgrinberg commented Aug 29, 2023

Uh oh!

Leonidas-from-XIV commented Aug 31, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Leonidas-from-XIV commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Leonidas-from-XIV commented Aug 28, 2023

Uh oh!

rgrinberg commented Aug 29, 2023

Uh oh!

Uh oh!

Uh oh!

rgrinberg commented Aug 29, 2023

Uh oh!

Leonidas-from-XIV commented Aug 29, 2023

Uh oh!

rgrinberg commented Aug 29, 2023

Uh oh!

Leonidas-from-XIV commented Aug 31, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Leonidas-from-XIV commented Aug 23, 2023 •

edited

Loading