From da70fd0b84bc491976c4ce808102acb847fbc4e0 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 30 Dec 2019 18:40:09 -0800 Subject: [PATCH] Move LoadHooks from Soong to Blueprint Move LoadHooks from Blueprint and run them during ParseBlueprintsFiles. This will allow them to add scoped module types that are visible to the rest of the Blueprints file. Requires passing the config object to ParseBlueprintsFiles. Test: all blueprint tests Change-Id: Ia2a2c9a0223d5458bfd48bd22ebed0fdbd0156c6 --- bootstrap/command.go | 2 +- context.go | 61 ++++++++++++++++++++++-------- context_test.go | 8 ++-- module_ctx.go | 89 +++++++++++++++++++++++++++++++++++++++++++- module_ctx_test.go | 2 +- visit_test.go | 2 +- 6 files changed, 141 insertions(+), 23 deletions(-) diff --git a/bootstrap/command.go b/bootstrap/command.go index bf6bbe9..44a26c0 100644 --- a/bootstrap/command.go +++ b/bootstrap/command.go @@ -140,7 +140,7 @@ func Main(ctx *blueprint.Context, config interface{}, extraNinjaFileDeps ...stri ctx.RegisterSingletonType("glob", globSingletonFactory(ctx)) - deps, errs := ctx.ParseFileList(filepath.Dir(bootstrapConfig.topLevelBlueprintsFile), filesToParse) + deps, errs := ctx.ParseFileList(filepath.Dir(bootstrapConfig.topLevelBlueprintsFile), filesToParse, config) if len(errs) > 0 { fatalErrors(errs) } diff --git a/context.go b/context.go index 0619953..0e4f586 100644 --- a/context.go +++ b/context.go @@ -631,17 +631,19 @@ type fileParseContext struct { // which the future output will depend is returned. This list will include both // Blueprints file paths as well as directory paths for cases where wildcard // subdirs are found. -func (c *Context) ParseBlueprintsFiles(rootFile string) (deps []string, errs []error) { +func (c *Context) ParseBlueprintsFiles(rootFile string, + config interface{}) (deps []string, errs []error) { + baseDir := filepath.Dir(rootFile) pathsToParse, err := c.ListModulePaths(baseDir) if err != nil { return nil, []error{err} } - return c.ParseFileList(baseDir, pathsToParse) + return c.ParseFileList(baseDir, pathsToParse, config) } -func (c *Context) ParseFileList(rootDir string, filePaths []string) (deps []string, - errs []error) { +func (c *Context) ParseFileList(rootDir string, filePaths []string, + config interface{}) (deps []string, errs []error) { if len(filePaths) < 1 { return nil, []error{fmt.Errorf("no paths provided to parse")} @@ -649,7 +651,12 @@ func (c *Context) ParseFileList(rootDir string, filePaths []string) (deps []stri c.dependenciesReady = false - moduleCh := make(chan *moduleInfo) + type newModuleInfo struct { + *moduleInfo + added chan<- struct{} + } + + moduleCh := make(chan newModuleInfo) errsCh := make(chan []error) doneCh := make(chan struct{}) var numErrs uint32 @@ -661,24 +668,45 @@ func (c *Context) ParseFileList(rootDir string, filePaths []string) (deps []stri return } + addedCh := make(chan struct{}) + + var addModule func(module *moduleInfo) []error + addModule = func(module *moduleInfo) (errs []error) { + moduleCh <- newModuleInfo{module, addedCh} + <-addedCh + var newModules []*moduleInfo + newModules, errs = runAndRemoveLoadHooks(c, config, module) + if len(errs) > 0 { + return errs + } + for _, n := range newModules { + errs = addModule(n) + if len(errs) > 0 { + return errs + } + } + return nil + } + for _, def := range file.Defs { - var module *moduleInfo - var errs []error switch def := def.(type) { case *parser.Module: - module, errs = c.processModuleDef(def, file.Name) + module, errs := c.processModuleDef(def, file.Name) + if len(errs) == 0 && module != nil { + errs = addModule(module) + } + + if len(errs) > 0 { + atomic.AddUint32(&numErrs, uint32(len(errs))) + errsCh <- errs + } + case *parser.Assignment: // Already handled via Scope object default: panic("unknown definition type") } - if len(errs) > 0 { - atomic.AddUint32(&numErrs, uint32(len(errs))) - errsCh <- errs - } else if module != nil { - moduleCh <- module - } } } @@ -698,7 +726,10 @@ loop: case newErrs := <-errsCh: errs = append(errs, newErrs...) case module := <-moduleCh: - newErrs := c.addModule(module) + newErrs := c.addModule(module.moduleInfo) + if module.added != nil { + module.added <- struct{}{} + } if len(newErrs) > 0 { errs = append(errs, newErrs...) } diff --git a/context_test.go b/context_test.go index 61e628e..d4e9bf7 100644 --- a/context_test.go +++ b/context_test.go @@ -168,7 +168,7 @@ func TestWalkDeps(t *testing.T) { ctx.RegisterModuleType("foo_module", newFooModule) ctx.RegisterModuleType("bar_module", newBarModule) - _, errs := ctx.ParseBlueprintsFiles("Blueprints") + _, errs := ctx.ParseBlueprintsFiles("Blueprints", nil) if len(errs) > 0 { t.Errorf("unexpected parse errors:") for _, err := range errs { @@ -260,7 +260,7 @@ func TestWalkDepsDuplicates(t *testing.T) { ctx.RegisterModuleType("foo_module", newFooModule) ctx.RegisterModuleType("bar_module", newBarModule) - _, errs := ctx.ParseBlueprintsFiles("Blueprints") + _, errs := ctx.ParseBlueprintsFiles("Blueprints", nil) if len(errs) > 0 { t.Errorf("unexpected parse errors:") for _, err := range errs { @@ -316,7 +316,7 @@ func TestCreateModule(t *testing.T) { ctx.RegisterModuleType("foo_module", newFooModule) ctx.RegisterModuleType("bar_module", newBarModule) - _, errs := ctx.ParseBlueprintsFiles("Blueprints") + _, errs := ctx.ParseBlueprintsFiles("Blueprints", nil) if len(errs) > 0 { t.Errorf("unexpected parse errors:") for _, err := range errs { @@ -499,7 +499,7 @@ func TestParseFailsForModuleWithoutName(t *testing.T) { ctx.RegisterModuleType("foo_module", newFooModule) ctx.RegisterModuleType("bar_module", newBarModule) - _, errs := ctx.ParseBlueprintsFiles("Blueprints") + _, errs := ctx.ParseBlueprintsFiles("Blueprints", nil) expectedErrs := []error{ errors.New(`Blueprints:6:4: property 'name' is missing from a module`), diff --git a/module_ctx.go b/module_ctx.go index 639cbf7..8b52b50 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -17,6 +17,7 @@ package blueprint import ( "fmt" "path/filepath" + "sync" "text/scanner" "github.com/google/blueprint/pathtools" @@ -120,7 +121,7 @@ type DynamicDependerModule interface { DynamicDependencies(DynamicDependerModuleContext) []string } -type BaseModuleContext interface { +type EarlyModuleContext interface { // Module returns the current module as a Module. It should rarely be necessary, as the module already has a // reference to itself. Module() Module @@ -178,6 +179,10 @@ type BaseModuleContext interface { // Namespace returns the Namespace object provided by the NameInterface set by Context.SetNameInterface, or the // default SimpleNameInterface if Context.SetNameInterface was not called. Namespace() Namespace +} + +type BaseModuleContext interface { + EarlyModuleContext // GetDirectDepWithTag returns the Module the direct dependency with the specified name, or nil if // none exists. It panics if the dependency does not have the specified tag. @@ -995,3 +1000,85 @@ type SimpleName struct { func (s *SimpleName) Name() string { return s.Properties.Name } + +// Load Hooks + +type LoadHookContext interface { + EarlyModuleContext + + // CreateModule creates a new module by calling the factory method for the specified moduleType, and applies + // the specified property structs to it as if the properties were set in a blueprint file. + CreateModule(ModuleFactory, ...interface{}) Module +} + +func (l *loadHookContext) CreateModule(factory ModuleFactory, props ...interface{}) Module { + module := l.context.newModule(factory) + + module.relBlueprintsFile = l.module.relBlueprintsFile + module.pos = l.module.pos + module.propertyPos = l.module.propertyPos + module.createdBy = l.module + + for _, p := range props { + err := proptools.AppendMatchingProperties(module.properties, p, nil) + if err != nil { + panic(err) + } + } + + l.newModules = append(l.newModules, module) + + return module.logicModule +} + +type loadHookContext struct { + baseModuleContext + newModules []*moduleInfo +} + +type LoadHook func(ctx LoadHookContext) + +// Load hooks need to be added by module factories, which don't have any parameter to get to the +// Context, and only produce a Module interface with no base implementation, so the load hooks +// must be stored in a global map. The key is a pointer allocated by the module factory, so there +// is no chance of collisions even if tests are running in parallel with multiple contexts. The +// contents should be short-lived, they are added during a module factory and removed immediately +// after the module factory returns. +var pendingHooks sync.Map + +func AddLoadHook(module Module, hook LoadHook) { + // Only one goroutine can be processing a given module, so no additional locking is required + // for the slice stored in the sync.Map. + v, exists := pendingHooks.Load(module) + if !exists { + v, _ = pendingHooks.LoadOrStore(module, new([]LoadHook)) + } + hooks := v.(*[]LoadHook) + *hooks = append(*hooks, hook) +} + +func runAndRemoveLoadHooks(ctx *Context, config interface{}, + module *moduleInfo) (newModules []*moduleInfo, errs []error) { + + if v, exists := pendingHooks.Load(module.logicModule); exists { + hooks := v.(*[]LoadHook) + mctx := &loadHookContext{ + baseModuleContext: baseModuleContext{ + context: ctx, + config: config, + module: module, + }, + } + + for _, hook := range *hooks { + hook(mctx) + newModules = append(newModules, mctx.newModules...) + errs = append(errs, mctx.errs...) + } + pendingHooks.Delete(module.logicModule) + + return newModules, errs + } + + return nil, nil +} diff --git a/module_ctx_test.go b/module_ctx_test.go index e7127ae..e72be8c 100644 --- a/module_ctx_test.go +++ b/module_ctx_test.go @@ -76,7 +76,7 @@ func TestAliases(t *testing.T) { ctx.MockFileSystem(mockFS) - _, errs := ctx.ParseFileList(".", []string{"Blueprints"}) + _, errs := ctx.ParseFileList(".", []string{"Blueprints"}, nil) if len(errs) > 0 { t.Errorf("unexpected parse errors:") for _, err := range errs { diff --git a/visit_test.go b/visit_test.go index 66a5295..efaadba 100644 --- a/visit_test.go +++ b/visit_test.go @@ -125,7 +125,7 @@ func setupVisitTest(t *testing.T) *Context { `), }) - _, errs := ctx.ParseBlueprintsFiles("Blueprints") + _, errs := ctx.ParseBlueprintsFiles("Blueprints", nil) if len(errs) > 0 { t.Errorf("unexpected parse errors:") for _, err := range errs {