Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): Only lock the packages selected in the workspace #7345

Merged
merged 7 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 Nargo.toml template if it doesn't exist, otherwise only writes if `allow_overwrite` is true,
// so it shouldn't lead to any 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);
Expand Down
41 changes: 16 additions & 25 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
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};
Expand All @@ -23,7 +21,7 @@
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
Expand All @@ -40,41 +38,31 @@
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<Workspace, ManifestError> {
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();

// No specific tickrate, max debounce time 1 seconds

Check warning on line 65 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (tickrate)
let mut debouncer = new_debouncer(Duration::from_secs(1), None, tx)?;

// Add a path to be watched. All files and directories at that path and
Expand All @@ -82,7 +70,7 @@
debouncer.watcher().watch(&workspace.root_dir, RecursiveMode::Recursive)?;

let mut screen = std::io::stdout();
write!(screen, "{}", termion::cursor::Save).unwrap();

Check warning on line 73 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (termion)
screen.flush().unwrap();
let _ = compile_workspace_full(workspace, compile_options);
for res in rx {
Expand All @@ -103,7 +91,7 @@
});

if noir_files_modified {
write!(screen, "{}{}", termion::cursor::Restore, termion::clear::AfterCursor).unwrap();

Check warning on line 94 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (termion)

Check warning on line 94 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (termion)
screen.flush().unwrap();
let _ = compile_workspace_full(workspace, compile_options);
}
Expand Down Expand Up @@ -334,8 +322,11 @@
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
Expand Down
4 changes: 1 addition & 3 deletions tooling/nargo_cli/src/cli/dap_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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.
//
Expand Down
33 changes: 19 additions & 14 deletions tooling/nargo_cli/src/cli/debug_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -52,17 +50,24 @@ pub(crate) struct DebugCommand {
skip_instrumentation: Option<bool>,
}

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 {
Expand Down
27 changes: 16 additions & 11 deletions tooling/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -46,14 +47,18 @@ pub(crate) struct ExecuteCommand {
oracle_resolver: Option<String>,
}

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.
Expand Down
27 changes: 14 additions & 13 deletions tooling/nargo_cli/src/cli/export_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)]
Expand All @@ -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);
Expand Down
37 changes: 20 additions & 17 deletions tooling/nargo_cli/src/cli/fmt_cmd.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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;
Expand Down
Loading
Loading