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

Use umbrella crate for minimal template #5155

Merged
merged 59 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
275b4d3
Use umbrella crate for minimal template
pgherveou Jul 26, 2024
6630adc
Add missing cargo changes
pgherveou Jul 26, 2024
93d6364
format cargo.toml with taplo
pgherveou Jul 26, 2024
694294d
Update cargo.toml formatting (2)
pgherveou Jul 26, 2024
a7c49f0
remove minimal/Cargo.toml
pgherveou Jul 26, 2024
b6d704d
Update cargo.toml
pgherveou Jul 26, 2024
64910cf
Add experimental feature flag
pgherveou Jul 26, 2024
1ef3029
Fix minimal template pallet::config check
pgherveou Jul 28, 2024
59167aa
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
pgherveou Jul 28, 2024
d96d3b2
a few fixes from kian
kianenigma Aug 13, 2024
4341776
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 13, 2024
8cea3d8
fix toml
kianenigma Aug 13, 2024
122d3f2
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 14, 2024
ce2238c
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 15, 2024
0cdcdcf
Merge branch 'master' of github.com:paritytech/polkadot-sdk into pg/u…
kianenigma Aug 15, 2024
472c7d9
fix and prdoc
kianenigma Aug 15, 2024
24d5e5c
Merge branch 'pg/use-umbrella-crate-for-minimal-template' of github.c…
kianenigma Aug 15, 2024
903ab6f
Merge branch 'master' of github.com:paritytech/polkadot-sdk into pg/u…
kianenigma Aug 16, 2024
4a4e46a
docs
kianenigma Aug 16, 2024
53fa677
fix feature
kianenigma Aug 16, 2024
24cf78c
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 16, 2024
e9cbcb3
Test fix CI clippy job
pgherveou Aug 20, 2024
525abf6
Tweak experimental flag for tests
pgherveou Aug 20, 2024
4a60415
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
pgherveou Aug 20, 2024
9b71984
Fix clippy
pgherveou Aug 21, 2024
ea08cc2
Fix clippy 2
pgherveou Aug 21, 2024
4867813
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
pgherveou Aug 21, 2024
9f60b3a
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 21, 2024
9867d6b
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 22, 2024
3e5f624
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 23, 2024
6a9ad9e
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
pgherveou Aug 23, 2024
3f88968
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 23, 2024
09fd0a5
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 25, 2024
055ee73
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 26, 2024
9076909
use no-default-features for anyhow
pgherveou Aug 26, 2024
e987766
taplo fix
pgherveou Aug 26, 2024
0f5e103
fix revive
pgherveou Aug 26, 2024
6093140
Do not add pallet-contracts when arch = riscv
pgherveou Aug 26, 2024
5d270ac
fixed anyhow dep
pgherveou Aug 26, 2024
3ede08d
Fix prdoc
pgherveou Aug 26, 2024
69968aa
Patch generate-umbrella for crate that do not support riscv
pgherveou Aug 26, 2024
7b1d5e6
Add missing pallet prdoc complains about
pgherveou Aug 26, 2024
bb7f6f9
add missing :
pgherveou Aug 26, 2024
2829fc5
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 26, 2024
3bc01aa
fix fmt
pgherveou Aug 26, 2024
69d998e
fix missing default features
pgherveou Aug 26, 2024
f87dd47
Revert "fix missing default features"
pgherveou Aug 27, 2024
1dc603d
Update Cargo.toml
pgherveou Aug 27, 2024
5928b87
fixes
pgherveou Aug 27, 2024
f6c0d2b
fix
pgherveou Aug 27, 2024
4f4256a
Rollback changes and update polkadot-sdk feature flags
pgherveou Aug 27, 2024
6c3f759
Update substrate/frame/revive/src/wasm/runtime.rs
pgherveou Aug 27, 2024
6fc20c3
rollback package.metadata.polkadot-sdk changes
pgherveou Aug 27, 2024
4677583
use runtime-full instead of runtime
pgherveou Aug 27, 2024
7031a43
Update umbrella
pgherveou Aug 27, 2024
80f4dd0
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
kianenigma Aug 27, 2024
5eff776
Update features runtime
pgherveou Aug 27, 2024
bb30654
Merge branch 'master' into pg/use-umbrella-crate-for-minimal-template
pgherveou Aug 28, 2024
fac0344
Update Cargo.lock
pgherveou Aug 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 3 additions & 34 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ ahash = { version = "0.8.2" }
alloy-primitives = { version = "0.4.2", default-features = false }
alloy-sol-types = { version = "0.4.2", default-features = false }
always-assert = { version = "0.1" }
anyhow = { version = "1.0.81" }
anyhow = { version = "1.0.81", default-features = false }
aquamarine = { version = "0.5.0" }
arbitrary = { version = "1.3.2" }
ark-bls12-377 = { version = "0.4.0", default-features = false }
Expand Down
2 changes: 1 addition & 1 deletion bridges/relays/utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ publish = false
workspace = true

