diff --git a/crates/nargo/src/manifest/mod.rs b/crates/nargo/src/manifest/mod.rs index ea51d622d82..483a0f6102a 100644 --- a/crates/nargo/src/manifest/mod.rs +++ b/crates/nargo/src/manifest/mod.rs @@ -64,6 +64,7 @@ pub struct WorkspaceConfig { #[allow(dead_code)] #[derive(Default, Debug, Deserialize, Clone)] pub struct PackageMetadata { + pub name: Option<String>, // Note: a package name is not needed unless there is a registry authors: Vec<String>, // If not compiler version is supplied, the latest is used diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index 5abef7bfb2a..9b3f4e459e4 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -36,7 +36,7 @@ fn check_from_path<B: Backend>( program_dir: &Path, compile_options: &CompileOptions, ) -> Result<(), CliError<B>> { - let mut context = resolve_root_manifest(program_dir)?; + let mut context = resolve_root_manifest(program_dir, None)?; check_crate_and_report_errors( &mut context, compile_options.deny_warnings, diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index c19ed5df3a6..cedf558bcb8 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -52,7 +52,7 @@ pub(crate) fn run<B: Backend>( } None => { let (program, _) = - compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?; + compile_circuit(backend, None, config.program_dir.as_ref(), &args.compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index da945e6e15e..d6d3deda55f 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -57,7 +57,7 @@ pub(crate) fn run<B: Backend>( // If contracts is set we're compiling every function in a 'contract' rather than just 'main'. if args.contracts { - let mut context = resolve_root_manifest(&config.program_dir)?; + let mut context = resolve_root_manifest(&config.program_dir, None)?; let result = compile_contracts(&mut context, &args.compile_options); let contracts = report_errors(result, &context, args.compile_options.deny_warnings)?; @@ -101,7 +101,8 @@ pub(crate) fn run<B: Backend>( ); } } else { - let (program, _) = compile_circuit(backend, &config.program_dir, &args.compile_options)?; + let (program, _) = + compile_circuit(backend, None, &config.program_dir, &args.compile_options)?; common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; @@ -119,10 +120,11 @@ pub(crate) fn run<B: Backend>( pub(crate) fn compile_circuit<B: Backend>( backend: &B, + package: Option<String>, program_dir: &Path, compile_options: &CompileOptions, ) -> Result<(CompiledProgram, Context), CliError<B>> { - let mut context = resolve_root_manifest(program_dir)?; + let mut context = resolve_root_manifest(program_dir, package)?; let result = compile_main(&mut context, compile_options); let mut program = report_errors(result, &context, compile_options.deny_warnings)?; diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index d8b764a6282..eaaea6d4ab3 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -61,7 +61,7 @@ fn execute_with_path<B: Backend>( prover_name: String, compile_options: &CompileOptions, ) -> Result<(Option<InputValue>, WitnessMap), CliError<B>> { - let (compiled_program, context) = compile_circuit(backend, program_dir, compile_options)?; + let (compiled_program, context) = compile_circuit(backend, None, program_dir, compile_options)?; let CompiledProgram { abi, circuit, debug } = compiled_program; // Parse the initial witness values from Prover.toml diff --git a/crates/nargo_cli/src/cli/gates_cmd.rs b/crates/nargo_cli/src/cli/gates_cmd.rs index 030daef9719..dc4a63c4105 100644 --- a/crates/nargo_cli/src/cli/gates_cmd.rs +++ b/crates/nargo_cli/src/cli/gates_cmd.rs @@ -28,7 +28,8 @@ fn count_gates_with_path<B: Backend, P: AsRef<Path>>( program_dir: P, compile_options: &CompileOptions, ) -> Result<(), CliError<B>> { - let (compiled_program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (compiled_program, _) = + compile_circuit(backend, None, program_dir.as_ref(), compile_options)?; let num_opcodes = compiled_program.circuit.opcodes.len(); println!( diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 346638835e3..92e9599cd8b 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -49,6 +49,9 @@ pub(crate) struct ProveCommand { #[clap(flatten)] compile_options: CompileOptions, + + #[clap(long)] + package: Option<String>, } pub(crate) fn run<B: Backend>( @@ -67,6 +70,7 @@ pub(crate) fn run<B: Backend>( args.proof_name, args.prover_name, args.verifier_name, + args.package, config.program_dir, proof_dir, circuit_build_path, @@ -83,6 +87,7 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>( proof_name: Option<String>, prover_name: String, verifier_name: String, + package: Option<String>, program_dir: P, proof_dir: P, circuit_build_path: Option<PathBuf>, @@ -104,7 +109,7 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>( } None => { let (program, context) = - compile_circuit(backend, program_dir.as_ref(), compile_options)?; + compile_circuit(backend, package, program_dir.as_ref(), compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index 5634c913c84..dcc4cce5734 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -40,7 +40,7 @@ fn run_tests<B: Backend>( test_name: &str, compile_options: &CompileOptions, ) -> Result<(), CliError<B>> { - let mut context = resolve_root_manifest(program_dir)?; + let mut context = resolve_root_manifest(program_dir, None)?; check_crate_and_report_errors( &mut context, compile_options.deny_warnings, diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index cc85159e488..f9068c66c9c 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -83,7 +83,8 @@ fn verify_with_path<B: Backend, P: AsRef<Path>>( (common_reference_string, program) } None => { - let (program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let (program, _) = + compile_circuit(backend, None, program_dir.as_ref(), compile_options)?; let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index 5fc158d1d7f..f5cc640ff5f 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -3,10 +3,10 @@ use std::{ path::{Path, PathBuf}, }; -use nargo::manifest::{Dependency, Manifest, PackageManifest}; +use nargo::manifest::{Dependency, Manifest, PackageManifest, WorkspaceConfig}; use noirc_driver::{add_dep, create_local_crate, create_non_local_crate}; use noirc_frontend::{ - graph::{CrateId, CrateType}, + graph::{CrateId, CrateName, CrateType}, hir::Context, }; use thiserror::Error; @@ -49,6 +49,20 @@ pub(crate) enum DependencyResolutionError { /// Use workspace as a dependency is not currently supported #[error("use workspace as a dependency is not currently supported")] WorkspaceDependency, + + /// Multiple workspace roots found in the same workspace + #[error("multiple workspace roots found in the same workspace:\n{}\n{}", root.display(), member.display())] + MultipleWorkspace { root: PathBuf, member: PathBuf }, + + /// Invalid character `-` in package name + #[error("invalid character `-` in package name")] + InvalidPackageName, + + #[error("package specification `{0}` did not match any packages")] + PackageNotFound(String), + + #[error("two packages named `{0}` in this workspace")] + PackageCollision(String), } #[derive(Debug, Clone)] @@ -72,6 +86,7 @@ struct CachedDep { /// XXX: Need to handle when a local package changes! pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, + package: Option<String>, ) -> Result<Context, DependencyResolutionError> { let mut context = Context::default(); @@ -79,37 +94,22 @@ pub(crate) fn resolve_root_manifest( let manifest = super::manifest::parse(&manifest_path)?; match manifest { - Manifest::Package(package) => { + Manifest::Package(inner) => { let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; - let crate_id = create_local_crate(&mut context, entry_path, crate_type); + let crate_id = create_local_crate(&mut context, entry_path, crate_type); let pkg_root = manifest_path.parent().expect("Every manifest path has a parent."); - resolve_manifest(&mut context, crate_id, package, pkg_root)?; + + resolve_package_manifest(&mut context, crate_id, inner, pkg_root)?; } Manifest::Workspace(workspace) => { - let config = workspace.config; - let members = config.members; - - let maybe_local = config - .default_member - .or_else(|| members.last().cloned()) - .map(|member| dir_path.join(member)); - - let default_member = match maybe_local { - Some(member) => member, - None => { - return Err(DependencyResolutionError::EmptyWorkspace { path: manifest_path }) - } - }; - - let (entry_path, _crate_type) = super::lib_or_bin(default_member)?; - let _local = create_local_crate(&mut context, entry_path, CrateType::Workspace); - - for member in members { - let path: PathBuf = dir_path.join(member); - let (entry_path, crate_type) = super::lib_or_bin(path)?; - create_non_local_crate(&mut context, entry_path, crate_type); - } + resolve_workspace_manifest( + &mut context, + package, + manifest_path, + dir_path, + workspace.config, + )?; } }; @@ -122,7 +122,7 @@ pub(crate) fn resolve_root_manifest( // We do not need to add stdlib, as it's implicitly // imported. However, it may be helpful to have the stdlib imported by the // package manager. -fn resolve_manifest( +fn resolve_package_manifest( context: &mut Context, parent_crate: CrateId, manifest: PackageManifest, @@ -154,11 +154,84 @@ fn resolve_manifest( return Err(DependencyResolutionError::RemoteDepWithLocalDep { dependency_path }); } // TODO: Why did it create a new resolver? - resolve_manifest(context, crate_id, dep_meta.manifest, &dependency_path)?; + resolve_package_manifest(context, crate_id, dep_meta.manifest, &dependency_path)?; } Ok(()) } +fn crate_name(name: Option<CrateName>) -> String { + name.map(|name| name.as_string()).unwrap_or_else(|| "[unnamed]".to_string()) +} + +fn resolve_workspace_manifest( + context: &mut Context, + mut local_package: Option<String>, + manifest_path: PathBuf, + dir_path: &Path, + workspace: WorkspaceConfig, +) -> Result<(), DependencyResolutionError> { + let members = workspace.members; + let mut packages = HashMap::new(); + + if members.is_empty() { + return Err(DependencyResolutionError::EmptyWorkspace { path: manifest_path }); + } + + for member in &members { + let member_path: PathBuf = dir_path.join(member); + let member_member_path = super::find_package_manifest(&member_path)?; + let member_manifest = super::manifest::parse(&member_member_path)?; + + match member_manifest { + Manifest::Package(inner) => { + let name = inner + .package + .name + .map(|name| { + CrateName::new(&name) + .map_err(|_name| DependencyResolutionError::InvalidPackageName) + }) + .transpose()?; + + if packages.insert(name.clone(), member_path).is_some() { + return Err(DependencyResolutionError::PackageCollision(crate_name(name))); + } + + if local_package.is_none() && workspace.default_member.as_ref() == Some(member) { + local_package = name.as_ref().map(CrateName::as_string); + } + } + Manifest::Workspace(_) => { + return Err(DependencyResolutionError::MultipleWorkspace { + root: manifest_path, + member: member_member_path, + }) + } + } + } + + let local_package = match local_package { + Some(local_package) => CrateName::new(&local_package) + .map_err(|_| DependencyResolutionError::InvalidPackageName)? + .into(), + None => packages.keys().last().expect("non-empty packages").clone(), + }; + + let local_crate = packages + .remove(&local_package) + .ok_or_else(|| DependencyResolutionError::PackageNotFound(crate_name(local_package)))?; + + let (entry_path, _crate_type) = super::lib_or_bin(local_crate)?; + create_local_crate(context, entry_path, CrateType::Workspace); + + for (_, package_path) in packages.drain() { + let (entry_path, crate_type) = super::lib_or_bin(package_path)?; + create_non_local_crate(context, entry_path, crate_type); + } + + Ok(()) +} + /// If the dependency is remote, download the dependency /// and return the directory path along with the metadata /// Needed to fill the CachedDep struct diff --git a/crates/nargo_cli/tests/test_data/workspace/crates/a/Nargo.toml b/crates/nargo_cli/tests/test_data/workspace/crates/a/Nargo.toml index dc0c2f8917c..5ff1a743e3d 100644 --- a/crates/nargo_cli/tests/test_data/workspace/crates/a/Nargo.toml +++ b/crates/nargo_cli/tests/test_data/workspace/crates/a/Nargo.toml @@ -1,4 +1,5 @@ [package] +name = "a" authors = [""] compiler_version = "0.8.0" diff --git a/crates/nargo_cli/tests/test_data/workspace/crates/b/Nargo.toml b/crates/nargo_cli/tests/test_data/workspace/crates/b/Nargo.toml index dc0c2f8917c..8ae69a781eb 100644 --- a/crates/nargo_cli/tests/test_data/workspace/crates/b/Nargo.toml +++ b/crates/nargo_cli/tests/test_data/workspace/crates/b/Nargo.toml @@ -1,4 +1,5 @@ [package] +name = "b" authors = [""] compiler_version = "0.8.0" diff --git a/crates/nargo_cli/tests/test_data/workspace_default_member/a/Nargo.toml b/crates/nargo_cli/tests/test_data/workspace_default_member/a/Nargo.toml index dc0c2f8917c..5ff1a743e3d 100644 --- a/crates/nargo_cli/tests/test_data/workspace_default_member/a/Nargo.toml +++ b/crates/nargo_cli/tests/test_data/workspace_default_member/a/Nargo.toml @@ -1,4 +1,5 @@ [package] +name = "a" authors = [""] compiler_version = "0.8.0" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/a/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/a/Nargo.toml index dc0c2f8917c..5ff1a743e3d 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/a/Nargo.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/a/Nargo.toml @@ -1,4 +1,5 @@ [package] +name = "a" authors = [""] compiler_version = "0.8.0" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/b/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/b/Nargo.toml index dc0c2f8917c..8ae69a781eb 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/b/Nargo.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/workspace/crates/b/Nargo.toml @@ -1,4 +1,5 @@ [package] +name = "b" authors = [""] compiler_version = "0.8.0" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/workspace_default_member/a/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/workspace_default_member/a/Nargo.toml index dc0c2f8917c..5ff1a743e3d 100644 --- a/crates/nargo_cli/tests/test_data_ssa_refactor/workspace_default_member/a/Nargo.toml +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/workspace_default_member/a/Nargo.toml @@ -1,4 +1,5 @@ [package] +name = "a" authors = [""] compiler_version = "0.8.0" diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index 271bcc19e67..f87a9b3347d 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/crates/noirc_frontend/src/graph/mod.rs @@ -21,7 +21,7 @@ impl CrateId { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct CrateName(SmolStr); impl CrateName {