Allow multiple dependencies on the same module

Allow adding multiple dependencies on the same module.  Adds a flag
to Context.walkDeps to determine whether dependencies should be
visited multiple times, and clarifies the behavior of
VisitDepsDepthFirst (continues to visit each module once, even if
there are multiple dependencies on it) and VisitDirectDeps (may
visit a module multiple times).

Test: TestWalkDepsDuplicates, TestVisit
Change-Id: I58afbe90490aca102d242d63e185386e1fe55d73
This commit is contained in:
Colin Cross 2018-06-20 11:16:37 -07:00
parent 3a16825a7c
commit 9607a9f248
4 changed files with 139 additions and 20 deletions

View file

@ -1401,12 +1401,6 @@ func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName s
}
if m := c.findMatchingVariant(module, possibleDeps); m != nil {
for _, dep := range module.directDeps {
if m == dep.module {
// TODO(ccross): what if adding a dependency with a different tag?
return nil
}
}
module.directDeps = append(module.directDeps, depInfo{m, tag})
atomic.AddUint32(&c.depsModified, 1)
return nil
@ -2386,7 +2380,7 @@ func (c *Context) processLocalBuildActions(out, in *localBuildActions,
return nil
}
func (c *Context) walkDeps(topModule *moduleInfo,
func (c *Context) walkDeps(topModule *moduleInfo, allowDuplicates bool,
visitDown func(depInfo, *moduleInfo) bool, visitUp func(depInfo, *moduleInfo)) {
visited := make(map[*moduleInfo]bool)
@ -2402,7 +2396,7 @@ func (c *Context) walkDeps(topModule *moduleInfo,
var walk func(module *moduleInfo)
walk = func(module *moduleInfo) {
for _, dep := range module.directDeps {
if !visited[dep.module] {
if allowDuplicates || !visited[dep.module] {
visited[dep.module] = true
visiting = dep.module
recurse := true
@ -2884,7 +2878,7 @@ func (c *Context) VisitDepsDepthFirst(module Module, visit func(Module)) {
}
}()
c.walkDeps(topModule, nil, func(dep depInfo, parent *moduleInfo) {
c.walkDeps(topModule, false, nil, func(dep depInfo, parent *moduleInfo) {
visiting = dep.module
visit(dep.module.logicModule)
})
@ -2902,7 +2896,7 @@ func (c *Context) VisitDepsDepthFirstIf(module Module, pred func(Module) bool, v
}
}()
c.walkDeps(topModule, nil, func(dep depInfo, parent *moduleInfo) {
c.walkDeps(topModule, false, nil, func(dep depInfo, parent *moduleInfo) {
if pred(dep.module.logicModule) {
visiting = dep.module
visit(dep.module.logicModule)

View file

@ -189,7 +189,7 @@ func TestWalkDeps(t *testing.T) {
var outputDown string
var outputUp string
topModule := ctx.modulesFromName("A", nil)[0]
ctx.walkDeps(topModule,
ctx.walkDeps(topModule, false,
func(dep depInfo, parent *moduleInfo) bool {
if dep.module.logicModule.(Walker).Walk() {
outputDown += ctx.ModuleName(dep.module.logicModule)
@ -203,10 +203,99 @@ func TestWalkDeps(t *testing.T) {
}
})
if outputDown != "CFG" {
t.Fatalf("unexpected walkDeps behaviour: %s\ndown should be: CFG", outputDown)
t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: CFG", outputDown)
}
if outputUp != "GFC" {
t.Fatalf("unexpected walkDeps behaviour: %s\nup should be: GFC", outputUp)
t.Errorf("unexpected walkDeps behaviour: %s\nup should be: GFC", 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
func TestWalkDepsDuplicates(t *testing.T) {
ctx := NewContext()
ctx.MockFileSystem(map[string][]byte{
"Blueprints": []byte(`
foo_module {
name: "A",
deps: ["B", "C"],
}
bar_module {
name: "B",
deps: ["D"],
}
foo_module {
name: "C",
deps: ["E", "F"],
}
foo_module {
name: "D",
}
foo_module {
name: "E",
deps: ["G"],
}
foo_module {
name: "F",
deps: ["G", "G"],
}
foo_module {
name: "G",
}
`),
})
ctx.RegisterModuleType("foo_module", newFooModule)
ctx.RegisterModuleType("bar_module", newBarModule)
_, errs := ctx.ParseBlueprintsFiles("Blueprints")
if len(errs) > 0 {
t.Errorf("unexpected parse errors:")
for _, err := range errs {
t.Errorf(" %s", err)
}
t.FailNow()
}
_, errs = ctx.ResolveDependencies(nil)
if len(errs) > 0 {
t.Errorf("unexpected dep errors:")
for _, err := range errs {
t.Errorf(" %s", err)
}
t.FailNow()
}
var outputDown string
var outputUp string
topModule := ctx.modulesFromName("A", nil)[0]
ctx.walkDeps(topModule, true,
func(dep depInfo, parent *moduleInfo) bool {
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)
}
})
if outputDown != "CEGFGG" {
t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: CEGFGG", outputDown)
}
if outputUp != "GEGGFC" {
t.Errorf("unexpected walkDeps behaviour: %s\nup should be: GEGGFC", outputUp)
}
}

View file

@ -304,6 +304,9 @@ func (m *baseModuleContext) OtherModuleErrorf(logicModule Module, format string,
})
}
// OtherModuleDependencyTag returns the dependency tag used to depend on a module, or nil if there is no dependency
// on the module. When called inside a Visit* method with current module being visited, and there are multiple
// dependencies on the module being visited, it returns the dependency tag used for the current dependency.
func (m *baseModuleContext) OtherModuleDependencyTag(logicModule Module) DependencyTag {
// fast path for calling OtherModuleDependencyTag from inside VisitDirectDeps
if logicModule == m.visitingDep.module.logicModule {
@ -319,8 +322,9 @@ func (m *baseModuleContext) OtherModuleDependencyTag(logicModule Module) Depende
return nil
}
// GetDirectDep returns the Module and DependencyTag for the direct dependency with the specified
// name, or nil if none exists.
// GetDirectDep returns the Module and DependencyTag for the direct dependency with the specified
// name, or nil if none exists. If there are multiple dependencies on the same module it returns
// the first DependencyTag.
func (m *baseModuleContext) GetDirectDep(name string) (Module, DependencyTag) {
for _, dep := range m.module.directDeps {
if dep.module.Name() == name {
@ -347,6 +351,8 @@ func (m *baseModuleContext) GetDirectDepWithTag(name string, tag DependencyTag)
return nil
}
// VisitDirectDeps calls visit for each direct dependency. If there are multiple direct dependencies on the same module
// visit will be called multiple times on that module and OtherModuleDependencyTag will return a different tag for each.
func (m *baseModuleContext) VisitDirectDeps(visit func(Module)) {
defer func() {
if r := recover(); r != nil {
@ -366,6 +372,9 @@ func (m *baseModuleContext) VisitDirectDeps(visit func(Module)) {
m.visitingDep = depInfo{}
}
// VisitDirectDepsIf calls pred for each direct dependency, and if pred returns true calls visit. If there are multiple
// direct dependencies on the same module pred and visit will be called multiple times on that module and
// OtherModuleDependencyTag will return a different tag for each.
func (m *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func(Module)) {
defer func() {
if r := recover(); r != nil {
@ -387,6 +396,10 @@ func (m *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func
m.visitingDep = depInfo{}
}
// VisitDepsDepthFirst calls visit for each transitive dependency, traversing the dependency tree in depth first order.
// visit will only be called once for any given module, even 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 first path found to the module.
func (m *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) {
defer func() {
if r := recover(); r != nil {
@ -395,7 +408,7 @@ func (m *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) {
}
}()
m.context.walkDeps(m.module, nil, func(dep depInfo, parent *moduleInfo) {
m.context.walkDeps(m.module, false, nil, func(dep depInfo, parent *moduleInfo) {
m.visitingParent = parent
m.visitingDep = dep
visit(dep.module.logicModule)
@ -405,6 +418,11 @@ func (m *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) {
m.visitingDep = depInfo{}
}
// VisitDepsDepthFirst calls pred for each transitive dependency, and if pred returns true calls visit, traversing the
// dependency tree in depth first order. visit will only be called once for any given module, even 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 first path found to the module. The return value of pred does
// not affect which branches of the tree are traversed.
func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
visit func(Module)) {
@ -415,7 +433,7 @@ func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
}
}()
m.context.walkDeps(m.module, nil, func(dep depInfo, parent *moduleInfo) {
m.context.walkDeps(m.module, false, nil, func(dep depInfo, parent *moduleInfo) {
if pred(dep.module.logicModule) {
m.visitingParent = parent
m.visitingDep = dep
@ -427,8 +445,12 @@ func (m *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool,
m.visitingDep = depInfo{}
}
// 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) {
m.context.walkDeps(m.module, func(dep depInfo, parent *moduleInfo) bool {
m.context.walkDeps(m.module, true, func(dep depInfo, parent *moduleInfo) bool {
m.visitingParent = parent
m.visitingDep = dep
return visit(dep.module.logicModule, parent.logicModule)

View file

@ -83,6 +83,9 @@ func visitMutator(ctx TopDownMutatorContext) {
// D
// |
// E
// / \
// \ /
// F
func setupVisitTest(t *testing.T) *Context {
ctx := NewContext()
ctx.RegisterModuleType("visit_module", newVisitModule)
@ -113,6 +116,11 @@ func setupVisitTest(t *testing.T) *Context {
visit_module {
name: "E",
visit: ["F", "F"],
}
visit_module {
name: "F",
}
`),
})
@ -142,10 +150,16 @@ func TestVisit(t *testing.T) {
ctx := setupVisitTest(t)
topModule := ctx.modulesFromName("A", nil)[0].logicModule.(*visitModule)
assertString(t, topModule.properties.VisitDepsDepthFirst, "EDCB")
assertString(t, topModule.properties.VisitDepsDepthFirstIf, "EDC")
assertString(t, topModule.properties.VisitDepsDepthFirst, "FEDCB")
assertString(t, topModule.properties.VisitDepsDepthFirstIf, "FEDC")
assertString(t, topModule.properties.VisitDirectDeps, "B")
assertString(t, topModule.properties.VisitDirectDepsIf, "")
eModule := ctx.modulesFromName("E", nil)[0].logicModule.(*visitModule)
assertString(t, eModule.properties.VisitDepsDepthFirst, "F")
assertString(t, eModule.properties.VisitDepsDepthFirstIf, "F")
assertString(t, eModule.properties.VisitDirectDeps, "FF")
assertString(t, eModule.properties.VisitDirectDepsIf, "FF")
}
func assertString(t *testing.T, got, expected string) {