Restrict genrules to disallow directories as input

While Bazel genrules will allow genrules to accept a directory as input,
the results can be unexpected to a user as changes to the contents of
the directory may not trigger a rebuild as expected. Restricting this
in Soong ensures that converted targets will behave as expected.

Test: CI
Change-Id: I8616f58d1df267005e6c0ca3f4730d06de52c0d9
This commit is contained in:
Liz Kammer
2022-01-28 15:13:39 -05:00
parent 1f7ce200a7
commit 619be4626b
4 changed files with 91 additions and 39 deletions

View File

@@ -1651,6 +1651,10 @@ func (c *deviceConfig) BuildBrokenVendorPropertyNamespace() bool {
return c.config.productVariables.BuildBrokenVendorPropertyNamespace return c.config.productVariables.BuildBrokenVendorPropertyNamespace
} }
func (c *deviceConfig) BuildBrokenInputDir(name string) bool {
return InList(name, c.config.productVariables.BuildBrokenInputDirModules)
}
func (c *deviceConfig) RequiresInsecureExecmemForSwiftshader() bool { func (c *deviceConfig) RequiresInsecureExecmemForSwiftshader() bool {
return c.config.productVariables.RequiresInsecureExecmemForSwiftshader return c.config.productVariables.RequiresInsecureExecmemForSwiftshader
} }

View File