[dependencies]
anyhow = { workspace = true }
anyhow = { workspace = true, default-features = true }
async-std = { workspace = true }
async-trait = { workspace = true }
backoff = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion cumulus/client/consensus/proposer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
workspace = true

[dependencies]
anyhow = { workspace = true }
anyhow = { workspace = true, default-features = true }
async-trait = { workspace = true }
thiserror = { workspace = true }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ use frame::{
runtime,
},
prelude::*,
runtime::{
apis::{self, impl_runtime_apis, ExtrinsicInclusionMode},
prelude::*,
},
runtime::{apis, prelude::*},
};
use sp_genesis_builder::PresetId;

Expand Down
27 changes: 27 additions & 0 deletions prdoc/pr_5155.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Use umbrella crate for minimal template

doc:
- audience: Runtime Dev
description: |
Minor additions to the `polkadot-sdk-frame` crate and making it ready for usage in more templates. This PR already integrates it in the minimal template.


crates:
- name: polkadot-sdk
bump: patch
- name: polkadot-sdk-frame
bump: patch
- name: sp-wasm-interface
bump: patch
- name: pallet-revive
bump: patch
- name: pallet-revive-fixtures
bump: patch
- name: frame-support
bump: patch
- name: pallet-balances
bump: patch

28 changes: 23 additions & 5 deletions scripts/generate-umbrella.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ def exclude(crate):

return False


def no_riscv_target(crate):
if crate.name == "pallet-contracts":
return True
return False

def main(path, version):
delete_umbrella(path)
workspace = Workspace.from_path(path)
Expand Down Expand Up @@ -89,13 +95,21 @@ def main(path, version):
all_crates = std_crates + nostd_crates
all_crates.sort(key=lambda x: x[0].name)
dependencies = {}

target = {}
no_arch_riscv_deps = {}
target["cfg(not(target_arch = \"riscv32\"))"] = {}
target["cfg(not(target_arch = \"riscv32\"))"]["dependencies"] = no_arch_riscv_deps

def add_dependency(crate, path):
deps = no_arch_riscv_deps if no_riscv_target(crate) else dependencies
deps[crate.name] = {"path": f"../{path}", "default-features": False, "optional": True}

for (crate, path) in nostd_crates:
dependencies[crate.name] = {"path": f"../{path}", "default-features": False, "optional": True}
add_dependency(crate, path)

for (crate, path) in std_crates:
dependencies[crate.name] = {"path": f"../{path}", "default-features": False, "optional": True}

add_dependency(crate, path)
# The empty features are filled by Zepter
features = {
"default": [ "std" ],
Expand Down Expand Up @@ -126,6 +140,7 @@ def main(path, version):
},
"dependencies": dependencies,
"features": features,
"target": target,
}

umbrella_dir = os.path.join(workspace.path, "umbrella")
Expand Down Expand Up @@ -158,7 +173,10 @@ def main(path, version):
use = crate.name.replace("-", "_")
desc = crate.description if crate.description.endswith(".") else crate.description + "."
f.write(f'\n/// {desc}')
f.write(f'\n#[cfg(feature = "{crate.name}")]\n')
if no_riscv_target(crate):
f.write(f'\n#[cfg(all(feature = "{crate.name}", not(target_arch = "riscv32")))]\n')
else:
f.write(f'\n#[cfg(feature = "{crate.name}")]\n')
f.write(f"pub use {use};\n")

print(f"Wrote {lib_path}")
Expand Down
7 changes: 5 additions & 2 deletions substrate/frame/contracts/fixtures/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,24 @@ edition.workspace = true
license.workspace = true
description = "Fixtures for testing contracts pallet."

[package.metadata.polkadot-sdk]
exclude-from-umbrella = true

[lints]
workspace = true

