From 63d53f1e712023061e840d12acd566ae22d7dcaa Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 3 Jan 2025 15:30:30 -0300 Subject: [PATCH 1/9] feat: lock on Nargo.toml on several nargo commands --- Cargo.lock | 11 ++++++++++ Cargo.toml | 2 -- tooling/nargo_cli/Cargo.toml | 1 + tooling/nargo_cli/build.rs | 23 --------------------- tooling/nargo_cli/src/cli/check_cmd.rs | 6 ++++++ tooling/nargo_cli/src/cli/compile_cmd.rs | 6 ++++++ tooling/nargo_cli/src/cli/execute_cmd.rs | 6 ++++++ tooling/nargo_cli/src/cli/export_cmd.rs | 11 ++++++++-- tooling/nargo_cli/src/cli/info_cmd.rs | 6 +++++- tooling/nargo_cli/src/lock.rs | 26 ++++++++++++++++++++++++ tooling/nargo_cli/src/main.rs | 1 + 11 files changed, 71 insertions(+), 28 deletions(-) create mode 100644 tooling/nargo_cli/src/lock.rs diff --git a/Cargo.lock b/Cargo.lock index 5bcf2cbf662..b5cb27069e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1780,6 +1780,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "fsevent-sys" version = "4.1.0" @@ -3007,6 +3017,7 @@ dependencies = [ "dirs", "file-lock", "fm", + "fs2", "fxhash", "iai", "iter-extended", diff --git a/Cargo.toml b/Cargo.toml index 9d061b9800d..c88851443e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -83,8 +83,6 @@ noir_lsp = { path = "tooling/lsp" } noir_debugger = { path = "tooling/debugger" } noirc_abi = { path = "tooling/noirc_abi" } noirc_artifacts = { path = "tooling/noirc_artifacts" } -bb_abstraction_leaks = { path = "tooling/bb_abstraction_leaks" } -acvm_cli = { path = "tooling/acvm_cli" } # Arkworks ark-bn254 = { version = "^0.5.0", default-features = false, features = [ diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index 1e41cea2d81..0d7298081e9 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -74,6 +74,7 @@ termion = "3.0.0" tracing-subscriber.workspace = true tracing-appender = "0.2.3" clap_complete = "4.5.36" +fs2 = "0.4.3" [target.'cfg(not(unix))'.dependencies] tokio-util = { version = "0.7.8", features = ["compat"] } diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 17ce6d4dbfd..8331bc6ea42 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -177,33 +177,13 @@ fn generate_test_cases( } let test_cases = test_cases.join("\n"); - // We need to isolate test cases in the same group, otherwise they overwrite each other's artifacts. - // On CI we use `cargo nextest`, which runs tests in different processes; for this we use a file lock. - // Locally we might be using `cargo test`, which run tests in the same process; in this case the file lock - // wouldn't work, becuase the process itself has the lock, and it looks like it can have N instances without - // any problems; for this reason we also use a `Mutex`. - let mutex_name = format! {"TEST_MUTEX_{}", test_name.to_uppercase()}; write!( test_file, r#" -lazy_static::lazy_static! {{ - /// Prevent concurrent tests in the matrix from overwriting the compilation artifacts in {test_dir} - static ref {mutex_name}: std::sync::Mutex<()> = std::sync::Mutex::new(()); -}} - {test_cases} fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner) {{ let test_program_dir = PathBuf::from("{test_dir}"); - // Ignore poisoning errors if some of the matrix cases failed. - let mutex_guard = {mutex_name}.lock().unwrap_or_else(|e| e.into_inner()); - - let file_guard = file_lock::FileLock::lock( - test_program_dir.join("Nargo.toml"), - true, - file_lock::FileOptions::new().read(true).write(true).append(true) - ).expect("failed to lock Nargo.toml"); - let mut nargo = Command::cargo_bin("nargo").unwrap(); nargo.arg("--program-dir").arg(test_program_dir); nargo.arg("{test_command}").arg("--force"); @@ -221,9 +201,6 @@ fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner }} {test_content} - - drop(file_guard); - drop(mutex_guard); }} "# ) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index c8695a8f626..29b72460ffe 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -1,4 +1,5 @@ use crate::errors::CliError; +use crate::lock::Lock; use clap::Args; use fm::FileManager; @@ -34,6 +35,8 @@ pub(crate) struct CheckCommand { pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; + let lock = Lock::lock(toml_path.clone()); + let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, @@ -57,6 +60,9 @@ pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliErro println!("[{}] Constraint system successfully built!", package.name); } } + + lock.unlock(); + Ok(()) } diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index f718021c351..7307f6409f9 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -21,6 +21,7 @@ use notify::{EventKind, RecursiveMode, Watcher}; use notify_debouncer_full::new_debouncer; use crate::errors::CliError; +use crate::lock::Lock; use super::fs::program::{read_program_from_file, save_contract_to_file, save_program_to_file}; use super::{NargoConfig, PackageOptions}; @@ -42,6 +43,9 @@ pub(crate) struct CompileCommand { pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliError> { let selection = args.package_options.package_selection(); + let toml_path = get_package_manifest(&config.program_dir)?; + let lock = Lock::lock(toml_path); + let workspace = read_workspace(&config.program_dir, selection)?; if args.watch { @@ -51,6 +55,8 @@ pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliEr compile_workspace_full(&workspace, &args.compile_options)?; } + lock.unlock(); + Ok(()) } diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 709caf71185..d815268dc6a 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -21,6 +21,7 @@ use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::{NargoConfig, PackageOptions}; use crate::cli::fs::program::read_program_from_file; use crate::errors::CliError; +use crate::lock::Lock; /// Executes a circuit to calculate its return value #[derive(Debug, Clone, Args)] @@ -48,6 +49,8 @@ pub(crate) struct ExecuteCommand { pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; + let lock = Lock::lock(toml_path.clone()); + let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, @@ -98,6 +101,9 @@ pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliEr } } } + + lock.unlock(); + Ok(()) } diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index cb92b987c4e..68e09f461a5 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -18,6 +18,7 @@ use noirc_driver::{ use clap::Args; use crate::errors::CliError; +use crate::lock::Lock; use super::check_cmd::check_crate_and_report_errors; @@ -36,6 +37,8 @@ pub(crate) struct ExportCommand { pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; + let lock = Lock::lock(toml_path.clone()); + let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, @@ -50,7 +53,7 @@ pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliErr let library_packages: Vec<_> = workspace.into_iter().filter(|package| package.is_library()).collect(); - library_packages + let result = library_packages .par_iter() .map(|package| { compile_exported_functions( @@ -61,7 +64,11 @@ pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliErr &args.compile_options, ) }) - .collect() + .collect(); + + lock.unlock(); + + result } fn compile_exported_functions( diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 6320cbbf350..f1ee152adfd 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -13,7 +13,7 @@ use prettytable::{row, table, Row}; use rayon::prelude::*; use serde::Serialize; -use crate::{cli::fs::inputs::read_inputs_from_file, errors::CliError}; +use crate::{cli::fs::inputs::read_inputs_from_file, errors::CliError, lock::Lock}; use super::{ compile_cmd::{compile_workspace_full, get_target_width}, @@ -49,6 +49,8 @@ pub(crate) struct InfoCommand { pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; + let lock = Lock::lock(toml_path.clone()); + let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, @@ -118,6 +120,8 @@ pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliE } } + lock.unlock(); + Ok(()) } diff --git a/tooling/nargo_cli/src/lock.rs b/tooling/nargo_cli/src/lock.rs new file mode 100644 index 00000000000..c25ed665adf --- /dev/null +++ b/tooling/nargo_cli/src/lock.rs @@ -0,0 +1,26 @@ +use std::{fs::File, path::PathBuf}; + +use fs2::FileExt as _; + +/// A lock used to lock the Nargo.toml file so two concurrent runs of nargo +/// commands (for example two `nargo execute`) don't overwrite output artifacts. +pub(crate) struct Lock { + file: File, +} + +impl Lock { + #[allow(clippy::self_named_constructors)] + pub(crate) fn lock(toml_path: PathBuf) -> Self { + let file = File::open(toml_path).expect("Expected Nargo.toml to exist"); + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on Nargo.toml..."); + } + + file.lock_exclusive().expect("Failed to lock Nargo.toml"); + Self { file } + } + + pub(crate) fn unlock(&self) { + self.file.unlock().expect("Failed to unlock Nargo.toml"); + } +} diff --git a/tooling/nargo_cli/src/main.rs b/tooling/nargo_cli/src/main.rs index a407d467ced..f2de82d4b42 100644 --- a/tooling/nargo_cli/src/main.rs +++ b/tooling/nargo_cli/src/main.rs @@ -9,6 +9,7 @@ mod cli; mod errors; +mod lock; use std::env; From cb0628b86a0e37db9d62d84e340c1a9ae254c340 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 3 Jan 2025 15:37:10 -0300 Subject: [PATCH 2/9] Remove file-lock from dev dependencies --- Cargo.lock | 11 ----------- tooling/nargo_cli/Cargo.toml | 3 +-- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b5cb27069e4..8352955ee1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1697,16 +1697,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "file-lock" -version = "2.1.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "040b48f80a749da50292d0f47a1e2d5bf1d772f52836c07f64bfccc62ba6e664" -dependencies = [ - "cc", - "libc", -] - [[package]] name = "filetime" version = "0.2.25" @@ -3015,7 +3005,6 @@ dependencies = [ "criterion", "dap", "dirs", - "file-lock", "fm", "fs2", "fxhash", diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index 0d7298081e9..11425151c02 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -86,7 +86,6 @@ dirs.workspace = true assert_cmd = "2.0.8" assert_fs = "1.0.10" predicates = "2.1.5" -file-lock = "2.1.11" fm.workspace = true criterion.workspace = true pprof.workspace = true @@ -102,7 +101,7 @@ light-poseidon = "0.2.0" ark-bn254-v04 = { package = "ark-bn254", version = "^0.4.0", default-features = false, features = [ "curve", ] } -ark-ff-v04 = { package = "ark-ff", version = "^0.4.0", default-features = false } +ark-ff-v04 = { package = "ark-ff", version = "^0.4.0", default-features = false } [[bench]] name = "criterion" From 8adebc7cd9017be4dfad3828c1f9746d48194384 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:05:28 -0300 Subject: [PATCH 3/9] Lock in one place --- tooling/nargo_cli/src/cli/check_cmd.rs | 4 ---- tooling/nargo_cli/src/cli/compile_cmd.rs | 4 ---- tooling/nargo_cli/src/cli/execute_cmd.rs | 4 ---- tooling/nargo_cli/src/cli/export_cmd.rs | 10 ++-------- tooling/nargo_cli/src/cli/info_cmd.rs | 5 +---- tooling/nargo_cli/src/cli/mod.rs | 23 +++++++++++++++++++++++ 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 29b72460ffe..3183c8e0077 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -1,5 +1,4 @@ use crate::errors::CliError; -use crate::lock::Lock; use clap::Args; use fm::FileManager; @@ -35,7 +34,6 @@ pub(crate) struct CheckCommand { pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let lock = Lock::lock(toml_path.clone()); let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( @@ -61,8 +59,6 @@ pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliErro } } - lock.unlock(); - Ok(()) } diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 0e0c671439d..53591635b93 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -43,8 +43,6 @@ pub(crate) struct CompileCommand { pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliError> { let selection = args.package_options.package_selection(); - let toml_path = get_package_manifest(&config.program_dir)?; - let lock = Lock::lock(toml_path); let workspace = read_workspace(&config.program_dir, selection)?; @@ -55,8 +53,6 @@ pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliEr compile_workspace_full(&workspace, &args.compile_options)?; } - lock.unlock(); - Ok(()) } diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index d815268dc6a..0b43bdfc517 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -21,7 +21,6 @@ use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::{NargoConfig, PackageOptions}; use crate::cli::fs::program::read_program_from_file; use crate::errors::CliError; -use crate::lock::Lock; /// Executes a circuit to calculate its return value #[derive(Debug, Clone, Args)] @@ -49,7 +48,6 @@ pub(crate) struct ExecuteCommand { pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let lock = Lock::lock(toml_path.clone()); let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( @@ -102,8 +100,6 @@ pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliEr } } - lock.unlock(); - Ok(()) } diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index 68e09f461a5..d576c28a30d 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -18,7 +18,6 @@ use noirc_driver::{ use clap::Args; use crate::errors::CliError; -use crate::lock::Lock; use super::check_cmd::check_crate_and_report_errors; @@ -37,7 +36,6 @@ pub(crate) struct ExportCommand { pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let lock = Lock::lock(toml_path.clone()); let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( @@ -53,7 +51,7 @@ pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliErr let library_packages: Vec<_> = workspace.into_iter().filter(|package| package.is_library()).collect(); - let result = library_packages + library_packages .par_iter() .map(|package| { compile_exported_functions( @@ -64,11 +62,7 @@ pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliErr &args.compile_options, ) }) - .collect(); - - lock.unlock(); - - result + .collect() } fn compile_exported_functions( diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index f1ee152adfd..4233c6b2d82 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -13,7 +13,7 @@ use prettytable::{row, table, Row}; use rayon::prelude::*; use serde::Serialize; -use crate::{cli::fs::inputs::read_inputs_from_file, errors::CliError, lock::Lock}; +use crate::{cli::fs::inputs::read_inputs_from_file, errors::CliError}; use super::{ compile_cmd::{compile_workspace_full, get_target_width}, @@ -49,7 +49,6 @@ pub(crate) struct InfoCommand { pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let lock = Lock::lock(toml_path.clone()); let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( @@ -120,8 +119,6 @@ pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliE } } - lock.unlock(); - Ok(()) } diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 43e3de9c6d0..3f10b795e89 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -118,6 +118,8 @@ enum NargoCommand { #[cfg(not(feature = "codegen-docs"))] #[tracing::instrument(level = "trace")] pub(crate) fn start_cli() -> eyre::Result<()> { + use crate::lock::Lock; + let NargoCli { command, mut config } = NargoCli::parse(); // If the provided `program_dir` is relative, make it absolute by joining it to the current directory. @@ -130,6 +132,25 @@ pub(crate) fn start_cli() -> eyre::Result<()> { config.program_dir = program_dir; } + let lock = match &command { + NargoCommand::Check(..) + | NargoCommand::Compile(..) + | NargoCommand::Execute(..) + | NargoCommand::Export(..) + | NargoCommand::Info(..) => { + let toml_path = config.program_dir.join("Nargo.toml"); + Some(Lock::lock(toml_path)) + } + NargoCommand::Fmt(..) + | NargoCommand::New(..) + | NargoCommand::Init(..) + | NargoCommand::Debug(..) + | NargoCommand::Test(..) + | NargoCommand::Lsp(..) + | NargoCommand::Dap(..) + | NargoCommand::GenerateCompletionScript(..) => None, + }; + match command { NargoCommand::New(args) => new_cmd::run(args, config), NargoCommand::Init(args) => init_cmd::run(args, config), @@ -146,6 +167,8 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::GenerateCompletionScript(args) => generate_completion_script_cmd::run(args), }?; + lock.map(|lock| lock.unlock()); + Ok(()) } From 3acf80129e168758fab036f5884d7e7ff51774c7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:08:35 -0300 Subject: [PATCH 4/9] Remove unused import --- tooling/nargo_cli/src/cli/compile_cmd.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 53591635b93..a5ebc59e27f 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -21,7 +21,6 @@ use notify::{EventKind, RecursiveMode, Watcher}; use notify_debouncer_full::new_debouncer; use crate::errors::CliError; -use crate::lock::Lock; use super::fs::program::{read_program_from_file, save_contract_to_file, save_program_to_file}; use super::{NargoConfig, PackageOptions}; From 5fa77561705d541d61d51a164fb0b22ad0a866e2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:14:13 -0300 Subject: [PATCH 5/9] Inline Lock's contents --- tooling/nargo_cli/src/cli/mod.rs | 51 +++++++++++++++++++++----------- tooling/nargo_cli/src/lock.rs | 26 ---------------- tooling/nargo_cli/src/main.rs | 1 - 3 files changed, 33 insertions(+), 45 deletions(-) delete mode 100644 tooling/nargo_cli/src/lock.rs diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 3f10b795e89..f5ee18643c6 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -118,7 +118,10 @@ enum NargoCommand { #[cfg(not(feature = "codegen-docs"))] #[tracing::instrument(level = "trace")] pub(crate) fn start_cli() -> eyre::Result<()> { - use crate::lock::Lock; + use std::fs::File; + + use fs2::FileExt as _; + use nargo_toml::get_package_manifest; let NargoCli { command, mut config } = NargoCli::parse(); @@ -132,23 +135,17 @@ pub(crate) fn start_cli() -> eyre::Result<()> { config.program_dir = program_dir; } - let lock = match &command { - NargoCommand::Check(..) - | NargoCommand::Compile(..) - | NargoCommand::Execute(..) - | NargoCommand::Export(..) - | NargoCommand::Info(..) => { - let toml_path = config.program_dir.join("Nargo.toml"); - Some(Lock::lock(toml_path)) + let lock_file = if needs_lock(&command) { + let toml_path = get_package_manifest(&config.program_dir)?; + let file = File::open(toml_path).expect("Expected Nargo.toml to exist"); + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on Nargo.toml..."); } - NargoCommand::Fmt(..) - | NargoCommand::New(..) - | NargoCommand::Init(..) - | NargoCommand::Debug(..) - | NargoCommand::Test(..) - | NargoCommand::Lsp(..) - | NargoCommand::Dap(..) - | NargoCommand::GenerateCompletionScript(..) => None, + + file.lock_exclusive().expect("Failed to lock Nargo.toml"); + Some(file) + } else { + None }; match command { @@ -167,7 +164,7 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::GenerateCompletionScript(args) => generate_completion_script_cmd::run(args), }?; - lock.map(|lock| lock.unlock()); + lock_file.map(|lock| lock.unlock().expect("Failed to unlock Nargo.toml")); Ok(()) } @@ -216,6 +213,24 @@ fn command_dir(cmd: &NargoCommand, program_dir: &Path) -> Result Ok(Some(nargo_toml::find_root(program_dir, workspace)?)) } +fn needs_lock(cmd: &NargoCommand) -> bool { + match cmd { + NargoCommand::Check(..) + | NargoCommand::Compile(..) + | NargoCommand::Execute(..) + | NargoCommand::Export(..) + | NargoCommand::Info(..) => true, + NargoCommand::Fmt(..) + | NargoCommand::New(..) + | NargoCommand::Init(..) + | NargoCommand::Debug(..) + | NargoCommand::Test(..) + | NargoCommand::Lsp(..) + | NargoCommand::Dap(..) + | NargoCommand::GenerateCompletionScript(..) => false, + } +} + #[cfg(test)] mod tests { use clap::Parser; diff --git a/tooling/nargo_cli/src/lock.rs b/tooling/nargo_cli/src/lock.rs deleted file mode 100644 index c25ed665adf..00000000000 --- a/tooling/nargo_cli/src/lock.rs +++ /dev/null @@ -1,26 +0,0 @@ -use std::{fs::File, path::PathBuf}; - -use fs2::FileExt as _; - -/// A lock used to lock the Nargo.toml file so two concurrent runs of nargo -/// commands (for example two `nargo execute`) don't overwrite output artifacts. -pub(crate) struct Lock { - file: File, -} - -impl Lock { - #[allow(clippy::self_named_constructors)] - pub(crate) fn lock(toml_path: PathBuf) -> Self { - let file = File::open(toml_path).expect("Expected Nargo.toml to exist"); - if file.try_lock_exclusive().is_err() { - eprintln!("Waiting for lock on Nargo.toml..."); - } - - file.lock_exclusive().expect("Failed to lock Nargo.toml"); - Self { file } - } - - pub(crate) fn unlock(&self) { - self.file.unlock().expect("Failed to unlock Nargo.toml"); - } -} diff --git a/tooling/nargo_cli/src/main.rs b/tooling/nargo_cli/src/main.rs index b67b2096e16..3ea167b7ffa 100644 --- a/tooling/nargo_cli/src/main.rs +++ b/tooling/nargo_cli/src/main.rs @@ -9,7 +9,6 @@ mod cli; mod errors; -mod lock; use std::env; From f9899787ada7410537375d20bf5869c0f6a11623 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:15:38 -0300 Subject: [PATCH 6/9] Undo whitespace changes --- tooling/nargo_cli/src/cli/check_cmd.rs | 2 -- tooling/nargo_cli/src/cli/compile_cmd.rs | 1 - tooling/nargo_cli/src/cli/execute_cmd.rs | 2 -- tooling/nargo_cli/src/cli/export_cmd.rs | 1 - 4 files changed, 6 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 3183c8e0077..c8695a8f626 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -34,7 +34,6 @@ pub(crate) struct CheckCommand { pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, @@ -58,7 +57,6 @@ pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliErro println!("[{}] Constraint system successfully built!", package.name); } } - Ok(()) } diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index a5ebc59e27f..0af05703c9a 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -42,7 +42,6 @@ pub(crate) struct CompileCommand { pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliError> { let selection = args.package_options.package_selection(); - let workspace = read_workspace(&config.program_dir, selection)?; if args.watch { diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 0b43bdfc517..709caf71185 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -48,7 +48,6 @@ pub(crate) struct ExecuteCommand { pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, @@ -99,7 +98,6 @@ pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliEr } } } - Ok(()) } diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index d576c28a30d..cb92b987c4e 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -36,7 +36,6 @@ pub(crate) struct ExportCommand { pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, From 0f0ef6136334a0ad96d71da6ee361d6edf8cf265 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:16:10 -0300 Subject: [PATCH 7/9] One more --- tooling/nargo_cli/src/cli/info_cmd.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 4233c6b2d82..6320cbbf350 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -49,7 +49,6 @@ pub(crate) struct InfoCommand { pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; - let selection = args.package_options.package_selection(); let workspace = resolve_workspace_from_toml( &toml_path, From 5d263d4ef259d31917afd26208897d08527f791d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:19:48 -0300 Subject: [PATCH 8/9] Clippy --- tooling/nargo_cli/src/cli/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index f5ee18643c6..8c72fde893c 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -164,7 +164,9 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::GenerateCompletionScript(args) => generate_completion_script_cmd::run(args), }?; - lock_file.map(|lock| lock.unlock().expect("Failed to unlock Nargo.toml")); + if let Some(lock_file) = lock_file { + lock_file.unlock().expect("Failed to unlock Nargo.toml"); + } Ok(()) } From 402e6eb1a352742f8ef7f973d8961e41dab851e0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 12:58:42 -0300 Subject: [PATCH 9/9] Shared lock on some commands --- tooling/nargo_cli/src/cli/mod.rs | 38 +++++++++++++++++++------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 8c72fde893c..0b725afcf4e 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -135,17 +135,26 @@ pub(crate) fn start_cli() -> eyre::Result<()> { config.program_dir = program_dir; } - let lock_file = if needs_lock(&command) { - let toml_path = get_package_manifest(&config.program_dir)?; - let file = File::open(toml_path).expect("Expected Nargo.toml to exist"); - if file.try_lock_exclusive().is_err() { - eprintln!("Waiting for lock on Nargo.toml..."); - } + let lock_file = match needs_lock(&command) { + Some(exclusive) => { + let toml_path = get_package_manifest(&config.program_dir)?; + let file = File::open(toml_path).expect("Expected Nargo.toml to exist"); + if exclusive { + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on Nargo.toml..."); + } + + file.lock_exclusive().expect("Failed to lock Nargo.toml"); + } else { + if file.try_lock_shared().is_err() { + eprintln!("Waiting for lock on Nargo.toml..."); + } - file.lock_exclusive().expect("Failed to lock Nargo.toml"); - Some(file) - } else { - None + file.lock_shared().expect("Failed to lock Nargo.toml"); + } + Some(file) + } + None => None, }; match command { @@ -215,21 +224,20 @@ fn command_dir(cmd: &NargoCommand, program_dir: &Path) -> Result Ok(Some(nargo_toml::find_root(program_dir, workspace)?)) } -fn needs_lock(cmd: &NargoCommand) -> bool { +fn needs_lock(cmd: &NargoCommand) -> Option { match cmd { NargoCommand::Check(..) | NargoCommand::Compile(..) | NargoCommand::Execute(..) | NargoCommand::Export(..) - | NargoCommand::Info(..) => true, + | NargoCommand::Info(..) => Some(true), + NargoCommand::Debug(..) | NargoCommand::Test(..) => Some(false), NargoCommand::Fmt(..) | NargoCommand::New(..) | NargoCommand::Init(..) - | NargoCommand::Debug(..) - | NargoCommand::Test(..) | NargoCommand::Lsp(..) | NargoCommand::Dap(..) - | NargoCommand::GenerateCompletionScript(..) => false, + | NargoCommand::GenerateCompletionScript(..) => None, } }