Merge remote-tracking branch 'aosp/upstream'
* aosp/upstream:
Support checking syntax of generated Blueprint files
Always emit rules for tests and add phony to run them
Bug: 155628860
Bug: 156428456
Test: treehugger
Change-Id: Ia0e0c8d8d07489c945d4e03d039273cc3f597ac0
diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go
index bb85e7d..79e5c8e 100644
--- a/bootstrap/bootstrap.go
+++ b/bootstrap/bootstrap.go
@@ -333,13 +333,11 @@
testSrcs = append(g.properties.TestSrcs, g.properties.Linux.TestSrcs...)
}
- if g.config.runGoTests {
- testArchiveFile := filepath.Join(testRoot(ctx, g.config),
- filepath.FromSlash(g.properties.PkgPath)+".a")
- g.testResultFile = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile,
- g.properties.PkgPath, srcs, genSrcs,
- testSrcs)
- }
+ testArchiveFile := filepath.Join(testRoot(ctx, g.config),
+ filepath.FromSlash(g.properties.PkgPath)+".a")
+ g.testResultFile = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile,
+ g.properties.PkgPath, srcs, genSrcs,
+ testSrcs)
buildGoPackage(ctx, g.pkgRoot, g.properties.PkgPath, g.archiveFile,
srcs, genSrcs)
@@ -436,9 +434,10 @@
testSrcs = append(g.properties.TestSrcs, g.properties.Linux.TestSrcs...)
}
+ testDeps := buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile,
+ name, srcs, genSrcs, testSrcs)
if g.config.runGoTests {
- deps = buildGoTest(ctx, testRoot(ctx, g.config), testArchiveFile,
- name, srcs, genSrcs, testSrcs)
+ deps = append(deps, testDeps...)
}
buildGoPackage(ctx, objDir, "main", archiveFile, srcs, genSrcs)
@@ -451,7 +450,9 @@
linkDeps = append(linkDeps, dep.GoPackageTarget())
libDir := dep.GoPkgRoot()
libDirFlags = append(libDirFlags, "-L "+libDir)
- deps = append(deps, dep.GoTestTargets()...)
+ if g.config.runGoTests {
+ deps = append(deps, dep.GoTestTargets()...)
+ }
})
linkArgs := map[string]string{}
@@ -635,17 +636,22 @@
var primaryBuilders []*goBinary
// blueprintTools contains blueprint go binaries that will be built in StageMain
var blueprintTools []string
- ctx.VisitAllModulesIf(isBootstrapBinaryModule,
- func(module blueprint.Module) {
- binaryModule := module.(*goBinary)
-
+ // blueprintTests contains the result files from the tests
+ var blueprintTests []string
+ ctx.VisitAllModules(func(module blueprint.Module) {
+ if binaryModule, ok := module.(*goBinary); ok {
if binaryModule.properties.Tool_dir {
blueprintTools = append(blueprintTools, binaryModule.InstallPath())
}
if binaryModule.properties.PrimaryBuilder {
primaryBuilders = append(primaryBuilders, binaryModule)
}
- })
+ }
+
+ if packageModule, ok := module.(goPackageProducer); ok {
+ blueprintTests = append(blueprintTests, packageModule.GoTestTargets()...)
+ }
+ })
var extraSharedFlagArray []string
if s.config.runGoTests {
@@ -759,6 +765,14 @@
Outputs: []string{"blueprint_tools"},
Inputs: blueprintTools,
})
+
+ // Add a phony target for running all of the tests
+ ctx.Build(pctx, blueprint.BuildParams{
+ Rule: blueprint.Phony,
+ Outputs: []string{"blueprint_tests"},
+ Inputs: blueprintTests,
+ })
+
}
}
diff --git a/context.go b/context.go
index 9a3253a..3018209 100644
--- a/context.go
+++ b/context.go
@@ -692,7 +692,7 @@
var scopedModuleFactories map[string]ModuleFactory
var addModule func(module *moduleInfo) []error
- addModule = func(module *moduleInfo) ([]error) {
+ addModule = func(module *moduleInfo) []error {
// Run any load hooks immediately before it is sent to the moduleCh and is
// registered by name. This allows load hooks to set and/or modify any aspect
// of the module (including names) using information that is not available when
@@ -716,7 +716,7 @@
for _, def := range file.Defs {
switch def := def.(type) {
case *parser.Module:
- module, errs := c.processModuleDef(def, file.Name, scopedModuleFactories)
+ module, errs := processModuleDef(def, file.Name, c.moduleFactories, scopedModuleFactories, c.ignoreUnknownModuleTypes)
if len(errs) == 0 && module != nil {
errs = addModule(module)
}
@@ -1349,7 +1349,7 @@
return strings.Join(variants, "\n ")
}
-func (c *Context) newModule(factory ModuleFactory) *moduleInfo {
+func newModule(factory ModuleFactory) *moduleInfo {
logicModule, properties := factory()
module := &moduleInfo{
@@ -1362,15 +1362,15 @@
return module
}
-func (c *Context) processModuleDef(moduleDef *parser.Module,
- relBlueprintsFile string, scopedModuleFactories map[string]ModuleFactory) (*moduleInfo, []error) {
+func processModuleDef(moduleDef *parser.Module,
+ relBlueprintsFile string, moduleFactories, scopedModuleFactories map[string]ModuleFactory, ignoreUnknownModuleTypes bool) (*moduleInfo, []error) {
- factory, ok := c.moduleFactories[moduleDef.Type]
+ factory, ok := moduleFactories[moduleDef.Type]
if !ok && scopedModuleFactories != nil {
factory, ok = scopedModuleFactories[moduleDef.Type]
}
if !ok {
- if c.ignoreUnknownModuleTypes {
+ if ignoreUnknownModuleTypes {
return nil, nil
}
@@ -1382,7 +1382,7 @@
}
}
- module := c.newModule(factory)
+ module := newModule(factory)
module.typeName = moduleDef.Type
module.relBlueprintsFile = relBlueprintsFile
diff --git a/module_ctx.go b/module_ctx.go
index 012142e..36e05a4 100644
--- a/module_ctx.go
+++ b/module_ctx.go
@@ -17,9 +17,11 @@
import (
"fmt"
"path/filepath"
+ "strings"
"sync"
"text/scanner"
+ "github.com/google/blueprint/parser"
"github.com/google/blueprint/pathtools"
"github.com/google/blueprint/proptools"
)
@@ -988,7 +990,7 @@
}
func (mctx *mutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) Module {
- module := mctx.context.newModule(factory)
+ module := newModule(factory)
module.relBlueprintsFile = mctx.module.relBlueprintsFile
module.pos = mctx.module.pos
@@ -1035,7 +1037,7 @@
}
func (l *loadHookContext) CreateModule(factory ModuleFactory, props ...interface{}) Module {
- module := l.context.newModule(factory)
+ module := newModule(factory)
module.relBlueprintsFile = l.module.relBlueprintsFile
module.pos = l.module.pos
@@ -1123,3 +1125,44 @@
return nil, nil
}
+
+// Check the syntax of a generated blueprint file.
+//
+// This is intended to perform a quick sanity check for generated blueprint
+// code to ensure that it is syntactically correct, where syntactically correct
+// means:
+// * No variable definitions.
+// * Valid module types.
+// * Valid property names.
+// * Valid values for the property type.
+//
+// It does not perform any semantic checking of properties, existence of referenced
+// files, or dependencies.
+//
+// At a low level it:
+// * Parses the contents.
+// * Invokes relevant factory to create Module instances.
+// * Unpacks the properties into the Module.
+// * Does not invoke load hooks or any mutators.
+//
+// The filename is only used for reporting errors.
+func CheckBlueprintSyntax(moduleFactories map[string]ModuleFactory, filename string, contents string) []error {
+ scope := parser.NewScope(nil)
+ file, errs := parser.Parse(filename, strings.NewReader(contents), scope)
+ if len(errs) != 0 {
+ return errs
+ }
+
+ for _, def := range file.Defs {
+ switch def := def.(type) {
+ case *parser.Module:
+ _, moduleErrs := processModuleDef(def, filename, moduleFactories, nil, false)
+ errs = append(errs, moduleErrs...)
+
+ default:
+ panic(fmt.Errorf("unknown definition type: %T", def))
+ }
+ }
+
+ return errs
+}
diff --git a/module_ctx_test.go b/module_ctx_test.go
index e72be8c..e98ae82 100644
--- a/module_ctx_test.go
+++ b/module_ctx_test.go
@@ -195,3 +195,89 @@
"\n 1:a, 2:a\n 1:a, 2:b\n 1:b, 2:a\n 1:b, 2:b")
})
}
+
+func expectedErrors(t *testing.T, errs []error, expectedMessages ...string) {
+ t.Helper()
+ if len(errs) != len(expectedMessages) {
+ t.Errorf("expected %d error, found: %q", len(expectedMessages), errs)
+ } else {
+ for i, expected := range expectedMessages {
+ err := errs[i]
+ if err.Error() != expected {
+ t.Errorf("expected error %q found %q", expected, err)
+ }
+ }
+ }
+}
+
+func TestCheckBlueprintSyntax(t *testing.T) {
+ factories := map[string]ModuleFactory{
+ "test": newModuleCtxTestModule,
+ }
+
+ t.Run("valid", func(t *testing.T) {
+ errs := CheckBlueprintSyntax(factories, "path/Blueprint", `
+test {
+ name: "test",
+}
+`)
+ expectedErrors(t, errs)
+ })
+
+ t.Run("syntax error", func(t *testing.T) {
+ errs := CheckBlueprintSyntax(factories, "path/Blueprint", `
+test {
+ name: "test",
+
+`)
+
+ expectedErrors(t, errs, `path/Blueprint:5:1: expected "}", found EOF`)
+ })
+
+ t.Run("unknown module type", func(t *testing.T) {
+ errs := CheckBlueprintSyntax(factories, "path/Blueprint", `
+test2 {
+ name: "test",
+}
+`)
+
+ expectedErrors(t, errs, `path/Blueprint:2:1: unrecognized module type "test2"`)
+ })
+
+ t.Run("unknown property name", func(t *testing.T) {
+ errs := CheckBlueprintSyntax(factories, "path/Blueprint", `
+test {
+ nam: "test",
+}
+`)
+
+ expectedErrors(t, errs, `path/Blueprint:3:5: unrecognized property "nam"`)
+ })
+
+ t.Run("invalid property type", func(t *testing.T) {
+ errs := CheckBlueprintSyntax(factories, "path/Blueprint", `
+test {
+ name: false,
+}
+`)
+
+ expectedErrors(t, errs, `path/Blueprint:3:8: can't assign bool value to string property "name"`)
+ })
+
+ t.Run("multiple failures", func(t *testing.T) {
+ errs := CheckBlueprintSyntax(factories, "path/Blueprint", `
+test {
+ name: false,
+}
+
+test2 {
+ name: false,
+}
+`)
+
+ expectedErrors(t, errs,
+ `path/Blueprint:3:8: can't assign bool value to string property "name"`,
+ `path/Blueprint:6:1: unrecognized module type "test2"`,
+ )
+ })
+}
