From ac469f5effee17a0a2b18902aad0387da6674976 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Thu, 20 May 2021 01:20:25 +0300 Subject: [PATCH] v.depgraph: fix detection of indirect module dependency cycles --- vlib/v/builder/builder.v | 6 ++- vlib/v/depgraph/depgraph.v | 66 ++++++++++++++++++++------- vlib/v/gen/native/tests/native_test.v | 8 +++- vlib/v/util/module.v | 13 ++---- 4 files changed, 66 insertions(+), 27 deletions(-) diff --git a/vlib/v/builder/builder.v b/vlib/v/builder/builder.v index d058403a03..61ab67669a 100644 --- a/vlib/v/builder/builder.v +++ b/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 } diff --git a/vlib/v/depgraph/depgraph.v b/vlib/v/depgraph/depgraph.v index d7bdb4a46c..5f60af5aa2 100644 --- a/vlib/v/depgraph/depgraph.v +++ b/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 } diff --git a/vlib/v/gen/native/tests/native_test.v b/vlib/v/gen/native/tests/native_test.v index 7b4cce66a8..ce81463b44 100644 --- a/vlib/v/gen/native/tests/native_test.v +++ b/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')) diff --git a/vlib/v/util/module.v b/vlib/v/util/module.v index 430851b1df..a2e1abd0fd 100644 --- a/vlib/v/util/module.v +++ b/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