diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 1940ce8e625..bb7f06692a6 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -5,20 +5,18 @@ use fm::FileManager; use iter_extended::btree_map; use nargo::{ errors::CompileError, insert_all_files_for_workspace_into_file_manager, ops::report_errors, - package::Package, parse_all, prepare_package, + package::Package, parse_all, prepare_package, workspace::Workspace, }; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; +use nargo_toml::PackageSelection; use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME}; -use noirc_driver::{ - check_crate, compute_function_abi, CompileOptions, CrateId, NOIR_ARTIFACT_VERSION_STRING, -}; +use noirc_driver::{check_crate, compute_function_abi, CompileOptions, CrateId}; use noirc_frontend::{ hir::{Context, ParsedFiles}, monomorphization::monomorphize, }; -use super::NargoConfig; use super::{fs::write_to_file, PackageOptions}; +use super::{LockType, WorkspaceCommand}; /// Check a local package and all of its dependencies for errors #[derive(Debug, Clone, Args)] @@ -39,15 +37,18 @@ pub(crate) struct CheckCommand { show_program_hash: bool, } -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, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; +impl WorkspaceCommand for CheckCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + fn lock_type(&self) -> LockType { + // Creates a `Prover.toml` template if it doesn't exist, otherwise only writes if `allow_overwrite` is true, + // so it shouldn't lead to accidental conflicts. Doesn't produce compilation artifacts. + LockType::None + } +} +pub(crate) fn run(args: CheckCommand, workspace: Workspace) -> Result<(), CliError> { let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let parsed_files = parse_all(&workspace_file_manager); diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 8a4b991a234..bb08d2675cb 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -8,9 +8,7 @@ use nargo::ops::{collect_errors, compile_contract, compile_program, report_error use nargo::package::Package; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; -use nargo_toml::{ - get_package_manifest, resolve_workspace_from_toml, ManifestError, PackageSelection, -}; +use nargo_toml::PackageSelection; use noirc_driver::DEFAULT_EXPRESSION_WIDTH; use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract}; @@ -23,7 +21,7 @@ use notify_debouncer_full::new_debouncer; use crate::errors::CliError; use super::fs::program::{read_program_from_file, save_contract_to_file, save_program_to_file}; -use super::{NargoConfig, PackageOptions}; +use super::{LockType, PackageOptions, WorkspaceCommand}; use rayon::prelude::*; /// Compile the program and its secret execution trace into ACIR format @@ -40,36 +38,26 @@ pub(crate) struct CompileCommand { watch: bool, } -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)?; +impl WorkspaceCommand for CompileCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + fn lock_type(&self) -> LockType { + LockType::Exclusive + } +} + +pub(crate) fn run(args: CompileCommand, workspace: Workspace) -> Result<(), CliError> { if args.watch { watch_workspace(&workspace, &args.compile_options) .map_err(|err| CliError::Generic(err.to_string()))?; } else { compile_workspace_full(&workspace, &args.compile_options)?; } - Ok(()) } -/// Read a given program directory into a workspace. -fn read_workspace( - program_dir: &Path, - selection: PackageSelection, -) -> Result { - let toml_path = get_package_manifest(program_dir)?; - - let workspace = resolve_workspace_from_toml( - &toml_path, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()), - )?; - - Ok(workspace) -} - /// Continuously recompile the workspace on any Noir file change event. fn watch_workspace(workspace: &Workspace, compile_options: &CompileOptions) -> notify::Result<()> { let (tx, rx) = std::sync::mpsc::channel(); @@ -334,8 +322,11 @@ mod tests { use nargo_toml::PackageSelection; use noirc_driver::{CompileOptions, CrateName}; - use crate::cli::compile_cmd::{get_target_width, parse_workspace, read_workspace}; use crate::cli::test_cmd::formatters::diagnostic_to_string; + use crate::cli::{ + compile_cmd::{get_target_width, parse_workspace}, + read_workspace, + }; /// Try to find the directory that Cargo sets when it is running; /// otherwise fallback to assuming the CWD is the root of the repository diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index 7fbf685688a..91db844ead3 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -23,8 +23,6 @@ use super::debug_cmd::compile_bin_package_for_debugging; use super::fs::inputs::read_inputs_from_file; use crate::errors::CliError; -use super::NargoConfig; - use noir_debugger::errors::{DapError, LoadError}; #[derive(Debug, Clone, Args)] @@ -255,7 +253,7 @@ fn run_preflight_check( Ok(()) } -pub(crate) fn run(args: DapCommand, _config: NargoConfig) -> Result<(), CliError> { +pub(crate) fn run(args: DapCommand) -> Result<(), CliError> { // When the --preflight-check flag is present, we run Noir's DAP server in "pre-flight mode", which test runs // the DAP initialization code without actually starting the DAP server. // diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 2ed30639d32..259abbbbfd0 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -12,18 +12,16 @@ use nargo::ops::{compile_program, compile_program_with_debug_instrumenter, repor use nargo::package::{CrateName, Package}; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; +use nargo_toml::PackageSelection; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::InputMap; -use noirc_driver::{ - file_manager_with_stdlib, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING, -}; +use noirc_driver::{file_manager_with_stdlib, CompileOptions, CompiledProgram}; use noirc_frontend::debug::DebugInstrumenter; use noirc_frontend::hir::ParsedFiles; use super::compile_cmd::get_target_width; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; -use super::NargoConfig; +use super::{LockType, WorkspaceCommand}; use crate::errors::CliError; /// Executes a circuit in debug mode @@ -52,17 +50,24 @@ pub(crate) struct DebugCommand { skip_instrumentation: Option, } -pub(crate) fn run(args: DebugCommand, config: NargoConfig) -> Result<(), CliError> { +impl WorkspaceCommand for DebugCommand { + fn package_selection(&self) -> PackageSelection { + self.package + .as_ref() + .cloned() + .map_or(PackageSelection::DefaultOrAll, PackageSelection::Selected) + } + + fn lock_type(&self) -> LockType { + // Always compiles fresh in-memory in debug mode, doesn't read or write the compilation artifacts. + // Reads the Prover.toml file and writes the witness at the end, but shouldn't conflict with others. + LockType::None + } +} + +pub(crate) fn run(args: DebugCommand, workspace: Workspace) -> Result<(), CliError> { let acir_mode = args.acir_mode; let skip_instrumentation = args.skip_instrumentation.unwrap_or(acir_mode); - - let toml_path = get_package_manifest(&config.program_dir)?; - let selection = args.package.map_or(PackageSelection::DefaultOrAll, PackageSelection::Selected); - let workspace = resolve_workspace_from_toml( - &toml_path, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; let target_dir = &workspace.target_directory_path(); let Some(package) = workspace.into_iter().find(|p| p.is_binary()) else { diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index cb471995be5..884ba06cdcb 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -9,16 +9,17 @@ use nargo::constants::PROVER_INPUT_FILE; use nargo::errors::try_to_diagnose_runtime_error; use nargo::foreign_calls::DefaultForeignCallBuilder; use nargo::package::Package; +use nargo::workspace::Workspace; use nargo::PrintOutput; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; +use nargo_toml::PackageSelection; use noirc_abi::input_parser::{Format, InputValue}; use noirc_abi::InputMap; use noirc_artifacts::debug::DebugArtifact; -use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; +use noirc_driver::{CompileOptions, CompiledProgram}; use super::compile_cmd::compile_workspace_full; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; -use super::{NargoConfig, PackageOptions}; +use super::{LockType, PackageOptions, WorkspaceCommand}; use crate::cli::fs::program::read_program_from_file; use crate::errors::CliError; @@ -46,14 +47,18 @@ pub(crate) struct ExecuteCommand { oracle_resolver: Option, } -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, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; +impl WorkspaceCommand for ExecuteCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + + fn lock_type(&self) -> LockType { + // Compiles artifacts. + LockType::Exclusive + } +} + +pub(crate) fn run(args: ExecuteCommand, workspace: Workspace) -> Result<(), CliError> { let target_dir = &workspace.target_directory_path(); // Compile the full workspace in order to generate any build artifacts. diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index cb92b987c4e..09fe11fdb74 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -10,10 +10,8 @@ use nargo::package::Package; use nargo::prepare_package; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; -use noirc_driver::{ - compile_no_check, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING, -}; +use nargo_toml::PackageSelection; +use noirc_driver::{compile_no_check, CompileOptions, CompiledProgram}; use clap::Args; @@ -22,7 +20,7 @@ use crate::errors::CliError; use super::check_cmd::check_crate_and_report_errors; use super::fs::program::save_program_to_file; -use super::{NargoConfig, PackageOptions}; +use super::{LockType, PackageOptions, WorkspaceCommand}; /// Exports functions marked with #[export] attribute #[derive(Debug, Clone, Args)] @@ -34,15 +32,18 @@ pub(crate) struct ExportCommand { compile_options: CompileOptions, } -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, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()), - )?; +impl WorkspaceCommand for ExportCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + + fn lock_type(&self) -> LockType { + // Writes the exported functions. + LockType::Exclusive + } +} +pub(crate) fn run(args: ExportCommand, workspace: Workspace) -> Result<(), CliError> { let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let parsed_files = parse_all(&workspace_file_manager); diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 7b29a90c5c0..1cdfb1e0c4f 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -1,15 +1,16 @@ use std::{fs::DirEntry, path::Path}; use clap::Args; -use nargo::{insert_all_files_for_workspace_into_file_manager, ops::report_errors}; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; +use nargo::{ + insert_all_files_for_workspace_into_file_manager, ops::report_errors, workspace::Workspace, +}; +use nargo_toml::PackageSelection; use noirc_errors::CustomDiagnostic; use noirc_frontend::{hir::def_map::parse_file, parser::ParserError}; use crate::errors::CliError; -use super::{NargoConfig, PackageOptions}; +use super::{LockType, PackageOptions, WorkspaceCommand}; /// Format the Noir files in a workspace #[derive(Debug, Clone, Args)] @@ -22,24 +23,26 @@ pub(crate) struct FormatCommand { pub(super) package_options: PackageOptions, } -pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { - let check_mode = args.check; +impl WorkspaceCommand for FormatCommand { + fn package_selection(&self) -> PackageSelection { + match self.package_options.package_selection() { + PackageSelection::DefaultOrAll => PackageSelection::All, + other => other, + } + } - let toml_path = get_package_manifest(&config.program_dir)?; - let selection = match args.package_options.package_selection() { - PackageSelection::DefaultOrAll => PackageSelection::All, - other => other, - }; - let workspace = resolve_workspace_from_toml( - &toml_path, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; + fn lock_type(&self) -> LockType { + // Writes source files, but doesn't touch compilation artifacts. + LockType::None + } +} +pub(crate) fn run(args: FormatCommand, workspace: Workspace) -> Result<(), CliError> { + let check_mode = args.check; let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let config = nargo_fmt::Config::read(&config.program_dir) + let config = nargo_fmt::Config::read(&workspace.root_dir) .map_err(|err| CliError::Generic(err.to_string()))?; let mut check_exit_code_one = false; diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index a41eb547e4f..4561a1221f6 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -4,14 +4,15 @@ use clap::Args; use iter_extended::vecmap; use nargo::{ constants::PROVER_INPUT_FILE, foreign_calls::DefaultForeignCallBuilder, package::Package, + workspace::Workspace, }; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; +use nargo_toml::PackageSelection; use noirc_abi::input_parser::Format; use noirc_artifacts::program::ProgramArtifact; use noirc_artifacts_info::{ count_opcodes_and_gates_in_program, show_info_report, FunctionInfo, InfoReport, ProgramInfo, }; -use noirc_driver::{CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; +use noirc_driver::CompileOptions; use prettytable::{row, Row}; use rayon::prelude::*; use serde::Serialize; @@ -21,7 +22,7 @@ use crate::errors::CliError; use super::{ compile_cmd::{compile_workspace_full, get_target_width}, fs::{inputs::read_inputs_from_file, program::read_program_from_file}, - NargoConfig, PackageOptions, + LockType, PackageOptions, WorkspaceCommand, }; /// Provides detailed information on each of a program's function (represented by a single circuit) @@ -50,15 +51,17 @@ pub(crate) struct InfoCommand { compile_options: CompileOptions, } -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, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; +impl WorkspaceCommand for InfoCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + + fn lock_type(&self) -> LockType { + LockType::Exclusive + } +} +pub(crate) fn run(mut args: InfoCommand, workspace: Workspace) -> Result<(), CliError> { if args.profile_execution { // Execution profiling is only relevant with the Brillig VM // as a constrained circuit should have totally flattened control flow (e.g. loops and if statements). diff --git a/tooling/nargo_cli/src/cli/lsp_cmd.rs b/tooling/nargo_cli/src/cli/lsp_cmd.rs index dc5d0995c06..64ca97abc95 100644 --- a/tooling/nargo_cli/src/cli/lsp_cmd.rs +++ b/tooling/nargo_cli/src/cli/lsp_cmd.rs @@ -7,7 +7,6 @@ use clap::Args; use noir_lsp::NargoLspService; use tower::ServiceBuilder; -use super::NargoConfig; use crate::errors::CliError; /// Starts the Noir LSP server @@ -18,7 +17,7 @@ use crate::errors::CliError; #[derive(Debug, Clone, Args)] pub(crate) struct LspCommand; -pub(crate) fn run(_args: LspCommand, _config: NargoConfig) -> Result<(), CliError> { +pub(crate) fn run() -> Result<(), CliError> { use tokio::runtime::Builder; let runtime = Builder::new_current_thread().enable_all().build().unwrap(); diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index d8e8993d8e4..efc6a351e33 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -1,11 +1,19 @@ use clap::{Args, Parser, Subcommand}; use const_format::formatcp; -use nargo_toml::{ManifestError, PackageSelection}; +use nargo::workspace::Workspace; +use nargo_toml::{ + get_package_manifest, resolve_workspace_from_toml, ManifestError, PackageSelection, +}; use noirc_driver::{CrateName, NOIR_ARTIFACT_VERSION_STRING}; -use std::path::{Path, PathBuf}; +use std::{ + fs::File, + path::{Path, PathBuf}, +}; use color_eyre::eyre; +use crate::errors::CliError; + mod fs; mod check_cmd; @@ -76,22 +84,6 @@ impl PackageOptions { self.package.clone().map_or(default_selection, PackageSelection::Selected) } - - /// Whether we need to look for the package manifest at the workspace level. - /// If a package is specified, it might not be the current package. - fn scope(&self) -> CommandScope { - if self.workspace || self.package.is_some() { - CommandScope::Workspace - } else { - CommandScope::CurrentPackage - } - } -} - -enum CommandScope { - Workspace, - CurrentPackage, - Any, } #[non_exhaustive] @@ -115,14 +107,30 @@ enum NargoCommand { GenerateCompletionScript(generate_completion_script_cmd::GenerateCompletionScriptCommand), } +/// Commands that can execute on the workspace level, or be limited to a selected package. +trait WorkspaceCommand { + /// Indicate which package the command will be applied to. + fn package_selection(&self) -> PackageSelection; + /// The kind of lock the command needs to take out on the selected packages. + fn lock_type(&self) -> LockType; +} + +/// What kind of lock to take out on the (selected) workspace members. +#[derive(Clone, Debug, PartialEq, Eq)] +#[allow(dead_code)] // Not using `Shared` at the moment, e.g. while we `debug` we can `compile` a different version. +enum LockType { + /// For commands that write artifacts. + Exclusive, + /// For commands that read artifacts, but never write them. + Shared, + /// For commands that cannot interfere with others. + None, +} + #[cfg(not(feature = "codegen-docs"))] #[tracing::instrument(level = "trace")] pub(crate) fn start_cli() -> eyre::Result<()> { - use std::fs::File; - use fm::NormalizePath; - use fs2::FileExt as _; - use nargo_toml::get_package_manifest; let NargoCli { command, mut config } = NargoCli::parse(); @@ -131,53 +139,22 @@ pub(crate) fn start_cli() -> eyre::Result<()> { config.program_dir = std::env::current_dir().unwrap().join(config.program_dir).normalize(); } - // Search through parent directories to find package root if necessary. - if let Some(program_dir) = command_dir(&command, &config.program_dir)? { - config.program_dir = program_dir; - } - - 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_shared().expect("Failed to lock Nargo.toml"); - } - Some(file) - } - None => None, - }; - match command { NargoCommand::New(args) => new_cmd::run(args, config), NargoCommand::Init(args) => init_cmd::run(args, config), - NargoCommand::Check(args) => check_cmd::run(args, config), - NargoCommand::Compile(args) => compile_cmd::run(args, config), - NargoCommand::Debug(args) => debug_cmd::run(args, config), - NargoCommand::Execute(args) => execute_cmd::run(args, config), - NargoCommand::Export(args) => export_cmd::run(args, config), - NargoCommand::Test(args) => test_cmd::run(args, config), - NargoCommand::Info(args) => info_cmd::run(args, config), - NargoCommand::Lsp(args) => lsp_cmd::run(args, config), - NargoCommand::Dap(args) => dap_cmd::run(args, config), - NargoCommand::Fmt(args) => fmt_cmd::run(args, config), + NargoCommand::Check(args) => with_workspace(args, config, check_cmd::run), + NargoCommand::Compile(args) => with_workspace(args, config, compile_cmd::run), + NargoCommand::Debug(args) => with_workspace(args, config, debug_cmd::run), + NargoCommand::Execute(args) => with_workspace(args, config, execute_cmd::run), + NargoCommand::Export(args) => with_workspace(args, config, export_cmd::run), + NargoCommand::Test(args) => with_workspace(args, config, test_cmd::run), + NargoCommand::Info(args) => with_workspace(args, config, info_cmd::run), + NargoCommand::Lsp(_) => lsp_cmd::run(), + NargoCommand::Dap(args) => dap_cmd::run(args), + NargoCommand::Fmt(args) => with_workspace(args, config, fmt_cmd::run), NargoCommand::GenerateCompletionScript(args) => generate_completion_script_cmd::run(args), }?; - if let Some(lock_file) = lock_file { - lock_file.unlock().expect("Failed to unlock Nargo.toml"); - } - Ok(()) } @@ -188,62 +165,88 @@ pub(crate) fn start_cli() -> eyre::Result<()> { Ok(()) } -/// Some commands have package options, which we use here to decide whether to -/// alter `--program-dir` to point at a manifest, depending on whether we want -/// to work on a specific package or the entire workspace. -fn command_scope(cmd: &NargoCommand) -> CommandScope { - match &cmd { - NargoCommand::Check(cmd) => cmd.package_options.scope(), - NargoCommand::Compile(cmd) => cmd.package_options.scope(), - NargoCommand::Execute(cmd) => cmd.package_options.scope(), - NargoCommand::Export(cmd) => cmd.package_options.scope(), - NargoCommand::Test(cmd) => cmd.package_options.scope(), - NargoCommand::Info(cmd) => cmd.package_options.scope(), - NargoCommand::Fmt(cmd) => cmd.package_options.scope(), - NargoCommand::Debug(cmd) => { - if cmd.package.is_some() { - CommandScope::Workspace - } else { - CommandScope::CurrentPackage - } - } - NargoCommand::New(..) - | NargoCommand::Init(..) - | NargoCommand::Lsp(..) - | NargoCommand::Dap(..) - | NargoCommand::GenerateCompletionScript(..) => CommandScope::Any, - } +/// Read a given program directory into a workspace. +fn read_workspace( + program_dir: &Path, + selection: PackageSelection, +) -> Result { + let toml_path = get_package_manifest(program_dir)?; + + let workspace = resolve_workspace_from_toml( + &toml_path, + selection, + Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()), + )?; + + Ok(workspace) } -/// A manifest directory we need to change into, if the command needs it. -fn command_dir(cmd: &NargoCommand, program_dir: &Path) -> Result, ManifestError> { - let workspace = match command_scope(cmd) { - CommandScope::Workspace => true, - CommandScope::CurrentPackage => false, - CommandScope::Any => return Ok(None), +/// Find the root directory, parse the workspace, lock the packages, then execute the command. +fn with_workspace(cmd: C, config: NargoConfig, run: R) -> Result<(), CliError> +where + C: WorkspaceCommand, + R: FnOnce(C, Workspace) -> Result<(), CliError>, +{ + // All commands need to run on the workspace level, because that's where the `target` directory is. + let workspace_dir = nargo_toml::find_root(&config.program_dir, true)?; + let package_dir = nargo_toml::find_root(&config.program_dir, false)?; + // Check if we're running inside the directory of a package, without having selected the entire workspace + // or a specific package; if that's the case then parse the package name to select it in the workspace. + let selection = match cmd.package_selection() { + PackageSelection::DefaultOrAll if workspace_dir != package_dir => { + let workspace = read_workspace(&package_dir, PackageSelection::DefaultOrAll)?; + let package = workspace.into_iter().next().expect("there should be exactly 1 package"); + PackageSelection::Selected(package.name.clone()) + } + other => other, + }; + // Parse the top level workspace with the member selected. + let workspace = read_workspace(&workspace_dir, selection)?; + // Lock manifests if the command needs it. + let _locks = match cmd.lock_type() { + LockType::None => None, + typ => Some(lock_workspace(&workspace, typ == LockType::Exclusive)?), }; - Ok(Some(nargo_toml::find_root(program_dir, workspace)?)) + run(cmd, workspace) } -/// Returns: -/// - `Some(true)` if an exclusive lock is needed -/// - `Some(false)` if an read lock is needed -/// - None if no lock is needed -fn needs_lock(cmd: &NargoCommand) -> Option { - match cmd { - NargoCommand::Check(check_command) => Some(check_command.allow_overwrite), - NargoCommand::Compile(..) - | NargoCommand::Execute(..) - | NargoCommand::Export(..) - | NargoCommand::Info(..) => Some(true), - NargoCommand::Debug(..) | NargoCommand::Test(..) => Some(false), - NargoCommand::Fmt(..) - | NargoCommand::New(..) - | NargoCommand::Init(..) - | NargoCommand::Lsp(..) - | NargoCommand::Dap(..) - | NargoCommand::GenerateCompletionScript(..) => None, +/// Lock the (selected) packages in the workspace. +/// The lock taken can be shared for commands that only read the artifacts, +/// or exclusive for the ones that (might) write artifacts as well. +fn lock_workspace(workspace: &Workspace, exclusive: bool) -> Result, CliError> { + use fs2::FileExt as _; + + struct LockedFile(File); + + impl Drop for LockedFile { + fn drop(&mut self) { + let _ = self.0.unlock(); + } + } + + let mut locks = Vec::new(); + for pkg in workspace.into_iter() { + let toml_path = get_package_manifest(&pkg.root_dir)?; + let path_display = toml_path.display(); + + let file = File::open(&toml_path) + .unwrap_or_else(|e| panic!("Expected {path_display} to exist: {e}")); + + if exclusive { + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on {path_display}..."); + } + file.lock_exclusive().unwrap_or_else(|e| panic!("Failed to lock {path_display}: {e}")); + } else { + if file.try_lock_shared().is_err() { + eprintln!("Waiting for lock on {path_display}...",); + } + file.lock_shared().unwrap_or_else(|e| panic!("Failed to lock {path_display}: {e}")); + } + + locks.push(LockedFile(file)); } + Ok(locks) } #[cfg(test)] diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 42e0d0d2fe3..3b395618273 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -18,13 +18,13 @@ use nargo::{ ops::TestStatus, package::Package, parse_all, prepare_package, workspace::Workspace, PrintOutput, }; -use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; -use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; +use nargo_toml::PackageSelection; +use noirc_driver::{check_crate, CompileOptions}; use noirc_frontend::hir::{FunctionNameMatch, ParsedFiles}; use crate::{cli::check_cmd::check_crate_and_report_errors, errors::CliError}; -use super::{NargoConfig, PackageOptions}; +use super::{LockType, PackageOptions, WorkspaceCommand}; pub(crate) mod formatters; @@ -70,6 +70,16 @@ pub(crate) struct TestCommand { quiet: bool, } +impl WorkspaceCommand for TestCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + fn lock_type(&self) -> LockType { + // Reads the code to compile tests in memory, but doesn't save artifacts. + LockType::None + } +} + #[derive(Debug, Copy, Clone, clap::ValueEnum)] enum Format { /// Print verbose output @@ -116,15 +126,7 @@ struct TestResult { const STACK_SIZE: usize = 4 * 1024 * 1024; -pub(crate) fn run(args: TestCommand, 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, - selection, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - )?; - +pub(crate) fn run(args: TestCommand, workspace: Workspace) -> Result<(), CliError> { let mut file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut file_manager); let parsed_files = parse_all(&file_manager); diff --git a/tooling/nargo_toml/src/lib.rs b/tooling/nargo_toml/src/lib.rs index e0b3fe19e0c..59bee569430 100644 --- a/tooling/nargo_toml/src/lib.rs +++ b/tooling/nargo_toml/src/lib.rs @@ -76,9 +76,9 @@ pub fn find_file_root(current_path: &Path) -> Result { } /// Returns the [PathBuf] of the directory containing the `Nargo.toml` by searching from `current_path` to the root of its [Path], -/// returning at the topmost directory found, i.e. the one corresponding to the entire workspace. +/// returning the topmost directory found, i.e. the one corresponding to the entire workspace. /// -/// Returns a [ManifestError] if no parent directories of `current_path` contain a manifest file. +/// Returns a [ManifestError] if none of the ancestor directories of `current_path` contain a manifest file. pub fn find_package_root(current_path: &Path) -> Result { let root = path_root(current_path); let manifest_path = find_package_manifest(&root, current_path)?;