Merge "ignore bp files from PRODUCT_SOURCE_ROOT_DIRS"

This commit is contained in:
Treehugger Robot 2023-03-29 17:04:37 +00:00 committed by Gerrit Code Review
commit 82aa0ffb51
4 changed files with 460 additions and 10 deletions

View file

@ -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,
})
}
}
}
@ -3515,7 +3613,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,

View file

@ -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)
}
}
})
}
}

View file

@ -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
}

View file

@ -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