v.depgraph: fix detection of indirect module dependency cycles

vlib/v/builder/builder.v

@@ -174,12 +174,12 @@ pub fn (mut b Builder) parse_imports() {
 pub fn (mut b Builder) resolve_deps() {
 	graph := b.import_graph()
 	deps_resolved := graph.resolve()
-	cycles := deps_resolved.display_cycles()
 	if b.pref.is_verbose {
 		eprintln('------ resolved dependencies graph: ------')
 		eprintln(deps_resolved.display())
 		eprintln('------------------------------------------')
 	}
+	cycles := deps_resolved.display_cycles()
 	if cycles.len > 1 {
 		verror('error: import cycle detected between the following modules: \n' + cycles)
 	}
@@ -210,6 +210,7 @@ pub fn (b &Builder) import_graph() &depgraph.DepGraph {
 	builtins := util.builtin_module_parts.clone()
 	mut graph := depgraph.new_dep_graph()
 	for p in b.parsed_files {
+		// eprintln('p.path: $p.path')
 		mut deps := []string{}
 		if p.mod.name !in builtins {
 			deps << 'builtin'
@@ -228,6 +229,9 @@ pub fn (b &Builder) import_graph() &depgraph.DepGraph {
 		}
 		graph.add(p.mod.name, deps)
 	}
+	$if trace_import_graph ? {
+		eprintln(graph.display())
+	}
 	return graph
 }
 

vlib/v/depgraph/depgraph.v

@@ -96,8 +96,10 @@ pub fn (graph &DepGraph) resolve() &DepGraph {
 		node_names.add(node.name, node.deps)
 		node_deps.add(node.name, node.deps)
 	}
+	mut iterations := 0
 	mut resolved := new_dep_graph()
 	for node_deps.size() != 0 {
+		iterations++
 		mut ready_set := []string{}
 		for name in node_deps.keys {
 			deps := node_deps.get(name)
@@ -130,31 +132,63 @@ pub fn (graph &DepGraph) last_node() DepGraphNode {
 }
 
 pub fn (graph &DepGraph) display() string {
-	mut out := '\n'
+	mut out := []string{}
 	for node in graph.nodes {
 		for dep in node.deps {
-			out += ' * $node.name -> $dep\n'
+			out << ' * $node.name -> $dep'
 		}
 	}
-	return out
+	return out.join('\n')
+}
+
+struct NodeNames {
+mut:
+	is_cycle map[string]bool
+	names    map[string][]string
 }
 
 pub fn (graph &DepGraph) display_cycles() string {
-	mut node_names := map[string]DepGraphNode{}
+	mut out := []string{}
+	mut nn := NodeNames{}
 	for node in graph.nodes {
-		node_names[node.name] = node
+		nn.names[node.name] = node.deps
 	}
-	mut out := '\n'
-	for node in graph.nodes {
-		for dep in node.deps {
-			if dep !in node_names {
-				continue
-			}
-			dn := node_names[dep]
-			if node.name in dn.deps {
-				out += ' * $node.name -> $dep\n'
-			}
+	for k, _ in nn.names {
+		mut cycle_names := []string{}
+		if k in nn.is_cycle {
+			continue
+		}
+		if nn.is_part_of_cycle(k, mut cycle_names) {
+			out << ' * ' + cycle_names.join(' -> ')
+			nn.is_cycle = map[string]bool{}
 		}
 	}
-	return out
+	return out.join('\n')
+}
+
+fn (mut nn NodeNames) is_part_of_cycle(name string, mut already_seen []string) bool {
+	if name in nn.is_cycle {
+		return nn.is_cycle[name]
+	}
+	if name in already_seen {
+		already_seen << name
+		nn.is_cycle[name] = true
+		return true
+	}
+	already_seen << name
+	deps := nn.names[name]
+	if deps.len == 0 {
+		nn.is_cycle[name] = false
+		return false
+	}
+	for d in deps {
+		mut d_already_seen := already_seen.clone()
+		if nn.is_part_of_cycle(d, mut d_already_seen) {
+			already_seen = d_already_seen.clone()
+			nn.is_cycle[name] = true
+			return true
+		}
+	}
+	nn.is_cycle[name] = false
+	return false
 }

vlib/v/gen/native/tests/native_test.v

@@ -2,6 +2,8 @@ import os
 import benchmark
 import term
 
+const is_verbose = os.getenv('VTEST_SHOW_CMD') != ''
+
 // TODO some logic copy pasted from valgrind_test.v and compiler_test.v, move to a module
 fn test_native() {
 	$if !amd64 {
@@ -33,7 +35,11 @@ fn test_native() {
 		relative_test_path := full_test_path.replace(vroot + '/', '')
 		work_test_path := '$wrkdir/x.v'
 		os.cp(full_test_path, work_test_path) or {}
-		res_native := os.execute('$vexe -o exe -native $work_test_path')
+		cmd := '$vexe -o exe -native $work_test_path'
+		if is_verbose {
+			println(cmd)
+		}
+		res_native := os.execute(cmd)
 		if res_native.exit_code != 0 {
 			bench.fail()
 			eprintln(bench.step_message_fail('native $test failed'))

vlib/v/util/module.v

@@ -3,6 +3,7 @@ module util
 import os
 import v.pref
 
+[if trace_mod_path_to_full_name]
 fn trace_mod_path_to_full_name(line string, mod string, file_path string, res string) {
 	eprintln('> $line ${@FN} mod: ${mod:-20} | file_path: ${file_path:-30} | result: $res')
 }
@@ -15,17 +16,13 @@ pub fn qualify_import(pref &pref.Preferences, mod string, file_path string) stri
 		try_path := os.join_path(search_path, mod_path)
 		if os.is_dir(try_path) {
 			if m1 := mod_path_to_full_name(mod, try_path) {
-				$if trace_mod_path_to_full_name ? {
-					trace_mod_path_to_full_name(@LINE, mod, try_path, m1)
-				}
+				trace_mod_path_to_full_name(@LINE, mod, try_path, m1)
 				return m1
 			}
 		}
 	}
 	if m1 := mod_path_to_full_name(mod, file_path) {
-		$if trace_mod_path_to_full_name ? {
-			trace_mod_path_to_full_name(@LINE, mod, file_path, m1)
-		}
+		trace_mod_path_to_full_name(@LINE, mod, file_path, m1)
 		return m1
 	}
 	return mod
@@ -42,9 +39,7 @@ pub fn qualify_module(mod string, file_path string) string {
 		return mod
 	}
 	if m1 := mod_path_to_full_name(mod, clean_file_path) {
-		$if trace_mod_path_to_full_name ? {
-			trace_mod_path_to_full_name(@LINE, mod, clean_file_path, m1)
-		}
+		trace_mod_path_to_full_name(@LINE, mod, clean_file_path, m1)
 		return m1
 	}
 	return mod