Use umbrella crate for minimal template #5155
Conversation
pgherveou
added
R0-silent
|
Some of the clippy warnings seem to come from the |
Yeah this is weird as the experimental feature seems to be propagated correctly, clippy works fine locally too 🤔 |
There was a problem hiding this comment.
I think we are waiting for a new release before we can merge this?
also would like to see https://github.com/paritytech/polkadot-sdk/pull/5155/files#r1703858118 addressed.
This reverts commit 69d998e.
| 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> { |
There was a problem hiding this comment.
Merge error? This shouldn't be necessary?
There was a problem hiding this comment.
Curious. I guess due to the difference in toolchain version then.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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 ...
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
note to self: maybe this can be rolled back as well
Co-authored-by: kianenigma <kian@parity.io> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
* master: (39 commits) short-term fix for para inherent weight overestimation (#5082) CI: Add backporting bot (#4795) Fix benchmark failures when using `insecure_zero_ed` flag (#5354) Command bot GHA v2 - /cmd <cmd> (#5457) Remove pallet::getter usage from treasury (#4962) Bump blake2b_simd from 1.0.1 to 1.0.2 (#5404) Bump rustversion from 1.0.14 to 1.0.17 (#5405) Bridge zombienet tests: remove old command (#5434) polkadot-parachain: Add omni-node variant with u64 block number (#5269) Refactor verbose test (#5506) Use umbrella crate for minimal template (#5155) IBP Coretime Polkadot bootnodes (#5499) rpc server: listen to `ipv6 socket` if available and `--experimental-rpc-endpoint` CLI option (#4792) Update approval-voting-regression-bench (#5504) change try-runtime rpc domains (#5443) polkadot-parachain-bin: Remove contracts parachain (#5471) Add feature to allow Aura collator to use full PoV size (#5393) Adding stkd bootnodes (#5470) Make `PendingConfigs` storage item public (#5467) frame-omni-bencher maintenance (#5466) ...
No description provided.