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

Remove most all usage of sp-std #5010

Merged
merged 13 commits into from
Jul 15, 2024
Merged

Conversation

jasl
Copy link
Contributor

@jasl jasl commented Jul 12, 2024

This should remove nearly all usage of sp-std except:

  • bridge and bridge-hubs
  • a few of frames re-export sp-std, keep them for now
  • there is a usage of sp_std::Writer, I don't have an idea how to move it

Please review proc-macro carefully. I'm not sure I'm doing it the right way.

Note: need /bot fmt

@jasl jasl requested review from athei, acatangiu, cheme, a team and koute as code owners July 12, 2024 15:51
@jasl
Copy link
Contributor Author

jasl commented Jul 12, 2024

This is a following PR of #5001

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Next time, some smaller chunks would be better :D

substrate/frame/timestamp/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/timestamp/src/lib.rs Outdated Show resolved Hide resolved
@bkchr
Copy link
Member

bkchr commented Jul 12, 2024

bot fmt

@command-bot
Copy link

command-bot bot commented Jul 12, 2024

@bkchr https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6689618 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-7d3974c0-c071-4a72-ab4c-d4886f99e410 to cancel this command or bot cancel to cancel all commands in this pull request.

Copy link

We are migrating the command bot to be a GitHub Action

Please, see the documentation on how to use it

@bkchr bkchr added the R0-silent Changes should not be mentioned in any release notes label Jul 12, 2024
@bkchr bkchr requested a review from ggwpez July 12, 2024 21:32
@command-bot
Copy link

command-bot bot commented Jul 12, 2024

@bkchr Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6689618 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6689618/artifacts/download.

let args = #crate_::validate_block::sp_std::boxed::Box::from_raw(
#crate_::validate_block::sp_std::slice::from_raw_parts_mut(
let args = #crate_::validate_block::alloc::boxed::Box::from_raw(
#crate_::validate_block::alloc::slice::from_raw_parts_mut(
Copy link
Member

Choose a reason for hiding this comment

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

It is not wrong, but also not particularly useful to re-export alloc for a macro.

Copy link
Member

Choose a reason for hiding this comment

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

If it is not re-exported here, the other crates would need to write extern crate alloc explicitly? Because the crate is not pulled in otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to re-export only selected types here instead of re-export the whole alloc crate

