From dcf82e41209fc5d61083739c004ccd6408b1604f Mon Sep 17 00:00:00 2001 From: Abhishek Shah Date: Fri, 8 Mar 2024 15:13:30 +0530 Subject: [PATCH] fix more runtime errors --- Cargo.lock | 1 + Cargo.toml | 1 + src/commands/add/mod.rs | 17 ++++++--- src/engines/pallet_engine/mod.rs | 23 ++++++++---- src/engines/pallet_engine/pallet_entry.rs | 1 + src/engines/pallet_engine/steps.rs | 43 ++++++++++++++--------- 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 982d60d9..ed4ddbd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4452,6 +4452,7 @@ dependencies = [ "toml_edit 0.22.6", "tracing-subscriber 0.3.18", "url", + "uuid", "walkdir", "zombienet-sdk", "zombienet-support", diff --git a/Cargo.toml b/Cargo.toml index 1c9fb56b..004b7b98 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ tracing-subscriber = { version = "0.3", optional = true } url = { version = "2.5", optional = true } zombienet-sdk = { git = "https://github.com/paritytech/zombienet-sdk", optional = true } zombienet-support = { git = "https://github.com/paritytech/zombienet-sdk", optional = true } +uuid = { version = "1.7.0", features = ["v4"] } [features] diff --git a/src/commands/add/mod.rs b/src/commands/add/mod.rs index a6b76bed..c8de71c5 100644 --- a/src/commands/add/mod.rs +++ b/src/commands/add/mod.rs @@ -10,7 +10,7 @@ pub(crate) struct AddArgs { #[command(subcommand)] /// Pallet to add to the runtime pub(crate) pallet: AddPallet, - #[arg(global = true, short)] + #[arg(global = true, short, long)] /// Runtime path; /// Cargo Manifest path will be inferred as `../Cargo.toml` pub(crate) runtime: Option, @@ -35,11 +35,20 @@ pub(crate) struct FrameArgs { impl AddArgs { pub(crate) fn execute(&self) -> anyhow::Result<()> { let runtime_path = match self.runtime { - Some(ref s) => PathBuf::from(s), + Some(ref s) => { + let path = PathBuf::from(s); + // println!("Using runtime path: {}", &path.display()); + if !path.exists() { + anyhow::bail!("Runtime path does not exist: {}", path.display()); + } + path + } None => { // TODO: Fetch runtime either from cache // Fix: This is a placeholder path, should not be used - PathBuf::from("my-app/runtime/src/lib.rs") + unimplemented!( + "provide a runtime path until cache is implemented: --runtime " + ); } }; let pallet = match self.pallet { @@ -48,7 +57,7 @@ impl AddArgs { eprintln!("Sorry, frame pallets cannot be added right now"); std::process::exit(1); // format!("FRAME-pallet-{name}") - }, + } }; pallet_engine::execute(self.pallet.clone(), runtime_path.clone())?; println!("Added {}\n-> to {}", pallet, runtime_path.display()); diff --git a/src/engines/pallet_engine/mod.rs b/src/engines/pallet_engine/mod.rs index edd3ef8c..cc666f6c 100644 --- a/src/engines/pallet_engine/mod.rs +++ b/src/engines/pallet_engine/mod.rs @@ -79,6 +79,12 @@ pub struct PalletEngine { /// Cursor for tracking where we are in the output cursor: usize, } +impl Drop for PalletEngine { + fn drop(&mut self) { + let output_dir = self.output.parent().unwrap(); + let _ = fs::remove_dir_all(output_dir); + } +} /// PalletDetails is data generated after parsing of a given `input` runtime file /// This will make observations as to which pallets are there, if there are instances of the pallets @@ -116,18 +122,19 @@ impl PalletEngine { /// Call this to finalize edits pub fn merge(self) -> anyhow::Result<()> { fs::copy(&self.output, &self.input)?; - fs::remove_file(self.output); + fs::remove_file(&self.output); Ok(()) } /// Create a new PalletEngine pub fn new(input: &PathBuf) -> anyhow::Result { - let tmp_dir = tempfile::TempDir::new()?; - let output: PathBuf = tmp_dir.path().join("temp_lib.rs"); + let tmp_dir = PathBuf::from(format!("/tmp/pallet_engine_{}", uuid::Uuid::new_v4())); + fs::create_dir(&tmp_dir).context("Failed to create temporary directory for PalletEngine")?; + let output: PathBuf = tmp_dir.join("out_lib.rs"); // Open the file specified in `output`. If non-empty, delete its contents. if output.exists() && output.is_file() { std::fs::remove_file(output.as_path())?; } - File::create(output.as_path()).context(format!( + File::create(&output).context(format!( "Failed to create PalletEngine with output: {}", output.display() ))?; @@ -363,7 +370,10 @@ impl PalletEngine { snip.push_str(&line?); snip.push('\n'); } - let mut file = OpenOptions::new().append(true).open(&self.output)?; + let mut file = OpenOptions::new() + .append(true) + .open(&self.output) + .context("fn append_lines_from - cannot open output")?; file.write_all(snip.as_bytes())?; Ok(()) } @@ -448,13 +458,12 @@ impl PalletEngine { let mut ultimate = i .pallets .last() - .expect("Fatal: No pallets defined in construct_runtime!") + .ok_or(anyhow!("Fatal: No pallets defined in construct_runtime!"))? .clone(); ultimate.index = new_pallet.index; ultimate.path.inner.segments[0].ident = new_pallet.path; ultimate.name = new_pallet.name; i.pallets.push(ultimate); - Ok(()) } RuntimeDeclaration::Explicit(e) => { todo!() diff --git a/src/engines/pallet_engine/pallet_entry.rs b/src/engines/pallet_engine/pallet_entry.rs index 94b755e8..a3fa7331 100644 --- a/src/engines/pallet_engine/pallet_entry.rs +++ b/src/engines/pallet_engine/pallet_entry.rs @@ -2,6 +2,7 @@ use syn::Ident; /// Format containing necessary information for appending pallets +#[derive(Debug)] pub(super) struct AddPalletEntry { pub(super) index: Option, pub(super) path: Ident, diff --git a/src/engines/pallet_engine/steps.rs b/src/engines/pallet_engine/steps.rs index 6bb307b6..4bf4a2d3 100644 --- a/src/engines/pallet_engine/steps.rs +++ b/src/engines/pallet_engine/steps.rs @@ -6,7 +6,9 @@ use log::{error, warn}; use proc_macro2::TokenStream as TokenStream2; use quote::quote; use Steps::*; +use super::State; /// Define the steps needed for a particular pallet insertion +#[derive(Debug)] pub(super) enum Steps { /// Import statements for pallet RuntimePalletImport(TokenStream2), @@ -25,13 +27,10 @@ pub(super) enum Steps { ChainspecGenesisImport(TokenStream2), /// Node specific imports if the above two are required NodePalletDependency(Dependency), - /// PalletEngine Specific Commands - Commands, -} -enum Commands { - SwitchToConfig, - SwitchToCRT, + /// PalletEngine State transitions + SwitchTo(State), } + macro_rules! steps { ($cmd:expr) => { steps.push($cmd); @@ -46,18 +45,18 @@ pub(super) fn step_builder(pallet: AddPallet) -> Result> { match pallet { // Adding a pallet-parachain-template requires 5 distinct steps AddPallet::Template => { - steps.push(RuntimePalletDependency(Dependency::runtime_template())); + // steps.push(RuntimePalletDependency(Dependency::runtime_template())); steps.push(RuntimePalletImport(quote!( pub use pallet_parachain_template; ))); - steps.push(SwitchToConfig); + steps.push(SwitchTo(State::Config)); steps.push(RuntimePalletConfiguration(quote!( /// Configure the pallet template in pallets/template. impl pallet_parachain_template::Config for Runtime { type RuntimeEvent = RuntimeEvent; } ))); - steps.push(SwitchToCrt); + steps.push(SwitchTo(State::ConstructRuntime)); steps.push(ConstructRuntimeEntry(AddPalletEntry::new( // Index None, @@ -67,7 +66,7 @@ pub(super) fn step_builder(pallet: AddPallet) -> Result> { // TODO (high priority): implement name conflict resolution strategy "Template", ))); - steps.push(NodePalletDependency(Dependency::node_template())) + // steps.push(NodePalletDependency(Dependency::node_template())) } AddPallet::Frame(_) => unimplemented!("Frame pallets not yet implemented"), }; @@ -76,6 +75,7 @@ pub(super) fn step_builder(pallet: AddPallet) -> Result> { /// Execute steps on PalletEngine. /// Each execution edits a file. /// Sequence of steps matters so take care when ordering them +/// Works only for Template pallets at the moment.. See config and CRT inserts pub(super) fn run_steps(mut pe: PalletEngine, steps: Vec) -> Result<()> { use super::State::*; pe.prepare_output()?; @@ -101,32 +101,40 @@ pub(super) fn run_steps(mut pe: PalletEngine, steps: Vec) -> Result<()> { } }; } - SwitchToConfig => pe.prepare_config()?, + SwitchTo(State::Config) => pe.prepare_config()?, RuntimePalletConfiguration(config) => { if pe.state != Config { // Not really a fatal error, but may cause unexpected behaviour warn!("Engine not in Config state, executing config insertion anyways"); } pe.insert_config(config)? - }, - SwitchToCRT => pe.prepare_crt()?, - ConstructRuntimeEntry(p) => pe.add_pallet_runtime(p)?, + } + SwitchTo(State::ConstructRuntime) => pe.prepare_crt()?, + ConstructRuntimeEntry(_entry) => { + // TODO : Switch to add_pallet_runtime + // pe.add_pallet_runtime(entry)? + pe.insert_str_runtime("\t\tTemplate: pallet_parachain_template = 100,")?; + } + // ListBenchmarks(step) => pe.insert(step), // ListBenchmarks(step) => pe.insert(step), // ChainspecGenesisConfig(step) => pe.insert(step), // ChainspecGenesisImport(step) => pe.insert(step), // NodePalletDependency(step) => pe.insert(step), - _ => { - unimplemented!() + step => { + unimplemented!("{step:?} unimplemented") } }; // -- match -- } // -- for -- + // Finalize runtime edits + pe.merge()?; + // TODO: Finalize toml and chainspec edits Ok(()) } mod dependency { use strum_macros::{Display, EnumString}; - #[derive(EnumString, Display)] + #[derive(EnumString, Display, Debug)] pub(in crate::engines::pallet_engine) enum Features { #[strum(serialize = "std")] Std, @@ -136,6 +144,7 @@ mod dependency { TryRuntime, Custom(String), } + #[derive(Debug)] pub(in crate::engines::pallet_engine) struct Dependency { features: Vec, path: String,