fix: checks for cyclic dependencies

tooling/nargo_toml/src/errors.rs

@@ -69,6 +69,9 @@ pub enum ManifestError {
 
     #[error(transparent)]
     SemverError(SemverError),
+
+    #[error("Cyclic package dependency found when processing {toml}")]
+    CyclicDependency { toml: PathBuf },
 }
 
 #[allow(clippy::enum_variant_names)]

tooling/nargo_toml/src/lib.rs

@@ -4,7 +4,7 @@
 #![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))]
 
 use std::{
-    collections::BTreeMap,
+    collections::{BTreeMap, HashSet},
     path::{Component, Path, PathBuf},
 };
 
@@ -99,7 +99,11 @@ struct PackageConfig {
 }
 
 impl PackageConfig {
-    fn resolve_to_package(&self, root_dir: &Path) -> Result<Package, ManifestError> {
+    fn resolve_to_package(
+        &self,
+        root_dir: &Path,
+        processed: &mut HashSet<String>,
+    ) -> Result<Package, ManifestError> {
         let name: CrateName = if let Some(name) = &self.package.name {
             name.parse().map_err(|_| ManifestError::InvalidPackageName {
                 toml: root_dir.join("Nargo.toml"),
@@ -115,7 +119,7 @@ impl PackageConfig {
                 toml: root_dir.join("Nargo.toml"),
                 name: name.into(),
             })?;
-            let resolved_dep = dep_config.resolve_to_dependency(root_dir)?;
+            let resolved_dep = dep_config.resolve_to_dependency(root_dir, processed)?;
 
             dependencies.insert(name, resolved_dep);
         }
@@ -263,7 +267,11 @@ enum DependencyConfig {
 }
 
 impl DependencyConfig {
-    fn resolve_to_dependency(&self, pkg_root: &Path) -> Result<Dependency, ManifestError> {
+    fn resolve_to_dependency(
+        &self,
+        pkg_root: &Path,
+        processed: &mut HashSet<String>,
+    ) -> Result<Dependency, ManifestError> {
         let dep = match self {
             Self::Github { git, tag, directory } => {
                 let dir_path = clone_git_repo(git, tag).map_err(ManifestError::GitError)?;
@@ -280,13 +288,13 @@ impl DependencyConfig {
                     dir_path
                 };
                 let toml_path = project_path.join("Nargo.toml");
-                let package = resolve_package_from_toml(&toml_path)?;
+                let package = resolve_package_from_toml(&toml_path, processed)?;
                 Dependency::Remote { package }
             }
             Self::Path { path } => {
                 let dir_path = pkg_root.join(path);
                 let toml_path = dir_path.join("Nargo.toml");
-                let package = resolve_package_from_toml(&toml_path)?;
+                let package = resolve_package_from_toml(&toml_path, processed)?;
                 Dependency::Local { package }
             }
         };
@@ -305,9 +313,10 @@ fn toml_to_workspace(
     nargo_toml: NargoToml,
     package_selection: PackageSelection,
 ) -> Result<Workspace, ManifestError> {
+    let mut resolved = HashSet::new();
     let workspace = match nargo_toml.config {
         Config::Package { package_config } => {
-            let member = package_config.resolve_to_package(&nargo_toml.root_dir)?;
+            let member = package_config.resolve_to_package(&nargo_toml.root_dir, &mut resolved)?;
             match &package_selection {
                 PackageSelection::Selected(selected_name) if selected_name != &member.name => {
                     return Err(ManifestError::MissingSelectedPackage(member.name))
@@ -325,7 +334,7 @@ fn toml_to_workspace(
             for (index, member_path) in workspace_config.members.into_iter().enumerate() {
                 let package_root_dir = nargo_toml.root_dir.join(&member_path);
                 let package_toml_path = package_root_dir.join("Nargo.toml");
-                let member = resolve_package_from_toml(&package_toml_path)?;
+                let member = resolve_package_from_toml(&package_toml_path, &mut resolved)?;
 
                 match &package_selection {
                     PackageSelection::Selected(selected_name) => {
@@ -382,17 +391,32 @@ fn read_toml(toml_path: &Path) -> Result<NargoToml, ManifestError> {
 }
 
 /// Resolves a Nargo.toml file into a `Package` struct as defined by our `nargo` core.
-fn resolve_package_from_toml(toml_path: &Path) -> Result<Package, ManifestError> {
+fn resolve_package_from_toml(
+    toml_path: &Path,
+    processed: &mut HashSet<String>,
+) -> Result<Package, ManifestError> {
+    // Checks for cyclic dependencies
+    let str_path = toml_path.to_str().expect("ICE - path is empty");
+    if processed.contains(str_path) {
+        return Err(ManifestError::CyclicDependency { toml: toml_path.to_path_buf() });
+    }
+    // Adds the package to the set of resolved packages
+    if let Some(str) = toml_path.to_str() {
+        processed.insert(str.to_string());
+    }
+
     let nargo_toml = read_toml(toml_path)?;
 
-    match nargo_toml.config {
+    let result = match nargo_toml.config {
         Config::Package { package_config } => {
-            package_config.resolve_to_package(&nargo_toml.root_dir)
+            package_config.resolve_to_package(&nargo_toml.root_dir, processed)
         }
         Config::Workspace { .. } => {
             Err(ManifestError::UnexpectedWorkspace(toml_path.to_path_buf()))
         }
-    }
+    };
+    processed.remove(str_path);
+    result
 }
 
 #[derive(Debug, PartialEq, Eq)]