-
Notifications
You must be signed in to change notification settings - Fork 27
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
Feature: Pallet Engine (Add pallet - MVP) #43
Changes from all commits
5f60e9a
3539460
ee058b3
e7b791a
c3dcfa0
60cee28
0cfec78
685f3c7
6dd21d1
4214d2c
7e81cc2
69d9491
2e6aef6
bc788de
83d2d2c
cc3ca54
0d40634
736deb4
85a7e47
e4eb10f
3e2c150
13c84bb
a588c92
d066526
8932833
7c2b7ff
70881b8
2182ade
023fdbf
c158721
a6e713f
ea01ead
8ab1ba9
1e6d690
7304889
aed5436
1f49718
49850b2
a39b1c2
a2532ad
19c7637
5a0e6b4
b80cb50
b122568
9ff1318
1bb0385
33c2a95
3c55dac
7996257
c6d1991
1d92314
5f806d8
4176187
f5ff852
96401ed
215b028
354116e
99f4b64
0e682f5
159b998
4309bb2
0363844
7e7478d
6f7ddd8
a939826
0754e6a
75fceb9
e93dc0c
44eed89
dd05112
e6733e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
#![cfg(feature = "parachain")] | ||
use std::path::PathBuf; | ||
|
||
use crate::engines::pallet_engine; | ||
use clap::{Args, Subcommand}; | ||
use cliclack::{intro, outro}; | ||
use console::style; | ||
|
||
#[derive(Args)] | ||
#[command(args_conflicts_with_subcommands = true)] | ||
pub(crate) struct AddArgs { | ||
#[command(subcommand)] | ||
commands: AddCommands, | ||
#[arg(global = true, short, long = "runtime")] | ||
/// Runtime path; for example: `sub0/runtime/src/lib.rs` | ||
/// Runtime cargo manifest path will be inferred as `(parent of lib.rs)/Cargo.toml` | ||
pub(crate) runtime_path: Option<String>, | ||
} | ||
#[derive(Subcommand)] | ||
#[command(subcommand_required = true)] | ||
pub(crate) enum AddCommands { | ||
#[command(subcommand)] | ||
weezy20 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// Add a pallet | ||
#[clap(alias = "p")] | ||
Pallet(AddPallet), | ||
} | ||
|
||
#[derive(Subcommand, Clone)] | ||
#[command(subcommand_required = true)] | ||
pub(crate) enum AddPallet { | ||
/// Insert `pallet-parachain-template` into the runtime. | ||
Template, | ||
/// Insert a frame-pallet into the runtime. | ||
Frame(FrameArgs), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally don’t like to have the FRAME logic there if is not working. It might be confusing for the user. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would the user be confused when they're met with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With git there is no need to rewrite things, just take it from here into another branch. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I have to disagree with that approach. Doesn't solve anything. |
||
} | ||
|
||
#[derive(Args, Clone)] | ||
pub(crate) struct FrameArgs { | ||
#[arg(short, long)] | ||
// TODO: Not ready for use | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, if doesn't work might be better delete it |
||
pub(crate) name: String, | ||
} | ||
|
||
impl AddArgs { | ||
pub(crate) fn execute(&self) -> anyhow::Result<()> { | ||
match self.commands { | ||
AddCommands::Pallet(ref cmd) => cmd.clone().execute(&self.runtime_path), | ||
} | ||
} | ||
} | ||
impl AddPallet { | ||
pub(crate) fn execute(self, runtime_path: &Option<String>) -> anyhow::Result<()> { | ||
let runtime_path = match runtime_path { | ||
Some(ref s) => { | ||
let path = PathBuf::from(s); | ||
if !path.exists() { | ||
anyhow::bail!("Invalid runtime path: {}", path.display()); | ||
} | ||
path | ||
}, | ||
None => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For UX, Is not better to use clap errors?. Will be add If i run
If I run
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. This makes more sense when we have a functional cache.. Just for the record,reason to use an Option here is to rely on automatic inference via the |
||
// TODO: Fetch runtime either from cache | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For TODOs is better to create an Issue, than lines of code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of it is already in the issue and PR description. These are just placeholder comments from where future work can be easily picked up where these were left off. |
||
unimplemented!( | ||
"provide a runtime path until feat:cache is implemented: --runtime <path>" | ||
); | ||
}, | ||
}; | ||
let pallet = match self { | ||
AddPallet::Template => "pallet-parachain-template".to_string(), | ||
AddPallet::Frame(FrameArgs { .. }) => { | ||
eprintln!("Sorry, frame pallets cannot be added right now"); | ||
std::process::exit(1); | ||
// format!("FRAME pallet-{name}") | ||
}, | ||
}; | ||
intro(format!( | ||
"{}: Adding pallet \"{}\"!", | ||
style(" Pop CLI ").black().on_magenta(), | ||
&pallet, | ||
))?; | ||
pallet_engine::execute(self, runtime_path.clone())?; | ||
outro(format!("Added {}\n-> to {}", pallet, runtime_path.display()))?; | ||
Ok(()) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
pub(crate) mod add; | ||
pub(crate) mod build; | ||
pub(crate) mod call; | ||
pub(crate) mod new; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
//! Dependency representations for Pallets | ||
use std::path::{self, Path, PathBuf}; | ||
use strum_macros::{Display, EnumString}; | ||
|
||
#[derive(EnumString, Display, Debug)] | ||
pub(super) enum Features { | ||
#[strum(serialize = "std")] | ||
Std, | ||
#[strum(serialize = "runtime-benchmarks")] | ||
RuntimeBenchmarks, | ||
#[strum(serialize = "try-runtime")] | ||
TryRuntime, | ||
/// Custom feature | ||
Custom(String), | ||
} | ||
#[derive(Display, Debug, Clone)] | ||
pub(super) enum Location { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is everything for Location needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
/// Local path is form `path = "X"` | ||
Local(std::path::PathBuf, Option<semver::Version>), | ||
/// `git = {url}` | ||
Git(reqwest::Url, Option<semver::Version>), | ||
weezy20 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// String should be of format `version = "X"` | ||
CratesIO(semver::Version), | ||
} | ||
impl From<reqwest::Url> for Location { | ||
fn from(url: reqwest::Url) -> Self { | ||
Self::Git(url, None) | ||
} | ||
} | ||
impl From<std::path::PathBuf> for Location { | ||
fn from(path: std::path::PathBuf) -> Self { | ||
Self::Local(path, None) | ||
} | ||
} | ||
impl From<(std::path::PathBuf, semver::Version)> for Location { | ||
fn from(info: (std::path::PathBuf, semver::Version)) -> Self { | ||
Self::Local(info.0, Some(info.1)) | ||
} | ||
} | ||
impl<'a> From<&'a std::path::Path> for Location { | ||
fn from(path: &'a std::path::Path) -> Self { | ||
Self::Local(path.to_path_buf(), None) | ||
} | ||
} | ||
impl<'a> From<(&'a std::path::Path, semver::Version)> for Location { | ||
fn from(info: (&'a std::path::Path, semver::Version)) -> Self { | ||
Self::Local(info.0.to_path_buf(), Some(info.1.into())) | ||
} | ||
} | ||
impl From<semver::Version> for Location { | ||
fn from(version: semver::Version) -> Self { | ||
Self::CratesIO(version) | ||
} | ||
} | ||
impl Into<String> for Location { | ||
fn into(self) -> String { | ||
match self { | ||
Location::Local(path, Some(version)) => { | ||
format!("{{ path = \"{}\", version = \"{}\" }}", path.display(), version) | ||
}, | ||
Location::Local(path, _) => format!("{{ path = \"{}\" }}", path.display()), | ||
Location::Git(url, Some(version)) => { | ||
format!("{{ git = \"{}\", version = \"{}\" }}", url, version) | ||
}, | ||
Location::Git(url, _) => format!("{{ git = \"{}\" }}", url), | ||
Location::CratesIO(version) => format!("{{ version = \"{}\" }}", version), | ||
} | ||
} | ||
} | ||
impl Into<toml_edit::Value> for Location { | ||
fn into(self) -> toml_edit::Value { | ||
let s = Into::<String>::into(self); | ||
let t = s | ||
.parse::<toml_edit::Value>() | ||
.expect("Location String parse as Toml Value failed"); | ||
toml_edit::Value::InlineTable( | ||
t.as_inline_table().expect(" Parsed Location -> ILT cast infallible").to_owned(), | ||
) | ||
} | ||
} | ||
|
||
#[derive(Debug)] | ||
pub(super) struct Dependency { | ||
/// Name for the dependency | ||
pub(super) name: String, | ||
/// Additional features that need to be enabled. Format -> {name}/{feature} | ||
pub(super) features: Vec<Features>, | ||
/// Maybe local path, git url, or from crates.io in which case we will use this for version | ||
pub(super) path: Location, | ||
/// Default features such as `std` are disabled by default for runtime pallet dependencies | ||
pub(super) default_features: bool, | ||
} | ||
|
||
impl Dependency { | ||
/// Generate the main dependency as an inline table | ||
pub(super) fn entry(&self) -> toml_edit::InlineTable { | ||
let mut t = toml_edit::Table::new(); | ||
let location = Into::<toml_edit::Value>::into(self.path.clone()); | ||
t.extend( | ||
location | ||
.as_inline_table() | ||
.expect("Location to String should produce valid inline table") | ||
.to_owned(), | ||
); | ||
t["default-features"] = toml_edit::value(self.default_features); | ||
t.into_inline_table() | ||
} | ||
/// Create dependencies required for adding a local pallet to runtime | ||
/// $(workspace_root)/pallets/pallet-name | ||
// Todo: Some values are hardcoded for now as this only deals with the pallet-parachain-template | ||
// Should be moved to args once that's not the case | ||
pub(super) fn local_template_runtime(pallet_name: &'static str, workspace: &PathBuf) -> Self { | ||
let name = format!("{pallet_name}"); | ||
Self { | ||
features: vec![Features::RuntimeBenchmarks, Features::TryRuntime, Features::Std], | ||
path: ( | ||
// The reason is, `pop new pallet` places a new pallet on $(workspace_root)/pallets | ||
workspace.join("pallets/").join(&name).to_path_buf(), | ||
semver::Version::new(1, 0, 0), | ||
) | ||
.into(), | ||
default_features: false, | ||
name, | ||
} | ||
} | ||
// TODO: Remove code - Node doesn't require template pallet deps by default | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As it says in the TODO: remove the code |
||
// but this maybe desirable for custom pallets. | ||
// /// Create dependencies required for adding a pallet-parachain-template to node | ||
// pub(super) fn template_node() -> Self { | ||
// Self { | ||
// features: vec![Features::RuntimeBenchmarks, Features::TryRuntime], | ||
// // TODO hardcode for now | ||
// path: format!("../pallets/pallet-parachain-template"), | ||
// default_features: true, | ||
// } | ||
// } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as Cargo.toml is infered maybe for UX we can add the whole runtime path too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow what you're saying