[dependencies]
frame-system = { workspace = true, default-features = true }
sp-runtime = { workspace = true, default-features = true }
anyhow = { workspace = true }
anyhow = { workspace = true, default-features = true }

[build-dependencies]
parity-wasm = { workspace = true }
tempfile = { workspace = true }
toml = { workspace = true }
twox-hash = { workspace = true, default-features = true }
polkavm-linker = { workspace = true, optional = true }
anyhow = { workspace = true }
anyhow = { workspace = true, default-features = true }

[features]
riscv = ["polkavm-linker"]
9 changes: 8 additions & 1 deletion substrate/frame/revive/fixtures/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ frame-system = { workspace = true, default-features = true, optional = true }
sp-runtime = { workspace = true, default-features = true, optional = true }
anyhow = { workspace = true }

[package.metadata.polkadot-sdk]
exclude-from-umbrella = true

[build-dependencies]
parity-wasm = { workspace = true }
tempfile = { workspace = true }
Expand All @@ -29,4 +32,8 @@ default = ["std"]
# we will remove this once there is an upstream toolchain
riscv = []
# only when std is enabled all fixtures are available
std = ["frame-system", "sp-runtime"]
std = [
"anyhow/std",
"frame-system",
"sp-runtime",
]
2 changes: 1 addition & 1 deletion substrate/frame/revive/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ impl<'a, E: Ext, M: PolkaVmInstance<E::T>> Runtime<'a, E, M> {
}
}

