From 740d487cf591c3342a75214ff1083951a4dc449b Mon Sep 17 00:00:00 2001
From: Nick Santos <nick.santos@docker.com>
Date: Tue, 25 Jun 2024 20:41:36 -0600
Subject: [PATCH] filter: fix a bug with parent dirs

if the Map() function excludes a directory,
but includes a file inside that directory,
then the walker must backtrack and include
the directory.

otherwise, buildkit gets confused and
sends "changes out of order" errors.

part of fixing
https://github.com/tilt-dev/tilt/issues/6393

Signed-off-by: Nick Santos <nick.santos@docker.com>
---
 filter.go      | 18 +++++-------------
 filter_test.go | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/filter.go b/filter.go
index 9cee8ad2..3b7f3150 100644
--- a/filter.go
+++ b/filter.go
@@ -197,7 +197,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
 			isDir = dirEntry.IsDir()
 		}
 
-		if fs.includeMatcher != nil || fs.excludeMatcher != nil {
+		if fs.includeMatcher != nil || fs.excludeMatcher != nil || fs.mapFn != nil {
 			for len(parentDirs) != 0 {
 				lastParentDir := parentDirs[len(parentDirs)-1].pathWithSep
 				if strings.HasPrefix(path, lastParentDir) {
@@ -298,7 +298,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
 			return walkErr
 		}
 
-		if fs.includeMatcher != nil || fs.excludeMatcher != nil {
+		if fs.includeMatcher != nil || fs.excludeMatcher != nil || fs.mapFn != nil {
 			defer func() {
 				if isDir {
 					parentDirs = append(parentDirs, dir)
@@ -310,8 +310,6 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
 			return nil
 		}
 
-		dir.calledFn = true
-
 		fi, err := dirEntry.Info()
 		if err != nil {
 			return err
@@ -333,6 +331,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
 					return nil
 				}
 			}
+
 			for i, parentDir := range parentDirs {
 				if parentDir.skipFn {
 					return filepath.SkipDir
@@ -354,15 +353,6 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
 					return ctx.Err()
 				default:
 				}
-				if fs.mapFn != nil {
-					result := fs.mapFn(parentStat.Path, parentStat)
-					if result == MapResultExclude {
-						continue
-					} else if result == MapResultSkipDir {
-						parentDirs[i].skipFn = true
-						return filepath.SkipDir
-					}
-				}
 
 				parentDirs[i].calledFn = true
 				if err := fn(parentStat.Path, &DirEntryInfo{Stat: parentStat}, nil); err == filepath.SkipDir {
@@ -372,6 +362,8 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
 					return err
 				}
 			}
+
+			dir.calledFn = true
 			if err := fn(stat.Path, &DirEntryInfo{Stat: stat}, nil); err != nil {
 				return err
 			}
diff --git a/filter_test.go b/filter_test.go
index 406425da..22299976 100644
--- a/filter_test.go
+++ b/filter_test.go
@@ -342,6 +342,7 @@ func TestWalkerMapSkipDirWithPattern(t *testing.T) {
 	assert.NoError(t, err)
 	defer os.RemoveAll(d)
 
+	// The include pattern takes precence over the map function.
 	b := &bytes.Buffer{}
 	err = Walk(context.Background(), d, &FilterOpt{
 		IncludePatterns: []string{"**/*.txt"},
@@ -354,11 +355,42 @@ func TestWalkerMapSkipDirWithPattern(t *testing.T) {
 	}, bufWalk(b))
 	assert.NoError(t, err)
 
-	assert.Equal(t, filepath.FromSlash(`dir y
+	assert.Equal(t, filepath.FromSlash(`dir x
+file x/a.txt
+dir y
 file y/b.txt
 `), b.String())
 }
 
+func TestWalkerMapPatternImpliesDir(t *testing.T) {
+	d, err := tmpDir(changeStream([]string{
+		"ADD x dir",
+		"ADD x/y dir",
+		"ADD x/y/a.txt file",
+		"ADD x/z dir",
+		"ADD x/z/b.txt file",
+	}))
+	assert.NoError(t, err)
+	defer os.RemoveAll(d)
+
+	b := &bytes.Buffer{}
+	err = Walk(context.Background(), d, &FilterOpt{
+		Map: func(_ string, s *types.Stat) MapResult {
+			if s.Path == "x/z/b.txt" {
+				return MapResultKeep
+			}
+
+			return MapResultExclude
+		},
+	}, bufWalk(b))
+	assert.NoError(t, err)
+
+	assert.Equal(t, filepath.FromSlash(`dir x
+dir x/z
+file x/z/b.txt
+`), b.String())
+}
+
 func TestWalkerPermissionDenied(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("os.Chmod not fully supported on Windows")