Skip to content

Commit

Permalink
separate directory findup logic
Browse files Browse the repository at this point in the history
  • Loading branch information
phated committed Jul 27, 2023
1 parent d871a8b commit 4963946
Show file tree
Hide file tree
Showing 17 changed files with 97 additions and 37 deletions.
12 changes: 6 additions & 6 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{errors::CliError, manifest::resolve_workspace, prepare_package};
use crate::{errors::CliError, manifest::resolve_workspace_in_directory, prepare_package};
use acvm::Backend;
use clap::Args;
use iter_extended::btree_map;
Expand Down Expand Up @@ -30,7 +30,7 @@ pub(crate) fn run<B: Backend>(
args: CheckCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;

for package in &workspace {
check_package(package, &args.compile_options)?;
Expand Down Expand Up @@ -111,7 +111,7 @@ mod tests {
use noirc_abi::{AbiParameter, AbiType, AbiVisibility, Sign};
use noirc_driver::CompileOptions;

use crate::manifest::resolve_workspace;
use crate::manifest::resolve_workspace_in_directory;

use super::create_input_toml_template;

Expand Down Expand Up @@ -166,7 +166,7 @@ d2 = ["", "", ""]
let paths = std::fs::read_dir(pass_dir).unwrap();
for path in paths.flatten() {
let path = path.path();
let workspace = resolve_workspace(&path, None).unwrap();
let workspace = resolve_workspace_in_directory(&path, None).unwrap();
for package in &workspace {
assert!(super::check_package(package, &config).is_ok(), "path: {}", path.display());
}
Expand All @@ -183,7 +183,7 @@ d2 = ["", "", ""]
let paths = std::fs::read_dir(fail_dir).unwrap();
for path in paths.flatten() {
let path = path.path();
let workspace = resolve_workspace(&path, None).unwrap();
let workspace = resolve_workspace_in_directory(&path, None).unwrap();
for package in &workspace {
assert!(
super::check_package(package, &config).is_err(),
Expand All @@ -204,7 +204,7 @@ d2 = ["", "", ""]
let paths = std::fs::read_dir(pass_dir).unwrap();
for path in paths.flatten() {
let path = path.path();
let workspace = resolve_workspace(&path, None).unwrap();
let workspace = resolve_workspace_in_directory(&path, None).unwrap();
for package in &workspace {
assert!(super::check_package(package, &config).is_ok(), "path: {}", path.display());
}
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::fs::{
};
use super::NargoConfig;
use crate::{cli::compile_cmd::compile_circuit, errors::CliError};
use crate::{manifest::resolve_workspace, prepare_package};
use crate::{manifest::resolve_workspace_in_directory, prepare_package};
use acvm::Backend;
use clap::Args;
use nargo::{
Expand All @@ -37,7 +37,7 @@ pub(crate) fn run<B: Backend>(
args: CodegenVerifierCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;

for package in &workspace {
let circuit_build_path = workspace.package_build_path(package);
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use clap::Args;
use nargo::ops::{preprocess_contract_function, preprocess_program};

use crate::errors::CliError;
use crate::manifest::resolve_workspace;
use crate::manifest::resolve_workspace_in_directory;
use crate::prepare_package;

use super::fs::{
Expand Down Expand Up @@ -57,7 +57,7 @@ pub(crate) fn run<B: Backend>(
args: CompileCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;
let circuit_dir = workspace.target_directory_path();

let mut common_reference_string = read_cached_common_reference_string();
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use super::compile_cmd::compile_circuit;
use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir};
use super::NargoConfig;
use crate::errors::CliError;
use crate::manifest::resolve_workspace;
use crate::manifest::resolve_workspace_in_directory;
use crate::prepare_package;

/// Executes a circuit to calculate its return value
Expand All @@ -42,7 +42,7 @@ pub(crate) fn run<B: Backend>(
args: ExecuteCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;
let witness_dir = &workspace.target_directory_path();

for package in &workspace {
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use noirc_driver::CompileOptions;
use noirc_frontend::graph::CrateName;

use crate::{
cli::compile_cmd::compile_circuit, errors::CliError, manifest::resolve_workspace,
cli::compile_cmd::compile_circuit, errors::CliError, manifest::resolve_workspace_in_directory,
prepare_package,
};

Expand All @@ -30,7 +30,7 @@ pub(crate) fn run<B: Backend>(
args: InfoCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;

for package in &workspace {
count_opcodes_and_gates_in_package(backend, package, &args.compile_options)?;
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use super::fs::{
proof::save_proof_to_dir,
};
use super::NargoConfig;
use crate::manifest::resolve_workspace;
use crate::manifest::resolve_workspace_in_directory;
use crate::prepare_package;
use crate::{cli::execute_cmd::execute_program, errors::CliError};

Expand Down Expand Up @@ -56,7 +56,7 @@ pub(crate) fn run<B: Backend>(
args: ProveCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;
let proof_dir = workspace.proofs_directory_path();

for package in &workspace {
Expand Down
6 changes: 3 additions & 3 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use noirc_frontend::{graph::CrateName, hir::Context, node_interner::FuncId};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

use crate::{
cli::check_cmd::check_crate_and_report_errors, errors::CliError, manifest::resolve_workspace,
prepare_package,
cli::check_cmd::check_crate_and_report_errors, errors::CliError,
manifest::resolve_workspace_in_directory, prepare_package,
};

use super::{compile_cmd::optimize_circuit, NargoConfig};
Expand Down Expand Up @@ -39,7 +39,7 @@ pub(crate) fn run<B: Backend>(
) -> Result<(), CliError<B>> {
let test_name: String = args.test_name.unwrap_or_else(|| "".to_owned());

let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;

for package in &workspace {
run_tests(backend, package, &test_name, args.show_output, &args.compile_options)?;
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/verify_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use super::fs::{
};
use super::NargoConfig;
use crate::errors::CliError;
use crate::manifest::resolve_workspace;
use crate::manifest::resolve_workspace_in_directory;
use crate::prepare_package;

use acvm::Backend;
Expand Down Expand Up @@ -46,7 +46,7 @@ pub(crate) fn run<B: Backend>(
args: VerifyCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let workspace = resolve_workspace(&config.program_dir, args.package)?;
let workspace = resolve_workspace_in_directory(&config.program_dir, args.package)?;
let proofs_dir = workspace.proofs_directory_path();
let proof_path = proofs_dir.join(&args.proof).with_extension(PROOF_EXT);

Expand Down
6 changes: 6 additions & 0 deletions crates/nargo_cli/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ pub(crate) enum ManifestError {
#[error("cannot find a Nargo.toml in {}", .0.display())]
MissingFile(PathBuf),

#[error("Cannot read file {0}. Does it exist?")]
ReadFailed(PathBuf),

#[error("Nargo.toml is missing a parent directory")]
MissingParent,

/// Package manifest is unreadable.
#[error("Nargo.toml is badly formed, could not parse.\n\n {0}")]
MalformedFile(#[from] toml::de::Error),
Expand Down
50 changes: 35 additions & 15 deletions crates/nargo_cli/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,37 +114,35 @@ impl DependencyConfig {
Self::Github { git, tag } => {
let dir_path = clone_git_repo(git, tag).map_err(ManifestError::GitError)?;
let (entry_path, crate_type) = super::lib_or_bin(&dir_path)?;
let workspace = resolve_workspace(&dir_path, None)?;
let toml_path = dir_path.join("Nargo.toml");
let workspace = resolve_workspace_from_toml(&toml_path, None)?;
Ok(package::Dependency::Remote { entry_path, crate_type, workspace })
}
Self::Path { path } => {
let dir_path = pkg_root.join(path);
let (entry_path, crate_type) = super::lib_or_bin(&dir_path)?;
let workspace = resolve_workspace(&dir_path, None)?;
let toml_path = dir_path.join("Nargo.toml");
let workspace = resolve_workspace_from_toml(&toml_path, None)?;
Ok(package::Dependency::Local { entry_path, crate_type, workspace })
}
}
}
}

fn toml_in_directory(dir_path: &Path) -> Result<NargoToml, ManifestError> {
let toml_path = super::find_package_manifest(dir_path)?;
let toml_as_string =
std::fs::read_to_string(toml_path).expect("ice: path given for toml file is invalid");
fn read_toml(toml_path: &Path) -> Result<NargoToml, ManifestError> {
let toml_as_string = std::fs::read_to_string(toml_path)
.map_err(|_| ManifestError::ReadFailed(toml_path.to_path_buf()))?;
let root_dir = toml_path.parent().ok_or(ManifestError::MissingParent)?;
let nargo_toml =
NargoToml { root_dir: dir_path.to_path_buf(), config: toml_as_string.try_into()? };
NargoToml { root_dir: root_dir.to_path_buf(), config: toml_as_string.try_into()? };

Ok(nargo_toml)
}

/// Resolves a Nargo.toml file into a `Workspace` struct as defined by our `nargo` core.
/// Recursively resolves filesystem locations and turns them into `Package` structs
pub(crate) fn resolve_workspace(
dir_path: &Path,
fn toml_to_workspace(
nargo_toml: NargoToml,
selected_package: Option<CrateName>,
) -> Result<Workspace, ManifestError> {
let nargo_toml = toml_in_directory(dir_path)?;

let workspace = match nargo_toml.config {
Config::Package { package_config } => {
let member = package_config.to_package(&nargo_toml.root_dir)?;
Expand All @@ -162,8 +160,8 @@ pub(crate) fn resolve_workspace(
let mut members = Vec::new();
let mut selected_package_index = None;
for (index, member_path) in workspace_config.members.into_iter().enumerate() {
let package_root_dir = nargo_toml.root_dir.join(&member_path);
let nargo_toml = toml_in_directory(&package_root_dir)?;
let package_toml_path = nargo_toml.root_dir.join(&member_path).join("Nargo.toml");
let nargo_toml = read_toml(&package_toml_path)?;
match nargo_toml.config {
Config::Workspace { .. } => {
return Err(ManifestError::AdditionalWorkspace(member_path))
Expand Down Expand Up @@ -206,6 +204,28 @@ pub(crate) fn resolve_workspace(
Ok(workspace)
}

/// Resolves a Nargo.toml file into a `Workspace` struct as defined by our `nargo` core.
/// Recursively resolves filesystem locations and turns them into `Package` structs
pub(crate) fn resolve_workspace_from_toml(
toml_path: &Path,
selected_package: Option<CrateName>,
) -> Result<Workspace, ManifestError> {
let nargo_toml = read_toml(toml_path)?;

toml_to_workspace(nargo_toml, selected_package)
}

/// Recursively resolves filesystem locations and turns them into `Package` structs
pub(crate) fn resolve_workspace_in_directory(
dir_path: &Path,
selected_package: Option<CrateName>,
) -> Result<Workspace, ManifestError> {
let toml_path = super::find_package_manifest(dir_path)?;
let nargo_toml = read_toml(&toml_path)?;

toml_to_workspace(nargo_toml, selected_package)
}

#[test]
fn parse_standard_toml() {
let src = r#"
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/tests/test_data_ssa_refactor/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ exclude = []


# List of tests (as their directory name) expecting to fail: if the test pass, we report an error.
fail = ["brillig_assert_fail", "workspace_fail"]
fail = ["brillig_assert_fail", "workspace_fail", "workspace_missing_toml"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[workspace]
members = ["crates/a", "crates/b"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "1"
y = "0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fn main(x : Field, y : pub Field) {
assert(x != y);
}

#[test]
fn a() {
main(1, 2);

// Uncomment to make test fail
// main(1, 1);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "b"
authors = [""]
compiler_version = "0.8.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "1"
y = "0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fn main(x : Field, y : pub Field) {
assert(x != y);
}

#[test]
fn b() {
main(1, 2);

// Uncomment to make test fail
// main(1, 1);
}

0 comments on commit 4963946

Please sign in to comment.