impl<'a, E: Ext, M: ?Sized + Memory<E::T>> Runtime<'a, E, M> {
impl<'a, E: Ext + 'a, M: ?Sized + Memory<E::T>> Runtime<'a, E, M> {
Copy link
Member

Choose a reason for hiding this comment

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

Merge error? This shouldn't be necessary?

Copy link
Member

Choose a reason for hiding this comment

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

Curious. I guess due to the difference in toolchain version then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so it does now when we build

SUBSTRATE_RUNTIME_TARGET=riscv cargo check -p minimal-template-runtime

because it pulls the umbrella crate that tries to build all the crates deps including pallet-revive to riscv.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kianenigma that explains why we are seeing all these compilations errors here and not in other PRs.
We could pick only the features we need in the Cargo.toml if we want to avoid that ...

Copy link
Member

Choose a reason for hiding this comment

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

Hm, yea only selecting the needed dependencies should work.
It's a bit annoying though, since the umbrella crate is supposed to make things easier. I guess it is still easier than selecting each dependency version manually.

Would it be possible to feature-gate the RISCV compilation?

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 gated the pallets that were causing issue, the long term solution is probably to generate the umbrella trait with feature flags that declare all their dependencies, so that you can hand pick the one you need and optimize your build time

Copy link
Member

Choose a reason for hiding this comment

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

yea only selecting the needed dependencies should work.
It's a bit annoying though, since the umbrella crate is supposed to make things easier.

Couldn't we encode the dependency graph into the features in the script? Like you only activate the pallets you want and then they automatically activate the other pallets they depend on.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would not support making the usage of umbrella crate in any of the templates sub-optimal, for the sake of SUBSTRATE_RUNTIME_TARGET=riscv which is more of an experimental thing.

All people out there will not face the same compilation issue, and people do use these templates as a source of truth, so it should be in the simplest form possible.

pgherveou marked this conversation as resolved.
Show resolved Hide resolved
pub fn new(ext: &'a mut E, input_data: Vec<u8>) -> Self {
Self {
ext,
Expand Down
27 changes: 20 additions & 7 deletions substrate/frame/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,21 +177,34 @@ pub mod runtime {
pub use frame_executive::*;

/// Macro to amalgamate the runtime into `struct Runtime`.
///
/// Consider using the new version of this [`frame_construct_runtime`].
pub use frame_support::construct_runtime;

/// Macro to amalgamate the runtime into `struct Runtime`.
///
/// This is the newer version of [`construct_runtime`].
pub use frame_support::runtime as frame_construct_runtime;

/// Macro to easily derive the `Config` trait of various pallet for `Runtime`.
pub use frame_support::derive_impl;

/// Macros to easily impl traits such as `Get` for types.
// TODO: using linking in the Get in the line above triggers an ICE :/
pub use frame_support::{ord_parameter_types, parameter_types};

/// For building genesis config.
pub use frame_support::genesis_builder_helper::{build_state, get_preset};

/// Const types that can easily be used in conjuncture with `Get`.
pub use frame_support::traits::{
ConstBool, ConstI128, ConstI16, ConstI32, ConstI64, ConstI8, ConstU128, ConstU16,
ConstU32, ConstU64, ConstU8,
};

/// Used for simple fee calculation.
pub use frame_support::weights::{self, FixedFee, NoFee};

/// Primary types used to parameterize `EnsureOrigin` and `EnsureRootWithArg`.
pub use frame_system::{
EnsureNever, EnsureNone, EnsureRoot, EnsureRootWithSuccess, EnsureSigned,
Expand All @@ -201,11 +214,16 @@ pub mod runtime {
/// Types to define your runtime version.
pub use sp_version::{create_runtime_str, runtime_version, RuntimeVersion};

#[cfg(feature = "std")]
pub use sp_version::NativeVersion;

/// Macro to implement runtime APIs.
pub use sp_api::impl_runtime_apis;

#[cfg(feature = "std")]
pub use sp_version::NativeVersion;
// Types often used in the runtime APIs.
pub use sp_core::OpaqueMetadata;
pub use sp_inherents::{CheckInherentsResult, InherentData};
pub use sp_runtime::{ApplyExtrinsicResult, ExtrinsicInclusionMode};
}

/// Types and traits for runtimes that implement runtime APIs.
Expand All @@ -223,11 +241,6 @@ pub mod runtime {
// moved to file similarly.
#[allow(ambiguous_glob_reexports)]
pub mod apis {
// Types often used in the runtime APIs.
pub use sp_core::OpaqueMetadata;
pub use sp_inherents::{CheckInherentsResult, InherentData};
pub use sp_runtime::{ApplyExtrinsicResult, ExtrinsicInclusionMode};

pub use frame_system_rpc_runtime_api::*;
pub use sp_api::{self, *};
pub use sp_block_builder::*;
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note to self: maybe this can be rolled back as well

Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ use frame_support::derive_impl;

mod common;

use common::outer_enums::{pallet, pallet2};

pub type Header = sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>;
pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic<u32, RuntimeCall, (), ()>;
Expand Down Expand Up @@ -75,8 +73,10 @@ frame_support::construct_runtime!(
}
);

#[cfg(feature = "experimental")]
#[test]
fn module_error_outer_enum_expand_explicit() {
use common::outer_enums::{pallet, pallet2};
// The Runtime has *all* parts explicitly defined.

// Check that all error types are propagated
Expand All @@ -90,9 +90,7 @@ fn module_error_outer_enum_expand_explicit() {
frame_system::Error::NonZeroRefCount => (),
frame_system::Error::CallFiltered => (),
frame_system::Error::MultiBlockMigrationsOngoing => (),
#[cfg(feature = "experimental")]
frame_system::Error::InvalidTask => (),
#[cfg(feature = "experimental")]
frame_system::Error::FailedTask => (),
frame_system::Error::NothingAuthorized => (),
frame_system::Error::Unauthorized => (),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ use frame_support::derive_impl;

mod common;

use common::outer_enums::{pallet, pallet2};

pub type Header = sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>;
pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic<u32, RuntimeCall, (), ()>;
Expand Down Expand Up @@ -75,8 +73,10 @@ frame_support::construct_runtime!(
}
);

#[cfg(feature = "experimental")]
#[test]
fn module_error_outer_enum_expand_implicit() {
use common::outer_enums::{pallet, pallet2};
// The Runtime has *all* parts implicitly defined.

// Check that all error types are propagated
Expand All @@ -90,9 +90,7 @@ fn module_error_outer_enum_expand_implicit() {
frame_system::Error::NonZeroRefCount => (),
frame_system::Error::CallFiltered => (),
frame_system::Error::MultiBlockMigrationsOngoing => (),
#[cfg(feature = "experimental")]
frame_system::Error::InvalidTask => (),
#[cfg(feature = "experimental")]
frame_system::Error::FailedTask => (),
frame_system::Error::NothingAuthorized => (),
frame_system::Error::Unauthorized => (),
Expand Down
6 changes: 5 additions & 1 deletion substrate/primitives/wasm-interface/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,9 @@ anyhow = { optional = true, workspace = true }

[features]
default = ["std"]
std = ["codec/std", "log/std"]
std = [
"anyhow?/std",
"codec/std",
"log/std",
]
wasmtime = ["anyhow", "dep:wasmtime"]
Loading
Loading