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

Add hints when dune exec x.exe instead of ./x.exe #3041

Merged
11 commits merged into from
Jan 20, 2020
Merged

Add hints when dune exec x.exe instead of ./x.exe #3041

11 commits merged into from
Jan 20, 2020

Conversation

voodoos
Copy link
Collaborator

@voodoos voodoos commented Jan 16, 2020

This fixes #1094, adding hints for users mistyping dune exec x.exe for dune exec ./x.exe when x is an executable defined at the root of the project.

Only meaningful changes are in bin/exec.ml

I am still new to the codebase and will be happy to enhance this PR is someone points me to a more idiomatic way to address the problem in Dune.

@voodoos voodoos requested a review from a user January 16, 2020 10:30
@voodoos voodoos force-pushed the feature/dune-exec-root-user-hint branch from 2ae2a5d to 5572dd6 Compare January 16, 2020 10:31
ghost
ghost suggested changes Jan 16, 2020
View reviewed changes
bin/exec.ml Outdated Show resolved Hide resolved
bin/exec.ml Outdated Show resolved Hide resolved
@aalekseyev
Copy link
Collaborator

Shouldn't we just support dune exec x.exe instead, to mean exactly the same as dune exec ./x.exe?

@ghost
Copy link

ghost commented Jan 16, 2020

We could but it's not really in line with the shell behaviour, or the way we interpret <prog> in (run <prog> ...).

@aalekseyev
Copy link
Collaborator

OK, that's convincing enough. I think a warning is a really big improvement, by the way!

@voodoos voodoos requested a review from a user January 16, 2020 16:10
@voodoos voodoos force-pushed the feature/dune-exec-root-user-hint branch 2 times, most recently from a59c624 to 38a8ee9 Compare January 20, 2020 10:43
ghost
ghost suggested changes Jan 20, 2020
View reviewed changes
bin/exec.ml Outdated Show resolved Hide resolved
bin/exec.ml Outdated Show resolved Hide resolved
@voodoos voodoos force-pushed the feature/dune-exec-root-user-hint branch from 4d09d79 to 9e9ac38 Compare January 20, 2020 14:15
voodoos and others added 11 commits January 20, 2020 15:17
@voodoos
Correct typo in comment
566ec25 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add hint for 'x.exe' instead of './x.exe'
373d758 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add corresponding test case
147c08f 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Reword comment
7e96a36 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add support for invocation from subfolders
5684152 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add corresponding tests
b21419f 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Remove one useledd 'map'
3c1e5b0 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add newlines at the end of test files
8f58e8e 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Update CHANGES.md
e0c1bad 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos @jeremiedimino
Correct comment
b487699 
Co-Authored-By: Jérémie Dimino <[email protected]>
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Un-open the List module
6a9f70b 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos voodoos force-pushed the feature/dune-exec-root-user-hint branch from 9e9ac38 to 6a9f70b Compare January 20, 2020 14:17
ghost
ghost approved these changes Jan 20, 2020
View reviewed changes
Copy link

@ghost ghost 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!

@ghost ghost merged commit ed3f540 into ocaml:master Jan 20, 2020
@voodoos voodoos mentioned this pull request Jan 20, 2020
Merged
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Feb 6, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
5d030d1 
…lugin, dune-private-libs and dune-glob (2.2.0)

CHANGES:

- `dune test` is now a command alias for `dune runtest`. This is to make the CLI
  less idiosyncratic (ocaml/dune#3006, @shonfeder)

- Allow to set menhir flags in the `env` stanza using the `menhir_flags` field.
  (ocaml/dune#2960, fix ocaml/dune#2924, @bschommer)

- By default, do not show the full command line of commands executed
  by `dune` when `dune` is executed inside `dune`. This is to make
  integration tests more reproducible (ocaml/dune#3042, @diml)

- `dune subst` now works even without opam files (ocaml/dune#2955, fixes ocaml/dune#2910,
  @fangyi-zhou and @diml)

- Hint when trying to execute an executable defined in the current directory
  without using the `./` prefix (ocaml/dune#3041, fixes ocaml/dune#1094, @voodoos).

- Extend the list of modifiers that can be nested under
  `with-accepted-exit-codes` with `chdir`,  `setenv`, `ignore-<outputs>`,
  `with-stdin-from` and `with-<outputs>-to` (ocaml/dune#3027, fixes ocaml/dune#3014, @voodoos)

- It is now an error to have a preprocessing dependency on a ppx rewriter
  library that is not marked as `(kind ppx_rewriter)` (ocaml/dune#3039, @snowleopard).

- Fix permissions of files promoted to the source tree when using the
  shared cache. In particular, make them writable by the user (ocaml/dune#3043,
  fixes ocaml/dune#3026, @diml)

- Only detect internal OCaml tools with `.opt` extensions. Previously, this
  detection applied to other binaries as well (@kit-ty-kate, @rgrinberg, ocaml/dune#3051).

- Give the user a proper error message when they try to promote into a source
  directory that doesn't exist. (ocaml/dune#3073, fix ocaml/dune#3069, @rgrinberg)

- Correctly build vendored packages in `-p` mode. These packages were
  incorrectly filtered out before. (ocaml/dune#3705, @diml)

- Do not install vendored packages (ocaml/dune#3704, @diml)

- `make` now prints a message explaining the main targets available
  (ocaml/dune#3085, fix ocaml/dune#3078, @diml)

- Add a `byte_complete` executable mode to build programs as
  self-contained bytecode programs
  (ocaml/dune#3076, fixes ocaml/dune#1519, @diml)
@rgrinberg rgrinberg mentioned this pull request Feb 6, 2020
Merged
mgree pushed a commit to mgree/opam-repository that referenced this pull request Feb 19, 2020
@rgrinberg @mgree
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
8476e6f 
…lugin, dune-private-libs and dune-glob (2.2.0)

CHANGES:

- `dune test` is now a command alias for `dune runtest`. This is to make the CLI
  less idiosyncratic (ocaml/dune#3006, @shonfeder)

- Allow to set menhir flags in the `env` stanza using the `menhir_flags` field.
  (ocaml/dune#2960, fix ocaml/dune#2924, @bschommer)

- By default, do not show the full command line of commands executed
  by `dune` when `dune` is executed inside `dune`. This is to make
  integration tests more reproducible (ocaml/dune#3042, @diml)

- `dune subst` now works even without opam files (ocaml/dune#2955, fixes ocaml/dune#2910,
  @fangyi-zhou and @diml)

- Hint when trying to execute an executable defined in the current directory
  without using the `./` prefix (ocaml/dune#3041, fixes ocaml/dune#1094, @voodoos).

- Extend the list of modifiers that can be nested under
  `with-accepted-exit-codes` with `chdir`,  `setenv`, `ignore-<outputs>`,
  `with-stdin-from` and `with-<outputs>-to` (ocaml/dune#3027, fixes ocaml/dune#3014, @voodoos)

- It is now an error to have a preprocessing dependency on a ppx rewriter
  library that is not marked as `(kind ppx_rewriter)` (ocaml/dune#3039, @snowleopard).

- Fix permissions of files promoted to the source tree when using the
  shared cache. In particular, make them writable by the user (ocaml/dune#3043,
  fixes ocaml/dune#3026, @diml)

- Only detect internal OCaml tools with `.opt` extensions. Previously, this
  detection applied to other binaries as well (@kit-ty-kate, @rgrinberg, ocaml/dune#3051).

- Give the user a proper error message when they try to promote into a source
  directory that doesn't exist. (ocaml/dune#3073, fix ocaml/dune#3069, @rgrinberg)

- Correctly build vendored packages in `-p` mode. These packages were
  incorrectly filtered out before. (ocaml/dune#3705, @diml)

- Do not install vendored packages (ocaml/dune#3704, @diml)

- `make` now prints a message explaining the main targets available
  (ocaml/dune#3085, fix ocaml/dune#3078, @diml)

- Add a `byte_complete` executable mode to build programs as
  self-contained bytecode programs
  (ocaml/dune#3076, fixes ocaml/dune#1519, @diml)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgrinberg rgrinberg Awaiting requested review from rgrinberg

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

Successfully merging this pull request may close these issues.

Change in dune exec incantation
2 participants
@voodoos @aalekseyev