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

Build script warnings don't respect --message-format=json. #14246

Closed
JarredAllen opened this issue Jul 12, 2024 · 2 comments
Closed

Build script warnings don't respect --message-format=json. #14246

JarredAllen opened this issue Jul 12, 2024 · 2 comments
Labels
A-build-scripts Area: build.rs scripts A-diagnostics Area: Error and warning messages generated by Cargo itself. A-json-output Area: JSON message output C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@JarredAllen
Copy link

Problem

When a build script prints cargo::warning=msg to stdout when running, cargo displays it as warning: <crate>: msg. This is great for human output, but causes problems when run with --message-format=json and passed to tooling that expects JSON data.

Steps

  1. Make a crate with a build.rs containing the line: println!("cargo::warning=msg");
  2. Run cargo check --message-fmt=json
  3. Observe the line of non-JSON in the middle of your JSON

Possible Solution(s)

It'd be cool if cargo could convert it into a JSON object of a similar format to warnings that cargo/rustc directly emit.

Notes

No response

Version

cargo 1.79.0 (ffa9cf99a 2024-06-03)
release: 1.79.0
commit-hash: ffa9cf99a594e59032757403d4c780b46dc2c43a
commit-date: 2024-06-03
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Ubuntu 22.4.0 (jammy) [64-bit]
@JarredAllen JarredAllen added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jul 12, 2024
@epage epage added A-diagnostics Area: Error and warning messages generated by Cargo itself. A-build-scripts Area: build.rs scripts A-json-output Area: JSON message output labels Jul 13, 2024
@epage
Copy link
Contributor

epage commented Jul 13, 2024

This applies to much more than build scripts but to all cargo warnings and errors. We have #8283 tracking that and I would be inclined to close this in favor of #8283.

@weihanglo
Copy link
Member

What about verbose (-vv) build script outputs? They are usually from an external build system and not possible to make it respect --message-format=json?

We have #8283 tracking that and I would be inclined to close this in favor of #8283.

Agree.

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2024
@epage epage mentioned this issue Jul 15, 2024
Open
@iulianbarbu iulianbarbu mentioned this issue Sep 25, 2024
Merged
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this issue Sep 27, 2024
@iulianbarbu
substrate/utils: enable wasm builder diagnostics propagation (#5838)
117cb93 
# 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
@iulianbarbu
substrate/utils: enable wasm builder diagnostics propagation (#5838)
da0bcb0 
# 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
@iulianbarbu
substrate/utils: enable wasm builder diagnostics propagation (#5838)
8bf8111 
# 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
@iulianbarbu @bkchr
substrate/utils: enable wasm builder diagnostics propagation (#5838)
58ade7a 
# 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
Assignees
No one assigned
Labels
A-build-scripts Area: build.rs scripts A-diagnostics Area: Error and warning messages generated by Cargo itself. A-json-output Area: JSON message output C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@epage @weihanglo @JarredAllen