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

display: add an option to not print the command even on errors #1733

Closed
tari3x opened this issue Jan 3, 2019 · 19 comments · Fixed by ocaml/opam-repository#14239
Closed

display: add an option to not print the command even on errors #1733

tari3x opened this issue Jan 3, 2019 · 19 comments · Fixed by ocaml/opam-repository#14239

Comments

@tari3x
Copy link

tari3x commented Jan 3, 2019

In the following output I never care about the huge ocamlopt command, I only care about the actual error. Would it be possible to add a flag to hide the command in this case? I am already running with "--display quiet".

(cd _build/default && /home/avatar/.opam/default/bin/ocamlopt.opt -w @a-4-29-40-41-42-44-45-48-58-59-60-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -I lib/.nullstellen_lib.objs -I lib/.nullstellen_lib.objs/.private -I /home/avatar/.opam/default/lib/async -I /home/avatar/.opam/default/lib/async_extra -I /home/avatar/.opam/default/lib/async_kernel -I /home/avatar/.opam/default/lib/async_rpc_kernel -I /home/avatar/.opam/default/lib/async_shell -I /home/avatar/.opam/default/lib/async_unix -I /home/avatar/.opam/default/lib/base -I /home/avatar/.opam/default/lib/base/caml -I /home/avatar/.opam/default/lib/base/md5 -I /home/avatar/.opam/default/lib/base/shadow_stdlib -I /home/avatar/.opam/default/lib/base_quickcheck -I /home/avatar/.opam/default/lib/bin_prot -I /home/avatar/.opam/default/lib/bin_prot/shape -I /home/avatar/.opam/default/lib/camlimages/core -I /home/avatar/.opam/default/lib/camlimages/png -I /home/avatar/.opam/default/lib/core -I /home/avatar/.opam/default/lib/core/uuid -I /home/avatar/.opam/default/lib/core_kernel -I /home/avatar/.opam/default/lib/core_kernel/base_for_tests -I /home/avatar/.opam/default/lib/core_kernel/uuid -I /home/avatar/.opam/default/lib/fieldslib -I /home/avatar/.opam/default/lib/jane-street-headers -I /home/avatar/.opam/default/lib/lacaml -I /home/avatar/.opam/default/lib/ocaml/threads -I /home/avatar/.opam/default/lib/parsexp -I /home/avatar/.opam/default/lib/ppx_assert/runtime-lib -I /home/avatar/.opam/default/lib/ppx_bench/runtime-lib -I /home/avatar/.opam/default/lib/ppx_compare/runtime-lib -I /home/avatar/.opam/default/lib/ppx_enumerate/runtime-lib -I /home/avatar/.opam/default/lib/ppx_expect/collector -I /home/avatar/.opam/default/lib/ppx_expect/common -I /home/avatar/.opam/default/lib/ppx_expect/config -I /home/avatar/.opam/default/lib/ppx_hash/runtime-lib -I /home/avatar/.opam/default/lib/ppx_inline_test/config -I /home/avatar/.opam/default/lib/ppx_inline_test/runtime-lib -I /home/avatar/.opam/default/lib/ppx_sexp_conv/runtime-lib -I /home/avatar/.opam/default/lib/protocol_version_header -I /home/avatar/.opam/default/lib/re2 -I /home/avatar/.opam/default/lib/re2/c -I /home/avatar/.opam/default/lib/sexplib -I /home/avatar/.opam/default/lib/sexplib/unix -I /home/avatar/.opam/default/lib/sexplib0 -I /home/avatar/.opam/default/lib/shell -I /home/avatar/.opam/default/lib/shell/filename_extended -I /home/avatar/.opam/default/lib/shell/low_level_process -I /home/avatar/.opam/default/lib/shell/shell_internal -I /home/avatar/.opam/default/lib/shell/string_extended -I /home/avatar/.opam/default/lib/shell/unix_extended -I /home/avatar/.opam/default/lib/spawn -I /home/avatar/.opam/default/lib/splittable_random -I /home/avatar/.opam/default/lib/stdio -I /home/avatar/.opam/default/lib/textutils/console -I /home/avatar/.opam/default/lib/typerep -I /home/avatar/.opam/default/lib/variantslib -intf-suffix .ml -no-alias-deps -opaque -open Nullstellen_lib -o lib/.nullstellen_lib.objs/nullstellen_lib__Polynomial.cmx -c -impl lib/polynomial.pp.ml)
File "lib/polynomial.ml", line 1:
Error: The implementation lib/polynomial.pp.ml
       does not match the interface lib/.nullstellen_lib.objs/nullstellen_lib__Polynomial.cmi:
       The value `func' is required but not provided
       File "lib/polynomial.mli", line 12, characters 0-22:
         Expected declaration
    ocamlopt lib/.nullstellen_lib.objs/nullstellen_lib__Animation.{cmx,o} (exit 2)
@tari3x
Copy link
Author

tari3x commented Jan 3, 2019

Once I asked, I realized that I can just "toggle-truncate-lines" in emacs, so I think I'm mostly fine.

@tari3x
Copy link
Author

tari3x commented Jan 3, 2019

Actually no, the buffer gets recreated and the setting wiped. So I'm back to being not fine, sadly.

@rgrinberg
Copy link
Member

I think that would be an improvement. Except not printing the command at all, I think we can just print it in "short mode" like we usually do. I'd like a second opinion from the other maintainers however.

@jberdine
Copy link
Contributor

jberdine commented Jan 3, 2019 via email

@tari3x
Copy link
Author

tari3x commented Jan 3, 2019

Indeed, this worked:

(add-hook 'compilation-mode-hook
          (quote (lambda nil (toggle-truncate-lines 1))))

@emillon
Copy link
Collaborator

emillon commented Jan 7, 2019

I agree with @rgrinberg that short mode is enough. If the actual command is required, one can always re-run -display verbose.

@ghost
Copy link

ghost commented Jan 7, 2019

It seems to me that having the full command line is useful in CI builds, but annoying during development. It'd be nice if we could reproduce this behaviour by default. Maybe we could add an option for it and make -p imply printing full command lines?

@rgrinberg
Copy link
Member

Hmm, I agree that making debugging in CI harder is a serious disadvantage. We don't run our tests with -p in CI with dune for example. So this would make things harder for us.

@ghost
Copy link

ghost commented Jan 8, 2019

Maybe in our CI we could pass a new --show-command option? I assume that our CI is an exception and that most other OCaml projects rely on the opam CI scripts, which should use -p.

@bobzhang
Copy link
Member

I think you can add a env variable support in CI, for the current output, the signal/noise is very low

@rgrinberg
Copy link
Member

One issue with just adding an env var for CI is that it will make everyone's current CI setups worse. People will have to observe a truncated error message, google why it's happening, update their CI setups to enable the variable, and re-run the tests.

To me this seem to be quite intrusive. Perhaps it makes sense to make this a dune-config or dune-project setting?

@ghost
Copy link

ghost commented Mar 26, 2019

The proposal is to show more in the CI by default, right? In that case, that seems good to me: you get all the information you can get when running inside the CI and less verbose output when running locally.

@ejgallego
Copy link
Collaborator

Isn't this more a problem of the IDE used than of Dune?

Having the full error line is very useful in quite a few situations, including the CI of projects using Dune.

@ghost
Copy link

ghost commented Mar 26, 2019

For the CI, I believe we all agree that having the full command line is useful. So we shouldn't change the behavior in the CI.

For local development, it seems to me that in general displaying the command line adds quite a bit of noise. So we should have a way to see it, but it shouldn't be the default.

@bobzhang
Copy link
Member

The thing is dune is too customized for Emacs. I used to be a long time Emacs user so I did not feel the pain.
The out of box experience, e.g, dune with vscode is not usable in my opinion: for a trivial build error, it has pages of message, most of them are not relevant, and you have to search those files and fix it.

@ejgallego
Copy link
Collaborator

So we shouldn't change the behavior in the CI.

I was not aware that the norm for Dune-based projects is to build in the CI with -p.

I do purposely avoid that in my projects as for example to build with warnings on.

@ghost
Copy link

ghost commented Mar 26, 2019

Just to be clear: there is special code in Dune to improve the experience when dune is executed inside emacs and we are happy to add special code to deal with vscode as well since it is now a widely used editor.

To detect whether dune is executed inside emacs, we look at the environment variable INSIDE_EMACS that is set by emacs. @bobzhang do you know if vscode sets a similar environment variable?

@ghost
Copy link

ghost commented Mar 26, 2019

@ejgallego, ack. Ideally, we would detect CI builds automatically by looking at environment variables. This way users would have nothing special to do.

@rgrinberg
Copy link
Member

Master contains improvements that elide the commands under certain circumstances:

  • The command omits recognizable location information
  • We are not in "CI" mode. This is detected with the CI env var. Which at least works for travis. Happy to further improve detection here.

rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue May 30, 2019
CHANGES:

- Restricted the set of variables available for expansion in the destination
  filename of `install` stanza to simplify implementation and avoid dependency
  cycles. (ocaml/dune#2073, @aalekseyev, @diml)

- [menhir] call menhir from context root build_dir (ocaml/dune#2067, @ejgallego,
  review by @diml, @rgrinberg)

- [coq] Add `coq.pp` stanza to help with pre-processing of grammar
  files (ocaml/dune#2054, @ejgallego, review by @rgrinberg)

- Add a new more generic form for the *promote* mode: `(promote
  (until-clean) (into <dir>))` (ocaml/dune#2068, @diml)

- Allow to promote only a subset of the targets via `(promote (only
  <pred>))`. For instance: `(promote (only *.mli))` (ocaml/dune#2068, @diml)

- Improve the behavior when a strict subset of the targets of a rule is already
  in the source tree for projects using the dune language < 1.10 (ocaml/dune#2068, fixes
  ocaml/dune#2061, @diml)

- With lang dune >= 1.10, rules in standard mode are no longer allowed to
  produce targets that are present in the source tree. This has been a warning
  for long enough (ocaml/dune#2068, @diml)

- Allow %{...} variables in pps flags (ocaml/dune#2076, @mlasson review by @diml and
  @aalekseyev).

- Add a 'cookies' option to ppx_rewriter/deriver flags in library stanzas. This
  allow to specify cookie requests from variables expanded at each invocation of
  the preprocessor. (ocaml/dune#2106, @mlasson @diml)

- Add more opam metadata and use it to generate `.opam` files. In particular, a
  `package` field has been added to specify package specific information.
  (ocaml/dune#2017, ocaml/dune#2091, @avsm, @jonludlam, @rgrinberg)

- Clean up the special support for `findlib.dynload`. Before, Dune would simply
  match on the library name. Now, we only match on the findlib package name when
  the library doesn't come from Dune. Someone writing a library called
  `findlib.dynload` with Dune would have to add `(special_builtin_support
  findlib_dynload)` to trigger the special behavior. (ocaml/dune#2115, @diml)

- Install the `future_syntax` preprocessor as `ocaml-syntax-shims.exe` (ocaml/dune#2125,
  @rgrinberg)

- Hide full command on errors and warnings in development and show them in CI.
  (detected using the `CI` environment variable). Commands for which the
  invocation might be omitted must output an error prefixed with `File `. Add an
  `--always-show-command-line` option to disable this behavior and always show
  the full command. (ocaml/dune#2120, fixes ocaml/dune#1733, @rgrinberg)

- In `dune-workspace` files, add the ability to choose the host context and to
  create duplicates of the default context with different settings. (ocaml/dune#2098,
  @TheLortex, review by @diml, @rgrinberg and @aalekseyev)

- Add support for hg in `dune subst` (ocaml/dune#2135, @diml)

- Don't build documentation for implementations of virtual libraries (ocaml/dune#2141,
  fixes ocaml/dune#2138, @jonludlam)

- Fix generation of the `-pp` flag in .merlin (ocaml/dune#2142, @rgrinberg)

- Make `dune subst` add a `(version ...)` field to the `dune-project`
  file (ocaml/dune#2148, @diml)

- Add the `%{os_type}` variable, which is a short-hand for
  `%{ocaml-config:os_type}` (ocaml/dune#1764, @diml)

- Allow `enabled_if` fields in `library` stanzas, restricted to the
  `%{os_type}`, `%{model}`, `%{architecture}`, `%{system}` variables (ocaml/dune#1764,
  ocaml/dune#2164 @diml, @rgrinberg)

- Fix `chdir` on external and source paths. Dune will also fail gracefully if
  the external or source path does not exist (ocaml/dune#2165, fixes ocaml/dune#2158, @rgrinberg)

- Support the `.cc` extension fro C++ sources (ocaml/dune#2195, fixes ocaml/dune#83, @rgrinberg)

- Run `ocamlformat` relative to the context root. This improves the locations of
  errors. (ocaml/dune#2196, fixes ocaml/dune#1370, @rgrinberg)

- Fix detection of `README`, `LICENSE`, `CHANGE`, and `HISTORY` files. These
  would be undetected whenever the project was nested in another workspace.
  (ocaml/dune#2194, @rgrinberg)

- Fix generation of `.merlin` whenever there's more than one stanza with the
  same ppx preprocessing specification (ocaml/dune#2209 ,fixes ocaml/dune#2206, @rgrinberg)

- Fix generation of `.meriln` in the presence of the `copy_files` stanza and
  preprocessing specifications of other stanazs. (ocaml/dune#2211, fixes ocaml/dune#2206,
  @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Jun 4, 2019
CHANGES:

- Restricted the set of variables available for expansion in the destination
  filename of `install` stanza to simplify implementation and avoid dependency
  cycles. (ocaml/dune#2073, @aalekseyev, @diml)

- [menhir] call menhir from context root build_dir (ocaml/dune#2067, @ejgallego,
  review by @diml, @rgrinberg)

- [coq] Add `coq.pp` stanza to help with pre-processing of grammar
  files (ocaml/dune#2054, @ejgallego, review by @rgrinberg)

- Add a new more generic form for the *promote* mode: `(promote
  (until-clean) (into <dir>))` (ocaml/dune#2068, @diml)

- Allow to promote only a subset of the targets via `(promote (only
  <pred>))`. For instance: `(promote (only *.mli))` (ocaml/dune#2068, @diml)

- Improve the behavior when a strict subset of the targets of a rule is already
  in the source tree for projects using the dune language < 1.10 (ocaml/dune#2068, fixes
  ocaml/dune#2061, @diml)

- With lang dune >= 1.10, rules in standard mode are no longer allowed to
  produce targets that are present in the source tree. This has been a warning
  for long enough (ocaml/dune#2068, @diml)

- Allow %{...} variables in pps flags (ocaml/dune#2076, @mlasson review by @diml and
  @aalekseyev).

- Add a 'cookies' option to ppx_rewriter/deriver flags in library stanzas. This
  allow to specify cookie requests from variables expanded at each invocation of
  the preprocessor. (ocaml/dune#2106, @mlasson @diml)

- Add more opam metadata and use it to generate `.opam` files. In particular, a
  `package` field has been added to specify package specific information.
  (ocaml/dune#2017, ocaml/dune#2091, @avsm, @jonludlam, @rgrinberg)

- Clean up the special support for `findlib.dynload`. Before, Dune would simply
  match on the library name. Now, we only match on the findlib package name when
  the library doesn't come from Dune. Someone writing a library called
  `findlib.dynload` with Dune would have to add `(special_builtin_support
  findlib_dynload)` to trigger the special behavior. (ocaml/dune#2115, @diml)

- Install the `future_syntax` preprocessor as `ocaml-syntax-shims.exe` (ocaml/dune#2125,
  @rgrinberg)

- Hide full command on errors and warnings in development and show them in CI.
  (detected using the `CI` environment variable). Commands for which the
  invocation might be omitted must output an error prefixed with `File `. Add an
  `--always-show-command-line` option to disable this behavior and always show
  the full command. (ocaml/dune#2120, fixes ocaml/dune#1733, @rgrinberg)

- In `dune-workspace` files, add the ability to choose the host context and to
  create duplicates of the default context with different settings. (ocaml/dune#2098,
  @TheLortex, review by @diml, @rgrinberg and @aalekseyev)

- Add support for hg in `dune subst` (ocaml/dune#2135, @diml)

- Don't build documentation for implementations of virtual libraries (ocaml/dune#2141,
  fixes ocaml/dune#2138, @jonludlam)

- Fix generation of the `-pp` flag in .merlin (ocaml/dune#2142, @rgrinberg)

- Make `dune subst` add a `(version ...)` field to the `dune-project`
  file (ocaml/dune#2148, @diml)

- Add the `%{os_type}` variable, which is a short-hand for
  `%{ocaml-config:os_type}` (ocaml/dune#1764, @diml)

- Allow `enabled_if` fields in `library` stanzas, restricted to the
  `%{os_type}`, `%{model}`, `%{architecture}`, `%{system}` variables (ocaml/dune#1764,
  ocaml/dune#2164 @diml, @rgrinberg)

- Fix `chdir` on external and source paths. Dune will also fail gracefully if
  the external or source path does not exist (ocaml/dune#2165, fixes ocaml/dune#2158, @rgrinberg)

- Support the `.cc` extension fro C++ sources (ocaml/dune#2195, fixes ocaml/dune#83, @rgrinberg)

- Run `ocamlformat` relative to the context root. This improves the locations of
  errors. (ocaml/dune#2196, fixes ocaml/dune#1370, @rgrinberg)

- Fix detection of `README`, `LICENSE`, `CHANGE`, and `HISTORY` files. These
  would be undetected whenever the project was nested in another workspace.
  (ocaml/dune#2194, @rgrinberg)

- Fix generation of `.merlin` whenever there's more than one stanza with the
  same ppx preprocessing specification (ocaml/dune#2209 ,fixes ocaml/dune#2206, @rgrinberg)

- Fix generation of `.merlin` in the presence of the `copy_files` stanza and
  preprocessing specifications of other stanazs. (ocaml/dune#2211, fixes ocaml/dune#2206,
  @rgrinberg)

- Run `refmt` from the context's root directory. This improves error messages in
  case of syntax errors. (ocaml/dune#2223, @rgrinberg)

- In .merlin files, don't pass `-dump-ast` to the `future_syntax` preprocessor.
  Merlin doesn't seem to like it when binary AST is generated by a `-pp`
  preprocessor. (ocaml/dune#2236, @aalekseyev)

- `dune install` will verify that all files mentioned in all .install files
  exist before trying to install anything. This prevents partial installation of
  packages (ocaml/dune#2230, @rgrinberg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants