Skip to content

Commit

Permalink
[sozo] Detect and manage manifests and artifacts discrepancies (#1672)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
remybar authored Mar 19, 2024
1 parent fe050bb commit 2aec873
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 16 deletions.
3 changes: 3 additions & 0 deletions bin/sozo/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<MigrateArgs>),
Expand Down
55 changes: 55 additions & 0 deletions bin/sozo/src/commands/clean.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
}
2 changes: 2 additions & 0 deletions bin/sozo/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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),
Expand Down
40 changes: 36 additions & 4 deletions bin/sozo/src/ops/migration/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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}"))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}"));
Expand All @@ -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`."
)
}
13 changes: 6 additions & 7 deletions crates/dojo-world/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down Expand Up @@ -251,11 +253,8 @@ impl BaseManifest {
let contract_dir = path.join("contracts");
let model_dir = path.join("models");

let world: Manifest<Class> =
toml::from_str(&fs::read_to_string(path.join("world.toml"))?).unwrap();
let base: Manifest<Class> =
toml::from_str(&fs::read_to_string(path.join("base.toml"))?).unwrap();

let world: Manifest<Class> = toml::from_str(&fs::read_to_string(path.join("world.toml"))?)?;
let base: Manifest<Class> = toml::from_str(&fs::read_to_string(path.join("base.toml"))?)?;
let contracts = elements_from_path::<DojoContract>(&contract_dir)?;
let models = elements_from_path::<DojoModel>(&model_dir)?;

Expand Down Expand Up @@ -596,7 +595,7 @@ where

for path in entries {
if path.is_file() {
let manifest: Manifest<T> = toml::from_str(&fs::read_to_string(path)?).unwrap();
let manifest: Manifest<T> = toml::from_str(&fs::read_to_string(path)?)?;
elements.push(manifest);
} else {
continue;
Expand All @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions crates/dojo-world/src/migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub enum MigrationError<S> {
Provider(#[from] ProviderError),
#[error(transparent)]
WaitingError(#[from] TransactionWaitingError),
#[error(transparent)]
ArtifactError(#[from] anyhow::Error),
}

/// Represents the type of migration that should be performed.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2aec873

Please sign in to comment.