Skip to content

Fix wasm-builder not exiting if compilation fails#3439

Merged
koute merged 2 commits intoparitytech:masterfrom
koute:master_fix_wasm_builder
Feb 22, 2024
Merged

Fix wasm-builder not exiting if compilation fails#3439
koute merged 2 commits intoparitytech:masterfrom
koute:master_fix_wasm_builder

Conversation

@koute
Copy link
Copy Markdown
Contributor

@koute koute commented Feb 22, 2024

This PR fixes a subtle bug in wasm-builder first introduced in #1851 (sorry, my bad! I should have caught this during review) where the status code of the cargo subprocess is not properly checked, which results in builds silently succeeding when they shouldn't (that is: if we successfully build a runtime blob, and then modify the code so that it won't compile, and recompile it again, then the build will succeed and silently use the old blob).

cc @athei This is the bug you were seeing.

[edit]Also fixes a similar PolkaVM-specific bug where I accidentally used the wrong comparison operator.[/edit]

@koute koute added R0-no-crate-publish-required The change does not require any crates to be re-published. I2-bug The node fails to follow expected behavior. labels Feb 22, 2024
@koute koute requested review from a team and athei February 22, 2024 06:39
@michalkucharczyk michalkucharczyk requested a review from a team February 22, 2024 09:04
@koute koute added this pull request to the merge queue Feb 22, 2024
Merged via the queue into paritytech:master with commit 8220828 Feb 22, 2024
@koute koute deleted the master_fix_wasm_builder branch February 22, 2024 10:29
girazoki pushed a commit to moondance-labs/polkadot-sdk that referenced this pull request Feb 22, 2024
This PR fixes a subtle bug in `wasm-builder` first introduced in
paritytech#1851 (sorry, my bad! I
should have caught this during review) where the status code of the
`cargo` subprocess is not properly checked, which results in builds
silently succeeding when they shouldn't (that is: if we successfully
build a runtime blob, and then modify the code so that it won't compile,
and recompile it again, then the build will succeed and silently use the
*old* blob).

cc @athei This is the bug you were seeing.

[edit]Also fixes a similar PolkaVM-specific bug where I accidentally
used the wrong comparison operator.[/edit]
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
This PR fixes a subtle bug in `wasm-builder` first introduced in
paritytech#1851 (sorry, my bad! I
should have caught this during review) where the status code of the
`cargo` subprocess is not properly checked, which results in builds
silently succeeding when they shouldn't (that is: if we successfully
build a runtime blob, and then modify the code so that it won't compile,
and recompile it again, then the build will succeed and silently use the
*old* blob).

cc @athei This is the bug you were seeing.

[edit]Also fixes a similar PolkaVM-specific bug where I accidentally
used the wrong comparison operator.[/edit]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I2-bug The node fails to follow expected behavior. R0-no-crate-publish-required The change does not require any crates to be re-published.

Projects

Status: done

Development

Successfully merging this pull request may close these issues.

4 participants