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

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

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
+	// GlobWithDeps returns a list of files and directories that match the
-	// of the patterns in excludes.  It also adds efficient dependencies to rerun the primary
+	// specified pattern but do not match any of the patterns in excludes.
-	// builder whenever a file matching the pattern as added or removed, without rerunning if a
+	// Any directories will have a '/' suffix.  It also adds efficient
-	// file that does not match the pattern is added to a searched directory.
+	// 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

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
+// Glob returns the list of files and directories that match the given pattern
-// the given exclude patterns, along with the list of directories and other
+// but do not match the given exclude patterns, along with the list of
-// dependencies that were searched to construct the file list.  The supported
+// directories and other dependencies that were searched to construct the file
-// glob and exclude patterns are equivalent to filepath.Glob, with an extension
+// list.  The supported glob and exclude patterns are equivalent to
-// that recursive glob (** matching zero or more complete path entries) is
+// filepath.Glob, with an extension that recursive glob (** matching zero or
-// supported.  Glob also returns a list of directories that were searched.
+// 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
+// GlobWithDepFile finds all files and directories that match glob.  Directories
-// against the contents of fileListFile, and rewrites fileListFile if it has changed.  It also
+// will have a trailing '/'.  It compares the list of matches against the
-// writes all of the the directories it traversed as a depenencies on fileListFile to depFile.
+// 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
+// The format of glob is either path/*.ext for a single directory glob, or
-// for a recursive glob.
+// path/**/*.ext for a recursive glob.
 //
 // Returns a list of file paths, and an error.
 //

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{"."},
 	},
 }

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
+	// GlobWithDeps returns a list of files and directories that match the
-	// of the patterns in excludes.  It also adds efficient dependencies to rerun the primary
+	// specified pattern but do not match any of the patterns in excludes.
-	// builder whenever a file matching the pattern as added or removed, without rerunning if a
+	// Any directories will have a '/' suffix. It also adds efficient
-	// file that does not match the pattern is added to a searched directory.
+	// 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