Merge "Make Glob return a GlobResult"

bootstrap/glob.go

@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"fmt"
 	"path/filepath"
-	"strings"
 
 	"github.com/google/blueprint"
 	"github.com/google/blueprint/pathtools"
@@ -132,8 +131,7 @@ func (s *globSingleton) GenerateBuildActions(ctx blueprint.SingletonContext) {
 			// We don't need to write the depfile because we're guaranteed that ninja
 			// will run the command at least once (to record it into the ninja_log), so
 			// the depfile will be loaded from that execution.
-			fileList := strings.Join(g.Files, "\n") + "\n"
+			err := pathtools.WriteFileIfChanged(absolutePath(fileListFile), g.FileList(), 0666)
-			err := pathtools.WriteFileIfChanged(absolutePath(fileListFile), []byte(fileList), 0666)
 			if err != nil {
 				panic(fmt.Errorf("error writing %s: %s", fileListFile, err))
 			}

glob.go

@@ -24,11 +24,8 @@ import (
 )
 
 type GlobPath struct {
-	Pattern  string
+	pathtools.GlobResult
-	Excludes []string
+	Name string
-	Files    []string
-	Deps     []string
-	Name     string
 }
 
 func verifyGlob(fileName, pattern string, excludes []string, g GlobPath) {
@@ -58,11 +55,11 @@ func (c *Context) glob(pattern string, excludes []string) ([]string, error) {
 		// Glob has already been done, double check it is identical
 		verifyGlob(fileName, pattern, excludes, g)
 		// Return a copy so that modifications don't affect the cached value.
-		return append([]string(nil), g.Files...), nil
+		return append([]string(nil), g.Matches...), nil
 	}
 
 	// Get a globbed file list
-	files, deps, err := c.fs.Glob(pattern, excludes, pathtools.FollowSymlinks)
+	result, err := c.fs.Glob(pattern, excludes, pathtools.FollowSymlinks)
 	if err != nil {
 		return nil, err
 	}
@@ -70,19 +67,19 @@ func (c *Context) glob(pattern string, excludes []string) ([]string, error) {
 	// Store the results
 	c.globLock.Lock()
 	if g, exists = c.globs[fileName]; !exists {
-		c.globs[fileName] = GlobPath{pattern, excludes, files, deps, fileName}
+		c.globs[fileName] = GlobPath{result, fileName}
 	}
 	c.globLock.Unlock()
 
-	// Getting the list raced with another goroutine, throw away the results and use theirs
 	if exists {
+		// Getting the list raced with another goroutine, throw away the results and use theirs
 		verifyGlob(fileName, pattern, excludes, g)
 		// Return a copy so that modifications don't affect the cached value.
-		return append([]string(nil), g.Files...), nil
+		return append([]string(nil), g.Matches...), nil
 	}
 
 	// Return a copy so that modifications don't affect the cached value.
-	return append([]string(nil), files...), nil
+	return append([]string(nil), result.Matches...), nil
 }
 
 func (c *Context) Globs() []GlobPath {

pathtools/fs.go

@@ -95,7 +95,7 @@ type FileSystem interface {
 	// Exists returns whether the file exists and whether it is a directory.  Follows symlinks.
 	Exists(name string) (bool, bool, error)
 
-	Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error)
+	Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (GlobResult, error)
 	glob(pattern string) (matches []string, err error)
 
 	// IsDir returns true if the path points to a directory, false it it points to a file.  Follows symlinks.
@@ -194,7 +194,7 @@ func (fs *osFs) IsSymlink(name string) (bool, error) {
 	}
 }
 
-func (fs *osFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error) {
+func (fs *osFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (GlobResult, error) {
 	return startGlob(fs, pattern, excludes, follow)
 }
 
@@ -346,7 +346,7 @@ func (m *mockFs) IsSymlink(name string) (bool, error) {
 	return false, os.ErrNotExist
 }
 
-func (m *mockFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error) {
+func (m *mockFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (GlobResult, error) {
 	return startGlob(m, pattern, excludes, follow)
 }
 

pathtools/glob.go

@@ -29,6 +29,25 @@ var GlobMultipleRecursiveErr = errors.New("pattern contains multiple '**'")
 var GlobLastRecursiveErr = errors.New("pattern has '**' as last path element")
 var GlobInvalidRecursiveErr = errors.New("pattern contains other characters between '**' and path separator")
 
+// GlobResult is a container holding the results of a call to Glob.
+type GlobResult struct {
+	// Pattern is the pattern that was passed to Glob.
+	Pattern string
+	// Excludes is the list of excludes that were passed to Glob.
+	Excludes []string
+
+	// Matches is the list of files or directories that matched the pattern but not the excludes.
+	Matches []string
+
+	// Deps is the list of files or directories that must be depended on to regenerate the glob.
+	Deps []string
+}
+
+// FileList returns the list of files matched by a glob for writing to an output file.
+func (result GlobResult) FileList() []byte {
+	return []byte(strings.Join(result.Matches, "\n") + "\n")
+}
+
 // 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
@@ -40,26 +59,26 @@ var GlobInvalidRecursiveErr = errors.New("pattern contains other characters betw
 // In general ModuleContext.GlobWithDeps or SingletonContext.GlobWithDeps
 // should be used instead, as they will automatically set up dependencies
 // to rerun the primary builder when the list of matching files changes.
-func Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, deps []string, err error) {
+func Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (GlobResult, error) {
 	return startGlob(OsFs, pattern, excludes, follow)
 }
 
 func startGlob(fs FileSystem, pattern string, excludes []string,
-	follow ShouldFollowSymlinks) (matches, deps []string, err error) {
+	follow ShouldFollowSymlinks) (GlobResult, error) {
 
 	if filepath.Base(pattern) == "**" {
-		return nil, nil, GlobLastRecursiveErr
+		return GlobResult{}, GlobLastRecursiveErr
-	} else {
-		matches, deps, err = glob(fs, pattern, false, follow)
 	}
 
+	matches, deps, err := glob(fs, pattern, false, follow)
+
 	if err != nil {
-		return nil, nil, err
+		return GlobResult{}, err
 	}
 
 	matches, err = filterExcludes(matches, excludes)
 	if err != nil {
-		return nil, nil, err
+		return GlobResult{}, err
 	}
 
 	// If the pattern has wildcards, we added dependencies on the
@@ -83,7 +102,7 @@ func startGlob(fs FileSystem, pattern string, excludes []string,
 			info, err = fs.Stat(match)
 		}
 		if err != nil {
-			return nil, nil, err
+			return GlobResult{}, err
 		}
 
 		if info.IsDir() {
@@ -91,7 +110,12 @@ func startGlob(fs FileSystem, pattern string, excludes []string,
 		}
 	}
 
-	return matches, deps, nil
+	return GlobResult{
+		Pattern:  pattern,
+		Excludes: excludes,
+		Matches:  matches,
+		Deps:     deps,
+	}, nil
 }
 
 // glob is a recursive helper function to handle globbing each level of the pattern individually,
@@ -328,18 +352,16 @@ func HasGlob(in []string) bool {
 // In general ModuleContext.GlobWithDeps or SingletonContext.GlobWithDeps
 // should be used instead, as they will automatically set up dependencies
 // to rerun the primary builder when the list of matching files changes.
-func GlobWithDepFile(glob, fileListFile, depFile string, excludes []string) (files []string, err error) {
+func GlobWithDepFile(glob, fileListFile, depFile string, excludes []string) ([]string, error) {
-	files, deps, err := Glob(glob, excludes, FollowSymlinks)
+	result, err := Glob(glob, excludes, FollowSymlinks)
 	if err != nil {
 		return nil, err
 	}
 
-	fileList := strings.Join(files, "\n") + "\n"
+	WriteFileIfChanged(fileListFile, result.FileList(), 0666)
+	deptools.WriteDepFile(depFile, fileListFile, result.Deps)
 
-	WriteFileIfChanged(fileListFile, []byte(fileList), 0666)
+	return result.Matches, nil
-	deptools.WriteDepFile(depFile, fileListFile, deps)
-
-	return
 }
 
 // WriteFileIfChanged wraps ioutil.WriteFile, but only writes the file if

pathtools/glob_test.go

@@ -723,7 +723,7 @@ func TestGlobDontFollowDanglingSymlinks(t *testing.T) {
 
 func testGlob(t *testing.T, fs FileSystem, testCase globTestCase, follow ShouldFollowSymlinks) {
 	t.Helper()
-	matches, deps, err := fs.Glob(testCase.pattern, testCase.excludes, follow)
+	result, err := fs.Glob(testCase.pattern, testCase.excludes, follow)
 	if err != testCase.err {
 		if err == nil {
 			t.Fatalf("missing error: %s", testCase.err)
@@ -733,22 +733,22 @@ func testGlob(t *testing.T, fs FileSystem, testCase globTestCase, follow ShouldF
 		return
 	}
 
-	if !reflect.DeepEqual(matches, testCase.matches) {
+	if !reflect.DeepEqual(result.Matches, testCase.matches) {
 		t.Errorf("incorrect matches list:")
 		t.Errorf(" pattern: %q", testCase.pattern)
 		if testCase.excludes != nil {
 			t.Errorf("excludes: %q", testCase.excludes)
 		}
-		t.Errorf("     got: %#v", matches)
+		t.Errorf("     got: %#v", result.Matches)
 		t.Errorf("expected: %#v", testCase.matches)
 	}
-	if !reflect.DeepEqual(deps, testCase.deps) {
+	if !reflect.DeepEqual(result.Deps, testCase.deps) {
 		t.Errorf("incorrect deps list:")
 		t.Errorf(" pattern: %q", testCase.pattern)
 		if testCase.excludes != nil {
 			t.Errorf("excludes: %q", testCase.excludes)
 		}
-		t.Errorf("     got: %#v", deps)
+		t.Errorf("     got: %#v", result.Deps)
 		t.Errorf("expected: %#v", testCase.deps)
 	}
 }
@@ -904,14 +904,14 @@ func TestMatch(t *testing.T) {
 
 			mock := MockFs(mockFiles)
 
-			matches, _, err := mock.Glob(test.pattern, nil, DontFollowSymlinks)
+			result, err := mock.Glob(test.pattern, nil, DontFollowSymlinks)
-			t.Log(test.name, test.pattern, matches)
+			t.Log(test.name, test.pattern, result.Matches)
 			if err != nil {
 				t.Fatal(err)
 			}
 
 			match := false
-			for _, x := range matches {
+			for _, x := range result.Matches {
 				if x == test.name {
 					match = true
 				}