From 2aec8739b6ac083e5f46cd871abee2f53dd1dd4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Baranx?= Date: Tue, 19 Mar 2024 13:26:47 +0700 Subject: [PATCH] =?UTF-8?q?[sozo]=C2=A0Detect=20and=20manage=20manifests?= =?UTF-8?q?=20and=20artifacts=20discrepancies=20=20(#1672)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit feat: detect and manage manifests and artifacts discrepancies `sozo` uses several file types (previously generated by `sozo build` and `sozo migrate`) as input for migration. All of these files may be corrupt or may have discrepancies due to an older format version. This PR introduces a new `sozo clean` command to clean the project of these files. `sozo clean --manifest-abis` cleans only manifest and ABI files. `sozo clean --artifacts` cleans only artifacts. When an anomaly is detected in a file, the user is informed of the problem and advised to run `sozo clean` to clean up the project. --- bin/sozo/src/args.rs | 3 ++ bin/sozo/src/commands/clean.rs | 55 ++++++++++++++++++++++++++ bin/sozo/src/commands/mod.rs | 2 + bin/sozo/src/ops/migration/mod.rs | 40 +++++++++++++++++-- crates/dojo-world/src/manifest.rs | 13 +++--- crates/dojo-world/src/migration/mod.rs | 9 ++--- 6 files changed, 106 insertions(+), 16 deletions(-) create mode 100644 bin/sozo/src/commands/clean.rs diff --git a/bin/sozo/src/args.rs b/bin/sozo/src/args.rs index 909c7ba10d..fc3c7c5f5b 100644 --- a/bin/sozo/src/args.rs +++ b/bin/sozo/src/args.rs @@ -9,6 +9,7 @@ use tracing_log::AsTrace; use crate::commands::auth::AuthArgs; use crate::commands::build::BuildArgs; +use crate::commands::clean::CleanArgs; use crate::commands::completions::CompletionsArgs; use crate::commands::dev::DevArgs; use crate::commands::events::EventsArgs; @@ -54,6 +55,8 @@ pub enum Commands { Build(BuildArgs), #[command(about = "Initialize a new project")] Init(InitArgs), + #[command(about = "Remove generated artifacts, manifests and abis")] + Clean(CleanArgs), #[command(about = "Run a migration, declaring and deploying contracts as necessary to \ update the world")] Migrate(Box), diff --git a/bin/sozo/src/commands/clean.rs b/bin/sozo/src/commands/clean.rs new file mode 100644 index 0000000000..5d0b12b0ff --- /dev/null +++ b/bin/sozo/src/commands/clean.rs @@ -0,0 +1,55 @@ +use std::fs; + +use anyhow::Result; +use camino::Utf8PathBuf; +use clap::Args; +use dojo_lang::compiler::{ABIS_DIR, MANIFESTS_DIR}; +use scarb::core::Config; + +#[derive(Debug, Args)] +pub struct CleanArgs { + #[arg(short, long)] + #[arg(help = "Remove manifests and abis only.")] + #[arg(long_help = "Remove manifests and abis only.")] + pub manifests_abis: bool, + + #[arg(short, long)] + #[arg(help = "Remove artifacts only.")] + #[arg(long_help = "Remove artifacts only.")] + pub artifacts: bool, +} + +impl CleanArgs { + pub fn clean_manifests_abis(&self, root_dir: &Utf8PathBuf) -> Result<()> { + let manifest_dir = root_dir.join(MANIFESTS_DIR); + let abis_dir = root_dir.join(ABIS_DIR); + + if manifest_dir.exists() { + fs::remove_dir_all(manifest_dir)?; + } + + if abis_dir.exists() { + fs::remove_dir_all(abis_dir)?; + } + + Ok(()) + } + + pub fn run(self, config: &Config) -> Result<()> { + let ws = scarb::ops::read_workspace(config.manifest_path(), config)?; + + let clean_manifests_abis = self.manifests_abis || !self.artifacts; + let clean_artifacts = self.artifacts || !self.manifests_abis; + + if clean_manifests_abis { + let manifest_dir = ws.manifest_path().parent().unwrap().to_path_buf(); + self.clean_manifests_abis(&manifest_dir)?; + } + + if clean_artifacts { + scarb::ops::clean(config)?; + } + + Ok(()) + } +} diff --git a/bin/sozo/src/commands/mod.rs b/bin/sozo/src/commands/mod.rs index 8281d5cc41..fce588da4f 100644 --- a/bin/sozo/src/commands/mod.rs +++ b/bin/sozo/src/commands/mod.rs @@ -5,6 +5,7 @@ use crate::args::Commands; pub(crate) mod auth; pub(crate) mod build; +pub(crate) mod clean; pub(crate) mod completions; pub(crate) mod dev; pub(crate) mod events; @@ -19,6 +20,7 @@ pub(crate) mod test; pub fn run(command: Commands, config: &Config) -> Result<()> { match command { Commands::Init(args) => args.run(config), + Commands::Clean(args) => args.run(config), Commands::Test(args) => args.run(config), Commands::Build(args) => args.run(config), Commands::Migrate(args) => args.run(config), diff --git a/bin/sozo/src/ops/migration/mod.rs b/bin/sozo/src/ops/migration/mod.rs index bba21db9c1..0452674cc2 100644 --- a/bin/sozo/src/ops/migration/mod.rs +++ b/bin/sozo/src/ops/migration/mod.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use anyhow::{anyhow, bail, Context, Result}; use camino::Utf8PathBuf; use dojo_lang::compiler::{ABIS_DIR, BASE_DIR, DEPLOYMENTS_DIR, MANIFESTS_DIR, OVERLAYS_DIR}; @@ -75,9 +77,14 @@ pub async fn execute( let target_dir = target_dir.join(ws.config().profile().as_str()); // Load local and remote World manifests. - let (local_manifest, remote_manifest) = - load_world_manifests(&manifest_dir, &account, world_address, &ui).await?; + load_world_manifests(&manifest_dir, &account, world_address, &ui).await.map_err(|e| { + ui.error(e.to_string()); + anyhow!( + "\n Use `sozo clean` to clean your project, or `sozo clean --manifests-abis` to \ + clean manifest and abi files only.\nThen, rebuild your project with `sozo build`.", + ) + })?; // Calculate diff between local and remote World manifests. @@ -330,12 +337,14 @@ where ui.print_step(1, "🌎", "Building World state..."); let mut local_manifest = - BaseManifest::load_from_path(&manifest_dir.join(MANIFESTS_DIR).join(BASE_DIR))?; + BaseManifest::load_from_path(&manifest_dir.join(MANIFESTS_DIR).join(BASE_DIR)) + .map_err(|_| anyhow!("Fail to load local manifest file."))?; let overlay_path = manifest_dir.join(MANIFESTS_DIR).join(OVERLAYS_DIR); if overlay_path.exists() { let overlay_manifest = - OverlayManifest::load_from_path(&manifest_dir.join(MANIFESTS_DIR).join(OVERLAYS_DIR))?; + OverlayManifest::load_from_path(&manifest_dir.join(MANIFESTS_DIR).join(OVERLAYS_DIR)) + .map_err(|_| anyhow!("Fail to load overlay manifest file."))?; // merge user defined changes to base manifest local_manifest.merge(overlay_manifest); @@ -429,6 +438,9 @@ where Err(MigrationError::ClassAlreadyDeclared) => { ui.print_sub(format!("Already declared: {:#x}", base.diff.local)); } + Err(MigrationError::ArtifactError(e)) => { + return Err(handle_artifact_error(&ui, base.artifact_path(), e)); + } Err(e) => { ui.verbose(format!("{e:?}")); return Err(e.into()); @@ -581,6 +593,9 @@ where Err(MigrationError::ContractAlreadyDeployed(contract_address)) => { Ok(ContractDeploymentOutput::AlreadyDeployed(contract_address)) } + Err(MigrationError::ArtifactError(e)) => { + return Err(handle_artifact_error(ui, contract.artifact_path(), e)); + } Err(e) => { ui.verbose(format!("{e:?}")); Err(anyhow!("Failed to migrate {contract_id}: {e}")) @@ -625,6 +640,9 @@ where ui.print_sub(format!("Already declared: {:#x}", c.diff.local)); continue; } + Err(MigrationError::ArtifactError(e)) => { + return Err(handle_artifact_error(ui, c.artifact_path(), e)); + } Err(e) => { ui.verbose(format!("{e:?}")); bail!("Failed to declare model {}: {e}", c.diff.name) @@ -705,6 +723,9 @@ where ui.print_sub(format!("Already deployed: {:#x}", contract_address)); deploy_output.push(None); } + Err(MigrationError::ArtifactError(e)) => { + return Err(handle_artifact_error(ui, contract.artifact_path(), e)); + } Err(e) => { ui.verbose(format!("{e:?}")); return Err(anyhow!("Failed to migrate {name}: {e}")); @@ -714,3 +735,14 @@ where Ok(deploy_output) } + +pub fn handle_artifact_error(ui: &Ui, artifact_path: &Path, error: anyhow::Error) -> anyhow::Error { + let path = artifact_path.to_string_lossy(); + let name = artifact_path.file_name().unwrap().to_string_lossy(); + ui.verbose(format!("{path}: {error:?}")); + + anyhow!( + "Discrepancy detected in {name}.\nUse `sozo clean` to clean your project or `sozo clean \ + --artifacts` to clean artifacts only.\nThen, rebuild your project with `sozo build`." + ) +} diff --git a/crates/dojo-world/src/manifest.rs b/crates/dojo-world/src/manifest.rs index 95e5f733a0..e4e0ce3d4d 100644 --- a/crates/dojo-world/src/manifest.rs +++ b/crates/dojo-world/src/manifest.rs @@ -51,6 +51,8 @@ pub enum AbstractManifestError { #[error(transparent)] Model(#[from] ModelError), #[error(transparent)] + TOML(#[from] toml::de::Error), + #[error(transparent)] IO(#[from] io::Error), } @@ -251,11 +253,8 @@ impl BaseManifest { let contract_dir = path.join("contracts"); let model_dir = path.join("models"); - let world: Manifest = - toml::from_str(&fs::read_to_string(path.join("world.toml"))?).unwrap(); - let base: Manifest = - toml::from_str(&fs::read_to_string(path.join("base.toml"))?).unwrap(); - + let world: Manifest = toml::from_str(&fs::read_to_string(path.join("world.toml"))?)?; + let base: Manifest = toml::from_str(&fs::read_to_string(path.join("base.toml"))?)?; let contracts = elements_from_path::(&contract_dir)?; let models = elements_from_path::(&model_dir)?; @@ -596,7 +595,7 @@ where for path in entries { if path.is_file() { - let manifest: Manifest = toml::from_str(&fs::read_to_string(path)?).unwrap(); + let manifest: Manifest = toml::from_str(&fs::read_to_string(path)?)?; elements.push(manifest); } else { continue; @@ -616,7 +615,7 @@ where let entry = entry?; let path = entry.path(); if path.is_file() { - let manifest: T = toml::from_str(&fs::read_to_string(path)?).unwrap(); + let manifest: T = toml::from_str(&fs::read_to_string(path)?)?; elements.push(manifest); } else { continue; diff --git a/crates/dojo-world/src/migration/mod.rs b/crates/dojo-world/src/migration/mod.rs index feeca7372d..26f61eae94 100644 --- a/crates/dojo-world/src/migration/mod.rs +++ b/crates/dojo-world/src/migration/mod.rs @@ -59,6 +59,8 @@ pub enum MigrationError { Provider(#[from] ProviderError), #[error(transparent)] WaitingError(#[from] TransactionWaitingError), + #[error(transparent)] + ArtifactError(#[from] anyhow::Error), } /// Represents the type of migration that should be performed. @@ -98,7 +100,7 @@ pub trait Declarable { S: Signer + Sync + Send, { let (flattened_class, casm_class_hash) = - prepare_contract_declaration_params(self.artifact_path()).unwrap(); + prepare_contract_declaration_params(self.artifact_path())?; match account .provider() @@ -146,10 +148,7 @@ pub trait Deployable: Declarable + Sync { let declare = match self.declare(account, txn_config).await { Ok(res) => Some(res), Err(MigrationError::ClassAlreadyDeclared) => None, - Err(e) => { - println!("{:?}", e); - return Err(e); - } + Err(e) => return Err(e), }; let base_class_hash = account