@@ -405,6 +405,13 @@ func PathsForModuleSrc(ctx ModuleMissingDepsPathContext, paths []string) Paths {
return PathsForModuleSrcExcludes(ctx, paths, nil) return PathsForModuleSrcExcludes(ctx, paths, nil)
} }
type SourceInput struct {
Context ModuleMissingDepsPathContext
Paths []string
ExcludePaths []string
IncludeDirs bool
}
// PathsForModuleSrcExcludes returns a Paths{} containing the resolved references in paths, minus // PathsForModuleSrcExcludes returns a Paths{} containing the resolved references in paths, minus
// those listed in excludes. Elements of paths and excludes are resolved as: // those listed in excludes. Elements of paths and excludes are resolved as:
// * filepath, relative to local module directory, resolves as a filepath relative to the local // * filepath, relative to local module directory, resolves as a filepath relative to the local
@@ -423,12 +430,21 @@ func PathsForModuleSrc(ctx ModuleMissingDepsPathContext, paths []string) Paths {
// missing dependencies // missing dependencies
// * otherwise, a ModuleError is thrown. // * otherwise, a ModuleError is thrown.
func PathsForModuleSrcExcludes(ctx ModuleMissingDepsPathContext, paths, excludes []string) Paths { func PathsForModuleSrcExcludes(ctx ModuleMissingDepsPathContext, paths, excludes []string) Paths {
ret, missingDeps := PathsAndMissingDepsForModuleSrcExcludes(ctx, paths, excludes) return PathsRelativeToModuleSourceDir(SourceInput{
if ctx.Config().AllowMissingDependencies() { Context: ctx,
ctx.AddMissingDependencies(missingDeps) Paths: paths,
ExcludePaths: excludes,
IncludeDirs: true,
})
}
func PathsRelativeToModuleSourceDir(input SourceInput) Paths {
ret, missingDeps := PathsAndMissingDepsRelativeToModuleSourceDir(input)
if input.Context.Config().AllowMissingDependencies() {
input.Context.AddMissingDependencies(missingDeps)
} else { } else {
for _, m := range missingDeps { for _, m := range missingDeps {
ctx.ModuleErrorf(`missing dependency on %q, is the property annotated with android:"path"?`, m) input.Context.ModuleErrorf(`missing dependency on %q, is the property annotated with android:"path"?`, m)
} }
} }
return ret return ret
@@ -543,23 +559,31 @@ func GetModuleFromPathDep(ctx ModuleWithDepsPathContext, moduleName, tag string)
// Properties passed as the paths argument must have been annotated with struct tag // Properties passed as the paths argument must have been annotated with struct tag
// `android:"path"` so that dependencies on SourceFileProducer modules will have already been handled by the // `android:"path"` so that dependencies on SourceFileProducer modules will have already been handled by the
// path_deps mutator. // path_deps mutator.
func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleWithDepsPathContext, paths, excludes []string) (Paths, []string) { func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleMissingDepsPathContext, paths, excludes []string) (Paths, []string) {
prefix := pathForModuleSrc(ctx).String() return PathsAndMissingDepsRelativeToModuleSourceDir(SourceInput{
Context: ctx,
Paths: paths,
ExcludePaths: excludes,
IncludeDirs: true,
})
}
func PathsAndMissingDepsRelativeToModuleSourceDir(input SourceInput) (Paths, []string) {
prefix := pathForModuleSrc(input.Context).String()
var expandedExcludes []string var expandedExcludes []string
if excludes != nil { if input.ExcludePaths != nil {
expandedExcludes = make([]string, 0, len(excludes)) expandedExcludes = make([]string, 0, len(input.ExcludePaths))
} }
var missingExcludeDeps []string var missingExcludeDeps []string
for _, e := range input.ExcludePaths {
for _, e := range excludes {
if m, t := SrcIsModuleWithTag(e); m != "" { if m, t := SrcIsModuleWithTag(e); m != "" {
modulePaths, err := getPathsFromModuleDep(ctx, e, m, t) modulePaths, err := getPathsFromModuleDep(input.Context, e, m, t)
if m, ok := err.(missingDependencyError); ok { if m, ok := err.(missingDependencyError); ok {
missingExcludeDeps = append(missingExcludeDeps, m.missingDeps...) missingExcludeDeps = append(missingExcludeDeps, m.missingDeps...)
} else if err != nil { } else if err != nil {
reportPathError(ctx, err) reportPathError(input.Context, err)
} else { } else {
expandedExcludes = append(expandedExcludes, modulePaths.Strings()...) expandedExcludes = append(expandedExcludes, modulePaths.Strings()...)
} }
@@ -568,19 +592,24 @@ func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleWithDepsPathContext, path
} }
} }
if paths == nil { if input.Paths == nil {
return nil, missingExcludeDeps return nil, missingExcludeDeps
} }
var missingDeps []string var missingDeps []string
expandedSrcFiles := make(Paths, 0, len(paths)) expandedSrcFiles := make(Paths, 0, len(input.Paths))
for _, s := range paths { for _, s := range input.Paths {
srcFiles, err := expandOneSrcPath(ctx, s, expandedExcludes) srcFiles, err := expandOneSrcPath(sourcePathInput{
context: input.Context,
path: s,
expandedExcludes: expandedExcludes,
includeDirs: input.IncludeDirs,
})
if depErr, ok := err.(missingDependencyError); ok { if depErr, ok := err.(missingDependencyError); ok {
missingDeps = append(missingDeps, depErr.missingDeps...) missingDeps = append(missingDeps, depErr.missingDeps...)
} else if err != nil { } else if err != nil {
reportPathError(ctx, err) reportPathError(input.Context, err)
} }
expandedSrcFiles = append(expandedSrcFiles, srcFiles...) expandedSrcFiles = append(expandedSrcFiles, srcFiles...)
} }
@@ -596,44 +625,59 @@ func (e missingDependencyError) Error() string {
return "missing dependencies: " + strings.Join(e.missingDeps, ", ") return "missing dependencies: " + strings.Join(e.missingDeps, ", ")
} }
type sourcePathInput struct {
context ModuleWithDepsPathContext
path string
expandedExcludes []string
includeDirs bool
}
// Expands one path string to Paths rooted from the module's local source // Expands one path string to Paths rooted from the module's local source
// directory, excluding those listed in the expandedExcludes. // directory, excluding those listed in the expandedExcludes.
// Expands globs, references to SourceFileProducer or OutputFileProducer modules using the ":name" and ":name{.tag}" syntax. // Expands globs, references to SourceFileProducer or OutputFileProducer modules using the ":name" and ":name{.tag}" syntax.
func expandOneSrcPath(ctx ModuleWithDepsPathContext, sPath string, expandedExcludes []string) (Paths, error) { func expandOneSrcPath(input sourcePathInput) (Paths, error) {
excludePaths := func(paths Paths) Paths { excludePaths := func(paths Paths) Paths {
if len(expandedExcludes) == 0 { if len(input.expandedExcludes) == 0 {
return paths return paths
} }
remainder := make(Paths, 0, len(paths)) remainder := make(Paths, 0, len(paths))
for _, p := range paths { for _, p := range paths {
if !InList(p.String(), expandedExcludes) { if !InList(p.String(), input.expandedExcludes) {
remainder = append(remainder, p) remainder = append(remainder, p)
} }
} }
return remainder return remainder
} }
if m, t := SrcIsModuleWithTag(sPath); m != "" { if m, t := SrcIsModuleWithTag(input.path); m != "" {
modulePaths, err := getPathsFromModuleDep(ctx, sPath, m, t) modulePaths, err := getPathsFromModuleDep(input.context, input.path, m, t)
if err != nil { if err != nil {
return nil, err return nil, err
} else { } else {
return excludePaths(modulePaths), nil return excludePaths(modulePaths), nil
} }
} else if pathtools.IsGlob(sPath) {
paths := GlobFiles(ctx, pathForModuleSrc(ctx, sPath).String(), expandedExcludes)
return PathsWithModuleSrcSubDir(ctx, paths, ""), nil
} else { } else {
p := pathForModuleSrc(ctx, sPath) p := pathForModuleSrc(input.context, input.path)
if exists, _, err := ctx.Config().fs.Exists(p.String()); err != nil { if pathtools.IsGlob(input.path) {
ReportPathErrorf(ctx, "%s: %s", p, err.Error()) paths := GlobFiles(input.context, p.String(), input.expandedExcludes)
} else if !exists && !ctx.Config().TestAllowNonExistentPaths { return PathsWithModuleSrcSubDir(input.context, paths, ""), nil
ReportPathErrorf(ctx, "module source path %q does not exist", p) } else {
} if exists, _, err := input.context.Config().fs.Exists(p.String()); err != nil {
ReportPathErrorf(input.context, "%s: %s", p, err.Error())
} else if !exists && !input.context.Config().TestAllowNonExistentPaths {
ReportPathErrorf(input.context, "module source path %q does not exist", p)
} else if !input.includeDirs {
if isDir, err := input.context.Config().fs.IsDir(p.String()); exists && err != nil {
ReportPathErrorf(input.context, "%s: %s", p, err.Error())
} else if isDir {
ReportPathErrorf(input.context, "module source path %q is a directory", p)
}
}
if InList(p.String(), expandedExcludes) { if InList(p.String(), input.expandedExcludes) {
return nil, nil return nil, nil
}
return Paths{p}, nil
} }
return Paths{p}, nil
} }
} }
@@ -1315,7 +1359,7 @@ func PathForModuleSrc(ctx ModuleMissingDepsPathContext, pathComponents ...string
// validatePath() will corrupt it, e.g. replace "//" with "/". If the path is not a module // validatePath() will corrupt it, e.g. replace "//" with "/". If the path is not a module
// reference then it will be validated by expandOneSrcPath anyway when it calls expandOneSrcPath. // reference then it will be validated by expandOneSrcPath anyway when it calls expandOneSrcPath.
p := strings.Join(pathComponents, string(filepath.Separator)) p := strings.Join(pathComponents, string(filepath.Separator))
paths, err := expandOneSrcPath(ctx, p, nil) paths, err := expandOneSrcPath(sourcePathInput{context: ctx, path: p, includeDirs: true})
if err != nil { if err != nil {
if depErr, ok := err.(missingDependencyError); ok { if depErr, ok := err.(missingDependencyError); ok {
if ctx.Config().AllowMissingDependencies() { if ctx.Config().AllowMissingDependencies() {

View File

@@ -421,9 +421,10 @@ type productVariables struct {
ShippingApiLevel *string `json:",omitempty"` ShippingApiLevel *string `json:",omitempty"`
BuildBrokenEnforceSyspropOwner bool `json:",omitempty"` BuildBrokenEnforceSyspropOwner bool `json:",omitempty"`
BuildBrokenTrebleSyspropNeverallow bool `json:",omitempty"` BuildBrokenTrebleSyspropNeverallow bool `json:",omitempty"`
BuildBrokenVendorPropertyNamespace bool `json:",omitempty"` BuildBrokenVendorPropertyNamespace bool `json:",omitempty"`
BuildBrokenInputDirModules []string `json:",omitempty"`
BuildDebugfsRestrictionsEnabled bool `json:",omitempty"` BuildDebugfsRestrictionsEnabled bool `json:",omitempty"`

View File

@@ -382,9 +382,12 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
addLocationLabel(toolFile, toolLocation{paths}) addLocationLabel(toolFile, toolLocation{paths})
} }
includeDirInPaths := ctx.DeviceConfig().BuildBrokenInputDir(g.Name())
var srcFiles android.Paths var srcFiles android.Paths
for _, in := range g.properties.Srcs { for _, in := range g.properties.Srcs {
paths, missingDeps := android.PathsAndMissingDepsForModuleSrcExcludes(ctx, []string{in}, g.properties.Exclude_srcs) paths, missingDeps := android.PathsAndMissingDepsRelativeToModuleSourceDir(android.SourceInput{
Context: ctx, Paths: []string{in}, ExcludePaths: g.properties.Exclude_srcs, IncludeDirs: includeDirInPaths,
})
if len(missingDeps) > 0 { if len(missingDeps) > 0 {
if !ctx.Config().AllowMissingDependencies() { if !ctx.Config().AllowMissingDependencies() {
panic(fmt.Errorf("should never get here, the missing dependencies %q should have been reported in DepsMutator", panic(fmt.Errorf("should never get here, the missing dependencies %q should have been reported in DepsMutator",