paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.8k stars 652 forks source link

substrate/utils: enable wasm builder diagnostics propagation #5838

Closed iulianbarbu closed 13 hours ago

iulianbarbu commented 3 days ago

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 initially and then here. 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.

bkchr commented 3 days ago

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.

Yeah that doesn't sound really nice. You could for example introduce a env variable to do this.

iulianbarbu commented 2 days ago

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.

Yeah that doesn't sound really nice. You could for example introduce a env variable to do this.

Great idea! Do you see it as a feature guarded by an env variable that should be either false/true? Will enable the change soon and also add some crate level docs to advertise this aspect.