@@ -73,7 +73,7 @@ fn decode_impl(
quote! {
let #name =
<
_fepsp::sp_std::prelude::Vec<(
_fepsp::alloc::vec::Vec<(
Copy link
Member

Choose a reason for hiding this comment

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

Also not really needed here.

@@ -147,8 +147,8 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result<TokenStream2> {
self,
voter_at: impl Fn(Self::VoterIndex) -> Option<A>,
target_at: impl Fn(Self::TargetIndex) -> Option<A>,
) -> Result<_fepsp::sp_std::prelude::Vec<_feps::Assignment<A, #weight_type>>, _feps::Error> {
let mut #assignment_name: _fepsp::sp_std::prelude::Vec<_feps::Assignment<A, #weight_type>> = Default::default();
) -> Result<_fepsp::alloc::vec::Vec<_feps::Assignment<A, #weight_type>>, _feps::Error> {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

fn check_extrinsics(&self, block: &#block) -> #scrate::inherent::CheckInherentsResult;
}

impl InherentDataExt for #scrate::inherent::InherentData {
fn create_extrinsics(&self) ->
#scrate::__private::sp_std::vec::Vec<<#block as #scrate::sp_runtime::traits::Block>::Extrinsic>
#scrate::__private::alloc::vec::Vec<<#block as #scrate::sp_runtime::traits::Block>::Extrinsic>
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@@ -105,25 +105,25 @@ pub fn expand_outer_origin(
#[derive(Clone)]
pub struct RuntimeOrigin {
pub caller: OriginCaller,
filter: #scrate::__private::sp_std::rc::Rc<Box<dyn Fn(&<#runtime as #system_path::Config>::RuntimeCall) -> bool>>,
filter: #scrate::__private::alloc::rc::Rc<#scrate::__private::alloc::boxed::Box<dyn Fn(&<#runtime as #system_path::Config>::RuntimeCall) -> bool>>,
Copy link
Member

Choose a reason for hiding this comment

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

likewise

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

nits about not re-exporting alloc

@jasl
Copy link
Contributor Author

jasl commented Jul 12, 2024

nits about not re-exporting alloc

Thank you, I'll fix build failed that CI reported now, and apply your suggest tomorrow

@jasl jasl force-pushed the remove-sp-std-usage branch 2 times, most recently from 8d3100f to e73513f Compare July 13, 2024 19:15
@jasl
Copy link
Contributor Author

jasl commented Jul 13, 2024

@ggwpez I updated proc-macro related staff. Does it look good now?
I also simulated commands on CI, and all compile issues should be resolved now.

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6696292

# Conflicts:
#	polkadot/runtime/parachains/src/assigner_on_demand/tests.rs
@@ -122,8 +122,8 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To
#[no_mangle]
unsafe fn validate_block(arguments: *mut u8, arguments_len: usize) -> u64 {
// We convert the `arguments` into a boxed slice and then into `Bytes`.
let args = #crate_::validate_block::sp_std::boxed::Box::from_raw(
#crate_::validate_block::sp_std::slice::from_raw_parts_mut(
let args = #crate_::validate_block::Box::from_raw(
Copy link
Member

Choose a reason for hiding this comment

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

Eventually we should move all the re-exports to a __private and #[doc(hidden)]` module, but for the beginning i think its fine.

@ggwpez ggwpez added this pull request to the merge queue Jul 15, 2024
Merged via the queue into paritytech:master with commit 7ecf3f7 Jul 15, 2024
156 of 160 checks passed
@jasl jasl deleted the remove-sp-std-usage branch July 15, 2024 14:29
@jasl jasl restored the remove-sp-std-usage branch July 15, 2024 14:29
@jasl jasl deleted the remove-sp-std-usage branch July 15, 2024 14:29
ordian added a commit that referenced this pull request Jul 17, 2024
* master:
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
jpserrat pushed a commit to jpserrat/polkadot-sdk that referenced this pull request Jul 18, 2024
This should remove nearly all usage of `sp-std` except:
- bridge and bridge-hubs
- a few of frames re-export `sp-std`, keep them for now
- there is a usage of `sp_std::Writer`, I don't have an idea how to move
it

Please review proc-macro carefully. I'm not sure I'm doing it the right
way.

Note: need `/bot fmt`

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
ordian added a commit that referenced this pull request Jul 18, 2024
* master: (125 commits)
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
  network/tx: Ban peers with tx that fail to decode (#5002)
  Try State Hook for Bounties (#4563)
  [statement-distribution] Add metrics for distributed statements in V2 (#4554)
  added sync command (#4818)
  Bridges V2 refactoring backport and `pallet_bridge_messages` simplifications (#4935)
  xcm-executor: Improve logging (#4996)
  Remove usage of `sp-std` on templates (#5001)
  ...
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
This should remove nearly all usage of `sp-std` except:
- bridge and bridge-hubs
- a few of frames re-export `sp-std`, keep them for now
- there is a usage of `sp_std::Writer`, I don't have an idea how to move
it

Please review proc-macro carefully. I'm not sure I'm doing it the right
way.

Note: need `/bot fmt`

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
ordian added a commit that referenced this pull request Aug 6, 2024
* master: (130 commits)
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
  network/tx: Ban peers with tx that fail to decode (#5002)
  Try State Hook for Bounties (#4563)
  [statement-distribution] Add metrics for distributed statements in V2 (#4554)
  added sync command (#4818)
  Bridges V2 refactoring backport and `pallet_bridge_messages` simplifications (#4935)
  xcm-executor: Improve logging (#4996)
  Remove usage of `sp-std` on templates (#5001)
  ...
github-merge-queue bot pushed a commit that referenced this pull request Sep 7, 2024
…5632)

The macro hygiene for the `format_runtime_string!` macro was broken
since #5010, which
resulted in the following build error under certain circumstances:

```console
  error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
      --> /home/clang/.cargo/registry/src/index.crates.io-6f17d22bba15001f/frame-benchmarking-36.0.0/src/v1.rs:1738:2
       |
  1738 | /     sp_runtime::format_runtime_string!(
  1739 | |         "\n* Pallet: {}\n\
  1740 | |         * Benchmark: {}\n\
  1741 | |         * Components: {:?}\n\
  ...    |
  1750 | |         error_message,
  1751 | |     )
       | |_____^ use of undeclared crate or module `alloc`
       |
       = note: this error originates in the macro `sp_runtime::format_runtime_string` (in Nightly builds, run with -Z macro-backtrace for more info)

  For more information about this error, try `rustc --explain E0433`.
```

This bug has been known already, but hasn't been fixed so far, see
#5213 and
https://substrate.stackexchange.com/questions/11786/use-of-undeclared-crate-or-module-alloc-when-upgrade-to-v1-13-0.

I have made a mini rust crate that can reproduce the bug, and it also
shows that this PR will fix the issue:
https://github.com/clangenb/sp-runtime-string-test.
github-actions bot pushed a commit that referenced this pull request Sep 7, 2024
…5632)

The macro hygiene for the `format_runtime_string!` macro was broken
since #5010, which
resulted in the following build error under certain circumstances:

```console
  error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
      --> /home/clang/.cargo/registry/src/index.crates.io-6f17d22bba15001f/frame-benchmarking-36.0.0/src/v1.rs:1738:2
       |
  1738 | /     sp_runtime::format_runtime_string!(
  1739 | |         "\n* Pallet: {}\n\
  1740 | |         * Benchmark: {}\n\
  1741 | |         * Components: {:?}\n\
  ...    |
  1750 | |         error_message,
  1751 | |     )
       | |_____^ use of undeclared crate or module `alloc`
       |
       = note: this error originates in the macro `sp_runtime::format_runtime_string` (in Nightly builds, run with -Z macro-backtrace for more info)

  For more information about this error, try `rustc --explain E0433`.
```

This bug has been known already, but hasn't been fixed so far, see
#5213 and
https://substrate.stackexchange.com/questions/11786/use-of-undeclared-crate-or-module-alloc-when-upgrade-to-v1-13-0.

I have made a mini rust crate that can reproduce the bug, and it also
shows that this PR will fix the issue:
https://github.com/clangenb/sp-runtime-string-test.

(cherry picked from commit 96fecc3)
github-actions bot pushed a commit that referenced this pull request Sep 7, 2024
…5632)

The macro hygiene for the `format_runtime_string!` macro was broken
since #5010, which
resulted in the following build error under certain circumstances:

```console
  error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
      --> /home/clang/.cargo/registry/src/index.crates.io-6f17d22bba15001f/frame-benchmarking-36.0.0/src/v1.rs:1738:2
       |
  1738 | /     sp_runtime::format_runtime_string!(
  1739 | |         "\n* Pallet: {}\n\
  1740 | |         * Benchmark: {}\n\
  1741 | |         * Components: {:?}\n\
  ...    |
  1750 | |         error_message,
  1751 | |     )
       | |_____^ use of undeclared crate or module `alloc`
       |
       = note: this error originates in the macro `sp_runtime::format_runtime_string` (in Nightly builds, run with -Z macro-backtrace for more info)

  For more information about this error, try `rustc --explain E0433`.
```

This bug has been known already, but hasn't been fixed so far, see
#5213 and
https://substrate.stackexchange.com/questions/11786/use-of-undeclared-crate-or-module-alloc-when-upgrade-to-v1-13-0.

I have made a mini rust crate that can reproduce the bug, and it also
shows that this PR will fix the issue:
https://github.com/clangenb/sp-runtime-string-test.

(cherry picked from commit 96fecc3)
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.

4 participants