diff --git a/context.go b/context.go index 56d4deb..2592289 100644 --- a/context.go +++ b/context.go @@ -142,6 +142,45 @@ type Context struct { // String values that can be used to gate build graph traversal includeTags *IncludeTags + + sourceRootDirs *SourceRootDirs +} + +// A container for String keys. The keys can be used to gate build graph traversal +type SourceRootDirs struct { + dirs []string +} + +func (dirs *SourceRootDirs) Add(names ...string) { + dirs.dirs = append(dirs.dirs, names...) +} + +func (dirs *SourceRootDirs) SourceRootDirAllowed(path string) (bool, string) { + sort.Slice(dirs.dirs, func(i, j int) bool { + return len(dirs.dirs[i]) < len(dirs.dirs[j]) + }) + last := len(dirs.dirs) + for i := range dirs.dirs { + // iterate from longest paths (most specific) + prefix := dirs.dirs[last-i-1] + disallowedPrefix := false + if len(prefix) >= 1 && prefix[0] == '-' { + prefix = prefix[1:] + disallowedPrefix = true + } + if strings.HasPrefix(path, prefix) { + if disallowedPrefix { + return false, prefix + } else { + return true, prefix + } + } + } + return true, "" +} + +func (c *Context) AddSourceRootDirs(dirs ...string) { + c.sourceRootDirs.Add(dirs...) } // A container for String keys. The keys can be used to gate build graph traversal @@ -427,6 +466,7 @@ func newContext() *Context { fs: pathtools.OsFs, finishedMutators: make(map[*mutatorInfo]bool), includeTags: &IncludeTags{}, + sourceRootDirs: &SourceRootDirs{}, outDir: nil, requiredNinjaMajor: 1, requiredNinjaMinor: 7, @@ -968,15 +1008,25 @@ func (c *Context) ParseBlueprintsFiles(rootFile string, return c.ParseFileList(baseDir, pathsToParse, config) } +type shouldVisitFileInfo struct { + shouldVisitFile bool + skippedModules []string + reasonForSkip string + errs []error +} + // Returns a boolean for whether this file should be analyzed // Evaluates to true if the file either // 1. does not contain a blueprint_package_includes // 2. contains a blueprint_package_includes and all requested tags are set // This should be processed before adding any modules to the build graph -func shouldVisitFile(c *Context, file *parser.File) (bool, []error) { +func shouldVisitFile(c *Context, file *parser.File) shouldVisitFileInfo { + skippedModules := []string{} + var blueprintPackageIncludes *PackageIncludes for _, def := range file.Defs { switch def := def.(type) { case *parser.Module: + skippedModules = append(skippedModules, def.Name()) if def.Type != "blueprint_package_includes" { continue } @@ -984,14 +1034,43 @@ func shouldVisitFile(c *Context, file *parser.File) (bool, []error) { if len(errs) > 0 { // This file contains errors in blueprint_package_includes // Visit anyways so that we can report errors on other modules in the file - return true, errs + return shouldVisitFileInfo{ + shouldVisitFile: true, + errs: errs, + } } logicModule, _ := c.cloneLogicModule(module) - pi := logicModule.(*PackageIncludes) - return pi.MatchesIncludeTags(c), []error{} + blueprintPackageIncludes = logicModule.(*PackageIncludes) } } - return true, []error{} + + if blueprintPackageIncludes != nil { + packageMatches := blueprintPackageIncludes.MatchesIncludeTags(c) + if !packageMatches { + return shouldVisitFileInfo{ + shouldVisitFile: false, + skippedModules: skippedModules, + reasonForSkip: fmt.Sprintf( + "module is defined in %q which contains a blueprint_package_includes module with unsatisfied tags", + file.Name, + ), + } + } + } + + shouldVisit, invalidatingPrefix := c.sourceRootDirs.SourceRootDirAllowed(file.Name) + if !shouldVisit { + return shouldVisitFileInfo{ + shouldVisitFile: shouldVisit, + skippedModules: skippedModules, + reasonForSkip: fmt.Sprintf( + "%q is a descendant of %q, and that path prefix was not included in PRODUCT_SOURCE_ROOT_DIRS", + file.Name, + invalidatingPrefix, + ), + } + } + return shouldVisitFileInfo{shouldVisitFile: true} } func (c *Context) ParseFileList(rootDir string, filePaths []string, @@ -1009,9 +1088,15 @@ func (c *Context) ParseFileList(rootDir string, filePaths []string, added chan<- struct{} } + type newSkipInfo struct { + shouldVisitFileInfo + file string + } + moduleCh := make(chan newModuleInfo) errsCh := make(chan []error) doneCh := make(chan struct{}) + skipCh := make(chan newSkipInfo) var numErrs uint32 var numGoroutines int32 @@ -1046,12 +1131,17 @@ func (c *Context) ParseFileList(rootDir string, filePaths []string, } return nil } - shouldVisit, errs := shouldVisitFile(c, file) + shouldVisitInfo := shouldVisitFile(c, file) + errs := shouldVisitInfo.errs if len(errs) > 0 { atomic.AddUint32(&numErrs, uint32(len(errs))) errsCh <- errs } - if !shouldVisit { + if !shouldVisitInfo.shouldVisitFile { + skipCh <- newSkipInfo{ + file: file.Name, + shouldVisitFileInfo: shouldVisitInfo, + } // TODO: Write a file that lists the skipped bp files return } @@ -1108,6 +1198,14 @@ loop: if n == 0 { break loop } + case skipped := <-skipCh: + nctx := newNamespaceContextFromFilename(skipped.file) + for _, name := range skipped.skippedModules { + c.nameInterface.NewSkippedModule(nctx, name, SkippedModuleInfo{ + filename: skipped.file, + reason: skipped.reasonForSkip, + }) + } } } @@ -3491,7 +3589,6 @@ func (c *Context) discoveredMissingDependencies(module *moduleInfo, depName stri func (c *Context) missingDependencyError(module *moduleInfo, depName string) (errs error) { err := c.nameInterface.MissingDependencyError(module.Name(), module.namespace(), depName) - return &BlueprintError{ Err: err, Pos: module.pos, diff --git a/context_test.go b/context_test.go index 2c0a6c0..2ea7801 100644 --- a/context_test.go +++ b/context_test.go @@ -1272,3 +1272,293 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) { }) } } + +func TestSourceRootDirAllowed(t *testing.T) { + type pathCase struct { + path string + decidingPrefix string + allowed bool + } + testcases := []struct { + desc string + rootDirs []string + pathCases []pathCase + }{ + { + desc: "simple case", + rootDirs: []string{ + "a", + "b/c/d", + "-c", + "-d/c/a", + "c/some_single_file", + }, + pathCases: []pathCase{ + { + path: "a", + decidingPrefix: "a", + allowed: true, + }, + { + path: "a/b/c", + decidingPrefix: "a", + allowed: true, + }, + { + path: "b", + decidingPrefix: "", + allowed: true, + }, + { + path: "b/c/d/a", + decidingPrefix: "b/c/d", + allowed: true, + }, + { + path: "c", + decidingPrefix: "c", + allowed: false, + }, + { + path: "c/a/b", + decidingPrefix: "c", + allowed: false, + }, + { + path: "c/some_single_file", + decidingPrefix: "c/some_single_file", + allowed: true, + }, + { + path: "d/c/a/abc", + decidingPrefix: "d/c/a", + allowed: false, + }, + }, + }, + { + desc: "root directory order matters", + rootDirs: []string{ + "-a", + "a/c/some_allowed_file", + "a/b/d/some_allowed_file", + "a/b", + "a/c", + "-a/b/d", + }, + pathCases: []pathCase{ + { + path: "a", + decidingPrefix: "a", + allowed: false, + }, + { + path: "a/some_disallowed_file", + decidingPrefix: "a", + allowed: false, + }, + { + path: "a/c/some_allowed_file", + decidingPrefix: "a/c/some_allowed_file", + allowed: true, + }, + { + path: "a/b/d/some_allowed_file", + decidingPrefix: "a/b/d/some_allowed_file", + allowed: true, + }, + { + path: "a/b/c", + decidingPrefix: "a/b", + allowed: true, + }, + { + path: "a/b/c/some_allowed_file", + decidingPrefix: "a/b", + allowed: true, + }, + { + path: "a/b/d", + decidingPrefix: "a/b/d", + allowed: false, + }, + }, + }, + } + for _, tc := range testcases { + dirs := SourceRootDirs{} + dirs.Add(tc.rootDirs...) + for _, pc := range tc.pathCases { + t.Run(fmt.Sprintf("%s: %s", tc.desc, pc.path), func(t *testing.T) { + allowed, decidingPrefix := dirs.SourceRootDirAllowed(pc.path) + if allowed != pc.allowed { + if pc.allowed { + t.Errorf("expected path %q to be allowed, but was not; root allowlist: %q", pc.path, tc.rootDirs) + } else { + t.Errorf("path %q was allowed unexpectedly; root allowlist: %q", pc.path, tc.rootDirs) + } + } + if decidingPrefix != pc.decidingPrefix { + t.Errorf("expected decidingPrefix to be %q, but got %q", pc.decidingPrefix, decidingPrefix) + } + }) + } + } +} + +func TestSourceRootDirs(t *testing.T) { + root_foo_bp := ` + foo_module { + name: "foo", + deps: ["foo_dir1", "foo_dir_ignored_special_case"], + } + ` + dir1_foo_bp := ` + foo_module { + name: "foo_dir1", + deps: ["foo_dir_ignored"], + } + ` + dir_ignored_foo_bp := ` + foo_module { + name: "foo_dir_ignored", + } + ` + dir_ignored_special_case_foo_bp := ` + foo_module { + name: "foo_dir_ignored_special_case", + } + ` + mockFs := map[string][]byte{ + "Android.bp": []byte(root_foo_bp), + "dir1/Android.bp": []byte(dir1_foo_bp), + "dir_ignored/Android.bp": []byte(dir_ignored_foo_bp), + "dir_ignored/special_case/Android.bp": []byte(dir_ignored_special_case_foo_bp), + } + fileList := []string{} + for f := range mockFs { + fileList = append(fileList, f) + } + testCases := []struct { + sourceRootDirs []string + expectedModuleDefs []string + unexpectedModuleDefs []string + expectedErrs []string + }{ + { + sourceRootDirs: []string{}, + expectedModuleDefs: []string{ + "foo", + "foo_dir1", + "foo_dir_ignored", + "foo_dir_ignored_special_case", + }, + }, + { + sourceRootDirs: []string{"-", ""}, + unexpectedModuleDefs: []string{ + "foo", + "foo_dir1", + "foo_dir_ignored", + "foo_dir_ignored_special_case", + }, + }, + { + sourceRootDirs: []string{"-"}, + unexpectedModuleDefs: []string{ + "foo", + "foo_dir1", + "foo_dir_ignored", + "foo_dir_ignored_special_case", + }, + }, + { + sourceRootDirs: []string{"dir1"}, + expectedModuleDefs: []string{ + "foo", + "foo_dir1", + "foo_dir_ignored", + "foo_dir_ignored_special_case", + }, + }, + { + sourceRootDirs: []string{"-dir1"}, + expectedModuleDefs: []string{ + "foo", + "foo_dir_ignored", + "foo_dir_ignored_special_case", + }, + unexpectedModuleDefs: []string{ + "foo_dir1", + }, + expectedErrs: []string{ + "Android.bp:2:2: \"foo\" depends on undefined module \"foo_dir1\"", + }, + }, + { + sourceRootDirs: []string{"-", "dir1"}, + expectedModuleDefs: []string{ + "foo_dir1", + }, + unexpectedModuleDefs: []string{ + "foo", + "foo_dir_ignored", + "foo_dir_ignored_special_case", + }, + expectedErrs: []string{ + "dir1/Android.bp:2:2: \"foo_dir1\" depends on undefined module \"foo_dir_ignored\"", + }, + }, + { + sourceRootDirs: []string{"-", "dir1", "dir_ignored/special_case/Android.bp"}, + expectedModuleDefs: []string{ + "foo_dir1", + "foo_dir_ignored_special_case", + }, + unexpectedModuleDefs: []string{ + "foo", + "foo_dir_ignored", + }, + expectedErrs: []string{ + "dir1/Android.bp:2:2: \"foo_dir1\" depends on undefined module \"foo_dir_ignored\"", + }, + }, + } + for _, tc := range testCases { + t.Run(fmt.Sprintf(`source root dirs are %q`, tc.sourceRootDirs), func(t *testing.T) { + ctx := NewContext() + ctx.MockFileSystem(mockFs) + ctx.RegisterModuleType("foo_module", newFooModule) + ctx.RegisterBottomUpMutator("deps", depsMutator) + ctx.AddSourceRootDirs(tc.sourceRootDirs...) + RegisterPackageIncludesModuleType(ctx) + ctx.ParseFileList(".", fileList, nil) + _, actualErrs := ctx.ResolveDependencies(nil) + + stringErrs := []string(nil) + for i := range actualErrs { + stringErrs = append(stringErrs, fmt.Sprint(actualErrs[i])) + } + if !reflect.DeepEqual(tc.expectedErrs, stringErrs) { + t.Errorf("expected to find errors %v; got %v", tc.expectedErrs, stringErrs) + } + for _, modName := range tc.expectedModuleDefs { + allMods := ctx.moduleGroupFromName(modName, nil) + if allMods == nil || len(allMods.modules) != 1 { + mods := modulesOrAliases{} + if allMods != nil { + mods = allMods.modules + } + t.Errorf("expected to find one definition for module %q, but got %v", modName, mods) + } + } + + for _, modName := range tc.unexpectedModuleDefs { + allMods := ctx.moduleGroupFromName(modName, nil) + if allMods != nil { + t.Errorf("expected to find no definitions for module %q, but got %v", modName, allMods.modules) + } + } + }) + } +} diff --git a/name_interface.go b/name_interface.go index 5e7e16e..c675ded 100644 --- a/name_interface.go +++ b/name_interface.go @@ -16,7 +16,9 @@ package blueprint import ( "fmt" + "os" "sort" + "strings" ) // This file exposes the logic of locating a module via a query string, to enable @@ -54,6 +56,9 @@ type NameInterface interface { // Gets called when a new module is created NewModule(ctx NamespaceContext, group ModuleGroup, module Module) (namespace Namespace, err []error) + // Gets called when a module was pruned from the build tree by SourceRootDirs + NewSkippedModule(ctx NamespaceContext, name string, skipInfo SkippedModuleInfo) + // Finds the module with the given name ModuleFromName(moduleName string, namespace Namespace) (group ModuleGroup, found bool) @@ -88,18 +93,29 @@ func newNamespaceContext(moduleInfo *moduleInfo) (ctx NamespaceContext) { return &namespaceContextImpl{moduleInfo.pos.Filename} } +func newNamespaceContextFromFilename(filename string) NamespaceContext { + return &namespaceContextImpl{filename} +} + func (ctx *namespaceContextImpl) ModulePath() string { return ctx.modulePath } +type SkippedModuleInfo struct { + filename string + reason string +} + // a SimpleNameInterface just stores all modules in a map based on name type SimpleNameInterface struct { - modules map[string]ModuleGroup + modules map[string]ModuleGroup + skippedModules map[string][]SkippedModuleInfo } func NewSimpleNameInterface() *SimpleNameInterface { return &SimpleNameInterface{ - modules: make(map[string]ModuleGroup), + modules: make(map[string]ModuleGroup), + skippedModules: make(map[string][]SkippedModuleInfo), } } @@ -118,8 +134,31 @@ func (s *SimpleNameInterface) NewModule(ctx NamespaceContext, group ModuleGroup, return nil, []error{} } +func (s *SimpleNameInterface) NewSkippedModule(ctx NamespaceContext, name string, info SkippedModuleInfo) { + if name == "" { + return + } + s.skippedModules[name] = append(s.skippedModules[name], info) +} + func (s *SimpleNameInterface) ModuleFromName(moduleName string, namespace Namespace) (group ModuleGroup, found bool) { group, found = s.modules[moduleName] + skipInfos, skipped := s.skippedModules[moduleName] + if skipped { + filesFound := make([]string, 0, len(skipInfos)) + reasons := make([]string, 0, len(skipInfos)) + for _, info := range skipInfos { + filesFound = append(filesFound, info.filename) + reasons = append(reasons, info.reason) + } + fmt.Fprintf( + os.Stderr, + "module %q was defined in files(s) [%v], but was skipped for reason(s) [%v]\n", + moduleName, + strings.Join(filesFound, ", "), + strings.Join(reasons, "; "), + ) + } return group, found } diff --git a/parser/ast.go b/parser/ast.go index fee2ec2..ea774e6 100644 --- a/parser/ast.go +++ b/parser/ast.go @@ -60,6 +60,8 @@ type Module struct { Type string TypePos scanner.Position Map + //TODO(delmerico) make this a private field once ag/21588220 lands + Name__internal_only *string } func (m *Module) Copy() *Module { @@ -86,6 +88,28 @@ func (m *Module) definitionTag() {} func (m *Module) Pos() scanner.Position { return m.TypePos } func (m *Module) End() scanner.Position { return m.Map.End() } +func (m *Module) Name() string { + if m.Name__internal_only != nil { + return *m.Name__internal_only + } + for _, prop := range m.Properties { + if prop.Name == "name" { + if stringProp, ok := prop.Value.(*String); ok { + name := stringProp.Value + m.Name__internal_only = &name + } else { + name := prop.Value.String() + m.Name__internal_only = &name + } + } + } + if m.Name__internal_only == nil { + name := "" + m.Name__internal_only = &name + } + return *m.Name__internal_only +} + // A Property is a name: value pair within a Map, which may be a top level Module. type Property struct { Name string