From 0b7e83e11aac64119ed78291e62d9c6b9f47a2a9 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 17 May 2016 14:58:05 -0700 Subject: [PATCH] Ask primary builder for module names Remove the assumed Name and Deps properties. Instead, ask the Module for the Name, and require them to use the existing replacements for Deps. Change-Id: I729045d86277686078b3aa0bba71c67d612ead2c --- bootstrap/bootstrap.go | 16 +++- context.go | 172 ++++++++++++++++++++--------------------- context_test.go | 26 +++++-- module_ctx.go | 27 ++++++- visit_test.go | 5 +- 5 files changed, 145 insertions(+), 101 deletions(-) diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index 709cba1..5b01988 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -192,7 +192,9 @@ func isBootstrapBinaryModule(module blueprint.Module) bool { // A goPackage is a module for building Go packages. type goPackage struct { + blueprint.SimpleName properties struct { + Deps []string PkgPath string Srcs []string TestSrcs []string @@ -224,10 +226,14 @@ func newGoPackageModuleFactory(config *Config) func() (blueprint.Module, []inter config: config, } module.properties.BuildStage = StageMain - return module, []interface{}{&module.properties} + return module, []interface{}{&module.properties, &module.SimpleName.Properties} } } +func (g *goPackage) DynamicDependencies(ctx blueprint.DynamicDependerModuleContext) []string { + return g.properties.Deps +} + func (g *goPackage) GoPkgPath() string { return g.properties.PkgPath } @@ -314,7 +320,9 @@ func (g *goPackage) GenerateBuildActions(ctx blueprint.ModuleContext) { // A goBinary is a module for building executable binaries from Go sources. type goBinary struct { + blueprint.SimpleName properties struct { + Deps []string Srcs []string TestSrcs []string PrimaryBuilder bool @@ -336,10 +344,14 @@ func newGoBinaryModuleFactory(config *Config, buildStage Stage) func() (blueprin config: config, } module.properties.BuildStage = buildStage - return module, []interface{}{&module.properties} + return module, []interface{}{&module.properties, &module.SimpleName.Properties} } } +func (g *goBinary) DynamicDependencies(ctx blueprint.DynamicDependerModuleContext) []string { + return g.properties.Deps +} + func (g *goBinary) GoTestTarget() string { return g.testArchiveFile } diff --git a/context.go b/context.go index c59b107..af2fc37 100644 --- a/context.go +++ b/context.go @@ -65,7 +65,8 @@ const maxErrors = 10 type Context struct { // set at instantiation moduleFactories map[string]ModuleFactory - moduleGroups map[string]*moduleGroup + moduleNames map[string]*moduleGroup + moduleGroups []*moduleGroup moduleInfo map[Module]*moduleInfo modulesSorted []*moduleInfo singletonInfo []*singletonInfo @@ -155,10 +156,6 @@ type moduleInfo struct { relBlueprintsFile string pos scanner.Position propertyPos map[string]scanner.Position - properties struct { - Name string - Deps []string - } variantName string variant variationMap @@ -191,8 +188,12 @@ type depInfo struct { tag DependencyTag } +func (module *moduleInfo) Name() string { + return module.group.name +} + func (module *moduleInfo) String() string { - s := fmt.Sprintf("module %q", module.properties.Name) + s := fmt.Sprintf("module %q", module.Name()) if module.variantName != "" { s += fmt.Sprintf(" variant %q", module.variantName) } @@ -262,7 +263,7 @@ type mutatorInfo struct { func NewContext() *Context { ctx := &Context{ moduleFactories: make(map[string]ModuleFactory), - moduleGroups: make(map[string]*moduleGroup), + moduleNames: make(map[string]*moduleGroup), moduleInfo: make(map[Module]*moduleInfo), moduleNinjaNames: make(map[string]*moduleGroup), fs: fs, @@ -975,15 +976,10 @@ func (c *Context) cloneLogicModule(origModule *moduleInfo) (Module, []interface{ panic(fmt.Sprintf("unrecognized module type %q during cloning", typeName)) } - props := []interface{}{ - &origModule.properties, - } newLogicModule, newProperties := factory() - newProperties = append(props, newProperties...) - if len(newProperties) != len(origModule.moduleProperties) { - panic("mismatched properties array length in " + origModule.properties.Name) + panic("mismatched properties array length in " + origModule.Name()) } for i := range newProperties { @@ -1001,7 +997,7 @@ func (c *Context) createVariations(origModule *moduleInfo, mutatorName string, if len(variationNames) == 0 { panic(fmt.Errorf("mutator %q passed zero-length variation list for module %q", - mutatorName, origModule.properties.Name)) + mutatorName, origModule.Name())) } newModules := []*moduleInfo{} @@ -1073,7 +1069,7 @@ func (c *Context) convertDepsToVariation(module *moduleInfo, if newDep == nil { errs = append(errs, &BlueprintError{ Err: fmt.Errorf("failed to find variation %q for module %q needed by %q", - variationName, dep.module.properties.Name, module.properties.Name), + variationName, dep.module.Name(), module.Name()), Pos: module.pos, }) continue @@ -1121,10 +1117,6 @@ func (c *Context) processModuleDef(moduleDef *parser.Module, relBlueprintsFile: relBlueprintsFile, } - props := []interface{}{ - &module.properties, - } - properties = append(props, properties...) module.moduleProperties = properties propertyMap, errs := unpackProperties(moduleDef.Properties, properties...) @@ -1142,10 +1134,10 @@ func (c *Context) processModuleDef(moduleDef *parser.Module, } func (c *Context) addModule(module *moduleInfo) []error { - name := module.properties.Name + name := module.logicModule.Name() c.moduleInfo[module.logicModule] = module - if group, present := c.moduleGroups[name]; present { + if group, present := c.moduleNames[name]; present { return []error{ &BlueprintError{ Err: fmt.Errorf("module %q already defined", name), @@ -1156,25 +1148,26 @@ func (c *Context) addModule(module *moduleInfo) []error { Pos: group.modules[0].pos, }, } - } else { - ninjaName := toNinjaName(module.properties.Name) - - // The sanitizing in toNinjaName can result in collisions, uniquify the name if it - // already exists - for i := 0; c.moduleNinjaNames[ninjaName] != nil; i++ { - ninjaName = toNinjaName(module.properties.Name) + strconv.Itoa(i) - } - - group := &moduleGroup{ - name: module.properties.Name, - ninjaName: ninjaName, - modules: []*moduleInfo{module}, - } - module.group = group - c.moduleGroups[name] = group - c.moduleNinjaNames[ninjaName] = group } + ninjaName := toNinjaName(name) + + // The sanitizing in toNinjaName can result in collisions, uniquify the name if it + // already exists + for i := 0; c.moduleNinjaNames[ninjaName] != nil; i++ { + ninjaName = toNinjaName(name) + strconv.Itoa(i) + } + + group := &moduleGroup{ + name: name, + ninjaName: ninjaName, + modules: []*moduleInfo{module}, + } + module.group = group + c.moduleNames[name] = group + c.moduleNinjaNames[ninjaName] = group + c.moduleGroups = append(c.moduleGroups, group) + return nil } @@ -1201,13 +1194,9 @@ func (c *Context) ResolveDependencies(config interface{}) []error { // Default dependencies handling. If the module implements the (deprecated) // DynamicDependerModule interface then this set consists of the union of those -// module names listed in its "deps" property, those returned by its -// DynamicDependencies method, and those added by calling AddDependencies or -// AddVariationDependencies on DynamicDependencyModuleContext. Otherwise it -// is simply those names listed in its "deps" property. +// module names returned by its DynamicDependencies method and those added by calling +// AddDependencies or AddVariationDependencies on DynamicDependencyModuleContext. func blueprintDepsMutator(ctx BottomUpMutatorContext) { - ctx.AddDependency(ctx.Module(), nil, ctx.moduleInfo().properties.Deps...) - if dynamicDepender, ok := ctx.Module().(DynamicDependerModule); ok { func() { defer func() { @@ -1228,11 +1217,11 @@ 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, group *moduleGroup) *moduleInfo { - if len(group.modules) == 1 { - return group.modules[0] +func (c *Context) findMatchingVariant(module *moduleInfo, possible []*moduleInfo) *moduleInfo { + if len(possible) == 1 { + return possible[0] } else { - for _, m := range group.modules { + for _, m := range possible { if m.variant.equal(module.dependencyVariant) { return m } @@ -1243,27 +1232,27 @@ func (c *Context) findMatchingVariant(module *moduleInfo, group *moduleGroup) *m } func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName string) []error { - if depName == module.properties.Name { + if depName == module.Name() { return []error{&BlueprintError{ Err: fmt.Errorf("%q depends on itself", depName), Pos: module.pos, }} } - depGroup, ok := c.moduleGroups[depName] - if !ok { + possibleDeps := c.modulesFromName(depName) + if possibleDeps == nil { if c.allowMissingDependencies { module.missingDeps = append(module.missingDeps, depName) return nil } return []error{&BlueprintError{ Err: fmt.Errorf("%q depends on undefined module %q", - module.properties.Name, depName), + module.Name(), depName), Pos: module.pos, }} } - if m := c.findMatchingVariant(module, depGroup); m != nil { + 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? @@ -1277,36 +1266,36 @@ func (c *Context) addDependency(module *moduleInfo, tag DependencyTag, depName s return []error{&BlueprintError{ Err: fmt.Errorf("dependency %q of %q missing variant %q", - depGroup.modules[0].properties.Name, module.properties.Name, + depName, module.Name(), c.prettyPrintVariant(module.dependencyVariant)), Pos: module.pos, }} } func (c *Context) findReverseDependency(module *moduleInfo, destName string) (*moduleInfo, []error) { - if destName == module.properties.Name { + if destName == module.Name() { return nil, []error{&BlueprintError{ Err: fmt.Errorf("%q depends on itself", destName), Pos: module.pos, }} } - destInfo, ok := c.moduleGroups[destName] - if !ok { + possibleDeps := c.modulesFromName(destName) + if possibleDeps == nil { return nil, []error{&BlueprintError{ Err: fmt.Errorf("%q has a reverse dependency on undefined module %q", - module.properties.Name, destName), + module.Name(), destName), Pos: module.pos, }} } - if m := c.findMatchingVariant(module, destInfo); m != nil { + if m := c.findMatchingVariant(module, possibleDeps); m != nil { return m, nil } return nil, []error{&BlueprintError{ Err: fmt.Errorf("reverse dependency %q of %q missing variant %q", - destName, module.properties.Name, + destName, module.Name(), c.prettyPrintVariant(module.dependencyVariant)), Pos: module.pos, }} @@ -1315,15 +1304,15 @@ func (c *Context) findReverseDependency(module *moduleInfo, destName string) (*m func (c *Context) addVariationDependency(module *moduleInfo, variations []Variation, tag DependencyTag, depName string, far bool) []error { - depGroup, ok := c.moduleGroups[depName] - if !ok { + possibleDeps := c.modulesFromName(depName) + if possibleDeps == nil { if c.allowMissingDependencies { module.missingDeps = append(module.missingDeps, depName) return nil } return []error{&BlueprintError{ Err: fmt.Errorf("%q depends on undefined module %q", - module.properties.Name, depName), + module.Name(), depName), Pos: module.pos, }} } @@ -1341,7 +1330,7 @@ func (c *Context) addVariationDependency(module *moduleInfo, variations []Variat newVariant[v.Mutator] = v.Variation } - for _, m := range depGroup.modules { + for _, m := range possibleDeps { var found bool if far { found = m.variant.subset(newVariant) @@ -1358,7 +1347,7 @@ func (c *Context) addVariationDependency(module *moduleInfo, variations []Variat // AddVariationDependency allows adding a dependency on itself, but only if // that module is earlier in the module list than this one, since we always // run GenerateBuildActions in order for the variants of a module - if depGroup == module.group && beforeInModuleList(module, m, module.group.modules) { + if m.group == module.group && beforeInModuleList(module, m, module.group.modules) { return []error{&BlueprintError{ Err: fmt.Errorf("%q depends on later version of itself", depName), Pos: module.pos, @@ -1372,7 +1361,7 @@ func (c *Context) addVariationDependency(module *moduleInfo, variations []Variat return []error{&BlueprintError{ Err: fmt.Errorf("dependency %q of %q missing variant %q", - depGroup.modules[0].properties.Name, module.properties.Name, + depName, module.Name(), c.prettyPrintVariant(newVariant)), Pos: module.pos, }} @@ -1389,14 +1378,14 @@ func (c *Context) addInterVariantDependency(origModule *moduleInfo, tag Dependen if m.logicModule == to { toInfo = m if fromInfo != nil { - panic(fmt.Errorf("%q depends on later version of itself", origModule.properties.Name)) + panic(fmt.Errorf("%q depends on later version of itself", origModule.Name())) } } } if fromInfo == nil || toInfo == nil { panic(fmt.Errorf("AddInterVariantDependency called for module %q on invalid variant", - origModule.properties.Name)) + origModule.Name())) } fromInfo.directDeps = append(fromInfo.directDeps, depInfo{toInfo, tag}) @@ -1530,8 +1519,8 @@ func (c *Context) updateDependencies() (errs []error) { nextModule := cycle[i] errs = append(errs, &BlueprintError{ Err: fmt.Errorf(" %q depends on %q", - curModule.properties.Name, - nextModule.properties.Name), + curModule.Name(), + nextModule.Name()), Pos: curModule.pos, }) curModule = nextModule @@ -1998,7 +1987,7 @@ func (c *Context) generateModuleBuildActions(config interface{}, for _, depName := range module.missingDeps { errs = append(errs, &BlueprintError{ Err: fmt.Errorf("%q depends on undefined module %q", - module.properties.Name, depName), + module.Name(), depName), Pos: module.pos, }) } @@ -2157,10 +2146,17 @@ func (c *Context) walkDeps(topModule *moduleInfo, type innerPanicError error +func (c *Context) modulesFromName(name string) []*moduleInfo { + if group := c.moduleNames[name]; group != nil { + return group.modules + } + return nil +} + func (c *Context) sortedModuleNames() []string { if c.cachedSortedModuleNames == nil { - c.cachedSortedModuleNames = make([]string, 0, len(c.moduleGroups)) - for moduleName := range c.moduleGroups { + c.cachedSortedModuleNames = make([]string, 0, len(c.moduleNames)) + for moduleName := range c.moduleNames { c.cachedSortedModuleNames = append(c.cachedSortedModuleNames, moduleName) } @@ -2181,8 +2177,8 @@ func (c *Context) visitAllModules(visit func(Module)) { }() for _, moduleName := range c.sortedModuleNames() { - group := c.moduleGroups[moduleName] - for _, module = range group.modules { + modules := c.modulesFromName(moduleName) + for _, module = range modules { visit(module.logicModule) } } @@ -2201,8 +2197,8 @@ func (c *Context) visitAllModulesIf(pred func(Module) bool, }() for _, moduleName := range c.sortedModuleNames() { - group := c.moduleGroups[moduleName] - for _, module := range group.modules { + modules := c.modulesFromName(moduleName) + for _, module := range modules { if pred(module.logicModule) { visit(module.logicModule) } @@ -2440,7 +2436,7 @@ func (c *Context) ModuleTypePropertyStructs() map[string][]interface{} { func (c *Context) ModuleName(logicModule Module) string { module := c.moduleInfo[logicModule] - return module.properties.Name + return module.Name() } func (c *Context) ModuleDir(logicModule Module) string { @@ -2817,8 +2813,8 @@ func (s depSorter) Len() int { } func (s depSorter) Less(i, j int) bool { - iName := s[i].module.properties.Name - jName := s[j].module.properties.Name + iName := s[i].module.Name() + jName := s[j].module.Name() if iName == jName { iName = s[i].module.variantName jName = s[j].module.variantName @@ -2837,8 +2833,8 @@ func (s moduleSorter) Len() int { } func (s moduleSorter) Less(i, j int) bool { - iName := s[i].properties.Name - jName := s[j].properties.Name + iName := s[i].Name() + jName := s[j].Name() if iName == jName { iName = s[i].variantName jName = s[j].variantName @@ -2885,11 +2881,11 @@ func (c *Context) writeAllModuleActions(nw *ninjaWriter) error { factoryName := factoryFunc.Name() infoMap := map[string]interface{}{ - "properties": module.properties, - "typeName": module.typeName, - "goFactory": factoryName, - "pos": relPos, - "variant": module.variantName, + "name": module.Name(), + "typeName": module.typeName, + "goFactory": factoryName, + "pos": relPos, + "variant": module.variantName, } err = headerTemplate.Execute(buf, infoMap) if err != nil { @@ -3098,7 +3094,7 @@ they were generated by the following Go packages: ` var moduleHeaderTemplate = `# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # -Module: {{.properties.Name}} +Module: {{.name}} Variant: {{.variant}} Type: {{.typeName}} Factory: {{.goFactory}} diff --git a/context_test.go b/context_test.go index 70a6daa..5d9f4bc 100644 --- a/context_test.go +++ b/context_test.go @@ -24,19 +24,25 @@ type Walker interface { } type fooModule struct { + SimpleName properties struct { - Foo string + Deps []string + Foo string } } func newFooModule() (Module, []interface{}) { m := &fooModule{} - return m, []interface{}{&m.properties} + return m, []interface{}{&m.properties, &m.SimpleName.Properties} } func (f *fooModule) GenerateBuildActions(ModuleContext) { } +func (f *fooModule) DynamicDependencies(ctx DynamicDependerModuleContext) []string { + return f.properties.Deps +} + func (f *fooModule) Foo() string { return f.properties.Foo } @@ -46,14 +52,20 @@ func (f *fooModule) Walk() bool { } type barModule struct { + SimpleName properties struct { - Bar bool + Deps []string + Bar bool } } func newBarModule() (Module, []interface{}) { m := &barModule{} - return m, []interface{}{&m.properties} + return m, []interface{}{&m.properties, &m.SimpleName.Properties} +} + +func (b *barModule) DynamicDependencies(ctx DynamicDependerModuleContext) []string { + return b.properties.Deps } func (b *barModule) GenerateBuildActions(ModuleContext) { @@ -74,12 +86,12 @@ func TestContextParse(t *testing.T) { r := bytes.NewBufferString(` foo_module { - name: "MyFooModule", + name: "MyFooModule", deps: ["MyBarModule"], } bar_module { - name: "MyBarModule", + name: "MyBarModule", } `) @@ -168,7 +180,7 @@ func TestWalkDeps(t *testing.T) { var outputDown string var outputUp string - topModule := ctx.moduleGroups["A"].modules[0] + topModule := ctx.modulesFromName("A")[0] ctx.walkDeps(topModule, func(dep depInfo, parent *moduleInfo) bool { if dep.module.logicModule.(Walker).Walk() { diff --git a/module_ctx.go b/module_ctx.go index 22896c2..673a1c0 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -84,6 +84,16 @@ import ( // or variants of the current Module must be synchronized by the implementation of // GenerateBuildActions. type Module interface { + // Name returns a string used to uniquely identify each module. The return + // value must be unique across all modules. It is only called once, during + // initial blueprint parsing. To change the name later a mutator must call + // MutatorContext.Rename + // + // In most cases, Name should return the contents of a "name:" property from + // the blueprint file. An embeddable SimpleName object can be used for this + // case. + Name() string + // GenerateBuildActions is called by the Context that created the Module // during its generate phase. This call should generate all Ninja build // actions (rules, pools, and build statements) needed to build the module. @@ -168,7 +178,7 @@ func (d *baseModuleContext) moduleInfo() *moduleInfo { } func (d *baseModuleContext) ModuleName() string { - return d.module.properties.Name + return d.module.Name() } func (d *baseModuleContext) ContainsProperty(name string) bool { @@ -248,7 +258,7 @@ type moduleContext struct { func (m *baseModuleContext) OtherModuleName(logicModule Module) string { module := m.context.moduleInfo[logicModule] - return module.properties.Name + return module.Name() } func (m *baseModuleContext) OtherModuleErrorf(logicModule Module, format string, @@ -640,3 +650,16 @@ func (mctx *mutatorContext) AddFarVariationDependencies(variations []Variation, func (mctx *mutatorContext) AddInterVariantDependency(tag DependencyTag, from, to Module) { mctx.context.addInterVariantDependency(mctx.module, tag, from, to) } + +// SimpleName is an embeddable object to implement the ModuleContext.Name method using a property +// called "name". Modules that embed it must also add SimpleName.Properties to their property +// structure list. +type SimpleName struct { + Properties struct { + Name string + } +} + +func (s *SimpleName) Name() string { + return s.Properties.Name +} diff --git a/visit_test.go b/visit_test.go index 51a9b63..e1768f1 100644 --- a/visit_test.go +++ b/visit_test.go @@ -20,6 +20,7 @@ import ( ) type visitModule struct { + SimpleName properties struct { Visit []string VisitDepsDepthFirst string `blueprint:"mutated"` @@ -31,7 +32,7 @@ type visitModule struct { func newVisitModule() (Module, []interface{}) { m := &visitModule{} - return m, []interface{}{&m.properties} + return m, []interface{}{&m.properties, &m.SimpleName.Properties} } func (f *visitModule) GenerateBuildActions(ModuleContext) { @@ -140,7 +141,7 @@ func setupVisitTest(t *testing.T) *Context { func TestVisit(t *testing.T) { ctx := setupVisitTest(t) - topModule := ctx.moduleGroups["A"].modules[0].logicModule.(*visitModule) + topModule := ctx.modulesFromName("A")[0].logicModule.(*visitModule) assertString(t, topModule.properties.VisitDepsDepthFirst, "EDCB") assertString(t, topModule.properties.VisitDepsDepthFirstIf, "EDC") assertString(t, topModule.properties.VisitDirectDeps, "B")