Fix dependency tracking and show shortest path dependency cycle #151
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,71 +1,152 @@ | ||
| use super::super::build_types::*; | ||
| use crate::helpers; | ||
| use std::collections::{HashMap, HashSet, VecDeque}; | ||
|
|
||
| pub fn find(modules: &Vec<(&String, &Module)>) -> Vec<String> { | ||
| find_shortest_cycle(modules) | ||
| } | ||
|
|
||
| fn find_shortest_cycle(modules: &Vec<(&String, &Module)>) -> Vec<String> { | ||
| let mut shortest_cycle: Vec<String> = Vec::new(); | ||
|
|
||
| // Build a graph representation for easier traversal | ||
| let mut graph: HashMap<String, Vec<String>> = HashMap::new(); | ||
| let mut in_degrees: HashMap<String, usize> = HashMap::new(); | ||
|
|
||
| // First pass: collect all nodes and initialize in-degrees | ||
| for (name, _) in modules { | ||
| graph.insert(name.to_string(), Vec::new()); | ||
| in_degrees.insert(name.to_string(), 0); | ||
| } | ||
|
|
||
| // Second pass: build the graph and count in-degrees | ||
| for (name, module) in modules { | ||
| let deps: Vec<String> = module.deps.iter().cloned().collect(); | ||
|
||
|
|
||
| // Update the graph | ||
| *graph.get_mut(&name.to_string()).unwrap() = deps.clone(); | ||
|
||
|
|
||
| // Update in-degrees | ||
| for dep in deps { | ||
| if let Some(count) = in_degrees.get_mut(&dep) { | ||
| *count += 1; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // OPTIMIZATION 1: Start with nodes that are more likely to be in cycles | ||
| // Sort nodes by their connectivity (in-degree + out-degree) | ||
| let mut start_nodes: Vec<String> = graph.keys().cloned().collect(); | ||
|
||
| start_nodes.sort_by(|a, b| { | ||
| let a_connectivity = in_degrees.get(a).unwrap_or(&0) + graph.get(a).map_or(0, |v| v.len()); | ||
|
Member
There was a problem hiding this comment. If |
||
| let b_connectivity = in_degrees.get(b).unwrap_or(&0) + graph.get(b).map_or(0, |v| v.len()); | ||
| b_connectivity.cmp(&a_connectivity) // Sort in descending order | ||
| }); | ||
|
|
||
| // OPTIMIZATION 2: Keep track of the current shortest cycle length for early termination | ||
| let mut current_shortest_length = usize::MAX; | ||
|
|
||
| // OPTIMIZATION 3: Cache nodes that have been checked and don't have cycles | ||
| let mut no_cycle_cache: HashSet<String> = HashSet::new(); | ||
|
|
||
| // Try BFS from each node to find the shortest cycle | ||
| for start_node in start_nodes { | ||
| // Skip nodes that we know don't have cycles | ||
| if no_cycle_cache.contains(&start_node) { | ||
| continue; | ||
| } | ||
|
|
||
| // Skip nodes with no outgoing edges or no incoming edges | ||
| if graph.get(&start_node).map_or(true, |v| v.is_empty()) | ||
| || in_degrees.get(&start_node).map_or(true, |&d| d == 0) | ||
| { | ||
| no_cycle_cache.insert(start_node.clone()); | ||
| continue; | ||
| } | ||
|
|
||
| if let Some(cycle) = find_cycle_bfs(&start_node, &graph, current_shortest_length) { | ||
| if shortest_cycle.is_empty() || cycle.len() < shortest_cycle.len() { | ||
| shortest_cycle = cycle.clone(); | ||
| current_shortest_length = cycle.len(); | ||
|
|
||
| // OPTIMIZATION 4: If we find a very short cycle (length <= 3), we can stop early | ||
| // as it's unlikely to find a shorter one | ||
| if cycle.len() <= 3 { | ||
| break; | ||
| } | ||
| } | ||
| } else { | ||
| // Cache this node as not having a cycle | ||
| no_cycle_cache.insert(start_node); | ||
| } | ||
| } | ||
|
|
||
| shortest_cycle | ||
| } | ||
|
|
||
| fn find_cycle_bfs( | ||
| start: &String, | ||
| graph: &HashMap<String, Vec<String>>, | ||
| max_length: usize, | ||
| ) -> Option<Vec<String>> { | ||
| // Use a BFS to find the shortest cycle | ||
| let mut queue = VecDeque::new(); | ||
| // Store node -> (distance, parent) | ||
| let mut visited: HashMap<String, (usize, Option<String>)> = HashMap::new(); | ||
|
|
||
| // Initialize with start node | ||
| visited.insert(start.clone(), (0, None)); | ||
| queue.push_back(start.clone()); | ||
|
|
||
| while let Some(current) = queue.pop_front() { | ||
| let (dist, _) = *visited.get(¤t).unwrap(); | ||
|
|
||
| // OPTIMIZATION: Early termination if we've gone too far | ||
| // If we're already at max_length, we won't find a shorter cycle from here | ||
| if dist >= max_length - 1 { | ||
| continue; | ||
| } | ||
|
|
||
| // Check all neighbors | ||
| if let Some(neighbors) = graph.get(¤t) { | ||
| for neighbor in neighbors { | ||
| // If we found the start node again, we have a cycle | ||
| if neighbor == start { | ||
| // Reconstruct the cycle | ||
| let mut path = Vec::new(); | ||
| path.push(start.clone()); | ||
|
|
||
| // Backtrack from current to start using parent pointers | ||
| let mut curr = current.clone(); | ||
| while curr != *start { | ||
| path.push(curr.clone()); | ||
| curr = visited.get(&curr).unwrap().1.clone().unwrap(); | ||
| } | ||
|
|
||
| return Some(path); | ||
| } | ||
|
|
||
| // If not visited, add to queue | ||
| if !visited.contains_key(neighbor) { | ||
| visited.insert(neighbor.clone(), (dist + 1, Some(current.clone()))); | ||
| queue.push_back(neighbor.clone()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| pub fn format(cycle: &[String]) -> String { | ||
| let mut cycle = cycle.to_vec(); | ||
| cycle.reverse(); | ||
| // add the first module to the end of the cycle | ||
| cycle.push(cycle[0].clone()); | ||
|
|
||
| cycle | ||
| .iter() | ||
| .map(|s| helpers::format_namespaced_module_name(s)) | ||
| .collect::<Vec<String>>() | ||
| .join("\n → ") | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -282,7 +282,7 @@ pub fn start( | |
| ) | ||
| .await | ||
| { | ||
| println!("{:?}", e) | ||
|
|
||
| } | ||
| }) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
log::warn!orlog::info!here instead?println!cannot be supressed by log levelsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log::error etc are prepended with
ERROR:it's a bit ugly if it's expected output of the application