Revendor updated fsutils fork; use same logic for IncludePatterns and…

… ExcludePatterns

Signed-off-by: Aaron Lehmann <[email protected]>

cache/contenthash/checksum.go

@@ -23,7 +23,6 @@ import (
 	digest "github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 	"github.com/tonistiigi/fsutil"
-	"github.com/tonistiigi/fsutil/prefix"
 	fstypes "github.com/tonistiigi/fsutil/types"
 )
 
@@ -480,9 +479,16 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
 	endsInSep := len(p) != 0 && p[len(p)-1] == filepath.Separator
 	p = keyPath(p)
 
-	rootedIncludePatterns := make([]string, len(opts.IncludePatterns))
-	for i, includePattern := range opts.IncludePatterns {
-		rootedIncludePatterns[i] = keyPath(includePattern)
+	var includePatternMatcher *fileutils.PatternMatcher
+	if len(opts.IncludePatterns) != 0 {
+		rootedIncludePatterns := make([]string, len(opts.IncludePatterns))
+		for i, includePattern := range opts.IncludePatterns {
+			rootedIncludePatterns[i] = keyPath(includePattern)
+		}
+		includePatternMatcher, err = fileutils.NewPatternMatcher(rootedIncludePatterns)
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid includepatterns: %s", opts.IncludePatterns)
+		}
 	}
 
 	var excludePatternMatcher *fileutils.PatternMatcher
@@ -511,18 +517,29 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
 	if opts.Wildcard {
 		iter = root.Seek([]byte{})
 		k, _, kOk = iter.Next()
-
 	} else {
 		k = convertPathToKey([]byte(p))
 		if _, kOk = root.Get(k); kOk {
 			iter = root.Seek(k)
 		}
 	}
 
-	lastIncludedDir := ""
-treeWalk:
+	var (
+		parentDirHeaders []*IncludedPath
+		lastMatchedDir   string
+	)
+
 	for kOk {
 		fn := string(convertKeyToPath(k))
+
+		for len(parentDirHeaders) != 0 {
+			lastParentDir := parentDirHeaders[len(parentDirHeaders)-1]
+			if strings.HasPrefix(fn, lastParentDir.Path+"/") {
+				break
+			}
+			parentDirHeaders = parentDirHeaders[:len(parentDirHeaders)-1]
+		}
+
 		dirHeader := false
 		if len(k) > 0 && k[len(k)-1] == byte(0) {
 			dirHeader = true
@@ -533,14 +550,28 @@ treeWalk:
 				continue
 			}
 		}
-		b, partialMatch, err := shouldIncludePath(p, fn, opts.Wildcard, rootedIncludePatterns, excludePatternMatcher, lastIncludedDir)
+		if opts.Wildcard {
+			if lastMatchedDir == "" || !strings.HasPrefix(fn, lastMatchedDir+"/") {
+				include, err := path.Match(p, fn)
+				if err != nil {
+					return nil, err
+				}
+				if !include {
+					k, _, kOk = iter.Next()
+					continue
+				}
+				lastMatchedDir = fn
+			}
+		} else if !strings.HasPrefix(fn+"/", p+"/") {
+			k, _, kOk = iter.Next()
+			continue
+		}
+
+		shouldInclude, err := shouldIncludePath(p, fn, includePatternMatcher, excludePatternMatcher)
 		if err != nil {
 			return nil, err
 		}
-		// Dir headers for parent dirs of an include pattern should be included
-		// in the digest because their metadata may be copied by a copy that
-		// includes files or subdirs underneath them.
-		if !b || dirHeader != partialMatch {
+		if !shouldInclude && !dirHeader {
 			k, _, kOk = iter.Next()
 			continue
 		}
@@ -554,27 +585,22 @@ treeWalk:
 		}
 
 		if cr.Type == CacheRecordTypeDir {
-			lastIncludedDir = fn
-
-			if excludePatternMatcher != nil {
-				dirSlash := fn + "/"
-				for _, pat := range excludePatternMatcher.Patterns() {
-					patStr := pat.String() + "/"
-					if strings.HasPrefix(patStr, dirSlash) {
-						// This dir has exclusions underneath it. Do not
-						// include the dir as a whole. Instead, continue
-						// walking and only include paths that match the
-						// filters.
-						k, _, kOk = iter.Next()
-						continue treeWalk
-					}
-				}
-			}
+			// We only hash dir headers and files, not dir contents. Hashing
+			// dir contents could be wrong if there are exclusions within the
+			// dir.
+			shouldInclude = false
 		}
 
