Make Glob return a GlobResult

Replace the individual matches and deps return values with a GlobResult
struct.  Use the GlobResult to create the file list file in GlobWithDepFile
(used by bpglob) and in the glob singleton, as the generated files must
match exactly so that soong_build is not rerun.

Bug: 159845846
Test: glob_test.go
Change-Id: I2159cc9d85f388073198eac7456e5bf43e813096

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,10 +24,7 @@ import (
 )
 
 type GlobPath struct {
-	Pattern  string
+	pathtools.GlobResult
-	Excludes []string
-	Files    []string
-	Deps     []string
 	Name string
 }
 
@@ -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
 				}