From b6c90239d601e7903028e0561ba601bc55fcfcbe Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Fri, 23 Feb 2018 14:49:45 -0800 Subject: [PATCH] Append / to directories in Glob results This makes it easy for users of Glob to detect whether the match is a file or directory. Doing the check at this level means that the filelist file used as a dependency will be updated if a directory is replaced with a file of the same name, or vice versa. Change-Id: I79ebba39327218bcdcf50b393498306119de9d6c --- context.go | 12 ++++++++++++ module_ctx.go | 10 ++++++---- pathtools/glob.go | 33 ++++++++++++++++++++++----------- pathtools/glob_test.go | 32 ++++++++++++++++---------------- singleton_ctx.go | 10 ++++++---- 5 files changed, 62 insertions(+), 35 deletions(-) diff --git a/context.go b/context.go index 2051365..ebe4e12 100644 --- a/context.go +++ b/context.go @@ -1017,6 +1017,12 @@ func (c *Context) findBuildBlueprints(dir string, build []string, } for _, foundBlueprints := range matches { + if strings.HasSuffix(foundBlueprints, "/") { + errs = append(errs, &BlueprintError{ + Err: fmt.Errorf("%q: is a directory", foundBlueprints), + Pos: buildPos, + }) + } blueprints = append(blueprints, foundBlueprints) } } @@ -1053,6 +1059,12 @@ func (c *Context) findSubdirBlueprints(dir string, subdirs []string, subdirsPos } for _, subBlueprints := range matches { + if strings.HasSuffix(subBlueprints, "/") { + errs = append(errs, &BlueprintError{ + Err: fmt.Errorf("%q: is a directory", subBlueprints), + Pos: subdirsPos, + }) + } blueprints = append(blueprints, subBlueprints) } } diff --git a/module_ctx.go b/module_ctx.go index 5e0faa9..fe6c12b 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -131,10 +131,12 @@ type BaseModuleContext interface { PropertyErrorf(property, fmt string, args ...interface{}) Failed() bool - // GlobWithDeps returns a list of files that match the specified pattern but do not match any - // of the patterns in excludes. It also adds efficient dependencies to rerun the primary - // builder whenever a file matching the pattern as added or removed, without rerunning if a - // file that does not match the pattern is added to a searched directory. + // GlobWithDeps returns a list of files and directories that match the + // specified pattern but do not match any of the patterns in excludes. + // Any directories will have a '/' suffix. It also adds efficient + // dependencies to rerun the primary builder whenever a file matching + // the pattern as added or removed, without rerunning if a file that + // does not match the pattern is added to a searched directory. GlobWithDeps(pattern string, excludes []string) ([]string, error) Fs() pathtools.FileSystem diff --git a/pathtools/glob.go b/pathtools/glob.go index 367d219..f7b32d7 100644 --- a/pathtools/glob.go +++ b/pathtools/glob.go @@ -28,12 +28,13 @@ import ( var GlobMultipleRecursiveErr = errors.New("pattern contains multiple **") var GlobLastRecursiveErr = errors.New("pattern ** as last path element") -// Glob returns the list of files that match the given pattern but do not match -// the given exclude patterns, along with the list of directories and other -// dependencies that were searched to construct the file list. The supported -// glob and exclude patterns are equivalent to filepath.Glob, with an extension -// that recursive glob (** matching zero or more complete path entries) is -// supported. Glob also returns a list of directories that were searched. +// Glob returns the list of files and directories that match the given pattern +// but do not match the given exclude patterns, along with the list of +// directories and other dependencies that were searched to construct the file +// list. The supported glob and exclude patterns are equivalent to +// filepath.Glob, with an extension that recursive glob (** matching zero or +// more complete path entries) is supported. Any directories in the matches +// list will have a '/' suffix. // // In general ModuleContext.GlobWithDeps or SingletonContext.GlobWithDeps // should be used instead, as they will automatically set up dependencies @@ -71,6 +72,14 @@ func startGlob(fs FileSystem, pattern string, excludes []string) (matches, deps deps = append(deps, matches...) } + for i, match := range matches { + if isDir, err := fs.IsDir(match); err != nil { + return nil, nil, fmt.Errorf("IsDir(%s): %s", match, err.Error()) + } else if isDir { + matches[i] = match + "/" + } + } + return matches, deps, nil } @@ -325,12 +334,14 @@ func HasGlob(in []string) bool { return false } -// GlobWithDepFile finds all files that match glob. It compares the list of files -// against the contents of fileListFile, and rewrites fileListFile if it has changed. It also -// writes all of the the directories it traversed as a depenencies on fileListFile to depFile. +// GlobWithDepFile finds all files and directories that match glob. Directories +// will have a trailing '/'. It compares the list of matches against the +// contents of fileListFile, and rewrites fileListFile if it has changed. It +// also writes all of the the directories it traversed as dependencies on +// fileListFile to depFile. // -// The format of glob is either path/*.ext for a single directory glob, or path/**/*.ext -// for a recursive glob. +// The format of glob is either path/*.ext for a single directory glob, or +// path/**/*.ext for a recursive glob. // // Returns a list of file paths, and an error. // diff --git a/pathtools/glob_test.go b/pathtools/glob_test.go index 85030a6..1d17e10 100644 --- a/pathtools/glob_test.go +++ b/pathtools/glob_test.go @@ -36,7 +36,7 @@ var globTestCases = []globTestCase{ // Current directory tests { pattern: "*", - matches: []string{"a", "b", "c", "d.ext", "e.ext"}, + matches: []string{"a/", "b/", "c/", "d.ext", "e.ext"}, deps: []string{"."}, }, { @@ -46,7 +46,7 @@ var globTestCases = []globTestCase{ }, { pattern: "*/a", - matches: []string{"a/a", "b/a"}, + matches: []string{"a/a/", "b/a"}, deps: []string{".", "a", "b", "c"}, }, { @@ -63,7 +63,7 @@ var globTestCases = []globTestCase{ // ./ directory tests { pattern: "./*", - matches: []string{"a", "b", "c", "d.ext", "e.ext"}, + matches: []string{"a/", "b/", "c/", "d.ext", "e.ext"}, deps: []string{"."}, }, { @@ -73,12 +73,12 @@ var globTestCases = []globTestCase{ }, { pattern: "./*/a", - matches: []string{"a/a", "b/a"}, + matches: []string{"a/a/", "b/a"}, deps: []string{".", "a", "b", "c"}, }, { pattern: "./[ac]/a", - matches: []string{"a/a"}, + matches: []string{"a/a/"}, deps: []string{".", "a", "c"}, }, @@ -112,12 +112,12 @@ var globTestCases = []globTestCase{ // no-wild tests { pattern: "a", - matches: []string{"a"}, + matches: []string{"a/"}, deps: []string{"a"}, }, { pattern: "a/a", - matches: []string{"a/a"}, + matches: []string{"a/a/"}, deps: []string{"a/a"}, }, @@ -136,17 +136,17 @@ var globTestCases = []globTestCase{ // recursive tests { pattern: "**/a", - matches: []string{"a", "a/a", "a/a/a", "b/a"}, + matches: []string{"a/", "a/a/", "a/a/a", "b/a"}, deps: []string{".", "a", "a/a", "a/b", "b", "c", "c/f", "c/g", "c/h"}, }, { pattern: "a/**/a", - matches: []string{"a/a", "a/a/a"}, + matches: []string{"a/a/", "a/a/a"}, deps: []string{"a", "a/a", "a/b"}, }, { pattern: "a/**/*", - matches: []string{"a/a", "a/b", "a/a/a", "a/b/b"}, + matches: []string{"a/a/", "a/b/", "a/a/a", "a/b/b"}, deps: []string{"a", "a/a", "a/b"}, }, @@ -208,19 +208,19 @@ var globTestCases = []globTestCase{ { pattern: "*/*", excludes: []string{"a/b"}, - matches: []string{"a/a", "b/a", "c/c", "c/f", "c/g", "c/h"}, + matches: []string{"a/a/", "b/a", "c/c", "c/f/", "c/g/", "c/h/"}, deps: []string{".", "a", "b", "c"}, }, { pattern: "*/*", excludes: []string{"a/b", "c/c"}, - matches: []string{"a/a", "b/a", "c/f", "c/g", "c/h"}, + matches: []string{"a/a/", "b/a", "c/f/", "c/g/", "c/h/"}, deps: []string{".", "a", "b", "c"}, }, { pattern: "*/*", excludes: []string{"c/*", "*/a"}, - matches: []string{"a/b"}, + matches: []string{"a/b/"}, deps: []string{".", "a", "b", "c"}, }, { @@ -268,13 +268,13 @@ var globTestCases = []globTestCase{ { pattern: "*/*", excludes: []string{"**/b"}, - matches: []string{"a/a", "b/a", "c/c", "c/f", "c/g", "c/h"}, + matches: []string{"a/a/", "b/a", "c/c", "c/f/", "c/g/", "c/h/"}, deps: []string{".", "a", "b", "c"}, }, { pattern: "*/*", excludes: []string{"a/**/*"}, - matches: []string{"b/a", "c/c", "c/f", "c/g", "c/h"}, + matches: []string{"b/a", "c/c", "c/f/", "c/g/", "c/h/"}, deps: []string{".", "a", "b", "c"}, }, { @@ -439,7 +439,7 @@ var globTestCases = []globTestCase{ }, { pattern: ".t*", - matches: []string{".test", ".testing"}, + matches: []string{".test/", ".testing"}, deps: []string{"."}, }, } diff --git a/singleton_ctx.go b/singleton_ctx.go index 28fecf0..bbfce00 100644 --- a/singleton_ctx.go +++ b/singleton_ctx.go @@ -65,10 +65,12 @@ type SingletonContext interface { AddNinjaFileDeps(deps ...string) - // GlobWithDeps returns a list of files that match the specified pattern but do not match any - // of the patterns in excludes. It also adds efficient dependencies to rerun the primary - // builder whenever a file matching the pattern as added or removed, without rerunning if a - // file that does not match the pattern is added to a searched directory. + // GlobWithDeps returns a list of files and directories that match the + // specified pattern but do not match any of the patterns in excludes. + // Any directories will have a '/' suffix. It also adds efficient + // dependencies to rerun the primary builder whenever a file matching + // the pattern as added or removed, without rerunning if a file that + // does not match the pattern is added to a searched directory. GlobWithDeps(pattern string, excludes []string) ([]string, error) Fs() pathtools.FileSystem