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

Update to Rust stable 1.84.1 #7625

Merged
merged 18 commits into from
Feb 20, 2025
Merged

Update to Rust stable 1.84.1 #7625

merged 18 commits into from
Feb 20, 2025

Conversation

athei
Copy link
Member

@athei athei commented Feb 19, 2025

Ref https://github.com/paritytech/ci_cd/issues/1107

We mainly need that so that we can finally compile the pallet_revive fixtures on stable. I did my best to keep the commits focused on one thing to make review easier.

All the changes are needed because rustc introduced more warnings or is more strict about existing ones. Most of the stuff could just be fixed and the commits should be pretty self explanatory. However, there are a few this that are notable:

non_local_definitions

A lot of runtimes to write impl blocks inside functions. This makes sense to reduce the amount of conditional compilation. I guess I could have moved them into a module instead. But I think allowing it here makes sense to avoid the code churn.

unexpected_cfgs

The FRAME macros emit code that references various features like std, runtime-benchmarks or try-runtime. If a create that uses those macros does not have those features we get this warning. Those were mostly when defining a mock runtime. I opted for silencing the warning in this case rather than adding not needed features.

For the benchmarking ui tests I opted for adding the runtime-benchmark feature to the Cargo.toml.

Failing UI test

I am bumping the trybuild version and regenerating the ui tests. The old version seems to be incompatible. This requires us to pass deny_warnings in CARGO_ENCODED_RUSTFLAGS as RUSTFLAGS is ignored in the new version.

Removing toolchain file from the pallet revive fixtures

This is no longer needed since the latest stable will compile them fine using the RUSTC_BOOTSTRAP=1.

@athei athei requested review from a team and koute as code owners February 19, 2025 14:34
@paritytech-review-bot paritytech-review-bot bot requested a review from a team February 19, 2025 14:36
@athei
Copy link
Member Author

athei commented Feb 19, 2025

/cmd prdoc --audience runtime_dev --bump patch

@athei athei added the R0-silent Changes should not be mentioned in any release notes label Feb 19, 2025
@paritytech-review-bot paritytech-review-bot bot requested a review from a team February 19, 2025 16:14
@bkchr
Copy link
Member

bkchr commented Feb 19, 2025

A lot of runtimes to write impl blocks inside functions. This makes sense to reduce the amount of conditional compilation. I guess I could have moved them into a module instead. But I think allowing it here makes sense to avoid the code churn.

Ser: polkadot-fellows/runtimes@c620a26 🙈

@@ -50,6 +50,8 @@
//!
//! ## Docs structure
#![doc = simple_mermaid::mermaid!("../mermaid/structure.mmd")]
// Frame macros reference features which this crate does not have
#![allow(unexpected_cfgs)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should create an issue for this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to eventually add those features to those crates? Because I thought we just want to allow it forever. Cause the features are essentially useless in those crates. I which you could selectively suppress the warnings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macros should not require these features. For that we have this which enables code, based on features being enabled in the frame-support crate. (The correct way to handle this)

@athei
Copy link
Member Author

athei commented Feb 20, 2025

A lot of runtimes to write impl blocks inside functions. This makes sense to reduce the amount of conditional compilation. I guess I could have moved them into a module instead. But I think allowing it here makes sense to avoid the code churn.

Ser: polkadot-fellows/runtimes@c620a26 🙈

Ooof. Thank you for your service.

@athei athei added this pull request to the merge queue Feb 20, 2025
Merged via the queue into master with commit e2d3da6 Feb 20, 2025
260 of 269 checks passed
@athei athei deleted the at/update-rust-2 branch February 20, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants