From 07e51619a2752d6a36bd149c8938b5d7cae5baa7 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 5 Mar 2019 12:46:40 -0800 Subject: [PATCH] Remove ModuleSrcPath ModuleSrcPath was designed as a type that ensured that modules only referenced sources inside the directory that contained the Android.bp file. In practice they don't work very well, because allowing filegroups and genrules as inputs to any module that takes a source path means that the path might end up being to a file in another source directory or to a generated file in the output directory. Remove ModuleSrcPath, replacing it with SourcePath in the places that need to explicitly refer to a path in the source tree, or Path where it may be a source path or a generated path. Make PathForModuleSrc return a Path instead of a SourcePath in preparation for consolidation with ctx.ExpandSources, which will make it possibly return paths to generated files. Test: All soong tests Change-Id: I973a78470ed14307eea5f6d0cc93942775a65715 --- android/config.go | 2 +- android/paths.go | 68 +++++++++++++++++++----------------------- android/paths_test.go | 26 +++++++++++----- cc/ndk_headers.go | 9 +++--- java/droiddoc.go | 18 +++++------ java/java_resources.go | 6 ++-- 6 files changed, 67 insertions(+), 62 deletions(-) diff --git a/android/config.go b/android/config.go index 92879de08..152073969 100644 --- a/android/config.go +++ b/android/config.go @@ -581,7 +581,7 @@ func (c *config) ApexKeyDir(ctx ModuleContext) SourcePath { if defaultCert == "" || filepath.Dir(defaultCert) == "build/target/product/security" { // When defaultCert is unset or is set to the testkeys path, use the APEX keys // that is under the module dir - return PathForModuleSrc(ctx).SourcePath + return pathForModuleSrc(ctx) } else { // If not, APEX keys are under the specified directory return PathForSource(ctx, filepath.Dir(defaultCert)) diff --git a/android/paths.go b/android/paths.go index a0ea7535b..a2908e1d0 100644 --- a/android/paths.go +++ b/android/paths.go @@ -254,10 +254,9 @@ func pathsForModuleSrcFromFullPath(ctx ModuleContext, paths []string, incDirs bo continue } - moduleSrcPath := ModuleSrcPath{srcPath} - moduleSrcPath.basePath.rel = srcPath.path + srcPath.basePath.rel = srcPath.path - ret = append(ret, moduleSrcPath) + ret = append(ret, srcPath) } return ret } @@ -633,9 +632,7 @@ func (p SourcePath) join(ctx PathContext, paths ...string) SourcePath { // SourcePath is the path to a resource overlay directory. func (p SourcePath) OverlayPath(ctx ModuleContext, path Path) OptionalPath { var relDir string - if moduleSrcPath, ok := path.(ModuleSrcPath); ok { - relDir = moduleSrcPath.path - } else if srcPath, ok := path.(SourcePath); ok { + if srcPath, ok := path.(SourcePath); ok { relDir = srcPath.path } else { reportPathErrorf(ctx, "Cannot find relative path for %s(%s)", reflect.TypeOf(path).Name(), path) @@ -747,35 +744,14 @@ func PathForIntermediates(ctx PathContext, paths ...string) OutputPath { return PathForOutput(ctx, ".intermediates", path) } -// ModuleSrcPath is a Path representing a file rooted from a module's local source dir -type ModuleSrcPath struct { - SourcePath -} +var _ genPathProvider = SourcePath{} +var _ objPathProvider = SourcePath{} +var _ resPathProvider = SourcePath{} -var _ Path = ModuleSrcPath{} -var _ genPathProvider = ModuleSrcPath{} -var _ objPathProvider = ModuleSrcPath{} -var _ resPathProvider = ModuleSrcPath{} - -// PathForModuleSrc returns a ModuleSrcPath representing the paths... under the +// PathForModuleSrc returns a Path representing the paths... under the // module's local source directory. -func PathForModuleSrc(ctx ModuleContext, paths ...string) ModuleSrcPath { - p, err := validatePath(paths...) - if err != nil { - reportPathError(ctx, err) - } - - srcPath, err := pathForSource(ctx, ctx.ModuleDir(), p) - if err != nil { - reportPathError(ctx, err) - } - - if pathtools.IsGlob(srcPath.String()) { - reportPathErrorf(ctx, "path may not contain a glob: %s", srcPath.String()) - } - - path := ModuleSrcPath{srcPath} - path.basePath.rel = p +func PathForModuleSrc(ctx ModuleContext, paths ...string) Path { + path := pathForModuleSrc(ctx, paths...) if exists, _, err := ctx.Fs().Exists(path.String()); err != nil { reportPathErrorf(ctx, "%s: %s", path, err.Error()) @@ -785,12 +761,28 @@ func PathForModuleSrc(ctx ModuleContext, paths ...string) ModuleSrcPath { return path } +func pathForModuleSrc(ctx ModuleContext, paths ...string) SourcePath { + p, err := validatePath(paths...) + if err != nil { + reportPathError(ctx, err) + } + + path, err := pathForSource(ctx, ctx.ModuleDir(), p) + if err != nil { + reportPathError(ctx, err) + } + + path.basePath.rel = p + + return path +} + // PathsWithModuleSrcSubDir takes a list of Paths and returns a new list of Paths where Rel() on each path // will return the path relative to subDir in the module's source directory. If any input paths are not located // inside subDir then a path error will be reported. func PathsWithModuleSrcSubDir(ctx ModuleContext, paths Paths, subDir string) Paths { paths = append(Paths(nil), paths...) - subDirFullPath := PathForModuleSrc(ctx, subDir) + subDirFullPath := pathForModuleSrc(ctx, subDir) for i, path := range paths { rel := Rel(ctx, subDirFullPath.String(), path.String()) paths[i] = subDirFullPath.join(ctx, rel) @@ -801,7 +793,7 @@ func PathsWithModuleSrcSubDir(ctx ModuleContext, paths Paths, subDir string) Pat // PathWithModuleSrcSubDir takes a Path and returns a Path where Rel() will return the path relative to subDir in the // module's source directory. If the input path is not located inside subDir then a path error will be reported. func PathWithModuleSrcSubDir(ctx ModuleContext, path Path, subDir string) Path { - subDirFullPath := PathForModuleSrc(ctx, subDir) + subDirFullPath := pathForModuleSrc(ctx, subDir) rel := Rel(ctx, subDirFullPath.String(), path.String()) return subDirFullPath.Join(ctx, rel) } @@ -815,15 +807,15 @@ func OptionalPathForModuleSrc(ctx ModuleContext, p *string) OptionalPath { return OptionalPathForPath(PathForModuleSrc(ctx, *p)) } -func (p ModuleSrcPath) genPathWithExt(ctx ModuleContext, subdir, ext string) ModuleGenPath { +func (p SourcePath) genPathWithExt(ctx ModuleContext, subdir, ext string) ModuleGenPath { return PathForModuleGen(ctx, subdir, pathtools.ReplaceExtension(p.path, ext)) } -func (p ModuleSrcPath) objPathWithExt(ctx ModuleContext, subdir, ext string) ModuleObjPath { +func (p SourcePath) objPathWithExt(ctx ModuleContext, subdir, ext string) ModuleObjPath { return PathForModuleObj(ctx, subdir, pathtools.ReplaceExtension(p.path, ext)) } -func (p ModuleSrcPath) resPathWithName(ctx ModuleContext, name string) ModuleResPath { +func (p SourcePath) resPathWithName(ctx ModuleContext, name string) ModuleResPath { // TODO: Use full directory if the new ctx is not the current ctx? return PathForModuleRes(ctx, p.path, name) } diff --git a/android/paths_test.go b/android/paths_test.go index cadf371bc..f5902b538 100644 --- a/android/paths_test.go +++ b/android/paths_test.go @@ -508,15 +508,27 @@ func TestPathForModuleInstall(t *testing.T) { } func TestDirectorySortedPaths(t *testing.T) { + config := TestConfig("out", nil) + + ctx := PathContextForTesting(config, map[string][]byte{ + "a.txt": nil, + "a/txt": nil, + "a/b/c": nil, + "a/b/d": nil, + "b": nil, + "b/b.txt": nil, + "a/a.txt": nil, + }) + makePaths := func() Paths { return Paths{ - PathForTesting("a.txt"), - PathForTesting("a/txt"), - PathForTesting("a/b/c"), - PathForTesting("a/b/d"), - PathForTesting("b"), - PathForTesting("b/b.txt"), - PathForTesting("a/a.txt"), + PathForSource(ctx, "a.txt"), + PathForSource(ctx, "a/txt"), + PathForSource(ctx, "a/b/c"), + PathForSource(ctx, "a/b/d"), + PathForSource(ctx, "b"), + PathForSource(ctx, "b/b.txt"), + PathForSource(ctx, "a/a.txt"), } } diff --git a/cc/ndk_headers.go b/cc/ndk_headers.go index 20241803f..9653a17dc 100644 --- a/cc/ndk_headers.go +++ b/cc/ndk_headers.go @@ -90,7 +90,7 @@ type headerModule struct { properties headerProperties installPaths android.Paths - licensePath android.ModuleSrcPath + licensePath android.Path } func getHeaderInstallDir(ctx android.ModuleContext, header android.Path, from string, @@ -204,7 +204,7 @@ type versionedHeaderModule struct { properties versionedHeaderProperties installPaths android.Paths - licensePath android.ModuleSrcPath + licensePath android.Path } func (m *versionedHeaderModule) GenerateAndroidBuildActions(ctx android.ModuleContext) { @@ -232,7 +232,8 @@ func (m *versionedHeaderModule) GenerateAndroidBuildActions(ctx android.ModuleCo processHeadersWithVersioner(ctx, fromSrcPath, toOutputPath, srcFiles, installPaths) } -func processHeadersWithVersioner(ctx android.ModuleContext, srcDir, outDir android.Path, srcFiles android.Paths, installPaths []android.WritablePath) android.Path { +func processHeadersWithVersioner(ctx android.ModuleContext, srcDir, outDir android.Path, + srcFiles android.Paths, installPaths []android.WritablePath) android.Path { // The versioner depends on a dependencies directory to simplify determining include paths // when parsing headers. This directory contains architecture specific directories as well // as a common directory, each of which contains symlinks to the actually directories to @@ -326,7 +327,7 @@ type preprocessedHeadersModule struct { properties preprocessedHeadersProperties installPaths android.Paths - licensePath android.ModuleSrcPath + licensePath android.Path } func (m *preprocessedHeadersModule) GenerateAndroidBuildActions(ctx android.ModuleContext) { diff --git a/java/droiddoc.go b/java/droiddoc.go index 777cd9c37..a85da0efd 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -935,15 +935,15 @@ func (d *Droiddoc) collectDoclavaDocsFlags(ctx android.ModuleContext, implicits }) if len(d.properties.Html_dirs) > 0 { - htmlDir := android.PathForModuleSrc(ctx, d.properties.Html_dirs[0]) - *implicits = append(*implicits, ctx.Glob(htmlDir.Join(ctx, "**/*").String(), nil)...) - args = args + " -htmldir " + htmlDir.String() + htmlDir := d.properties.Html_dirs[0] + *implicits = append(*implicits, ctx.ExpandSources([]string{filepath.Join(d.properties.Html_dirs[0], "**/*")}, nil)...) + args = args + " -htmldir " + htmlDir } if len(d.properties.Html_dirs) > 1 { - htmlDir2 := android.PathForModuleSrc(ctx, d.properties.Html_dirs[1]) - *implicits = append(*implicits, ctx.Glob(htmlDir2.Join(ctx, "**/*").String(), nil)...) - args = args + " -htmldir2 " + htmlDir2.String() + htmlDir2 := d.properties.Html_dirs[1] + *implicits = append(*implicits, ctx.ExpandSources([]string{filepath.Join(htmlDir2, "**/*")}, nil)...) + args = args + " -htmldir2 " + htmlDir2 } if len(d.properties.Html_dirs) > 2 { @@ -1791,9 +1791,9 @@ func ExportedDroiddocDirFactory() android.Module { func (d *ExportedDroiddocDir) DepsMutator(android.BottomUpMutatorContext) {} func (d *ExportedDroiddocDir) GenerateAndroidBuildActions(ctx android.ModuleContext) { - path := android.PathForModuleSrc(ctx, String(d.properties.Path)) - d.dir = path - d.deps = ctx.Glob(path.Join(ctx, "**/*").String(), nil) + path := String(d.properties.Path) + d.dir = android.PathForModuleSrc(ctx, path) + d.deps = ctx.ExpandSources([]string{filepath.Join(path, "**/*")}, nil) } // diff --git a/java/java_resources.go b/java/java_resources.go index 6c1fd39d0..462296302 100644 --- a/java/java_resources.go +++ b/java/java_resources.go @@ -39,10 +39,10 @@ func ResourceDirsToJarArgs(ctx android.ModuleContext, var excludeFiles []string for _, exclude := range excludeResourceDirs { - dirs := ctx.Glob(android.PathForModuleSrc(ctx).Join(ctx, exclude).String(), nil) + dirs := ctx.Glob(android.PathForSource(ctx, ctx.ModuleDir()).Join(ctx, exclude).String(), nil) for _, dir := range dirs { excludeDirs = append(excludeDirs, dir.String()) - excludeFiles = append(excludeFiles, dir.(android.ModuleSrcPath).Join(ctx, "**/*").String()) + excludeFiles = append(excludeFiles, dir.(android.SourcePath).Join(ctx, "**/*").String()) } } @@ -52,7 +52,7 @@ func ResourceDirsToJarArgs(ctx android.ModuleContext, for _, resourceDir := range resourceDirs { // resourceDir may be a glob, resolve it first - dirs := ctx.Glob(android.PathForModuleSrc(ctx).Join(ctx, resourceDir).String(), excludeDirs) + dirs := ctx.Glob(android.PathForSource(ctx, ctx.ModuleDir()).Join(ctx, resourceDir).String(), excludeDirs) for _, dir := range dirs { files := ctx.GlobFiles(filepath.Join(dir.String(), "**/*"), excludeFiles)