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

Provide Cargo messages as JSON messages #8283

Open
ehuss opened this issue May 26, 2020 · 4 comments
Open

Provide Cargo messages as JSON messages #8283

ehuss opened this issue May 26, 2020 · 4 comments
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-json-output Area: JSON message output C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@ehuss
Copy link
Contributor

ehuss commented May 26, 2020

Describe the problem you are trying to solve
When integrating Cargo with an editor or IDE, it is common to use JSON to get structured messages from rustc. However, Cargo's own messages (such as errors, warnings, etc.) are sent as plain text, making it difficult for the development environment to know how to display these messages.

Describe the solution you'd like
It would be nice if Cargo's messages were sent as JSON when --message-format=json is used.

Notes
There are many decisions to be made on the behavior and exact structure to use.

@ehuss ehuss added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` A-json-output Area: JSON message output labels May 26, 2020
@mchernyavsky
Copy link
Contributor

Is it OK to simply change Cargo to start sending JSON with --message-format=json? That could break or disrupt tools, though I'm not sure how likely that is, or what the consequences would be. Cargo could include something like rustc's --json flag to configure JSON behavior, but it would be nice to avoid if possible to keep things simple.

Don't forget about other JSON-related formats for --message-format (e.g., json-diagnostic-rendered-ansi) in case if you choose to include a new flag.

Should Cargo re-use rustc's "compiler-message" structure for warnings and errors, or should it add a new one? It might be difficult to shoe-horn Cargo's format (which uses a stack of causes) to rustc's format. The drawback of using a separate structure is that then there would need to be multiple ways to handle "messages".

Is it ok to mark build tool messages as messages from the compiler? (I'm talking about "compiler-message" reason)

What other activities and messages should be JSON messages? See #8165 (comment) for some examples. Cargo has many different things it prints (status messages, "downloading", etc.), and there are quite a few action points ("build started", "unit started", "unit finished", etc.) that could be useful to have as JSON.

Currently, in IJ-Rust we show only Compiling status and Building progress (#8165 (comment)), but it's a good idea to show other status / progress messages too (e.g., Updating / Fetch, Downloaded / Downloading). Does cargo build uses Checkout progress?

Also, in IJ-Rust we parse raw Running executable_path and Doc-tests package_name messages to get target names. We use target names as test suite names:

It would be nice to have JSON messages for these statuses. It's not necessary for IJ since we already have libtest support, but it may be useful for anyone else who wants to write it from scratch.

Also, I think JSON messages for statuses should have "rendered" field like other diagnostic messages, in order to be able to show the "original" output in a separate window.

Should Cargo intercept non-JSON output from rustc and wrap it in JSON (#8179)?

👍 , but I think it can be discussed in a separate issue after this issue is closed.

@mchernyavsky
Copy link
Contributor

In continuation of #8165 (comment)

I'm uncertain about the best way to express "progress". There are different stages (updating indexes, resolving, downloading, compiling, etc.). Using "unit count" may not be the best option due to that, and also that could make it difficult in the future if Cargo is more lazy or dynamic when deciding what to build (although maybe the unit count could be documented as an "estimate"?).

Cargo already prints "unit count" (11 / 13 on the screenshot below). Do you want to remove it from the progress bar?

image

An option is to have an initial "build-started" JSON message with the estimated unit count?

An alternative is to have Cargo compute some estimated progress amount as a number from 0 to 1. It could then include that in the JSON messages (or maybe as a separate message).

For my purpose, I only need a number from 0 to 1 + some text to show (e.g., "Building a, b, c"), but as mentioned in #8165 (comment), an initial JSON message with the unit count + start / finish messages for each unit should be enough to calculate progress amount and list of active units.

I'm also wondering about whether to show progress for the downloading phase separately, or should it just be part of a single "progress" progression? Downloading can be quite slow.

I think single progression with different phases (Fetch-Downloading-Building) is more useful for users, but I'm not sure.

I wonder, do any other build tools have a similar concept?

As I understand, Gradle downloads dependencies during the "compile" task (x MB/3.62 MB is updated in-place):

image

image

@Nemo157
Copy link
Member

Nemo157 commented Aug 13, 2020

I would love to have Cargo emit useful json messages as well. I was just looking at https://github.com/Nemo157/cargo-build-tree again and was wondering what would be required to actually have it support showing what was currently in-progress, having unit-started and unit-finished messages would be great for this.

The fact that a build-finished message was added seems to indicate that it is ok for Cargo to simply start emitting its own messages with --message-format=json.

@gilescope
Copy link
Contributor

It would be nice if all the json messages had a rendered field. The error and warning messages do but if you want to augment cargo in any way you have to parse all the messages and output the same things as cargo dose currently so it doesn't look unusual (except for the messages which already have a rendered field). Noticed the need for this in a couple of tools now so thought I would mention it in case it would be helpful also to others.

@ehuss ehuss added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Jun 19, 2023
@epage epage added the A-diagnostics Area: Error and warning messages generated by Cargo itself. label Nov 4, 2023
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this issue Sep 27, 2024
# Description

`substrate-wasm-builder` can be a build dependency for crates which
develop FRAME runtimes. I had a tough time seeing errors happening in
such crates (e.g. runtimes from the `templates` directory) in my IDE. I
use a combination of rust-analyzer + nvim-lsp + nvim-lspconfig +
rustacean.vim and all of this stack is not able to correctly parse
errors emitted during the `build` phase.

As a matter of fact there is also a cargo issue tracking specifically
this issue where cargo doesn't propagate the `--message-format` type to
the build phase: [here](rust-lang/cargo#14246)
initially and then
[here](rust-lang/cargo#8283). It feels like a
solution for this use case isn't very close, so if it comes to runtimes
development (both as an SDK user and developer), enabling wasm builder
to emit diagnostics messages friendly to IDEs would be useful for
regular workflows where IDEs are used for finding errors instead of
manually running `cargo` commands.

## Integration

It can be an issue if Substrate/FRAME SDKs users and developers rely on
the runtimes' crates build phase output in certain ways. Emitting
compilation messages as json will pollute the regular compilation output
so people that would manually run `cargo build` or `cargo check` on
their crates will have a tougher time extracting the non JSON output.

## Review Notes

Rust IDEs based on rust-analyzer rely on cargo check/clippy to extract
diagnostic information. The information is generated by passing flags
like `--messages-format=json` to the `cargo` commands. The messages are
extracted by rust-analyzer and published to LSP clients that will
populate UIs accordingly.

We need to build against the wasm target by using `message-format=json`
too so that IDEs can show the errors for crates that have a build
dependency on `substrate-wasm-builder`.

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this issue Sep 27, 2024
# Description

`substrate-wasm-builder` can be a build dependency for crates which
develop FRAME runtimes. I had a tough time seeing errors happening in
such crates (e.g. runtimes from the `templates` directory) in my IDE. I
use a combination of rust-analyzer + nvim-lsp + nvim-lspconfig +
rustacean.vim and all of this stack is not able to correctly parse
errors emitted during the `build` phase.

As a matter of fact there is also a cargo issue tracking specifically
this issue where cargo doesn't propagate the `--message-format` type to
the build phase: [here](rust-lang/cargo#14246)
initially and then
[here](rust-lang/cargo#8283). It feels like a
solution for this use case isn't very close, so if it comes to runtimes
development (both as an SDK user and developer), enabling wasm builder
to emit diagnostics messages friendly to IDEs would be useful for
regular workflows where IDEs are used for finding errors instead of
manually running `cargo` commands.

## Integration

It can be an issue if Substrate/FRAME SDKs users and developers rely on
the runtimes' crates build phase output in certain ways. Emitting
compilation messages as json will pollute the regular compilation output
so people that would manually run `cargo build` or `cargo check` on
their crates will have a tougher time extracting the non JSON output.

## Review Notes

Rust IDEs based on rust-analyzer rely on cargo check/clippy to extract
diagnostic information. The information is generated by passing flags
like `--messages-format=json` to the `cargo` commands. The messages are
extracted by rust-analyzer and published to LSP clients that will
populate UIs accordingly.

We need to build against the wasm target by using `message-format=json`
too so that IDEs can show the errors for crates that have a build
dependency on `substrate-wasm-builder`.

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this issue Sep 27, 2024
# Description

`substrate-wasm-builder` can be a build dependency for crates which
develop FRAME runtimes. I had a tough time seeing errors happening in
such crates (e.g. runtimes from the `templates` directory) in my IDE. I
use a combination of rust-analyzer + nvim-lsp + nvim-lspconfig +
rustacean.vim and all of this stack is not able to correctly parse
errors emitted during the `build` phase.

As a matter of fact there is also a cargo issue tracking specifically
this issue where cargo doesn't propagate the `--message-format` type to
the build phase: [here](rust-lang/cargo#14246)
initially and then
[here](rust-lang/cargo#8283). It feels like a
solution for this use case isn't very close, so if it comes to runtimes
development (both as an SDK user and developer), enabling wasm builder
to emit diagnostics messages friendly to IDEs would be useful for
regular workflows where IDEs are used for finding errors instead of
manually running `cargo` commands.

## Integration

It can be an issue if Substrate/FRAME SDKs users and developers rely on
the runtimes' crates build phase output in certain ways. Emitting
compilation messages as json will pollute the regular compilation output
so people that would manually run `cargo build` or `cargo check` on
their crates will have a tougher time extracting the non JSON output.

## Review Notes

Rust IDEs based on rust-analyzer rely on cargo check/clippy to extract
diagnostic information. The information is generated by passing flags
like `--messages-format=json` to the `cargo` commands. The messages are
extracted by rust-analyzer and published to LSP clients that will
populate UIs accordingly.

We need to build against the wasm target by using `message-format=json`
too so that IDEs can show the errors for crates that have a build
dependency on `substrate-wasm-builder`.

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this issue Sep 28, 2024
# Description

`substrate-wasm-builder` can be a build dependency for crates which
develop FRAME runtimes. I had a tough time seeing errors happening in
such crates (e.g. runtimes from the `templates` directory) in my IDE. I
use a combination of rust-analyzer + nvim-lsp + nvim-lspconfig +
rustacean.vim and all of this stack is not able to correctly parse
errors emitted during the `build` phase.

As a matter of fact there is also a cargo issue tracking specifically
this issue where cargo doesn't propagate the `--message-format` type to
the build phase: [here](rust-lang/cargo#14246)
initially and then
[here](rust-lang/cargo#8283). It feels like a
solution for this use case isn't very close, so if it comes to runtimes
development (both as an SDK user and developer), enabling wasm builder
to emit diagnostics messages friendly to IDEs would be useful for
regular workflows where IDEs are used for finding errors instead of
manually running `cargo` commands.

## Integration

It can be an issue if Substrate/FRAME SDKs users and developers rely on
the runtimes' crates build phase output in certain ways. Emitting
compilation messages as json will pollute the regular compilation output
so people that would manually run `cargo build` or `cargo check` on
their crates will have a tougher time extracting the non JSON output.

## Review Notes

Rust IDEs based on rust-analyzer rely on cargo check/clippy to extract
diagnostic information. The information is generated by passing flags
like `--messages-format=json` to the `cargo` commands. The messages are
extracted by rust-analyzer and published to LSP clients that will
populate UIs accordingly.

We need to build against the wasm target by using `message-format=json`
too so that IDEs can show the errors for crates that have a build
dependency on `substrate-wasm-builder`.

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-json-output Area: JSON message output C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

5 participants