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

dune build --always-show-command-line and --verbose has wrong output #5546

Closed
Alizter opened this issue Apr 5, 2022 · 1 comment · Fixed by ocaml/opam-repository#21217
Closed

Comments

@Alizter
Copy link
Collaborator

Alizter commented Apr 5, 2022

Here are a few issues regarding the output of --always-show-command-line and --verbose.

Expected Behavior

Here is an example rule from a not too important project:

File "test-suite/test_suite_rules.sexp", line 6883, characters 1-403:
6883 |  (rule
6884 |   (alias runtest)
6885 |   (targets rexample.vo rexample.v.log)
6886 |   (deps .csdp.cache ../micromega/rexample.v .././../theories/micromega/Lra.vo
6887 |         .././../theories/Reals/Reals.vo)
6888 |   (action (with-outputs-to rexample.v.log (with-accepted-exit-codes 0 (run coqc -boot -I ../../../install/default/lib -R ../../theories Coq -R ../prerequisite TestSuite -Q ../../user-contrib/Ltac2 Ltac2   rexample.v)))))
(cd _build/default/test-suite/micromega && ../../../install/default/bin/coqc -boot -I ../../../install/default/lib -R ../../theories Coq -R ../prerequisite TestSuite -Q ../../user-contrib/Ltac2 Ltac2 rexample.v &> _build/default/test-suite/micromega/rexample.v.log
Command exited with code 1.                                     

This was run with dune test --always-show-command-line. The output of the actual command however is a little wrong.

  • It seems that there is a missing parenthesis before &> in the output.
  • There is no leading $ for the subshell.

Using --verbose gives a slightly different output:

$ (cd _build/default/test-suite/micromega && ../../../install/default/bin/coqc -boot -I ../../../install/default/lib -R ../../theories Coq -R ../prerequisite TestSuite -Q ../../user-contrib/Ltac2 Ltac2 rexample.v &> _build/default/test-suite/micromega/rexample.v.log

Here the parenthesis problem is the same, however the $ has been inserted. However there is a space after the $ which should not be there.

Reproduction

I don't have a reproduction case, but his appears to be an issue with the printing of the shell command for with-outputs-to. The $ issues for --verbose and --always-show-command-line seem to be unrelated but nonetheless wrong on all examples I have tried.

Specifications

  • Version of dune (output of dune --version): 3.0.3
  • Version of ocaml (output of ocamlc --version) 4.12.1
  • Operating system (distribution and version): Probably not relevant
@Alizter
Copy link
Collaborator Author

Alizter commented Apr 6, 2022

The $ for subshell seems to be mistaken, however the missing parenthesises is real and I have submitted a patch.

rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Apr 15, 2022
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info and dune-action-plugin (3.1.0)

CHANGES:

- Add `sourcehut` as an option for defining project sources in dune-project
  files. For example, `(source (sourcehut user/repo))`. (ocaml/dune#5564, @rgrinberg)

- Add `dune coq top` command for running a Coq toplevel (ocaml/dune#5457, @rlepigre)

- Fix dune exec dumping database in wrong directory (ocaml/dune#5544, @bobot)

- Always output absolute paths for locations in RPC reported diagnostics
  (ocaml/dune#5539, @rgrinberg)

- Add `(deps <deps>)` in ctype field (ocaml/dune#5346, @bobot)

- Add `(include <file>)` constructor to dependency specifications. This can be
  used to introduce dynamic dependencies (ocaml/dune#5442, @anmonteiro)

- Ensure that `dune describe` computes a transitively closed set of
  libraries (ocaml/dune#5395, @esope)

- Add direct dependencies to $ dune describe output (ocaml/dune#5412, @esope)

- Show auto-detected concurrency on Windows too (ocaml/dune#5502, @MisterDA)

- Fix operations that remove folders with absolute path. This happens when
  using esy (ocaml/dune#5507, @EduardoRFS)

- Dune will not fail if some directories are non-empty when uninstalling.
  (ocaml/dune#5543, fixes ocaml/dune#5542, @nojb)

- `coqdep` now depends only on the filesystem layout of the .v files,
  and not on their contents (ocaml/dune#5547, helps with ocaml/dune#5100, @ejgallego)

- The mdx stanza 0.2 can now be used with `(implicit_transitive_deps false)`
  (ocaml/dune#5558, fixes ocaml/dune#5499, @emillon)

- Fix missing parenthesis in printing of corresponding terminal command for
  `(with-outputs-to )` (ocaml/dune#5551, fixes ocaml/dune#5546, @Alizter)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment