diff --git a/substrate/frame/support/procedural/src/construct_runtime/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/mod.rs index cfd582d0e4..e20cb61b7a 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/mod.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/mod.rs @@ -156,10 +156,7 @@ use parse::{ use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; use quote::quote; -use std::{ - collections::{HashMap, HashSet}, - str::FromStr, -}; +use std::{collections::HashSet, str::FromStr}; use syn::{Ident, Result}; /// The fixed name of the system pallet. @@ -324,11 +321,15 @@ fn decl_all_pallets<'a>( features: &HashSet<&str>, ) -> TokenStream2 { let mut types = TokenStream2::new(); - let mut names_by_feature = features + + // Every feature set to the pallet names that should be included by this feature set. + let mut features_to_names = features .iter() + .map(|f| *f) .powerset() - .map(|feat| (feat, Vec::new())) - .collect::>(); + .map(|feat| (HashSet::from_iter(feat), Vec::new())) + .collect::, Vec<_>)>>(); + for pallet_declaration in pallet_declarations { let type_name = &pallet_declaration.name; let pallet = &pallet_declaration.path; @@ -346,21 +347,22 @@ fn decl_all_pallets<'a>( types.extend(type_decl); if pallet_declaration.cfg_pattern.is_empty() { - for names in names_by_feature.values_mut() { + for (_, names) in features_to_names.iter_mut() { names.push(&pallet_declaration.name); } } else { - for (feature_set, names) in &mut names_by_feature { + for (feature_set, names) in &mut features_to_names { // Rust tidbit: if we have multiple `#[cfg]` feature on the same item, then the // predicates listed in all `#[cfg]` attributes are effectively joined by `and()`, // meaning that all of them must match in order to activate the item let is_feature_active = pallet_declaration.cfg_pattern.iter().all(|expr| { expr.eval(|pred| match pred { - Predicate::Feature(f) => feature_set.contains(&f), - Predicate::Test => feature_set.contains(&&"test"), + Predicate::Feature(f) => feature_set.contains(f), + Predicate::Test => feature_set.contains(&"test"), _ => false, }) }); + if is_feature_active { names.push(&pallet_declaration.name); } @@ -368,11 +370,34 @@ fn decl_all_pallets<'a>( } } - let all_pallets_without_system = names_by_feature.iter().map(|(feature_set, names)| { - let mut feature_set = feature_set.iter().collect::>(); - let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter(); - let feature_set = feature_set.into_iter(); - let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]); + // All possible features. This will be used below for the empty feature set. + let mut all_features = features_to_names + .iter() + .flat_map(|f| f.0.iter().cloned()) + .collect::>(); + let attribute_to_names = features_to_names + .into_iter() + .map(|(mut features, names)| { + // If this is the empty feature set, it needs to be changed to negate all available + // features. So, we ensure that there is some type declared when all features are not + // enabled. + if features.is_empty() { + let test_cfg = all_features.remove("test").then_some(quote!(test)).into_iter(); + let features = all_features.iter(); + let attr = quote!(#[cfg(all( #(not(#test_cfg)),* #(not(feature = #features)),* ))]); + + (attr, names) + } else { + let test_cfg = features.remove("test").then_some(quote!(test)).into_iter(); + let features = features.iter(); + let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #features),* ))]); + + (attr, names) + } + }) + .collect::>(); + + let all_pallets_without_system = attribute_to_names.iter().map(|(attr, names)| { let names = names.iter().filter(|n| **n != SYSTEM_PALLET_NAME); quote! { #attr @@ -382,11 +407,7 @@ fn decl_all_pallets<'a>( } }); - let all_pallets_with_system = names_by_feature.iter().map(|(feature_set, names)| { - let mut feature_set = feature_set.iter().collect::>(); - let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter(); - let feature_set = feature_set.into_iter(); - let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]); + let all_pallets_with_system = attribute_to_names.iter().map(|(attr, names)| { quote! { #attr /// All pallets included in the runtime as a nested tuple of types. @@ -394,28 +415,19 @@ fn decl_all_pallets<'a>( } }); - let all_pallets_without_system_reversed = - names_by_feature.iter().map(|(feature_set, names)| { - let mut feature_set = feature_set.iter().collect::>(); - let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter(); - let feature_set = feature_set.into_iter(); - let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]); - let names = names.iter().filter(|n| **n != SYSTEM_PALLET_NAME).rev(); - quote! { - #attr - /// All pallets included in the runtime as a nested tuple of types in reversed order. - /// Excludes the System pallet. - #[deprecated(note = "Using reverse pallet orders is deprecated. use only \ - `AllPalletWithSystem or AllPalletsWithoutSystem`")] - pub type AllPalletsWithoutSystemReversed = ( #(#names,)* ); - } - }); + let all_pallets_without_system_reversed = attribute_to_names.iter().map(|(attr, names)| { + let names = names.iter().filter(|n| **n != SYSTEM_PALLET_NAME).rev(); + quote! { + #attr + /// All pallets included in the runtime as a nested tuple of types in reversed order. + /// Excludes the System pallet. + #[deprecated(note = "Using reverse pallet orders is deprecated. use only \ + `AllPalletWithSystem or AllPalletsWithoutSystem`")] + pub type AllPalletsWithoutSystemReversed = ( #(#names,)* ); + } + }); - let all_pallets_with_system_reversed = names_by_feature.iter().map(|(feature_set, names)| { - let mut feature_set = feature_set.iter().collect::>(); - let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter(); - let feature_set = feature_set.into_iter(); - let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]); + let all_pallets_with_system_reversed = attribute_to_names.iter().map(|(attr, names)| { let names = names.iter().rev(); quote! { #attr @@ -426,35 +438,19 @@ fn decl_all_pallets<'a>( } }); - let system_pallet = - match names_by_feature[&Vec::new()].iter().find(|n| **n == SYSTEM_PALLET_NAME) { - Some(name) => name, - None => - return syn::Error::new( - proc_macro2::Span::call_site(), - "`System` pallet declaration is missing. \ - Please add this line: `System: frame_system,`", - ) - .into_compile_error(), - }; - - let all_pallets_reversed_with_system_first = - names_by_feature.iter().map(|(feature_set, names)| { - let mut feature_set = feature_set.iter().collect::>(); - let test_cfg = feature_set.remove(&&&"test").then_some(quote!(test)).into_iter(); - let feature_set = feature_set.into_iter(); - let attr = quote!(#[cfg(all( #(#test_cfg),* #(feature = #feature_set),* ))]); - let names = std::iter::once(system_pallet) - .chain(names.iter().rev().filter(|n| **n != SYSTEM_PALLET_NAME)); - quote! { - #attr - /// All pallets included in the runtime as a nested tuple of types in reversed order. - /// With the system pallet first. - #[deprecated(note = "Using reverse pallet orders is deprecated. use only \ - `AllPalletWithSystem or AllPalletsWithoutSystem`")] - pub type AllPalletsReversedWithSystemFirst = ( #(#names,)* ); - } - }); + let all_pallets_reversed_with_system_first = attribute_to_names.iter().map(|(attr, names)| { + let system = quote::format_ident!("{}", SYSTEM_PALLET_NAME); + let names = std::iter::once(&system) + .chain(names.iter().rev().filter(|n| **n != SYSTEM_PALLET_NAME).cloned()); + quote! { + #attr + /// All pallets included in the runtime as a nested tuple of types in reversed order. + /// With the system pallet first. + #[deprecated(note = "Using reverse pallet orders is deprecated. use only \ + `AllPalletWithSystem or AllPalletsWithoutSystem`")] + pub type AllPalletsReversedWithSystemFirst = ( #(#names,)* ); + } + }); quote!( #types diff --git a/substrate/frame/support/test/Cargo.toml b/substrate/frame/support/test/Cargo.toml index dd23d7e6b0..d7d3bfc98f 100644 --- a/substrate/frame/support/test/Cargo.toml +++ b/substrate/frame/support/test/Cargo.toml @@ -47,9 +47,10 @@ std = [ "sp-version/std", ] try-runtime = ["frame-support/try-runtime"] -# WARNING: CI only execute pallet test with this feature, -# if the feature intended to be used outside, CI and this message need to be updated. -conditional-storage = [] +# WARNING: +# Only CI runs with this feature enabled. This feature is for testing stuff related to the FRAME macros +# in conjunction with rust features. +frame-feature-testing = [] # Disable ui tests disable-ui-tests = [] no-metadata-docs = ["frame-support/no-metadata-docs"] diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index 907a52f118..d683399482 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -334,22 +334,22 @@ pub mod pallet { #[pallet::storage] #[pallet::getter(fn conditional_value)] - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] pub type ConditionalValue = StorageValue<_, u32>; - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] #[pallet::storage] #[pallet::getter(fn conditional_map)] pub type ConditionalMap = StorageMap<_, Twox64Concat, u16, u32, OptionQuery, GetDefault, ConstU32<12>>; - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] #[pallet::storage] #[pallet::getter(fn conditional_double_map)] pub type ConditionalDoubleMap = StorageDoubleMap<_, Blake2_128Concat, u8, Twox64Concat, u16, u32>; - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] #[pallet::storage] #[pallet::getter(fn conditional_nmap)] pub type ConditionalNMap = @@ -550,7 +550,9 @@ pub mod pallet2 { #[frame_support::pallet] pub mod pallet3 { #[pallet::config] - pub trait Config: frame_system::Config {} + pub trait Config: frame_system::Config::Origin> { + type Origin; + } #[pallet::pallet] pub struct Pallet(_); @@ -612,6 +614,11 @@ impl pallet2::Config for Runtime { impl pallet4::Config for Runtime {} +#[cfg(feature = "frame-feature-testing")] +impl pallet3::Config for Runtime { + type Origin = Origin; +} + pub type Header = sp_runtime::generic::Header; pub type Block = sp_runtime::generic::Block; pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; @@ -626,7 +633,7 @@ frame_support::construct_runtime!( System: frame_system exclude_parts { Pallet, Storage }, Example: pallet, Example2: pallet2 exclude_parts { Call }, - #[cfg(feature = "example3")] + #[cfg(feature = "frame-feature-testing")] Example3: pallet3, Example4: pallet4 use_parts { Call }, } @@ -1024,7 +1031,7 @@ fn storage_expand() { Err(pallet::Error::::NonExistentStorageValue), ); - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] { pallet::ConditionalValue::::put(1); pallet::ConditionalMap::::insert(1, 2); @@ -1164,8 +1171,10 @@ fn migrate_from_pallet_version_to_storage_version() { AllPalletsWithSystem, >(&db_weight); - // 4 pallets, 2 writes and every write costs 5 weight. - assert_eq!(Weight::from_ref_time(4 * 2 * 5), weight); + let pallet_num = if cfg!(feature = "frame-feature-testing") { 5 } else { 4 }; + + // `pallet_num` pallets, 2 writes and every write costs 5 weight. + assert_eq!(Weight::from_ref_time(pallet_num * 2 * 5), weight); // All pallet versions should be removed assert!(sp_io::storage::get(&pallet_version_key(Example::name())).is_none()); @@ -1332,7 +1341,7 @@ fn metadata() { default: vec![1, 1], docs: vec![], }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] StorageEntryMetadata { name: "ConditionalValue", modifier: StorageEntryModifier::Optional, @@ -1340,7 +1349,7 @@ fn metadata() { default: vec![0], docs: vec![], }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] StorageEntryMetadata { name: "ConditionalMap", modifier: StorageEntryModifier::Optional, @@ -1352,7 +1361,7 @@ fn metadata() { default: vec![0], docs: vec![], }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] StorageEntryMetadata { name: "ConditionalDoubleMap", modifier: StorageEntryModifier::Optional, @@ -1367,7 +1376,7 @@ fn metadata() { default: vec![0], docs: vec![], }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] StorageEntryMetadata { name: "ConditionalNMap", modifier: StorageEntryModifier::Optional, @@ -1489,6 +1498,16 @@ fn metadata() { constants: vec![], error: None, }, + #[cfg(feature = "frame-feature-testing")] + PalletMetadata { + index: 3, + name: "Example3", + storage: None, + calls: None, + event: None, + constants: vec![], + error: None, + }, ]; let empty_doc = pallets[0].event.as_ref().unwrap().ty.type_info().docs().is_empty() && @@ -1633,7 +1652,7 @@ fn test_storage_info() { max_values: None, max_size: Some(16 + 1 + 8 + 2 + 16), }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] { StorageInfo { pallet_name: b"Example".to_vec(), @@ -1643,7 +1662,7 @@ fn test_storage_info() { max_size: Some(4), } }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] { StorageInfo { pallet_name: b"Example".to_vec(), @@ -1653,7 +1672,7 @@ fn test_storage_info() { max_size: Some(8 + 2 + 4), } }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] { StorageInfo { pallet_name: b"Example".to_vec(), @@ -1663,7 +1682,7 @@ fn test_storage_info() { max_size: Some(16 + 1 + 8 + 2 + 4), } }, - #[cfg(feature = "conditional-storage")] + #[cfg(feature = "frame-feature-testing")] { StorageInfo { pallet_name: b"Example".to_vec(), @@ -1730,27 +1749,42 @@ fn assert_type_all_pallets_reversed_with_system_first_is_correct() { // Just ensure the 2 types are same. #[allow(deprecated)] fn _a(_t: AllPalletsReversedWithSystemFirst) {} + #[cfg(not(feature = "frame-feature-testing"))] fn _b(t: (System, Example4, Example2, Example)) { _a(t) } + #[cfg(feature = "frame-feature-testing")] + fn _b(t: (System, Example4, Example3, Example2, Example)) { + _a(t) + } } #[test] fn assert_type_all_pallets_with_system_is_correct() { // Just ensure the 2 types are same. fn _a(_t: AllPalletsWithSystem) {} + #[cfg(not(feature = "frame-feature-testing"))] fn _b(t: (System, Example, Example2, Example4)) { _a(t) } + #[cfg(feature = "frame-feature-testing")] + fn _b(t: (System, Example, Example2, Example3, Example4)) { + _a(t) + } } #[test] fn assert_type_all_pallets_without_system_is_correct() { // Just ensure the 2 types are same. fn _a(_t: AllPalletsWithoutSystem) {} + #[cfg(not(feature = "frame-feature-testing"))] fn _b(t: (Example, Example2, Example4)) { _a(t) } + #[cfg(feature = "frame-feature-testing")] + fn _b(t: (Example, Example2, Example3, Example4)) { + _a(t) + } } #[test] @@ -1758,9 +1792,14 @@ fn assert_type_all_pallets_with_system_reversed_is_correct() { // Just ensure the 2 types are same. #[allow(deprecated)] fn _a(_t: AllPalletsWithSystemReversed) {} + #[cfg(not(feature = "frame-feature-testing"))] fn _b(t: (Example4, Example2, Example, System)) { _a(t) } + #[cfg(feature = "frame-feature-testing")] + fn _b(t: (Example4, Example3, Example2, Example, System)) { + _a(t) + } } #[test] @@ -1768,9 +1807,14 @@ fn assert_type_all_pallets_without_system_reversed_is_correct() { // Just ensure the 2 types are same. #[allow(deprecated)] fn _a(_t: AllPalletsWithoutSystemReversed) {} + #[cfg(not(feature = "frame-feature-testing"))] fn _b(t: (Example4, Example2, Example)) { _a(t) } + #[cfg(feature = "frame-feature-testing")] + fn _b(t: (Example4, Example3, Example2, Example)) { + _a(t) + } } #[test] diff --git a/substrate/scripts/ci/gitlab/pipeline/test.yml b/substrate/scripts/ci/gitlab/pipeline/test.yml index 8036e2cec2..50916a37fc 100644 --- a/substrate/scripts/ci/gitlab/pipeline/test.yml +++ b/substrate/scripts/ci/gitlab/pipeline/test.yml @@ -251,7 +251,7 @@ test-frame-support: RUN_UI_TESTS: 1 script: - rusty-cachier snapshot create - - time cargo test -p frame-support-test --features=conditional-storage,no-metadata-docs --manifest-path ./frame/support/test/Cargo.toml --test pallet # does not reuse cache 1 min 44 sec + - time cargo test -p frame-support-test --features=frame-feature-testing,no-metadata-docs --manifest-path ./frame/support/test/Cargo.toml --test pallet # does not reuse cache 1 min 44 sec - SUBSTRATE_TEST_TIMEOUT=1 time cargo test -p substrate-test-utils --release --verbose --locked -- --ignored timeout - rusty-cachier cache upload