Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions rewatch/src/build/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,28 @@ pub fn compile(
.collect::<Vec<(&String, &Module)>>(),
);

compile_errors.push_str(&format!(
"\n{}\n{}\n",
let guidance = "\nHow to fix:\n- Extract shared code into a new module both depend on.\n- If a namespace entry is part of the cycle, import submodules directly instead of the entry.\n";
Copy link
Member

@tsnobip tsnobip Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if what you mean by

If a namespace entry is part of the cycle, import submodules directly instead of the entry

Do you have any example?
Do you mean you should drop the namespace and directly use the name of the module? Submodule feels a bit weird here given it can be a file module, not necessarily a submodule, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, was going to remove that one, I didn't find a good way to word it. Essentially it's about when you access a module through the main namespace file. But I'll just remove this hint.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, for some reason this used to give me error ms using bsb but not with rewatch.

I think indeed that it adds more confusion than it solves, better without it!

let message = format!(
"\n{}\n{}\n{}",
style("Can't continue... Found a circular dependency in your code:").red(),
dependency_cycle::format(&cycle)
))
dependency_cycle::format(&cycle, build_state),
guidance
);

// Append the error to the logs of all packages involved in the cycle,
// so editor tooling can surface it from .compiler.log
let mut touched_packages = AHashSet::<String>::new();
for module_name in cycle.iter() {
if let Some(module) = build_state.get_module(module_name) {
if touched_packages.insert(module.package_name.clone()) {
if let Some(package) = build_state.get_package(&module.package_name) {
logs::append(package, &message);
}
}
}
}

compile_errors.push_str(&message)
}
if !compile_errors.is_empty() {
break;
Expand Down
51 changes: 43 additions & 8 deletions rewatch/src/build/compile/dependency_cycle.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use super::super::build_types::*;
use crate::helpers;
use ahash::AHashSet;
use std::collections::{HashMap, HashSet, VecDeque};
use std::{
collections::{HashMap, HashSet, VecDeque},
path::Path,
};

pub fn find(modules: &Vec<(&String, &Module)>) -> Vec<String> {
find_shortest_cycle(modules)
Expand Down Expand Up @@ -139,15 +142,47 @@ fn find_cycle_bfs(
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());
pub fn format(cycle: &[String], build_state: &BuildCommandState) -> String {
// Mirror dependency_cycle::format semantics: reverse then close the loop
let mut nodes = cycle.to_vec();
if nodes.is_empty() {
return String::new();
}
nodes.reverse();
nodes.push(nodes[0].clone());

let root = build_state.get_root_config().path.parent().unwrap();

cycle
nodes
.iter()
.map(|s| helpers::format_namespaced_module_name(s))
.map(|name| {
let display_name = helpers::format_namespaced_module_name(name);

match build_state.get_module(name) {
Some(Module {
source_type: SourceType::SourceFile(source_file),
package_name,
..
}) => {
if let Some(package) = build_state.get_package(package_name) {
let abs_path = Path::new(&package.path).join(&source_file.implementation.path);
let rel_path = abs_path
.strip_prefix(root)
.unwrap_or(&abs_path)
.to_string_lossy()
.replace('\\', "/");
format!("{} ({})", display_name, rel_path)
} else {
display_name
}
}
Some(Module {
source_type: SourceType::MlMap(_),
..
})
| None => display_name,
}
})
.collect::<Vec<String>>()
.join("\n → ")
}
14 changes: 9 additions & 5 deletions rewatch/tests/snapshots/dependency-cycle.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ The field 'bsc-flags' found in the package config of '@testrepo/deprecated-confi
Use 'compiler-flags' instead.

Can't continue... Found a circular dependency in your code:
Dep01
→ Dep02
→ NS
→ NewNamespace.NS_alias
→ Dep01
Dep01 (packages/dep01/src/Dep01.res)
→ Dep02 (packages/dep02/src/Dep02.res)
→ NS (packages/new-namespace/src/NS.res)
→ NewNamespace.NS_alias (packages/new-namespace/src/NS_alias.res)
→ Dep01 (packages/dep01/src/Dep01.res)

How to fix:
- Extract shared code into a new module both depend on.
- If a namespace entry is part of the cycle, import submodules directly instead of the entry.

Incremental build failed. Error:  Failed to Compile. See Errors Above
Expand Down
Loading