Skip to content

Commit

Permalink
display the cycle in the error
Browse files Browse the repository at this point in the history
  • Loading branch information
guipublic committed Dec 19, 2023
1 parent b774a0c commit b162050
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
4 changes: 2 additions & 2 deletions tooling/nargo_toml/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ pub enum ManifestError {
#[error(transparent)]
SemverError(SemverError),

#[error("Cyclic package dependency found when processing {toml}")]
CyclicDependency { toml: PathBuf },
#[error("Cyclic package dependency found when processing {cycle}")]
CyclicDependency { cycle: String },
}

#[allow(clippy::enum_variant_names)]
Expand Down
29 changes: 20 additions & 9 deletions tooling/nargo_toml/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))]

use std::{
collections::{BTreeMap, HashSet},
collections::BTreeMap,
path::{Component, Path, PathBuf},
};

Expand Down Expand Up @@ -123,7 +123,7 @@ impl PackageConfig {
fn resolve_to_package(
&self,
root_dir: &Path,
processed: &mut HashSet<String>,
processed: &mut Vec<String>,
) -> Result<Package, ManifestError> {
let name: CrateName = if let Some(name) = &self.package.name {
name.parse().map_err(|_| ManifestError::InvalidPackageName {
Expand Down Expand Up @@ -290,7 +290,7 @@ impl DependencyConfig {
fn resolve_to_dependency(
&self,
pkg_root: &Path,
processed: &mut HashSet<String>,
processed: &mut Vec<String>,
) -> Result<Dependency, ManifestError> {
let dep = match self {
Self::Github { git, tag, directory } => {
Expand Down Expand Up @@ -333,7 +333,7 @@ fn toml_to_workspace(
nargo_toml: NargoToml,
package_selection: PackageSelection,
) -> Result<Workspace, ManifestError> {
let mut resolved = HashSet::new();
let mut resolved = Vec::new();
let workspace = match nargo_toml.config {
Config::Package { package_config } => {
let member = package_config.resolve_to_package(&nargo_toml.root_dir, &mut resolved)?;
Expand Down Expand Up @@ -413,16 +413,25 @@ 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,
processed: &mut HashSet<String>,
processed: &mut Vec<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() });
if processed.contains(&str_path.to_string()) {
let mut cycle = false;
let mut message = String::new();
for toml in processed {
cycle = cycle || toml == str_path;
if cycle {
message += &format!("{} referencing ", toml);
}
}
message += str_path;
return Err(ManifestError::CyclicDependency { cycle: message });
}
// Adds the package to the set of resolved packages
if let Some(str) = toml_path.to_str() {
processed.insert(str.to_string());
processed.push(str.to_string());
}

let nargo_toml = read_toml(toml_path)?;
Expand All @@ -435,7 +444,9 @@ fn resolve_package_from_toml(
Err(ManifestError::UnexpectedWorkspace(toml_path.to_path_buf()))
}
};
processed.remove(str_path);
let pos =
processed.iter().position(|toml| toml == str_path).expect("added package must be here");
processed.remove(pos);
result
}

Expand Down

0 comments on commit b162050

Please sign in to comment.