-		includedPaths = append(includedPaths, &IncludedPath{Path: fn, Record: cr})
-		if cr.Type == CacheRecordTypeDir {
-			iter = root.Seek(append(k, 0, 0xff))
+		if !shouldInclude {
+			if cr.Type == CacheRecordTypeDirHeader {
+				// We keep track of non-included parent dir headers in case an
+				// include pattern matches a file inside one of these dirs.
+				parentDirHeaders = append(parentDirHeaders, &IncludedPath{Path: fn, Record: cr})
+			}
+		} else {
+			includedPaths = append(includedPaths, parentDirHeaders...)
+			parentDirHeaders = nil
+			includedPaths = append(includedPaths, &IncludedPath{Path: fn, Record: cr})
 		}
 		k, _, kOk = iter.Next()
 	}
@@ -588,51 +614,30 @@ treeWalk:
 func shouldIncludePath(
 	p string,
 	candidate string,
-	wildcard bool,
-	rootedIncludePatterns []string,
+	includePatternMatcher *fileutils.PatternMatcher,
 	excludePatternMatcher *fileutils.PatternMatcher,
-	lastIncludedDir string,
-) (bool, bool, error) {
-	if wildcard {
-		include, err := path.Match(p, candidate)
+) (bool, error) {
+	if includePatternMatcher != nil {
+		m, err := includePatternMatcher.Matches(filepath.FromSlash(candidate))
 		if err != nil {
-			return include, false, err
-		}
-		if !include {
-			return false, false, nil
-		}
-	} else if !strings.HasPrefix(candidate+"/", p+"/") {
-		return false, false, nil
-	}
-
-	partial := false
-	if len(rootedIncludePatterns) != 0 &&
-		(lastIncludedDir == "" ||
-			!strings.HasPrefix(candidate, lastIncludedDir+"/")) {
-		partial = true
-		matched := false
-		for _, pattern := range rootedIncludePatterns {
-			if ok, partialMatch := prefix.Match(pattern, candidate, true); ok {
-				matched = true
-				if !partialMatch {
-					partial = false
-					break
-				}
-			}
+			return false, errors.Wrap(err, "failed to match includepatterns")
 		}
-		if !matched {
-			return false, false, nil
+		if !m {
+			return false, nil
 		}
 	}
 
-	if excludePatternMatcher == nil {
-		return true, partial, nil
-	}
-	m, err := excludePatternMatcher.Matches(candidate)
-	if err != nil {
-		return false, partial, errors.Wrap(err, "failed to match excludepatterns")
+	if excludePatternMatcher != nil {
+		m, err := excludePatternMatcher.Matches(filepath.FromSlash(candidate))
+		if err != nil {
+			return false, errors.Wrap(err, "failed to match excludepatterns")
+		}
+		if m {
+			return false, nil
+		}
 	}
-	return !m, partial, nil
+
+	return true, nil
 }
 
 func (cc *cacheContext) checksumNoFollow(ctx context.Context, m *mount, p string) (*CacheRecord, error) {

cache/contenthash/checksum_test.go

@@ -31,9 +31,10 @@ import (
 )
 
 const (
-	dgstFileData0     = digest.Digest("sha256:cd8e75bca50f2d695f220d0cb0997d8ead387e4f926e8669a92d7f104cc9885b")
-	dgstDirD0         = digest.Digest("sha256:d47454417d2c554067fbefe5f5719edc49f3cfe969c36b62e34a187a4da0cc9a")
-	dgstDirD0Modified = digest.Digest("sha256:555ffa3028630d97ba37832b749eda85ab676fd64ffb629fbf0f4ec8c1e3bff1")
+	dgstFileData0       = digest.Digest("sha256:cd8e75bca50f2d695f220d0cb0997d8ead387e4f926e8669a92d7f104cc9885b")
+	dgstDirD0           = digest.Digest("sha256:d47454417d2c554067fbefe5f5719edc49f3cfe969c36b62e34a187a4da0cc9a")
+	dgstDirD0FileByFile = digest.Digest("sha256:231c3293e329de47fec9e79056686477891fd1f244ed7b1c1fa668489a1f0d50")
+	dgstDirD0Modified   = digest.Digest("sha256:555ffa3028630d97ba37832b749eda85ab676fd64ffb629fbf0f4ec8c1e3bff1")
 )
 
 func TestChecksumSymlinkNoParentScan(t *testing.T) {
@@ -189,7 +190,7 @@ func TestChecksumWildcardOrFilter(t *testing.T) {
 
 	dgst, err = cc.Checksum(context.TODO(), ref, "x/d?", ChecksumOpts{Wildcard: true}, nil)
 	require.NoError(t, err)
-	require.Equal(t, digest.FromBytes(append([]byte("d0"), []byte(dgstDirD0)...)), dgst)
+	require.Equal(t, dgstDirD0FileByFile, dgst)
 
 	dgst, err = cc.Checksum(context.TODO(), ref, "x/d?/def", ChecksumOpts{FollowLinks: true, Wildcard: true}, nil)
 	require.NoError(t, err)
@@ -486,7 +487,8 @@ func TestChecksumIncludeExclude(t *testing.T) {
 	dgstD1Star, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d1/*"}}, nil)
 	require.NoError(t, err)
 
-	// Nothing matches pattern, but d2's metadata should be captured in the checksum
+	// Nothing matches pattern, but d2's metadata should be captured in the
+	// checksum if d2 exists
 	dgstD2Foo, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d2/foo"}}, nil)
 	require.NoError(t, err)
 
@@ -546,7 +548,7 @@ func TestChecksumIncludeExclude(t *testing.T) {
 
 	dgstD2Foo2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d2/foo"}}, nil)
 	require.NoError(t, err)
-	require.NotEqual(t, dgstD2Foo, dgstD2Foo2)
+	require.Equal(t, dgstD2Foo, dgstD2Foo2)
 
 	err = ref.Release(context.TODO())
 	require.NoError(t, err)

go.mod

@@ -74,6 +74,8 @@ replace (
 	github.com/golang/protobuf => github.com/golang/protobuf v1.3.5
 	github.com/hashicorp/go-immutable-radix => github.com/tonistiigi/go-immutable-radix v0.0.0-20170803185627-826af9ccf0fe
 	github.com/jaguilar/vt100 => github.com/tonistiigi/vt100 v0.0.0-20190402012908-ad4c4a574305
+	// temporary, until https://github.com/tonistiigi/fsutil/pull/103 is merged
+	github.com/tonistiigi/fsutil => github.com/aaronlehmann/fsutil v0.0.0-20210524204258-ace084b6f5d7
 	// genproto: corresponds to containerd
 	google.golang.org/genproto => google.golang.org/genproto v0.0.0-20200224152610-e50cd9704f63
 	// grpc: corresponds to protobuf

go.sum

@@ -61,6 +61,8 @@ github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAE
 github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
 github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
 github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d/go.mod h1:HI8ITrYtUY+O+ZhtlqUnD8+KwNPOyugEhfP9fdUIaEQ=
+github.com/aaronlehmann/fsutil v0.0.0-20210524204258-ace084b6f5d7 h1:8dVjjxH0rtFdRWsfgmKOs18lgLxbxbpYDe7i7PJMOyA=
+github.com/aaronlehmann/fsutil v0.0.0-20210524204258-ace084b6f5d7/go.mod h1:4Bcxev2PKmz1bFF6Mg3opy8w4kYQVvEXQGKVkQkP2Ac=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
@@ -629,8 +631,6 @@ github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635/go.mod h1:hkRG
 github.com/tchap/go-patricia v2.2.6+incompatible/go.mod h1:bmLyhP68RS6kStMGxByiQ23RP/odRBOTVjwp2cDyi6I=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
-github.com/tonistiigi/fsutil v0.0.0-20210503153615-5c8be85fc731 h1:mQRv1skMLUpSW6elAWmJHWXTz42PQ3cB4R8Z4iA29yk=
-github.com/tonistiigi/fsutil v0.0.0-20210503153615-5c8be85fc731/go.mod h1:4Bcxev2PKmz1bFF6Mg3opy8w4kYQVvEXQGKVkQkP2Ac=
 github.com/tonistiigi/go-immutable-radix v0.0.0-20170803185627-826af9ccf0fe h1:pd7hrFSqUPxYS9IB+UMG1AB/8EXGXo17ssx0bSQ5L6Y=
 github.com/tonistiigi/go-immutable-radix v0.0.0-20170803185627-826af9ccf0fe/go.mod h1:/+MCh11CJf2oz0BXmlmqyopK/ad1rKkcOXPoYuPCJYU=
 github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea h1:SXhTLE6pb6eld/v/cCndK0AMpt1wiVFb/YYmqB3/QG0=