diff --git a/context.go b/context.go index 11ee6f0383f1d5ed1c410b8373109c35c2cf1d98..919377cb117429f9b7454f241f09829507472da8 100644 --- a/context.go +++ b/context.go @@ -2397,15 +2397,15 @@ func (c *Context) walkDeps(topModule *moduleInfo, allowDuplicates bool, walk = func(module *moduleInfo) { for _, dep := range module.directDeps { if allowDuplicates || !visited[dep.module] { - visited[dep.module] = true visiting = dep.module recurse := true if visitDown != nil { recurse = visitDown(dep, module) } - if recurse { + if recurse && !visited[dep.module] { walk(dep.module) } + visited[dep.module] = true if visitUp != nil { visitUp(dep, module) } diff --git a/context_test.go b/context_test.go index c08b0b72dd8406b05ded8a49f24a8adb441a1805..bc21a1c219a082a38c24ac9fcb6cfefd058381af 100644 --- a/context_test.go +++ b/context_test.go @@ -122,9 +122,9 @@ func TestContextParse(t *testing.T) { } } -// |---B===D - represents a non-walkable edge +// |===B---D - represents a non-walkable edge // A = represents a walkable edge -// |===C---E===G +// |===C===E---G // | | A should not be visited because it's the root node. // |===F===| B, D and E should not be walked. func TestWalkDeps(t *testing.T) { @@ -191,31 +191,29 @@ func TestWalkDeps(t *testing.T) { topModule := ctx.modulesFromName("A", nil)[0] ctx.walkDeps(topModule, false, func(dep depInfo, parent *moduleInfo) bool { + outputDown += ctx.ModuleName(dep.module.logicModule) if dep.module.logicModule.(Walker).Walk() { - outputDown += ctx.ModuleName(dep.module.logicModule) return true } return false }, func(dep depInfo, parent *moduleInfo) { - if dep.module.logicModule.(Walker).Walk() { - outputUp += ctx.ModuleName(dep.module.logicModule) - } + outputUp += ctx.ModuleName(dep.module.logicModule) }) - if outputDown != "CFG" { - t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: CFG", outputDown) + if outputDown != "BCEFG" { + t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEFG", outputDown) } - if outputUp != "GFC" { - t.Errorf("unexpected walkDeps behaviour: %s\nup should be: GFC", outputUp) + if outputUp != "BEGFC" { + t.Errorf("unexpected walkDeps behaviour: %s\nup should be: BEGFC", outputUp) } } -// |---B===D - represents a non-walkable edge -// A = represents a walkable edge -// |===C===E===\ -// | | A should not be visited because it's the root node. -// |===F===G B, D should not be walked. -// \===/ G should be visited multiple times +// |===B---D - represents a non-walkable edge +// A = represents a walkable edge +// |===C===E===\ A should not be visited because it's the root node. +// | | B, D should not be walked. +// |===F===G===H G should be visited multiple times +// \===/ H should only be visited once func TestWalkDepsDuplicates(t *testing.T) { ctx := NewContext() ctx.MockFileSystem(map[string][]byte{ @@ -251,6 +249,11 @@ func TestWalkDepsDuplicates(t *testing.T) { foo_module { name: "G", + deps: ["H"], + } + + foo_module { + name: "H", } `), }) @@ -280,22 +283,20 @@ func TestWalkDepsDuplicates(t *testing.T) { topModule := ctx.modulesFromName("A", nil)[0] ctx.walkDeps(topModule, true, func(dep depInfo, parent *moduleInfo) bool { + outputDown += ctx.ModuleName(dep.module.logicModule) if dep.module.logicModule.(Walker).Walk() { - outputDown += ctx.ModuleName(dep.module.logicModule) return true } return false }, func(dep depInfo, parent *moduleInfo) { - if dep.module.logicModule.(Walker).Walk() { - outputUp += ctx.ModuleName(dep.module.logicModule) - } + outputUp += ctx.ModuleName(dep.module.logicModule) }) - if outputDown != "CEGFGG" { - t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: CEGFGG", outputDown) + if outputDown != "BCEGHFGG" { + t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEGHFGG", outputDown) } - if outputUp != "GEGGFC" { - t.Errorf("unexpected walkDeps behaviour: %s\nup should be: GEGGFC", outputUp) + if outputUp != "BHGEGGFC" { + t.Errorf("unexpected walkDeps behaviour: %s\nup should be: BHGEGGFC", outputUp) } } diff --git a/module_ctx.go b/module_ctx.go index 200fd9a4d09548f2cdd800d38e217801e67f9a90..62646f1f61f775717161e97960b4205bcabc8b5d 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -164,7 +164,7 @@ type ModuleContext interface { VisitDirectDepsIf(pred func(Module) bool, visit func(Module)) VisitDepsDepthFirst(visit func(Module)) VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module)) - WalkDeps(visit func(Module, Module) bool) + WalkDeps(visit func(child, parent Module) bool) ModuleSubDir() string @@ -446,10 +446,10 @@ func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool, } // WalkDeps calls visit for each transitive dependency, traversing the dependency tree in top down order. visit may be -// called multiple times for the same module if there are multiple paths through the dependency tree to the module or -// multiple direct dependencies with different tags. OtherModuleDependencyTag will return the tag for the currently -// visited path to the module. If visit returns false WalkDeps will not continue recursing down the current path. -func (m *baseModuleContext) WalkDeps(visit func(Module, Module) bool) { +// called multiple times for the same (child, parent) pair if there are multiple direct dependencies between the +// child and parent with different tags. OtherModuleDependencyTag will return the tag for the currently visited +// (child, parent) pair. If visit returns false WalkDeps will not continue recursing down to child. +func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) { m.context.walkDeps(m.module, true, func(dep depInfo, parent *moduleInfo) bool { m.visitingParent = parent m.visitingDep = dep