diff --git a/context.go b/context.go index a4fea26..e1bd892 100644 --- a/context.go +++ b/context.go @@ -163,16 +163,65 @@ type moduleAlias struct { target *moduleInfo } +func (m *moduleAlias) alias() *moduleAlias { return m } +func (m *moduleAlias) module() *moduleInfo { return nil } +func (m *moduleAlias) moduleOrAliasTarget() *moduleInfo { return m.target } +func (m *moduleAlias) moduleOrAliasVariant() variant { return m.variant } + +func (m *moduleInfo) alias() *moduleAlias { return nil } +func (m *moduleInfo) module() *moduleInfo { return m } +func (m *moduleInfo) moduleOrAliasTarget() *moduleInfo { return m } +func (m *moduleInfo) moduleOrAliasVariant() variant { return m.variant } + +type moduleOrAlias interface { + alias() *moduleAlias + module() *moduleInfo + moduleOrAliasTarget() *moduleInfo + moduleOrAliasVariant() variant +} + +type modulesOrAliases []moduleOrAlias + +func (l modulesOrAliases) firstModule() *moduleInfo { + for _, moduleOrAlias := range l { + if m := moduleOrAlias.module(); m != nil { + return m + } + } + panic(fmt.Errorf("no first module!")) +} + +func (l modulesOrAliases) lastModule() *moduleInfo { + for i := range l { + if m := l[len(l)-1-i].module(); m != nil { + return m + } + } + panic(fmt.Errorf("no last module!")) +} + type moduleGroup struct { name string ninjaName string - modules []*moduleInfo - aliases []*moduleAlias + modules modulesOrAliases namespace Namespace } +func (group *moduleGroup) moduleOrAliasByVariantName(name string) moduleOrAlias { + for _, module := range group.modules { + if module.moduleOrAliasVariant().name == name { + return module + } + } + return nil +} + +func (group *moduleGroup) moduleByVariantName(name string) *moduleInfo { + return group.moduleOrAliasByVariantName(name).module() +} + type moduleInfo struct { // set during Parse typeName string @@ -201,8 +250,7 @@ type moduleInfo struct { waitingCount int // set during each runMutator - splitModules []*moduleInfo - pendingAliases []pendingAlias + splitModules modulesOrAliases // set during PrepareBuildActions actionDefs localBuildActions @@ -275,10 +323,10 @@ func (vm variationMap) clone() variationMap { } // Compare this variationMap to another one. Returns true if the every entry in this map -// is either the same in the other map or doesn't exist in the other map. -func (vm variationMap) subset(other variationMap) bool { +// exists and has the same value in the other map. +func (vm variationMap) subsetOf(other variationMap) bool { for k, v1 := range vm { - if v2, ok := other[k]; ok && v1 != v2 { + if v2, ok := other[k]; !ok || v1 != v2 { return false } } @@ -1256,14 +1304,14 @@ func newVariant(module *moduleInfo, mutatorName string, variationName string, } func (c *Context) createVariations(origModule *moduleInfo, mutatorName string, - defaultVariationName *string, variationNames []string, local bool) ([]*moduleInfo, []error) { + defaultVariationName *string, variationNames []string, local bool) (modulesOrAliases, []error) { if len(variationNames) == 0 { panic(fmt.Errorf("mutator %q passed zero-length variation list for module %q", mutatorName, origModule.Name())) } - newModules := []*moduleInfo{} + var newModules modulesOrAliases var errs []error @@ -1312,37 +1360,19 @@ func (c *Context) convertDepsToVariation(module *moduleInfo, if dep.module.logicModule == nil { var newDep *moduleInfo for _, m := range dep.module.splitModules { - if m.variant.variations[mutatorName] == variationName { - newDep = m + if m.moduleOrAliasVariant().variations[mutatorName] == variationName { + newDep = m.moduleOrAliasTarget() break } } - if newDep == nil { - // check aliases against variationNam - for _, a := range dep.module.pendingAliases { - if a.fromVariant.variations[mutatorName] == variationName { - newDep = a.target - break - } - } - } if newDep == nil && defaultVariationName != nil { // give it a second chance; match with defaultVariationName for _, m := range dep.module.splitModules { - if m.variant.variations[mutatorName] == *defaultVariationName { - newDep = m + if m.moduleOrAliasVariant().variations[mutatorName] == *defaultVariationName { + newDep = m.moduleOrAliasTarget() break } } - if newDep == nil { - // check aliases against defaultVariationName - for _, a := range dep.module.pendingAliases { - if a.fromVariant.variations[mutatorName] == *defaultVariationName { - newDep = a.target - break - } - } - } } if newDep == nil { errs = append(errs, &BlueprintError{ @@ -1372,14 +1402,14 @@ func (c *Context) prettyPrintVariant(variations variationMap) string { func (c *Context) prettyPrintGroupVariants(group *moduleGroup) string { var variants []string - for _, mod := range group.modules { - variants = append(variants, c.prettyPrintVariant(mod.variant.variations)) + for _, moduleOrAlias := range group.modules { + if mod := moduleOrAlias.module(); mod != nil { + variants = append(variants, c.prettyPrintVariant(mod.variant.variations)) + } else if alias := moduleOrAlias.alias(); alias != nil { + variants = append(variants, c.prettyPrintVariant(alias.variant.variations)+ + "(alias to "+c.prettyPrintVariant(alias.target.variant.variations)+")") + } } - for _, mod := range group.aliases { - variants = append(variants, c.prettyPrintVariant(mod.variant.variations)+ - "(alias to "+c.prettyPrintVariant(mod.target.variant.variations)+")") - } - sort.Strings(variants) return strings.Join(variants, "\n ") } @@ -1458,7 +1488,7 @@ func (c *Context) addModule(module *moduleInfo) []error { group := &moduleGroup{ name: name, - modules: []*moduleInfo{module}, + modules: modulesOrAliases{module}, } module.group = group namespace, errs := c.nameInterface.NewModule( @@ -1542,34 +1572,23 @@ func blueprintDepsMutator(ctx BottomUpMutatorContext) { } } -// findMatchingVariant searches the moduleGroup for a module with the same variant as module, -// and returns the matching module, or nil if one is not found. -func (c *Context) findMatchingVariant(module *moduleInfo, possible *moduleGroup, reverse bool) *moduleInfo { - if len(possible.modules) == 1 { - return possible.modules[0] - } else { - var variantToMatch variationMap - if !reverse { - // For forward dependency, ignore local variants by matching against - // dependencyVariant which doesn't have the local variants - variantToMatch = module.variant.dependencyVariations - } else { - // For reverse dependency, use all the variants - variantToMatch = module.variant.variations - } - for _, m := range possible.modules { - if m.variant.variations.equal(variantToMatch) { - return m - } - } - for _, m := range possible.aliases { - if m.variant.variations.equal(variantToMatch) { - return m.target +// findExactVariantOrSingle searches the moduleGroup for a module with the same variant as module, +// and returns the matching module, or nil if one is not found. A group with exactly one module +// is always considered matching. +func findExactVariantOrSingle(module *moduleInfo, possible *moduleGroup, reverse bool) *moduleInfo { + found, _ := findVariant(module, possible, nil, false, reverse) + if found == nil { + for _, moduleOrAlias := range possible.modules { + if m := moduleOrAlias.module(); m != nil { + if found != nil { + // more than one possible match, give up + return nil + } + found = m } } } - - return nil + return found } func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName string) []error { @@ -1589,7 +1608,7 @@ func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName s return c.discoveredMissingDependencies(module, depName) } - if m := c.findMatchingVariant(module, possibleDeps, false); m != nil { + if m := findExactVariantOrSingle(module, possibleDeps, false); m != nil { module.newDirectDeps = append(module.newDirectDeps, depInfo{m, tag}) atomic.AddUint32(&c.depsModified, 1) return nil @@ -1626,7 +1645,7 @@ func (c *Context) findReverseDependency(module *moduleInfo, destName string) (*m }} } - if m := c.findMatchingVariant(module, possibleDeps, true); m != nil { + if m := findExactVariantOrSingle(module, possibleDeps, true); m != nil { return m, nil } @@ -1644,7 +1663,7 @@ func (c *Context) findReverseDependency(module *moduleInfo, destName string) (*m }} } -func (c *Context) findVariant(module *moduleInfo, possibleDeps *moduleGroup, variations []Variation, far bool, reverse bool) (*moduleInfo, variationMap) { +func findVariant(module *moduleInfo, possibleDeps *moduleGroup, variations []Variation, far bool, reverse bool) (*moduleInfo, variationMap) { // We can't just append variant.Variant to module.dependencyVariant.variantName and // compare the strings because the result won't be in mutator registration order. // Create a new map instead, and then deep compare the maps. @@ -1668,7 +1687,7 @@ func (c *Context) findVariant(module *moduleInfo, possibleDeps *moduleGroup, var check := func(variant variationMap) bool { if far { - return variant.subset(newVariant) + return newVariant.subsetOf(variant) } else { return variant.equal(newVariant) } @@ -1676,21 +1695,12 @@ func (c *Context) findVariant(module *moduleInfo, possibleDeps *moduleGroup, var var foundDep *moduleInfo for _, m := range possibleDeps.modules { - if check(m.variant.variations) { - foundDep = m + if check(m.moduleOrAliasVariant().variations) { + foundDep = m.moduleOrAliasTarget() break } } - if foundDep == nil { - for _, m := range possibleDeps.aliases { - if check(m.variant.variations) { - foundDep = m.target - break - } - } - } - return foundDep, newVariant } @@ -1705,7 +1715,7 @@ func (c *Context) addVariationDependency(module *moduleInfo, variations []Variat return c.discoveredMissingDependencies(module, depName) } - foundDep, newVariant := c.findVariant(module, possibleDeps, variations, far, false) + foundDep, newVariant := findVariant(module, possibleDeps, variations, far, false) if foundDep == nil { if c.allowMissingDependencies { @@ -1748,14 +1758,16 @@ func (c *Context) addInterVariantDependency(origModule *moduleInfo, tag Dependen } var fromInfo, toInfo *moduleInfo - for _, m := range origModule.splitModules { - if m.logicModule == from { - fromInfo = m - } - if m.logicModule == to { - toInfo = m - if fromInfo != nil { - panic(fmt.Errorf("%q depends on later version of itself", origModule.Name())) + for _, moduleOrAlias := range origModule.splitModules { + if m := moduleOrAlias.module(); m != nil { + if m.logicModule == from { + fromInfo = m + } + if m.logicModule == to { + toInfo = m + if fromInfo != nil { + panic(fmt.Errorf("%q depends on later version of itself", origModule.Name())) + } } } } @@ -1987,7 +1999,9 @@ func (c *Context) updateDependencies() (errs []error) { if dep == module { break } - deps[dep] = true + if depModule := dep.module(); depModule != nil { + deps[depModule] = true + } } for _, dep := range module.directDeps { @@ -2229,7 +2243,7 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, errsCh := make(chan []error) globalStateCh := make(chan globalStateChange) - newVariationsCh := make(chan []*moduleInfo) + newVariationsCh := make(chan modulesOrAliases) done := make(chan bool) c.depsModified = 0 @@ -2239,8 +2253,6 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, panic("split module found in sorted module list") } - module.pendingAliases = nil - mctx := &mutatorContext{ baseModuleContext: baseModuleContext{ context: c, @@ -2302,8 +2314,10 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, newModules = append(newModules, globalStateChange.newModules...) deps = append(deps, globalStateChange.deps...) case newVariations := <-newVariationsCh: - for _, m := range newVariations { - newModuleInfo[m.logicModule] = m + for _, moduleOrAlias := range newVariations { + if m := moduleOrAlias.module(); m != nil { + newModuleInfo[m.logicModule] = m + } } case <-done: return @@ -2327,31 +2341,27 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, for _, group := range c.moduleGroups { for i := 0; i < len(group.modules); i++ { - module := group.modules[i] + module := group.modules[i].module() + if module == nil { + // Existing alias, skip it + continue + } // Update module group to contain newly split variants if module.splitModules != nil { group.modules, i = spliceModules(group.modules, i, module.splitModules) } - // Create any new aliases. - for _, alias := range module.pendingAliases { - group.aliases = append(group.aliases, &moduleAlias{ - variant: alias.fromVariant, - target: alias.target, - }) - } - // Fix up any remaining dependencies on modules that were split into variants // by replacing them with the first variant for j, dep := range module.directDeps { if dep.module.logicModule == nil { - module.directDeps[j].module = dep.module.splitModules[0] + module.directDeps[j].module = dep.module.splitModules.firstModule() } } if module.createdBy != nil && module.createdBy.logicModule == nil { - module.createdBy = module.createdBy.splitModules[0] + module.createdBy = module.createdBy.splitModules.firstModule() } // Add in any new direct dependencies that were added by the mutator @@ -2360,26 +2370,30 @@ func (c *Context) runMutator(config interface{}, mutator *mutatorInfo, } findAliasTarget := func(variant variant) *moduleInfo { - for _, alias := range group.aliases { - if alias.variant.variations.equal(variant.variations) { - return alias.target + for _, moduleOrAlias := range group.modules { + if alias := moduleOrAlias.alias(); alias != nil { + if alias.variant.variations.equal(variant.variations) { + return alias.target + } } } return nil } // Forward or delete any dangling aliases. - for i := 0; i < len(group.aliases); i++ { - alias := group.aliases[i] - - if alias.target.logicModule == nil { - newTarget := findAliasTarget(alias.target.variant) - if newTarget != nil { - alias.target = newTarget - } else { - // The alias was left dangling, remove it. - group.aliases = append(group.aliases[:i], group.aliases[i+1:]...) - i-- + // Use a manual loop instead of range because len(group.modules) can + // change inside the loop + for i := 0; i < len(group.modules); i++ { + if alias := group.modules[i].alias(); alias != nil { + if alias.target.logicModule == nil { + newTarget := findAliasTarget(alias.target.variant) + if newTarget != nil { + alias.target = newTarget + } else { + // The alias was left dangling, remove it. + group.modules = append(group.modules[:i], group.modules[i+1:]...) + i-- + } } } } @@ -2454,15 +2468,15 @@ func (c *Context) cloneModules() { // Removes modules[i] from the list and inserts newModules... where it was located, returning // the new slice and the index of the last inserted element -func spliceModules(modules []*moduleInfo, i int, newModules []*moduleInfo) ([]*moduleInfo, int) { +func spliceModules(modules modulesOrAliases, i int, newModules modulesOrAliases) (modulesOrAliases, int) { spliceSize := len(newModules) newLen := len(modules) + spliceSize - 1 - var dest []*moduleInfo + var dest modulesOrAliases if cap(modules) >= len(modules)-1+len(newModules) { // We can fit the splice in the existing capacity, do everything in place dest = modules[:newLen] } else { - dest = make([]*moduleInfo, newLen) + dest = make(modulesOrAliases, newLen) copy(dest, modules[:i]) } @@ -2719,14 +2733,8 @@ func (c *Context) moduleMatchingVariant(module *moduleInfo, name string) *module } for _, m := range group.modules { - if module.variant.name == m.variant.name { - return m - } - } - - for _, m := range group.aliases { - if module.variant.name == m.variant.name { - return m.target + if module.variant.variations.equal(m.moduleOrAliasVariant().variations) { + return m.moduleOrAliasTarget() } } @@ -2823,8 +2831,10 @@ func (c *Context) visitAllModules(visit func(Module)) { }() for _, moduleGroup := range c.sortedModuleGroups() { - for _, module = range moduleGroup.modules { - visit(module.logicModule) + for _, moduleOrAlias := range moduleGroup.modules { + if module = moduleOrAlias.module(); module != nil { + visit(module.logicModule) + } } } } @@ -2842,9 +2852,11 @@ func (c *Context) visitAllModulesIf(pred func(Module) bool, }() for _, moduleGroup := range c.sortedModuleGroups() { - for _, module := range moduleGroup.modules { - if pred(module.logicModule) { - visit(module.logicModule) + for _, moduleOrAlias := range moduleGroup.modules { + if module = moduleOrAlias.module(); module != nil { + if pred(module.logicModule) { + visit(module.logicModule) + } } } } @@ -2862,8 +2874,10 @@ func (c *Context) visitAllModuleVariants(module *moduleInfo, } }() - for _, variant = range module.group.modules { - visit(variant.logicModule) + for _, moduleOrAlias := range module.group.modules { + if variant = moduleOrAlias.module(); variant != nil { + visit(variant.logicModule) + } } } @@ -3207,12 +3221,11 @@ func (c *Context) VisitDepsDepthFirstIf(module Module, pred func(Module) bool, v } func (c *Context) PrimaryModule(module Module) Module { - return c.moduleInfo[module].group.modules[0].logicModule + return c.moduleInfo[module].group.modules.firstModule().logicModule } func (c *Context) FinalModule(module Module) Module { - modules := c.moduleInfo[module].group.modules - return modules[len(modules)-1].logicModule + return c.moduleInfo[module].group.modules.lastModule().logicModule } func (c *Context) VisitAllModuleVariants(module Module, @@ -3772,15 +3785,15 @@ func (c *Context) writeLocalBuildActions(nw *ninjaWriter, return nil } -func beforeInModuleList(a, b *moduleInfo, list []*moduleInfo) bool { +func beforeInModuleList(a, b *moduleInfo, list modulesOrAliases) bool { found := false if a == b { return false } for _, l := range list { - if l == a { + if l.module() == a { found = true - } else if l == b { + } else if l.module() == b { return found } } diff --git a/context_test.go b/context_test.go index 0541c06..91089e6 100644 --- a/context_test.go +++ b/context_test.go @@ -238,7 +238,7 @@ func TestWalkDeps(t *testing.T) { t.FailNow() } - topModule := ctx.moduleGroupFromName("A", nil).modules[0] + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule() outputDown, outputUp := walkDependencyGraph(ctx, topModule, false) if outputDown != "BCEFG" { t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEFG", outputDown) @@ -319,7 +319,7 @@ func TestWalkDepsDuplicates(t *testing.T) { t.FailNow() } - topModule := ctx.moduleGroupFromName("A", nil).modules[0] + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule() outputDown, outputUp := walkDependencyGraph(ctx, topModule, true) if outputDown != "BCEGHFGG" { t.Errorf("unexpected walkDeps behaviour: %s\ndown should be: BCEGHFGG", outputDown) @@ -386,7 +386,7 @@ func TestWalkDepsDuplicates_IgnoreFirstPath(t *testing.T) { t.FailNow() } - topModule := ctx.moduleGroupFromName("A", nil).modules[0] + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule() outputDown, outputUp := walkDependencyGraph(ctx, topModule, true) expectedDown := "BDCDE" if outputDown != expectedDown { @@ -432,10 +432,10 @@ func TestCreateModule(t *testing.T) { t.FailNow() } - a := ctx.moduleGroupFromName("A", nil).modules[0].logicModule.(*fooModule) - b := ctx.moduleGroupFromName("B", nil).modules[0].logicModule.(*barModule) - c := ctx.moduleGroupFromName("C", nil).modules[0].logicModule.(*barModule) - d := ctx.moduleGroupFromName("D", nil).modules[0].logicModule.(*fooModule) + a := ctx.moduleGroupFromName("A", nil).modules.firstModule().logicModule.(*fooModule) + b := ctx.moduleGroupFromName("B", nil).modules.firstModule().logicModule.(*barModule) + c := ctx.moduleGroupFromName("C", nil).modules.firstModule().logicModule.(*barModule) + d := ctx.moduleGroupFromName("D", nil).modules.firstModule().logicModule.(*fooModule) checkDeps := func(m Module, expected string) { var deps []string @@ -606,3 +606,235 @@ func TestParseFailsForModuleWithoutName(t *testing.T) { t.Errorf("Incorrect errors; expected:\n%s\ngot:\n%s", expectedErrs, errs) } } + +func Test_findVariant(t *testing.T) { + module := &moduleInfo{ + variant: variant{ + name: "normal_local", + variations: variationMap{ + "normal": "normal", + "local": "local", + }, + dependencyVariations: variationMap{ + "normal": "normal", + }, + }, + } + + type alias struct { + variant variant + target int + } + + makeDependencyGroup := func(in ...interface{}) *moduleGroup { + group := &moduleGroup{ + name: "dep", + } + for _, x := range in { + switch m := x.(type) { + case *moduleInfo: + m.group = group + group.modules = append(group.modules, m) + case alias: + // aliases may need to target modules that haven't been processed + // yet, put an empty alias in for now. + group.modules = append(group.modules, nil) + default: + t.Fatalf("unexpected type %T", x) + } + } + + for i, x := range in { + switch m := x.(type) { + case *moduleInfo: + // already added in the first pass + case alias: + group.modules[i] = &moduleAlias{ + variant: m.variant, + target: group.modules[m.target].moduleOrAliasTarget(), + } + default: + t.Fatalf("unexpected type %T", x) + } + } + + return group + } + + tests := []struct { + name string + possibleDeps *moduleGroup + variations []Variation + far bool + reverse bool + want string + }{ + { + name: "AddVariationDependencies(nil)", + // A dependency that matches the non-local variations of the module + possibleDeps: makeDependencyGroup( + &moduleInfo{ + variant: variant{ + name: "normal", + variations: variationMap{ + "normal": "normal", + }, + }, + }, + ), + variations: nil, + far: false, + reverse: false, + want: "normal", + }, + { + name: "AddVariationDependencies(nil) to alias", + // A dependency with an alias that matches the non-local variations of the module + possibleDeps: makeDependencyGroup( + alias{ + variant: variant{ + name: "normal", + variations: variationMap{ + "normal": "normal", + }, + }, + target: 1, + }, + &moduleInfo{ + variant: variant{ + name: "normal_a", + variations: variationMap{ + "normal": "normal", + "a": "a", + }, + }, + }, + ), + variations: nil, + far: false, + reverse: false, + want: "normal_a", + }, + { + name: "AddVariationDependencies(a)", + // A dependency with local variations + possibleDeps: makeDependencyGroup( + &moduleInfo{ + variant: variant{ + name: "normal_a", + variations: variationMap{ + "normal": "normal", + "a": "a", + }, + }, + }, + ), + variations: []Variation{{"a", "a"}}, + far: false, + reverse: false, + want: "normal_a", + }, + { + name: "AddFarVariationDependencies(far)", + // A dependency with far variations + possibleDeps: makeDependencyGroup( + &moduleInfo{ + variant: variant{ + name: "", + variations: nil, + }, + }, + &moduleInfo{ + variant: variant{ + name: "far", + variations: variationMap{ + "far": "far", + }, + }, + }, + ), + variations: []Variation{{"far", "far"}}, + far: true, + reverse: false, + want: "far", + }, + { + name: "AddFarVariationDependencies(far) to alias", + // A dependency with far variations and aliases + possibleDeps: makeDependencyGroup( + alias{ + variant: variant{ + name: "far", + variations: variationMap{ + "far": "far", + }, + }, + target: 2, + }, + &moduleInfo{ + variant: variant{ + name: "far_a", + variations: variationMap{ + "far": "far", + "a": "a", + }, + }, + }, + &moduleInfo{ + variant: variant{ + name: "far_b", + variations: variationMap{ + "far": "far", + "b": "b", + }, + }, + }, + ), + variations: []Variation{{"far", "far"}}, + far: true, + reverse: false, + want: "far_b", + }, + { + name: "AddFarVariationDependencies(far, b) to missing", + // A dependency with far variations and aliases + possibleDeps: makeDependencyGroup( + alias{ + variant: variant{ + name: "far", + variations: variationMap{ + "far": "far", + }, + }, + target: 1, + }, + &moduleInfo{ + variant: variant{ + name: "far_a", + variations: variationMap{ + "far": "far", + "a": "a", + }, + }, + }, + ), + variations: []Variation{{"far", "far"}, {"a", "b"}}, + far: true, + reverse: false, + want: "nil", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, _ := findVariant(module, tt.possibleDeps, tt.variations, tt.far, tt.reverse) + if g, w := got == nil, tt.want == "nil"; g != w { + t.Fatalf("findVariant() got = %v, want %v", got, tt.want) + } + if got != nil { + if g, w := got.String(), fmt.Sprintf("module %q variant %q", "dep", tt.want); g != w { + t.Errorf("findVariant() got = %v, want %v", g, w) + } + } + }) + } +} diff --git a/module_ctx.go b/module_ctx.go index c992c0a..50fbcf0 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -510,7 +510,7 @@ func (m *baseModuleContext) OtherModuleDependencyVariantExists(variations []Vari if possibleDeps == nil { return false } - found, _ := m.context.findVariant(m.module, possibleDeps, variations, false, false) + found, _ := findVariant(m.module, possibleDeps, variations, false, false) return found != nil } @@ -519,7 +519,7 @@ func (m *baseModuleContext) OtherModuleReverseDependencyVariantExists(name strin if possibleDeps == nil { return false } - found, _ := m.context.findVariant(m.module, possibleDeps, nil, false, true) + found, _ := findVariant(m.module, possibleDeps, nil, false, true) return found != nil } @@ -643,11 +643,11 @@ func (m *baseModuleContext) WalkDeps(visit func(child, parent Module) bool) { } func (m *baseModuleContext) PrimaryModule() Module { - return m.module.group.modules[0].logicModule + return m.module.group.modules.firstModule().logicModule } func (m *baseModuleContext) FinalModule() Module { - return m.module.group.modules[len(m.module.group.modules)-1].logicModule + return m.module.group.modules.lastModule().logicModule } func (m *baseModuleContext) VisitAllModuleVariants(visit func(Module)) { @@ -722,8 +722,8 @@ type mutatorContext struct { reverseDeps []reverseDep rename []rename replace []replace - newVariations []*moduleInfo // new variants of existing modules - newModules []*moduleInfo // brand new modules + newVariations modulesOrAliases // new variants of existing modules + newModules []*moduleInfo // brand new modules defaultVariation *string } @@ -921,7 +921,7 @@ func (mctx *mutatorContext) createVariations(variationNames []string, local bool } for _, module := range modules { - ret = append(ret, module.logicModule) + ret = append(ret, module.module().logicModule) } if mctx.newVariations != nil { @@ -937,25 +937,30 @@ func (mctx *mutatorContext) createVariations(variationNames []string, local bool } func (mctx *mutatorContext) AliasVariation(variationName string) { - for _, alias := range mctx.module.pendingAliases { - if alias.fromVariant.variations.equal(mctx.module.variant.variations) { - panic(fmt.Errorf("AliasVariation already called")) + for _, moduleOrAlias := range mctx.module.splitModules { + if alias := moduleOrAlias.alias(); alias != nil { + if alias.variant.variations.equal(mctx.module.variant.variations) { + panic(fmt.Errorf("AliasVariation already called")) + } } } for _, variant := range mctx.newVariations { - if variant.variant.variations[mctx.name] == variationName { - mctx.module.pendingAliases = append(mctx.module.pendingAliases, pendingAlias{ - fromVariant: mctx.module.variant, - target: variant, - }) + if variant.moduleOrAliasVariant().variations[mctx.name] == variationName { + alias := &moduleAlias{ + variant: mctx.module.variant, + target: variant.moduleOrAliasTarget(), + } + // Prepend the alias so that AddFarVariationDependencies subset match matches + // the alias before matching the first variation. + mctx.module.splitModules = append(modulesOrAliases{alias}, mctx.module.splitModules...) return } } var foundVariations []string for _, variant := range mctx.newVariations { - foundVariations = append(foundVariations, variant.variant.variations[mctx.name]) + foundVariations = append(foundVariations, variant.moduleOrAliasVariant().variations[mctx.name]) } panic(fmt.Errorf("no %q variation in module variations %q", variationName, foundVariations)) } @@ -963,17 +968,22 @@ func (mctx *mutatorContext) AliasVariation(variationName string) { func (mctx *mutatorContext) CreateAliasVariation(aliasVariationName, targetVariationName string) { newVariant := newVariant(mctx.module, mctx.name, aliasVariationName, false) - for _, alias := range mctx.module.pendingAliases { - if alias.fromVariant.variations.equal(newVariant.variations) { - panic(fmt.Errorf("can't alias %q to %q, already aliased to %q", aliasVariationName, targetVariationName, alias.target.variant.name)) + for _, moduleOrAlias := range mctx.module.splitModules { + if moduleOrAlias.moduleOrAliasVariant().variations.equal(newVariant.variations) { + if alias := moduleOrAlias.alias(); alias != nil { + panic(fmt.Errorf("can't alias %q to %q, already aliased to %q", aliasVariationName, targetVariationName, alias.target.variant.name)) + } else { + panic(fmt.Errorf("can't alias %q to %q, there is already a variant with that name", aliasVariationName, targetVariationName)) + } } } for _, variant := range mctx.newVariations { - if variant.variant.variations[mctx.name] == targetVariationName { - mctx.module.pendingAliases = append(mctx.module.pendingAliases, pendingAlias{ - fromVariant: newVariant, - target: variant, + if variant.moduleOrAliasVariant().variations[mctx.name] == targetVariationName { + // Append the alias here so that it comes after any aliases created by AliasVariation. + mctx.module.splitModules = append(mctx.module.splitModules, &moduleAlias{ + variant: newVariant, + target: variant.moduleOrAliasTarget(), }) return } @@ -981,7 +991,7 @@ func (mctx *mutatorContext) CreateAliasVariation(aliasVariationName, targetVaria var foundVariations []string for _, variant := range mctx.newVariations { - foundVariations = append(foundVariations, variant.variant.variations[mctx.name]) + foundVariations = append(foundVariations, variant.moduleOrAliasVariant().variations[mctx.name]) } panic(fmt.Errorf("no %q variation in module variations %q", targetVariationName, foundVariations)) } diff --git a/module_ctx_test.go b/module_ctx_test.go index 22666dc..62a89c1 100644 --- a/module_ctx_test.go +++ b/module_ctx_test.go @@ -131,12 +131,8 @@ func TestAliasVariation(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barB := ctx.moduleGroupFromName("bar", nil).modules[1] - - if g, w := barB.variant.name, "b"; g != w { - t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b") if g, w := foo.forwardDeps, []*moduleInfo{barB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -155,12 +151,8 @@ func TestAliasVariation(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barBB := ctx.moduleGroupFromName("bar", nil).modules[3] - - if g, w := barBB.variant.name, "b_b"; g != w { - t.Fatalf("expected bar.modules[3] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barBB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b_b") if g, w := foo.forwardDeps, []*moduleInfo{barBB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -179,12 +171,8 @@ func TestAliasVariation(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barAB := ctx.moduleGroupFromName("bar", nil).modules[1] - - if g, w := barAB.variant.name, "a_b"; g != w { - t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barAB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("a_b") if g, w := foo.forwardDeps, []*moduleInfo{barAB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -270,12 +258,8 @@ func TestCreateAliasVariations(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barB := ctx.moduleGroupFromName("bar", nil).modules[1] - - if g, w := barB.variant.name, "b"; g != w { - t.Fatalf("expected bar.modules[1] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b") if g, w := foo.forwardDeps, []*moduleInfo{barB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) @@ -294,12 +278,8 @@ func TestCreateAliasVariations(t *testing.T) { run(ctx) - foo := ctx.moduleGroupFromName("foo", nil).modules[0] - barBB := ctx.moduleGroupFromName("bar", nil).modules[3] - - if g, w := barBB.variant.name, "b_b"; g != w { - t.Fatalf("expected bar.modules[3] variant to be %q, got %q", w, g) - } + foo := ctx.moduleGroupFromName("foo", nil).moduleByVariantName("") + barBB := ctx.moduleGroupFromName("bar", nil).moduleByVariantName("b_b") if g, w := foo.forwardDeps, []*moduleInfo{barBB}; !reflect.DeepEqual(g, w) { t.Fatalf("expected foo deps to be %q, got %q", w, g) diff --git a/name_interface.go b/name_interface.go index 1849e9d..5e7e16e 100644 --- a/name_interface.go +++ b/name_interface.go @@ -109,7 +109,7 @@ func (s *SimpleNameInterface) NewModule(ctx NamespaceContext, group ModuleGroup, return nil, []error{ // seven characters at the start of the second line to align with the string "error: " fmt.Errorf("module %q already defined\n"+ - " %s <-- previous definition here", name, group.modules[0].pos), + " %s <-- previous definition here", name, group.modules.firstModule().pos), } } @@ -130,7 +130,7 @@ func (s *SimpleNameInterface) Rename(oldName string, newName string, namespace N // seven characters at the start of the second line to align with the string "error: " fmt.Errorf("renaming module %q to %q conflicts with existing module\n"+ " %s <-- existing module defined here", - oldName, newName, existingGroup.modules[0].pos), + oldName, newName, existingGroup.modules.firstModule().pos), } } diff --git a/splice_modules_test.go b/splice_modules_test.go index 059ff41..473999a 100644 --- a/splice_modules_test.go +++ b/splice_modules_test.go @@ -29,82 +29,82 @@ var ( ) var spliceModulesTestCases = []struct { - in []*moduleInfo + in modulesOrAliases at int - with []*moduleInfo - out []*moduleInfo + with modulesOrAliases + out modulesOrAliases outAt int reallocate bool }{ { // Insert at the beginning - in: []*moduleInfo{testModuleA, testModuleB, testModuleC}, + in: modulesOrAliases{testModuleA, testModuleB, testModuleC}, at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE, testModuleB, testModuleC}, outAt: 1, reallocate: true, }, { // Insert in the middle - in: []*moduleInfo{testModuleA, testModuleB, testModuleC}, + in: modulesOrAliases{testModuleA, testModuleB, testModuleC}, at: 1, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleD, testModuleE, testModuleC}, outAt: 2, reallocate: true, }, { // Insert at the end - in: []*moduleInfo{testModuleA, testModuleB, testModuleC}, + in: modulesOrAliases{testModuleA, testModuleB, testModuleC}, at: 2, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleB, testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleB, testModuleD, testModuleE}, outAt: 3, reallocate: true, }, { // Insert over a single element - in: []*moduleInfo{testModuleA}, + in: modulesOrAliases{testModuleA}, at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE}, outAt: 1, reallocate: true, }, { // Insert at the beginning without reallocating - in: []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3], + in: modulesOrAliases{testModuleA, testModuleB, testModuleC, nil}[0:3], at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE, testModuleB, testModuleC}, outAt: 1, reallocate: false, }, { // Insert in the middle without reallocating - in: []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3], + in: modulesOrAliases{testModuleA, testModuleB, testModuleC, nil}[0:3], at: 1, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleD, testModuleE, testModuleC}, outAt: 2, reallocate: false, }, { // Insert at the end without reallocating - in: []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3], + in: modulesOrAliases{testModuleA, testModuleB, testModuleC, nil}[0:3], at: 2, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleA, testModuleB, testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleA, testModuleB, testModuleD, testModuleE}, outAt: 3, reallocate: false, }, { // Insert over a single element without reallocating - in: []*moduleInfo{testModuleA, nil}[0:1], + in: modulesOrAliases{testModuleA, nil}[0:1], at: 0, - with: []*moduleInfo{testModuleD, testModuleE}, - out: []*moduleInfo{testModuleD, testModuleE}, + with: modulesOrAliases{testModuleD, testModuleE}, + out: modulesOrAliases{testModuleD, testModuleE}, outAt: 1, reallocate: false, }, @@ -112,7 +112,7 @@ var spliceModulesTestCases = []struct { func TestSpliceModules(t *testing.T) { for _, testCase := range spliceModulesTestCases { - in := make([]*moduleInfo, len(testCase.in), cap(testCase.in)) + in := make(modulesOrAliases, len(testCase.in), cap(testCase.in)) copy(in, testCase.in) origIn := in got, gotAt := spliceModules(in, testCase.at, testCase.with) @@ -139,6 +139,6 @@ func TestSpliceModules(t *testing.T) { } } -func sameArray(a, b []*moduleInfo) bool { +func sameArray(a, b modulesOrAliases) bool { return &a[0:cap(a)][cap(a)-1] == &b[0:cap(b)][cap(b)-1] } diff --git a/visit_test.go b/visit_test.go index efaadba..1c74b93 100644 --- a/visit_test.go +++ b/visit_test.go @@ -149,13 +149,13 @@ func setupVisitTest(t *testing.T) *Context { func TestVisit(t *testing.T) { ctx := setupVisitTest(t) - topModule := ctx.moduleGroupFromName("A", nil).modules[0].logicModule.(*visitModule) + topModule := ctx.moduleGroupFromName("A", nil).modules.firstModule().logicModule.(*visitModule) 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.moduleGroupFromName("E", nil).modules[0].logicModule.(*visitModule) + eModule := ctx.moduleGroupFromName("E", nil).modules.firstModule().logicModule.(*visitModule) assertString(t, eModule.properties.VisitDepsDepthFirst, "F") assertString(t, eModule.properties.VisitDepsDepthFirstIf, "F") assertString(t, eModule.properties.VisitDirectDeps, "FF")