From 619be4626b42bf23b6861818e3fbdb4961d04250 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Fri, 28 Jan 2022 15:13:39 -0500 Subject: [PATCH] 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 --- android/config.go | 4 ++ android/paths.go | 114 ++++++++++++++++++++++++++++++-------------- android/variable.go | 7 +-- genrule/genrule.go | 5 +- 4 files changed, 91 insertions(+), 39 deletions(-) diff --git a/android/config.go b/android/config.go index 4a7e0d968..3c8224bf9 100644 --- a/android/config.go +++ b/android/config.go @@ -1651,6 +1651,10 @@ func (c *deviceConfig) BuildBrokenVendorPropertyNamespace() bool { return c.config.productVariables.BuildBrokenVendorPropertyNamespace } +func (c *deviceConfig) BuildBrokenInputDir(name string) bool { + return InList(name, c.config.productVariables.BuildBrokenInputDirModules) +} + func (c *deviceConfig) RequiresInsecureExecmemForSwiftshader() bool { return c.config.productVariables.RequiresInsecureExecmemForSwiftshader } diff --git a/android/paths.go b/android/paths.go index 4c69de706..05caebd18 100644 --- a/android/paths.go +++ b/android/paths.go @@ -405,6 +405,13 @@ func PathsForModuleSrc(ctx ModuleMissingDepsPathContext, paths []string) Paths { 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 // 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 @@ -423,12 +430,21 @@ func PathsForModuleSrc(ctx ModuleMissingDepsPathContext, paths []string) Paths { // missing dependencies // * otherwise, a ModuleError is thrown. func PathsForModuleSrcExcludes(ctx ModuleMissingDepsPathContext, paths, excludes []string) Paths { - ret, missingDeps := PathsAndMissingDepsForModuleSrcExcludes(ctx, paths, excludes) - if ctx.Config().AllowMissingDependencies() { - ctx.AddMissingDependencies(missingDeps) + return PathsRelativeToModuleSourceDir(SourceInput{ + Context: ctx, + 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 { 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 @@ -543,23 +559,31 @@ func GetModuleFromPathDep(ctx ModuleWithDepsPathContext, moduleName, tag string) // 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 // path_deps mutator. -func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleWithDepsPathContext, paths, excludes []string) (Paths, []string) { - prefix := pathForModuleSrc(ctx).String() +func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleMissingDepsPathContext, paths, excludes []string) (Paths, []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 - if excludes != nil { - expandedExcludes = make([]string, 0, len(excludes)) + if input.ExcludePaths != nil { + expandedExcludes = make([]string, 0, len(input.ExcludePaths)) } var missingExcludeDeps []string - - for _, e := range excludes { + for _, e := range input.ExcludePaths { 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 { missingExcludeDeps = append(missingExcludeDeps, m.missingDeps...) } else if err != nil { - reportPathError(ctx, err) + reportPathError(input.Context, err) } else { expandedExcludes = append(expandedExcludes, modulePaths.Strings()...) } @@ -568,19 +592,24 @@ func PathsAndMissingDepsForModuleSrcExcludes(ctx ModuleWithDepsPathContext, path } } - if paths == nil { + if input.Paths == nil { return nil, missingExcludeDeps } var missingDeps []string - expandedSrcFiles := make(Paths, 0, len(paths)) - for _, s := range paths { - srcFiles, err := expandOneSrcPath(ctx, s, expandedExcludes) + expandedSrcFiles := make(Paths, 0, len(input.Paths)) + for _, s := range input.Paths { + srcFiles, err := expandOneSrcPath(sourcePathInput{ + context: input.Context, + path: s, + expandedExcludes: expandedExcludes, + includeDirs: input.IncludeDirs, + }) if depErr, ok := err.(missingDependencyError); ok { missingDeps = append(missingDeps, depErr.missingDeps...) } else if err != nil { - reportPathError(ctx, err) + reportPathError(input.Context, err) } expandedSrcFiles = append(expandedSrcFiles, srcFiles...) } @@ -596,44 +625,59 @@ func (e missingDependencyError) Error() string { 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 // directory, excluding those listed in the expandedExcludes. // 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 { - if len(expandedExcludes) == 0 { + if len(input.expandedExcludes) == 0 { return paths } remainder := make(Paths, 0, len(paths)) for _, p := range paths { - if !InList(p.String(), expandedExcludes) { + if !InList(p.String(), input.expandedExcludes) { remainder = append(remainder, p) } } return remainder } - if m, t := SrcIsModuleWithTag(sPath); m != "" { - modulePaths, err := getPathsFromModuleDep(ctx, sPath, m, t) + if m, t := SrcIsModuleWithTag(input.path); m != "" { + modulePaths, err := getPathsFromModuleDep(input.context, input.path, m, t) if err != nil { return nil, err } else { return excludePaths(modulePaths), nil } - } else if pathtools.IsGlob(sPath) { - paths := GlobFiles(ctx, pathForModuleSrc(ctx, sPath).String(), expandedExcludes) - return PathsWithModuleSrcSubDir(ctx, paths, ""), nil } else { - p := pathForModuleSrc(ctx, sPath) - if exists, _, err := ctx.Config().fs.Exists(p.String()); err != nil { - ReportPathErrorf(ctx, "%s: %s", p, err.Error()) - } else if !exists && !ctx.Config().TestAllowNonExistentPaths { - ReportPathErrorf(ctx, "module source path %q does not exist", p) - } + p := pathForModuleSrc(input.context, input.path) + if pathtools.IsGlob(input.path) { + paths := GlobFiles(input.context, p.String(), input.expandedExcludes) + return PathsWithModuleSrcSubDir(input.context, paths, ""), nil + } 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) { - return nil, nil + if InList(p.String(), input.expandedExcludes) { + 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 // reference then it will be validated by expandOneSrcPath anyway when it calls expandOneSrcPath. 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 depErr, ok := err.(missingDependencyError); ok { if ctx.Config().AllowMissingDependencies() { diff --git a/android/variable.go b/android/variable.go index 627d9bdf2..68f19b993 100644 --- a/android/variable.go +++ b/android/variable.go @@ -421,9 +421,10 @@ type productVariables struct { ShippingApiLevel *string `json:",omitempty"` - BuildBrokenEnforceSyspropOwner bool `json:",omitempty"` - BuildBrokenTrebleSyspropNeverallow bool `json:",omitempty"` - BuildBrokenVendorPropertyNamespace bool `json:",omitempty"` + BuildBrokenEnforceSyspropOwner bool `json:",omitempty"` + BuildBrokenTrebleSyspropNeverallow bool `json:",omitempty"` + BuildBrokenVendorPropertyNamespace bool `json:",omitempty"` + BuildBrokenInputDirModules []string `json:",omitempty"` BuildDebugfsRestrictionsEnabled bool `json:",omitempty"` diff --git a/genrule/genrule.go b/genrule/genrule.go index 2ddd70ebd..c52ddee53 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -382,9 +382,12 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { addLocationLabel(toolFile, toolLocation{paths}) } + includeDirInPaths := ctx.DeviceConfig().BuildBrokenInputDir(g.Name()) var srcFiles android.Paths 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 !ctx.Config().AllowMissingDependencies() { panic(fmt.Errorf("should never get here, the missing dependencies %q should have been reported in DepsMutator",