From b942414bb40a15a27a6695a863e7ae6092d90aab Mon Sep 17 00:00:00 2001 From: pedrocarlo Date: Mon, 9 Jun 2025 19:01:56 -0300 Subject: [PATCH 1/3] add anyhow to workspace dependency --- Cargo.lock | 4 ++-- Cargo.toml | 1 + cli/Cargo.toml | 2 +- tests/Cargo.toml | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f81e1a76..8d5c06fea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -152,9 +152,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.97" +version = "1.0.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dcfed56ad506cb2c684a14971b8861fdc3baaaae314b9e5f9bb532cbe3ba7a4f" +checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" [[package]] name = "arrayref" diff --git a/Cargo.toml b/Cargo.toml index 9fe7c5c96..330c331a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,6 +56,7 @@ strum = { version = "0.26", features = ["derive"] } strum_macros = "0.26" serde = "1.0" serde_json = "1.0" +anyhow = "1.0.98" [profile.release] debug = "line-tables-only" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 3f3e28693..9f317b639 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -19,7 +19,7 @@ path = "main.rs" [dependencies] -anyhow = "1.0.75" +anyhow.workspace = true cfg-if = "1.0.0" clap = { version = "4.5.31", features = ["derive"] } clap_complete = { version = "=4.5.47", features = ["unstable-dynamic"] } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 02a220244..8e103cb56 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -15,7 +15,7 @@ name = "integration_tests" path = "integration/mod.rs" [dependencies] -anyhow = "1.0.75" +anyhow.workspace = true env_logger = "0.10.1" limbo_core = { path = "../core" } rusqlite = { version = "0.34", features = ["bundled"] } From f3ef60e1f14da60f6b5f291a7a95e92c9bffb6e3 Mon Sep 17 00:00:00 2001 From: pedrocarlo Date: Mon, 9 Jun 2025 19:45:19 -0300 Subject: [PATCH 2/3] use anyhow in simulator for lazily evaluated error context instead of eager evaluation with `.or` --- Cargo.lock | 1 + simulator/Cargo.toml | 1 + simulator/main.rs | 27 +++++----- simulator/runner/bugbase.rs | 99 +++++++++++++++++++------------------ simulator/runner/cli.rs | 8 +-- 5 files changed, 71 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d5c06fea..f6fbb4826 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1955,6 +1955,7 @@ name = "limbo_sim" version = "0.0.22-pre.1" dependencies = [ "anarchist-readable-name-generator-lib", + "anyhow", "chrono", "clap", "dirs 6.0.0", diff --git a/simulator/Cargo.toml b/simulator/Cargo.toml index 68296a8a3..852a39cb7 100644 --- a/simulator/Cargo.toml +++ b/simulator/Cargo.toml @@ -34,4 +34,5 @@ dirs = "6.0.0" chrono = { version = "0.4.40", features = ["serde"] } tracing = "0.1.41" tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } +anyhow.workspace = true diff --git a/simulator/main.rs b/simulator/main.rs index 4cbf7ba10..dfce93bae 100644 --- a/simulator/main.rs +++ b/simulator/main.rs @@ -1,4 +1,5 @@ #![allow(clippy::arc_with_non_send_sync, dead_code)] +use anyhow::anyhow; use clap::Parser; use generation::plan::{Interaction, InteractionPlan, InteractionPlanState}; use generation::ArbitraryFrom; @@ -49,14 +50,14 @@ impl Paths { } } -fn main() -> Result<(), String> { +fn main() -> anyhow::Result<()> { init_logger(); let mut cli_opts = SimulatorCLI::parse(); cli_opts.validate()?; match cli_opts.subcommand { Some(SimulatorCommand::List) => { - let mut bugbase = BugBase::load().map_err(|e| format!("{:?}", e))?; + let mut bugbase = BugBase::load()?; bugbase.list_bugs() } Some(SimulatorCommand::Loop { n, short_circuit }) => { @@ -76,7 +77,7 @@ fn main() -> Result<(), String> { Ok(()) } Some(SimulatorCommand::Test { filter }) => { - let mut bugbase = BugBase::load().map_err(|e| format!("{:?}", e))?; + let mut bugbase = BugBase::load()?; let bugs = bugbase.load_bugs()?; let mut bugs = bugs .into_iter() @@ -127,11 +128,11 @@ fn main() -> Result<(), String> { } } -fn testing_main(cli_opts: &SimulatorCLI) -> Result<(), String> { +fn testing_main(cli_opts: &SimulatorCLI) -> anyhow::Result<()> { let mut bugbase = if cli_opts.disable_bugbase { None } else { - Some(BugBase::load().map_err(|e| format!("{:?}", e))?) + Some(BugBase::load()?) }; let last_execution = Arc::new(Mutex::new(Execution::new(0, 0, 0))); @@ -237,7 +238,7 @@ fn run_simulator( env: SimulatorEnv, plans: Vec, last_execution: Arc>, -) -> Result<(), String> { +) -> anyhow::Result<()> { std::panic::set_hook(Box::new(move |info| { tracing::error!("panic occurred"); @@ -357,7 +358,7 @@ fn run_simulator( .add_bug(seed, plans[0].clone(), Some(error.clone()), cli_opts) .unwrap(); } - Err(format!("failed with error: '{}'", error)) + Err(anyhow!("failed with error: '{}'", error)) } else { tracing::info!( "shrinking succeeded, reduced the plan from {} to {}", @@ -375,7 +376,7 @@ fn run_simulator( ) .unwrap(); } - Err(format!("failed with error: '{}'", e1)) + Err(anyhow!("failed with error: '{}'", e1)) } } (_, SandboxedResult::Correct) => { @@ -388,7 +389,7 @@ fn run_simulator( .add_bug(seed, plans[0].clone(), Some(error.clone()), cli_opts) .unwrap(); } - Err(format!("failed with error: '{}'", error)) + Err(anyhow!("failed with error: '{}'", error)) } } } @@ -404,7 +405,7 @@ fn doublecheck( plans: &[InteractionPlan], last_execution: Arc>, result: SandboxedResult, -) -> Result<(), String> { +) -> anyhow::Result<()> { let env = SimulatorEnv::new(seed, cli_opts, &paths.doublecheck_db); let env = Arc::new(Mutex::new(env)); @@ -468,7 +469,7 @@ fn doublecheck( .add_bug(seed, plans[0].clone(), Some(e.clone()), cli_opts) .unwrap(); } - Err(format!("doublecheck failed: '{}'", e)) + Err(anyhow!("doublecheck failed: '{}'", e)) } } } @@ -480,7 +481,7 @@ fn differential_testing( paths: &Paths, plans: Vec, last_execution: Arc>, -) -> Result<(), String> { +) -> anyhow::Result<()> { let env = Arc::new(Mutex::new(SimulatorEnv::new(seed, cli_opts, &paths.db))); let rusqlite_env = Arc::new(Mutex::new(SimulatorEnv::new( seed, @@ -518,7 +519,7 @@ fn differential_testing( .add_bug(seed, plans[0].clone(), Some(error.clone()), cli_opts) .unwrap(); } - Err(format!("simulation failed: '{}'", error)) + Err(anyhow!("simulation failed: '{}'", error)) } } } diff --git a/simulator/runner/bugbase.rs b/simulator/runner/bugbase.rs index c11cf0afd..9dba0c49e 100644 --- a/simulator/runner/bugbase.rs +++ b/simulator/runner/bugbase.rs @@ -6,6 +6,7 @@ use std::{ time::SystemTime, }; +use anyhow::{anyhow, Context}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; @@ -71,7 +72,7 @@ pub(crate) struct BugBase { impl BugBase { /// Create a new bug base. - fn new(path: PathBuf) -> Result { + fn new(path: PathBuf) -> anyhow::Result { let mut bugs = HashMap::new(); // list all the bugs in the path as directories if let Ok(entries) = std::fs::read_dir(&path) { @@ -82,10 +83,12 @@ impl BugBase { .to_string_lossy() .to_string() .parse::() - .or(Err(format!( - "failed to parse seed from directory name {}", - entry.file_name().to_string_lossy() - )))?; + .with_context(|| { + format!( + "failed to parse seed from directory name {}", + entry.file_name().to_string_lossy() + ) + })?; bugs.insert(seed, Bug::Unloaded { seed }); } } @@ -95,15 +98,14 @@ impl BugBase { } /// Load the bug base from one of the potential paths. - pub(crate) fn load() -> Result { + pub(crate) fn load() -> anyhow::Result { let potential_paths = vec![ // limbo project directory BugBase::get_limbo_project_dir()?, // home directory - dirs::home_dir().ok_or("should be able to get home directory".to_string())?, + dirs::home_dir().with_context(|| "should be able to get home directory")?, // current directory - std::env::current_dir() - .or(Err("should be able to get current directory".to_string()))?, + std::env::current_dir().with_context(|| "should be able to get current directory")?, ]; for path in &potential_paths { @@ -121,19 +123,18 @@ impl BugBase { } } - Err("failed to create bug base".to_string()) + Err(anyhow!("failed to create bug base")) } /// Load the bug base from one of the potential paths. - pub(crate) fn interactive_load() -> Result { + pub(crate) fn interactive_load() -> anyhow::Result { let potential_paths = vec![ // limbo project directory BugBase::get_limbo_project_dir()?, // home directory - dirs::home_dir().ok_or("should be able to get home directory".to_string())?, + dirs::home_dir().with_context(|| "should be able to get home directory")?, // current directory - std::env::current_dir() - .or(Err("should be able to get current directory".to_string()))?, + std::env::current_dir().with_context(|| "should be able to get current directory")?, ]; for path in potential_paths { @@ -157,21 +158,21 @@ impl BugBase { let choice = choice .trim() .parse::() - .or(Err(format!("invalid choice {choice}")))?; + .with_context(|| format!("invalid choice {choice}"))?; let path = match choice { 1 => BugBase::get_limbo_project_dir()?.join(".bugbase"), 2 => { - let home = std::env::var("HOME").or(Err("failed to get home directory"))?; + let home = std::env::var("HOME").with_context(|| "failed to get home directory")?; PathBuf::from(home).join(".bugbase") } 3 => PathBuf::from(".bugbase"), - _ => return Err(format!("invalid choice {choice}")), + _ => anyhow::bail!(format!("invalid choice {choice}")), }; if path.exists() { unreachable!("bug base already exists at {}", path.display()); } else { - std::fs::create_dir_all(&path).or(Err("failed to create bug base"))?; + std::fs::create_dir_all(&path).with_context(|| "failed to create bug base")?; tracing::info!("bug base created at {}", path.display()); BugBase::new(path) } @@ -184,7 +185,7 @@ impl BugBase { plan: InteractionPlan, error: Option, cli_options: &SimulatorCLI, - ) -> Result<(), String> { + ) -> anyhow::Result<()> { tracing::debug!("adding bug with seed {}", seed); let bug = self.get_bug(seed); @@ -221,7 +222,7 @@ impl BugBase { } /// Save a bug to the bug base. - fn save_bug(&self, seed: u64) -> Result<(), String> { + fn save_bug(&self, seed: u64) -> anyhow::Result<()> { let bug = self.get_bug(seed); match bug { @@ -231,57 +232,59 @@ impl BugBase { Some(Bug::Loaded(bug)) => { let bug_path = self.path.join(seed.to_string()); std::fs::create_dir_all(&bug_path) - .or(Err("should be able to create bug directory".to_string()))?; + .with_context(|| "should be able to create bug directory")?; let seed_path = bug_path.join("seed.txt"); std::fs::write(&seed_path, seed.to_string()) - .or(Err("should be able to write seed file".to_string()))?; + .with_context(|| "should be able to write seed file")?; let plan_path = bug_path.join("plan.json"); std::fs::write( &plan_path, serde_json::to_string_pretty(&bug.plan) - .or(Err("should be able to serialize plan".to_string()))?, + .with_context(|| "should be able to serialize plan")?, ) - .or(Err("should be able to write plan file".to_string()))?; + .with_context(|| "should be able to write plan file")?; let readable_plan_path = bug_path.join("plan.sql"); std::fs::write(&readable_plan_path, bug.plan.to_string()) - .or(Err("should be able to write readable plan file".to_string()))?; + .with_context(|| "should be able to write readable plan file")?; let runs_path = bug_path.join("runs.json"); std::fs::write( &runs_path, serde_json::to_string_pretty(&bug.runs) - .or(Err("should be able to serialize runs".to_string()))?, + .with_context(|| "should be able to serialize runs")?, ) - .or(Err("should be able to write runs file".to_string()))?; + .with_context(|| "should be able to write runs file")?; } } Ok(()) } - pub(crate) fn load_bug(&mut self, seed: u64) -> Result { + pub(crate) fn load_bug(&mut self, seed: u64) -> anyhow::Result { let seed_match = self.bugs.get(&seed); match seed_match { - None => Err(format!("No bugs found for seed {}", seed)), + None => anyhow::bail!("No bugs found for seed {}", seed), Some(Bug::Unloaded { .. }) => { let plan = std::fs::read_to_string(self.path.join(seed.to_string()).join("plan.json")) - .or(Err(format!( - "should be able to read plan file at {}", - self.path.join(seed.to_string()).join("plan.json").display() - )))?; + .with_context(|| { + format!( + "should be able to read plan file at {}", + self.path.join(seed.to_string()).join("plan.json").display() + ) + })?; let plan: InteractionPlan = serde_json::from_str(&plan) - .or(Err("should be able to deserialize plan".to_string()))?; + .with_context(|| "should be able to deserialize plan")?; let runs = std::fs::read_to_string(self.path.join(seed.to_string()).join("runs.json")) - .or(Err("should be able to read runs file".to_string()))?; + .with_context(|| "should be able to read runs file")?; let runs: Vec = serde_json::from_str(&runs) - .or(Err("should be able to deserialize runs".to_string()))?; + .with_context(|| "should be able to deserialize runs")?; let bug = LoadedBug { seed, @@ -307,13 +310,13 @@ impl BugBase { &mut self, seed: u64, cli_options: &SimulatorCLI, - ) -> Result<(), String> { + ) -> anyhow::Result<()> { let bug = self.get_bug(seed); match bug { None => { tracing::debug!("removing bug base entry for {}", seed); std::fs::remove_dir_all(self.path.join(seed.to_string())) - .or(Err("should be able to remove bug directory".to_string()))?; + .with_context(|| "should be able to remove bug directory")?; } Some(_) => { let mut bug = self.load_bug(seed)?; @@ -326,7 +329,7 @@ impl BugBase { self.bugs.insert(seed, Bug::Loaded(bug.clone())); // Save the bug to the bug base. self.save_bug(seed) - .or(Err("should be able to save bug".to_string()))?; + .with_context(|| "should be able to save bug")?; tracing::debug!("Updated bug with seed {}", seed); } } @@ -334,7 +337,7 @@ impl BugBase { Ok(()) } - pub(crate) fn load_bugs(&mut self) -> Result, String> { + pub(crate) fn load_bugs(&mut self) -> anyhow::Result> { let seeds = self.bugs.keys().map(|seed| *seed).collect::>(); seeds @@ -343,7 +346,7 @@ impl BugBase { .collect::, _>>() } - pub(crate) fn list_bugs(&mut self) -> Result<(), String> { + pub(crate) fn list_bugs(&mut self) -> anyhow::Result<()> { let bugs = self.load_bugs()?; for bug in bugs { println!("seed: {}", bug.seed); @@ -393,31 +396,31 @@ impl BugBase { } impl BugBase { - pub(crate) fn get_current_commit_hash() -> Result { + pub(crate) fn get_current_commit_hash() -> anyhow::Result { let output = Command::new("git") .args(["rev-parse", "HEAD"]) .output() - .or(Err("should be able to get the commit hash".to_string()))?; + .with_context(|| "should be able to get the commit hash")?; let commit_hash = String::from_utf8(output.stdout) - .or(Err("commit hash should be valid utf8".to_string()))? + .with_context(|| "commit hash should be valid utf8")? .trim() .to_string(); Ok(commit_hash) } - pub(crate) fn get_limbo_project_dir() -> Result { + pub(crate) fn get_limbo_project_dir() -> anyhow::Result { Ok(PathBuf::from( String::from_utf8( Command::new("git") .args(["rev-parse", "--git-dir"]) .output() - .or(Err("should be able to get the git path".to_string()))? + .with_context(|| "should be able to get the git path")? .stdout, ) - .or(Err("commit hash should be valid utf8".to_string()))? + .with_context(|| "commit hash should be valid utf8")? .trim() .strip_suffix(".git") - .ok_or("should be able to strip .git suffix".to_string())?, + .with_context(|| "should be able to strip .git suffix")?, )) } } diff --git a/simulator/runner/cli.rs b/simulator/runner/cli.rs index ba6c3b895..b47baafc8 100644 --- a/simulator/runner/cli.rs +++ b/simulator/runner/cli.rs @@ -93,12 +93,12 @@ pub enum SimulatorCommand { } impl SimulatorCLI { - pub fn validate(&mut self) -> Result<(), String> { + pub fn validate(&mut self) -> anyhow::Result<()> { if self.minimum_tests < 1 { - return Err("minimum size must be at least 1".to_string()); + anyhow::bail!("minimum size must be at least 1"); } if self.maximum_tests < 1 { - return Err("maximum size must be at least 1".to_string()); + anyhow::bail!("maximum size must be at least 1"); } if self.minimum_tests > self.maximum_tests { @@ -112,7 +112,7 @@ impl SimulatorCLI { } if self.seed.is_some() && self.load.is_some() { - return Err("Cannot set seed and load plan at the same time".to_string()); + anyhow::bail!("Cannot set seed and load plan at the same time"); } Ok(()) From 82538dbe8e31c43de33c80480279a8f8eecfe4c5 Mon Sep 17 00:00:00 2001 From: pedrocarlo Date: Mon, 9 Jun 2025 21:42:05 -0300 Subject: [PATCH 3/3] add integrity_check to sim --- simulator/main.rs | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/simulator/main.rs b/simulator/main.rs index dfce93bae..5cf8de426 100644 --- a/simulator/main.rs +++ b/simulator/main.rs @@ -1,5 +1,5 @@ #![allow(clippy::arc_with_non_send_sync, dead_code)] -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use clap::Parser; use generation::plan::{Interaction, InteractionPlan, InteractionPlanState}; use generation::ArbitraryFrom; @@ -144,7 +144,7 @@ fn testing_main(cli_opts: &SimulatorCLI) -> anyhow::Result<()> { return Ok(()); } - let result = if cli_opts.differential { + let mut result = if cli_opts.differential { differential_testing( seed, bugbase.as_mut(), @@ -165,6 +165,17 @@ fn testing_main(cli_opts: &SimulatorCLI) -> anyhow::Result<()> { ) }; + if let Err(err) = integrity_check(&paths.db) { + tracing::error!("Integrity Check failed:\n{}", err); + if result.is_err() { + result = result.with_context(|| anyhow!("Integrity Check failed:\n{}", err)); + } else { + result = Err(err); + } + } else if let Some(bugbase) = bugbase.as_mut() { + bugbase.mark_successful_run(seed, cli_opts).unwrap(); + }; + // Print the seed, the locations of the database and the plan file at the end again for easily accessing them. println!("seed: {}", seed); println!("path: {}", paths.base.display()); @@ -279,9 +290,6 @@ fn run_simulator( SandboxedResult::Correct => { tracing::info!("simulation succeeded"); println!("simulation succeeded"); - if let Some(bugbase) = bugbase { - bugbase.mark_successful_run(seed, cli_opts).unwrap(); - } Ok(()) } SandboxedResult::Panicked { @@ -457,9 +465,6 @@ fn doublecheck( Ok(_) => { tracing::info!("doublecheck succeeded"); println!("doublecheck succeeded"); - if let Some(bugbase) = bugbase { - bugbase.mark_successful_run(seed, cli_opts)?; - } Ok(()) } Err(e) => { @@ -507,9 +512,6 @@ fn differential_testing( SandboxedResult::Correct => { tracing::info!("simulation succeeded, output of Limbo conforms to SQLite"); println!("simulation succeeded, output of Limbo conforms to SQLite"); - if let Some(bugbase) = bugbase { - bugbase.mark_successful_run(seed, cli_opts).unwrap(); - } Ok(()) } SandboxedResult::Panicked { error, .. } | SandboxedResult::FoundBug { error, .. } => { @@ -739,3 +741,25 @@ const BANNER: &str = r#" \____________________________/ "#; + +fn integrity_check(db_path: &Path) -> anyhow::Result<()> { + let conn = rusqlite::Connection::open(db_path)?; + let mut stmt = conn.prepare("SELECT * FROM pragma_integrity_check;")?; + let mut rows = stmt.query(())?; + let mut result: Vec = Vec::new(); + + while let Some(row) = rows.next()? { + result.push(row.get(0)?); + } + if result.is_empty() { + anyhow::bail!("integrity_check should return `ok` or a list of problems") + } + if !result[0].eq_ignore_ascii_case("ok") { + // Build a list of problems + result + .iter_mut() + .for_each(|row| *row = format!("- {}", row)); + anyhow::bail!(result.join("\n")) + } + Ok(()) +}