Fix dependency tracking and show shortest path dependency cycle #151
Conversation
| @@ -330,7 +330,7 @@ pub fn incremental_build( | |||
| pb.finish(); | |||
| } | |||
|
|
|||
| log::error!("Could not parse source files: {}", &err); | |||
| println!("Could not parse source files: {}", &err); | |||
There was a problem hiding this comment.
Perhaps log::warn! or log::info! here instead? println! cannot be supressed by log levels
There was a problem hiding this comment.
The log::error etc are prepended with ERROR: it's a bit ugly if it's expected output of the application
| @@ -405,7 +405,7 @@ pub fn incremental_build( | |||
| pb.finish(); | |||
| if !compile_errors.is_empty() { | |||
| if helpers::contains_ascii_characters(&compile_warnings) { | |||
| log::error!("{}", &compile_warnings); | |||
| println!("{}", &compile_warnings); | |||
| @@ -417,7 +417,7 @@ pub fn incremental_build( | |||
| default_timing.unwrap_or(compile_duration).as_secs_f64() | |||
| ); | |||
| } | |||
| log::error!("{}", &compile_errors); | |||
| println!("{}", &compile_errors); | |||
| @@ -282,7 +282,7 @@ pub fn start( | |||
| ) | |||
| .await | |||
| { | |||
| log::error!("{:?}", e) | |||
| println!("{:?}", e) | |||
| let deps: Vec<String> = module.deps.iter().cloned().collect(); | ||
|
|
||
| // Update the graph | ||
| *graph.get_mut(&name.to_string()).unwrap() = deps.clone(); |
There was a problem hiding this comment.
I don't think you have to do this clone here?
There was a problem hiding this comment.
We couldn't, but I could swap the next statement with this one and only borrow there 👍
| return stack; | ||
| // Second pass: build the graph and count in-degrees | ||
| for (name, module) in modules { | ||
| let deps: Vec<String> = module.deps.iter().cloned().collect(); |
There was a problem hiding this comment.
It could be that you can do into_iter() which will consume the iterator straight away? Could be the other way around though - not 100% sure, but one of them is more optimal
| stack.push(module_name.to_string()) | ||
| // 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(); |
There was a problem hiding this comment.
I think you can first filter out all graph keys that don't have any edges connected to them here. I think the check might be happening later, but we can just drop them here.
Also - I'm not sure if you need to clone these here
| // 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()); |
There was a problem hiding this comment.
If in_degrees is 0 here, or you can't get them, we should ignore them, they can't be part of a cycle by definition
No description provided.