From 620ac87d5620ad519f072b6f13b03e128bc09a13 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 21 Apr 2021 15:58:35 -0700 Subject: [PATCH 1/7] Add IncludePatterns and ExcludePatterns options for Copy Allow include patterns and exclude patterns to be specified, similarly to Walker. There is a bit of extra complexity to handle the case of a pattern like `a/*/c`. In this case, creating the directories `a` and `a/b` may need to be delayed until we encounter `a/b/c`. --- copy/copy.go | 246 ++++++++++++++++++++++++++++++++++++++----- copy/copy_test.go | 108 +++++++++++++++++++ prefix/match.go | 32 ++++++ prefix/match_test.go | 57 ++++++++++ walker.go | 31 +----- walker_test.go | 51 --------- 6 files changed, 419 insertions(+), 106 deletions(-) create mode 100644 prefix/match.go create mode 100644 prefix/match_test.go diff --git a/copy/copy.go b/copy/copy.go index 02b3dc9..2053ecb 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -11,7 +11,9 @@ import ( "time" "github.com/containerd/continuity/fs" + "github.com/docker/docker/pkg/fileutils" "github.com/pkg/errors" + "github.com/tonistiigi/fsutil/prefix" ) var bufferPool = &sync.Pool{ @@ -86,7 +88,10 @@ func Copy(ctx context.Context, srcRoot, src, dstRoot, dst string, opts ...Opt) e return err } - c := newCopier(ci.Chown, ci.Utime, ci.Mode, ci.XAttrErrorHandler) + c, err := newCopier(ci.Chown, ci.Utime, ci.Mode, ci.XAttrErrorHandler, ci.IncludePatterns, ci.ExcludePatterns) + if err != nil { + return err + } srcs := []string{src} if ci.AllowWildcards { @@ -109,7 +114,8 @@ func Copy(ctx context.Context, srcRoot, src, dstRoot, dst string, opts ...Opt) e if err != nil { return err } - if err := c.copy(ctx, srcFollowed, dst, false); err != nil { + includeAll := len(c.includePatterns) == 0 + if err := c.copy(ctx, srcFollowed, "", dst, false, includeAll); err != nil { return err } } @@ -162,6 +168,10 @@ type CopyInfo struct { XAttrErrorHandler XAttrErrorHandler CopyDirContents bool FollowLinks bool + // Include only files/dirs matching at least one of these patterns + IncludePatterns []string + // Exclude files/dir matching any of these patterns (even if they match an include pattern) + ExcludePatterns []string } type Opt func(*CopyInfo) @@ -197,48 +207,110 @@ func AllowXAttrErrors(ci *CopyInfo) { WithXAttrErrorHandler(h)(ci) } +func WithIncludePattern(includePattern string) Opt { + return func(ci *CopyInfo) { + ci.IncludePatterns = append(ci.IncludePatterns, includePattern) + } +} + +func WithExcludePattern(excludePattern string) Opt { + return func(ci *CopyInfo) { + ci.ExcludePatterns = append(ci.ExcludePatterns, excludePattern) + } +} + type copier struct { - chown Chowner - utime *time.Time - mode *int - inodes map[uint64]string - xattrErrorHandler XAttrErrorHandler + chown Chowner + utime *time.Time + mode *int + inodes map[uint64]string + xattrErrorHandler XAttrErrorHandler + includePatterns []string + excludePatternMatcher *fileutils.PatternMatcher } -func newCopier(chown Chowner, tm *time.Time, mode *int, xeh XAttrErrorHandler) *copier { +func newCopier(chown Chowner, tm *time.Time, mode *int, xeh XAttrErrorHandler, includePatterns, excludePatterns []string) (*copier, error) { if xeh == nil { xeh = func(dst, src, key string, err error) error { return err } } - return &copier{inodes: map[uint64]string{}, chown: chown, utime: tm, xattrErrorHandler: xeh, mode: mode} + + var pm *fileutils.PatternMatcher + if len(excludePatterns) != 0 { + var err error + pm, err = fileutils.NewPatternMatcher(excludePatterns) + if err != nil { + return nil, errors.Wrapf(err, "invalid excludepatterns: %s", excludePatterns) + } + } + + return &copier{ + inodes: map[uint64]string{}, + chown: chown, + utime: tm, + xattrErrorHandler: xeh, + mode: mode, + includePatterns: includePatterns, + excludePatternMatcher: pm, + }, nil } // dest is always clean -func (c *copier) copy(ctx context.Context, src, target string, overwriteTargetMetadata bool) error { +func (c *copier) copy(ctx context.Context, src, srcComponents, target string, overwriteTargetMetadata, includeAll bool) error { select { case <-ctx.Done(): return ctx.Err() default: } + fi, err := os.Lstat(src) if err != nil { return errors.Wrapf(err, "failed to stat %s", src) } + var include bool + if srcComponents != "" { + if !includeAll { + include, includeAll, err = c.include(srcComponents, fi) + if err != nil { + return err + } + if !include { + return nil + } + } + exclude, err := c.exclude(srcComponents, fi) + if err != nil { + return err + } + if exclude { + return nil + } + } + if !fi.IsDir() { + if include { + if err := c.createParentDirs(src, srcComponents, target, overwriteTargetMetadata); err != nil { + return err + } + } if err := ensureEmptyFileTarget(target); err != nil { return err } + } else if includeAll { + if err := c.createParentDirs(src, srcComponents, target, overwriteTargetMetadata); err != nil { + return err + } } copyFileInfo := true switch { case fi.IsDir(): - if created, err := c.copyDirectory(ctx, src, target, fi, overwriteTargetMetadata); err != nil { + if created, err := c.copyDirectory(ctx, src, srcComponents, target, fi, overwriteTargetMetadata, includeAll); err != nil { return err - } else if !overwriteTargetMetadata { + } else if !overwriteTargetMetadata || !includeAll { copyFileInfo = created } case (fi.Mode() & os.ModeType) == 0: @@ -282,26 +354,127 @@ func (c *copier) copy(ctx context.Context, src, target string, overwriteTargetMe return nil } -func (c *copier) copyDirectory(ctx context.Context, src, dst string, stat os.FileInfo, overwriteTargetMetadata bool) (bool, error) { +func (c *copier) include(path string, fi os.FileInfo) (bool, bool, error) { + matched := false + partial := true + for _, pattern := range c.includePatterns { + if fi.IsDir() { + pattern = strings.TrimSuffix(pattern, string(filepath.Separator)) + } + + if ok, p := prefix.Match(pattern, path); ok { + matched = true + if !p { + partial = false + break + } + } + } + + if !matched { + return false, false, nil + } + if fi.IsDir() { + return true, !partial, nil + } + return !partial, !partial, nil +} + +func (c *copier) exclude(path string, fi os.FileInfo) (bool, error) { + if c.excludePatternMatcher == nil { + return false, nil + } + + m, err := c.excludePatternMatcher.Matches(path) + if err != nil { + return false, errors.Wrap(err, "failed to match excludepatterns") + } + if m { + if fi.IsDir() && c.excludePatternMatcher.Exclusions() { + dirSlash := path + string(filepath.Separator) + for _, pat := range c.excludePatternMatcher.Patterns() { + if !pat.Exclusion() { + continue + } + patStr := pat.String() + string(filepath.Separator) + if strings.HasPrefix(patStr, dirSlash) { + return false, nil + } + } + } + return true, nil + } + + return false, nil +} + +// Delayed creation of parent directories when a file or dir matches an include +// pattern. +func (c *copier) createParentDirs(src, srcComponents, target string, overwriteTargetMetadata bool) error { + if len(c.includePatterns) == 0 { + return nil + } + + count := strings.Count(srcComponents, string(filepath.Separator)) + if count != 0 { + srcPaths := []string{src} + targetPaths := []string{target} + for i := 0; i != count; i++ { + srcParentDir, _ := filepath.Split(srcPaths[len(srcPaths)-1]) + if len(srcParentDir) > 1 { + srcParentDir = strings.TrimSuffix(srcParentDir, string(filepath.Separator)) + } + srcPaths = append(srcPaths, srcParentDir) + + targetParentDir, _ := filepath.Split(targetPaths[len(targetPaths)-1]) + if len(targetParentDir) > 1 { + targetParentDir = strings.TrimSuffix(targetParentDir, string(filepath.Separator)) + } + targetPaths = append(targetPaths, targetParentDir) + } + for i := count; i > 0; i-- { + fi, err := os.Stat(srcPaths[i]) + if err != nil { + return errors.Wrapf(err, "failed to stat %s", src) + } + if !fi.IsDir() { + return errors.Errorf("%s is not a directory", srcPaths[i]) + } + + created, err := copyDirectoryOnly(srcPaths[i], targetPaths[i], fi, overwriteTargetMetadata) + if err != nil { + return err + } + if created { + if err := c.copyFileInfo(fi, targetPaths[i]); err != nil { + return errors.Wrap(err, "failed to copy file info") + } + + if err := copyXAttrs(targetPaths[i], srcPaths[i], c.xattrErrorHandler); err != nil { + return errors.Wrap(err, "failed to copy xattrs") + } + } + } + } + return nil +} + +func (c *copier) copyDirectory(ctx context.Context, src, srcComponents, dst string, stat os.FileInfo, overwriteTargetMetadata, includeAll bool) (bool, error) { if !stat.IsDir() { return false, errors.Errorf("source is not directory") } created := false - if st, err := os.Lstat(dst); err != nil { - if !os.IsNotExist(err) { - return false, err - } - created = true - if err := os.Mkdir(dst, stat.Mode()); err != nil { - return created, errors.Wrapf(err, "failed to mkdir %s", dst) - } - } else if !st.IsDir() { - return false, errors.Errorf("cannot copy to non-directory: %s", dst) - } else if overwriteTargetMetadata { - if err := os.Chmod(dst, stat.Mode()); err != nil { - return false, errors.Wrapf(err, "failed to chmod on %s", dst) + // If there are no include patterns or this directory matched an include + // pattern exactly, go ahead and create the directory. Otherwise, delay to + // handle include patterns like a/*/c where we do not want to create a/b + // until we encounter a/b/c. + if includeAll { + var err error + created, err = copyDirectoryOnly(src, dst, stat, overwriteTargetMetadata) + if err != nil { + return created, err } } @@ -311,7 +484,7 @@ func (c *copier) copyDirectory(ctx context.Context, src, dst string, stat os.Fil } for _, fi := range fis { - if err := c.copy(ctx, filepath.Join(src, fi.Name()), filepath.Join(dst, fi.Name()), true); err != nil { + if err := c.copy(ctx, filepath.Join(src, fi.Name()), filepath.Join(srcComponents, fi.Name()), filepath.Join(dst, fi.Name()), true, includeAll); err != nil { return false, err } } @@ -319,6 +492,25 @@ func (c *copier) copyDirectory(ctx context.Context, src, dst string, stat os.Fil return created, nil } +func copyDirectoryOnly(src, dst string, stat os.FileInfo, overwriteTargetMetadata bool) (bool, error) { + if st, err := os.Lstat(dst); err != nil { + if !os.IsNotExist(err) { + return false, err + } + if err := os.Mkdir(dst, stat.Mode()); err != nil { + return false, errors.Wrapf(err, "failed to mkdir %s", dst) + } + return true, nil + } else if !st.IsDir() { + return false, errors.Errorf("cannot copy to non-directory: %s", dst) + } else if overwriteTargetMetadata { + if err := os.Chmod(dst, stat.Mode()); err != nil { + return false, errors.Wrapf(err, "failed to chmod on %s", dst) + } + } + return false, nil +} + func ensureEmptyFileTarget(dst string) error { fi, err := os.Lstat(dst) if err != nil { diff --git a/copy/copy_test.go b/copy/copy_test.go index b1456dc..37db673 100644 --- a/copy/copy_test.go +++ b/copy/copy_test.go @@ -10,6 +10,7 @@ import ( "github.com/containerd/continuity/fs/fstest" "github.com/pkg/errors" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -352,3 +353,110 @@ func testCopy(apply fstest.Applier) error { return fstest.CheckDirectoryEqual(t1, t2) } + +func TestCopyIncludeExclude(t *testing.T) { + t1, err := ioutil.TempDir("", "test") + require.NoError(t, err) + defer os.RemoveAll(t1) + + apply := fstest.Apply( + fstest.CreateDir("bar", 0755), + fstest.CreateFile("bar/foo", []byte("foo-contents"), 0755), + fstest.CreateDir("bar/baz", 0755), + fstest.CreateFile("bar/baz/foo3", []byte("foo3-contents"), 0755), + fstest.CreateFile("foo2", []byte("foo2-contents"), 0755), + ) + + require.NoError(t, apply.Apply(t1)) + + testCases := []struct { + name string + opts []Opt + expectedResults []string + }{ + { + name: "include bar", + opts: []Opt{WithIncludePattern("bar")}, + expectedResults: []string{"bar", "bar/foo", "bar/baz", "bar/baz/foo3"}, + }, + { + name: "include *", + opts: []Opt{WithIncludePattern("*")}, + expectedResults: []string{"bar", "bar/foo", "bar/baz", "bar/baz/foo3", "foo2"}, + }, + { + name: "include bar/foo", + opts: []Opt{WithIncludePattern("bar/foo")}, + expectedResults: []string{"bar", "bar/foo"}, + }, + { + name: "include bar/foo and foo*", + opts: []Opt{WithIncludePattern("bar/foo"), WithIncludePattern("foo*")}, + expectedResults: []string{"bar", "bar/foo", "foo2"}, + }, + { + name: "include b*", + opts: []Opt{WithIncludePattern("b*")}, + expectedResults: []string{"bar", "bar/foo", "bar/baz", "bar/baz/foo3"}, + }, + { + name: "include bar/f*", + opts: []Opt{WithIncludePattern("bar/f*")}, + expectedResults: []string{"bar", "bar/foo"}, + }, + { + name: "include bar/g*", + opts: []Opt{WithIncludePattern("bar/g*")}, + expectedResults: nil, + }, + { + name: "include b*/f*", + opts: []Opt{WithIncludePattern("b*/f*")}, + expectedResults: []string{"bar", "bar/foo"}, + }, + { + name: "include b*/foo", + opts: []Opt{WithIncludePattern("b*/foo")}, + expectedResults: []string{"bar", "bar/foo"}, + }, + { + name: "include b*/", + opts: []Opt{WithIncludePattern("b*/")}, + expectedResults: []string{"bar", "bar/foo", "bar/baz", "bar/baz/foo3"}, + }, + { + name: "include bar/*/foo3", + opts: []Opt{WithIncludePattern("bar/*/foo3")}, + expectedResults: []string{"bar", "bar/baz", "bar/baz/foo3"}, + }, + { + name: "exclude bar*, !bar/baz", + opts: []Opt{WithExcludePattern("bar*"), WithExcludePattern("!bar/baz")}, + expectedResults: []string{"bar", "bar/baz", "bar/baz/foo3", "foo2"}, + }, + { + name: "include bar, exclude bar/baz", + opts: []Opt{WithIncludePattern("bar"), WithExcludePattern("bar/baz")}, + expectedResults: []string{"bar", "bar/foo"}, + }, + } + + for _, tc := range testCases { + t2, err := ioutil.TempDir("", "test") + require.NoError(t, err) + defer os.RemoveAll(t2) + + err = Copy(context.Background(), t1, "/", t2, "/", tc.opts...) + require.NoError(t, err) + + var results []string + for _, path := range []string{"bar", "bar/foo", "bar/baz", "bar/baz/foo3", "foo2"} { + _, err := os.Stat(filepath.Join(t2, path)) + if err == nil { + results = append(results, path) + } + } + + assert.Equal(t, tc.expectedResults, results, tc.name) + } +} diff --git a/prefix/match.go b/prefix/match.go new file mode 100644 index 0000000..8a1e81b --- /dev/null +++ b/prefix/match.go @@ -0,0 +1,32 @@ +package prefix + +import ( + "path/filepath" + "strings" +) + +func Match(pattern, name string) (bool, bool) { + count := strings.Count(name, string(filepath.Separator)) + partial := false + if strings.Count(pattern, string(filepath.Separator)) > count { + pattern = trimUntilIndex(pattern, string(filepath.Separator), count) + partial = true + } + m, _ := filepath.Match(pattern, name) + return m, partial +} + +func trimUntilIndex(str, sep string, count int) string { + s := str + i := 0 + c := 0 + for { + idx := strings.Index(s, sep) + s = s[idx+len(sep):] + i += idx + len(sep) + c++ + if c > count { + return str[:i-len(sep)] + } + } +} diff --git a/prefix/match_test.go b/prefix/match_test.go new file mode 100644 index 0000000..3a3b704 --- /dev/null +++ b/prefix/match_test.go @@ -0,0 +1,57 @@ +package prefix + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMatch(t *testing.T) { + ok, partial := Match("foo", "foo") + assert.Equal(t, true, ok) + assert.Equal(t, false, partial) + + ok, partial = Match("foo/bar/baz", "foo") + assert.Equal(t, true, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("foo/bar/baz", "foo/bar") + assert.Equal(t, true, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("foo/bar/baz", "foo/bax") + assert.Equal(t, false, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("foo/bar/baz", "foo/bar/baz") + assert.Equal(t, true, ok) + assert.Equal(t, false, partial) + + ok, partial = Match("f*", "foo") + assert.Equal(t, true, ok) + assert.Equal(t, false, partial) + + ok, partial = Match("foo/bar/*", "foo") + assert.Equal(t, true, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("foo/*/baz", "foo") + assert.Equal(t, true, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("*/*/baz", "foo") + assert.Equal(t, true, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("*/bar/baz", "foo/bar") + assert.Equal(t, true, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("*/bar/baz", "foo/bax") + assert.Equal(t, false, ok) + assert.Equal(t, true, partial) + + ok, partial = Match("*/*/baz", "foo/bar/baz") + assert.Equal(t, true, ok) + assert.Equal(t, false, partial) +} diff --git a/walker.go b/walker.go index b10383e..f3e377e 100644 --- a/walker.go +++ b/walker.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/pkg/fileutils" "github.com/pkg/errors" + "github.com/tonistiigi/fsutil/prefix" "github.com/tonistiigi/fsutil/types" ) @@ -96,8 +97,8 @@ func Walk(ctx context.Context, p string, opt *WalkOpt, fn filepath.WalkFunc) err if !skip { matched := false partial := true - for _, p := range includePatterns { - if ok, p := matchPrefix(p, path); ok { + for _, pattern := range includePatterns { + if ok, p := prefix.Match(pattern, path); ok { matched = true if !p { partial = false @@ -190,32 +191,6 @@ func (s *StatInfo) Sys() interface{} { return s.Stat } -func matchPrefix(pattern, name string) (bool, bool) { - count := strings.Count(name, string(filepath.Separator)) - partial := false - if strings.Count(pattern, string(filepath.Separator)) > count { - pattern = trimUntilIndex(pattern, string(filepath.Separator), count) - partial = true - } - m, _ := filepath.Match(pattern, name) - return m, partial -} - -func trimUntilIndex(str, sep string, count int) string { - s := str - i := 0 - c := 0 - for { - idx := strings.Index(s, sep) - s = s[idx+len(sep):] - i += idx + len(sep) - c++ - if c > count { - return str[:i-len(sep)] - } - } -} - func isNotExist(err error) bool { return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) } diff --git a/walker_test.go b/walker_test.go index 77cbe1a..8b9f2d3 100644 --- a/walker_test.go +++ b/walker_test.go @@ -149,7 +149,6 @@ func TestWalkerExclude(t *testing.T) { dir foo file foo/bar2 `, string(b.Bytes())) - } func TestWalkerFollowLinks(t *testing.T) { @@ -234,56 +233,6 @@ file _foo2 `, string(b.Bytes())) } -func TestMatchPrefix(t *testing.T) { - ok, partial := matchPrefix("foo", "foo") - assert.Equal(t, true, ok) - assert.Equal(t, false, partial) - - ok, partial = matchPrefix("foo/bar/baz", "foo") - assert.Equal(t, true, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("foo/bar/baz", "foo/bar") - assert.Equal(t, true, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("foo/bar/baz", "foo/bax") - assert.Equal(t, false, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("foo/bar/baz", "foo/bar/baz") - assert.Equal(t, true, ok) - assert.Equal(t, false, partial) - - ok, partial = matchPrefix("f*", "foo") - assert.Equal(t, true, ok) - assert.Equal(t, false, partial) - - ok, partial = matchPrefix("foo/bar/*", "foo") - assert.Equal(t, true, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("foo/*/baz", "foo") - assert.Equal(t, true, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("*/*/baz", "foo") - assert.Equal(t, true, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("*/bar/baz", "foo/bar") - assert.Equal(t, true, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("*/bar/baz", "foo/bax") - assert.Equal(t, false, ok) - assert.Equal(t, true, partial) - - ok, partial = matchPrefix("*/*/baz", "foo/bar/baz") - assert.Equal(t, true, ok) - assert.Equal(t, false, partial) -} - func bufWalk(buf *bytes.Buffer) filepath.WalkFunc { return func(path string, fi os.FileInfo, err error) error { stat, ok := fi.Sys().(*types.Stat) From 08e52415f9b15220e25782865c69b28ea15e1ca4 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 28 Apr 2021 13:24:02 -0700 Subject: [PATCH 2/7] Rename includeAll to skipIncludePatterns --- copy/copy.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 2053ecb..6781f39 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -114,8 +114,8 @@ func Copy(ctx context.Context, srcRoot, src, dstRoot, dst string, opts ...Opt) e if err != nil { return err } - includeAll := len(c.includePatterns) == 0 - if err := c.copy(ctx, srcFollowed, "", dst, false, includeAll); err != nil { + skipIncludePatterns := len(c.includePatterns) == 0 + if err := c.copy(ctx, srcFollowed, "", dst, false, skipIncludePatterns); err != nil { return err } } @@ -257,7 +257,7 @@ func newCopier(chown Chowner, tm *time.Time, mode *int, xeh XAttrErrorHandler, i } // dest is always clean -func (c *copier) copy(ctx context.Context, src, srcComponents, target string, overwriteTargetMetadata, includeAll bool) error { +func (c *copier) copy(ctx context.Context, src, srcComponents, target string, overwriteTargetMetadata, skipIncludePatterns bool) error { select { case <-ctx.Done(): return ctx.Err() @@ -271,8 +271,8 @@ func (c *copier) copy(ctx context.Context, src, srcComponents, target string, ov var include bool if srcComponents != "" { - if !includeAll { - include, includeAll, err = c.include(srcComponents, fi) + if !skipIncludePatterns { + include, skipIncludePatterns, err = c.include(srcComponents, fi) if err != nil { return err } @@ -298,7 +298,7 @@ func (c *copier) copy(ctx context.Context, src, srcComponents, target string, ov if err := ensureEmptyFileTarget(target); err != nil { return err } - } else if includeAll { + } else if skipIncludePatterns { if err := c.createParentDirs(src, srcComponents, target, overwriteTargetMetadata); err != nil { return err } @@ -308,9 +308,9 @@ func (c *copier) copy(ctx context.Context, src, srcComponents, target string, ov switch { case fi.IsDir(): - if created, err := c.copyDirectory(ctx, src, srcComponents, target, fi, overwriteTargetMetadata, includeAll); err != nil { + if created, err := c.copyDirectory(ctx, src, srcComponents, target, fi, overwriteTargetMetadata, skipIncludePatterns); err != nil { return err - } else if !overwriteTargetMetadata || !includeAll { + } else if !overwriteTargetMetadata || !skipIncludePatterns { copyFileInfo = created } case (fi.Mode() & os.ModeType) == 0: @@ -459,7 +459,7 @@ func (c *copier) createParentDirs(src, srcComponents, target string, overwriteTa return nil } -func (c *copier) copyDirectory(ctx context.Context, src, srcComponents, dst string, stat os.FileInfo, overwriteTargetMetadata, includeAll bool) (bool, error) { +func (c *copier) copyDirectory(ctx context.Context, src, srcComponents, dst string, stat os.FileInfo, overwriteTargetMetadata, skipIncludePatterns bool) (bool, error) { if !stat.IsDir() { return false, errors.Errorf("source is not directory") } @@ -470,7 +470,7 @@ func (c *copier) copyDirectory(ctx context.Context, src, srcComponents, dst stri // pattern exactly, go ahead and create the directory. Otherwise, delay to // handle include patterns like a/*/c where we do not want to create a/b // until we encounter a/b/c. - if includeAll { + if skipIncludePatterns { var err error created, err = copyDirectoryOnly(src, dst, stat, overwriteTargetMetadata) if err != nil { @@ -484,7 +484,7 @@ func (c *copier) copyDirectory(ctx context.Context, src, srcComponents, dst stri } for _, fi := range fis { - if err := c.copy(ctx, filepath.Join(src, fi.Name()), filepath.Join(srcComponents, fi.Name()), filepath.Join(dst, fi.Name()), true, includeAll); err != nil { + if err := c.copy(ctx, filepath.Join(src, fi.Name()), filepath.Join(srcComponents, fi.Name()), filepath.Join(dst, fi.Name()), true, skipIncludePatterns); err != nil { return false, err } } From e5bab588b143d9f6dbca70375209537cfec01f23 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 28 Apr 2021 13:25:44 -0700 Subject: [PATCH 3/7] Use "require" instead of "assert" --- copy/copy_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/copy/copy_test.go b/copy/copy_test.go index 37db673..959e67c 100644 --- a/copy/copy_test.go +++ b/copy/copy_test.go @@ -10,7 +10,6 @@ import ( "github.com/containerd/continuity/fs/fstest" "github.com/pkg/errors" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -457,6 +456,6 @@ func TestCopyIncludeExclude(t *testing.T) { } } - assert.Equal(t, tc.expectedResults, results, tc.name) + require.Equal(t, tc.expectedResults, results, tc.name) } } From b39921c8a87d94689b4ca6d032da30162719744f Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 28 Apr 2021 13:33:12 -0700 Subject: [PATCH 4/7] Add slashSeparator argument to Match --- copy/copy.go | 2 +- prefix/match.go | 20 +++++++++++++++----- prefix/match_test.go | 24 ++++++++++++------------ walker.go | 2 +- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 6781f39..c9dee91 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -362,7 +362,7 @@ func (c *copier) include(path string, fi os.FileInfo) (bool, bool, error) { pattern = strings.TrimSuffix(pattern, string(filepath.Separator)) } - if ok, p := prefix.Match(pattern, path); ok { + if ok, p := prefix.Match(pattern, path, false); ok { matched = true if !p { partial = false diff --git a/prefix/match.go b/prefix/match.go index 8a1e81b..8e263b8 100644 --- a/prefix/match.go +++ b/prefix/match.go @@ -1,18 +1,28 @@ package prefix import ( + "path" "path/filepath" "strings" ) -func Match(pattern, name string) (bool, bool) { - count := strings.Count(name, string(filepath.Separator)) +func Match(pattern, name string, slashSeparator bool) (bool, bool) { + separator := filepath.Separator + if slashSeparator { + separator = '/' + } + count := strings.Count(name, string(separator)) partial := false - if strings.Count(pattern, string(filepath.Separator)) > count { - pattern = trimUntilIndex(pattern, string(filepath.Separator), count) + if strings.Count(pattern, string(separator)) > count { + pattern = trimUntilIndex(pattern, string(separator), count) partial = true } - m, _ := filepath.Match(pattern, name) + var m bool + if slashSeparator { + m, _ = path.Match(pattern, name) + } else { + m, _ = filepath.Match(pattern, name) + } return m, partial } diff --git a/prefix/match_test.go b/prefix/match_test.go index 3a3b704..bbb253b 100644 --- a/prefix/match_test.go +++ b/prefix/match_test.go @@ -7,51 +7,51 @@ import ( ) func TestMatch(t *testing.T) { - ok, partial := Match("foo", "foo") + ok, partial := Match("foo", "foo", false) assert.Equal(t, true, ok) assert.Equal(t, false, partial) - ok, partial = Match("foo/bar/baz", "foo") + ok, partial = Match("foo/bar/baz", "foo", false) assert.Equal(t, true, ok) assert.Equal(t, true, partial) - ok, partial = Match("foo/bar/baz", "foo/bar") + ok, partial = Match("foo/bar/baz", "foo/bar", false) assert.Equal(t, true, ok) assert.Equal(t, true, partial) - ok, partial = Match("foo/bar/baz", "foo/bax") + ok, partial = Match("foo/bar/baz", "foo/bax", false) assert.Equal(t, false, ok) assert.Equal(t, true, partial) - ok, partial = Match("foo/bar/baz", "foo/bar/baz") + ok, partial = Match("foo/bar/baz", "foo/bar/baz", false) assert.Equal(t, true, ok) assert.Equal(t, false, partial) - ok, partial = Match("f*", "foo") + ok, partial = Match("f*", "foo", false) assert.Equal(t, true, ok) assert.Equal(t, false, partial) - ok, partial = Match("foo/bar/*", "foo") + ok, partial = Match("foo/bar/*", "foo", false) assert.Equal(t, true, ok) assert.Equal(t, true, partial) - ok, partial = Match("foo/*/baz", "foo") + ok, partial = Match("foo/*/baz", "foo", false) assert.Equal(t, true, ok) assert.Equal(t, true, partial) - ok, partial = Match("*/*/baz", "foo") + ok, partial = Match("*/*/baz", "foo", false) assert.Equal(t, true, ok) assert.Equal(t, true, partial) - ok, partial = Match("*/bar/baz", "foo/bar") + ok, partial = Match("*/bar/baz", "foo/bar", false) assert.Equal(t, true, ok) assert.Equal(t, true, partial) - ok, partial = Match("*/bar/baz", "foo/bax") + ok, partial = Match("*/bar/baz", "foo/bax", false) assert.Equal(t, false, ok) assert.Equal(t, true, partial) - ok, partial = Match("*/*/baz", "foo/bar/baz") + ok, partial = Match("*/*/baz", "foo/bar/baz", false) assert.Equal(t, true, ok) assert.Equal(t, false, partial) } diff --git a/walker.go b/walker.go index f3e377e..cc6143d 100644 --- a/walker.go +++ b/walker.go @@ -98,7 +98,7 @@ func Walk(ctx context.Context, p string, opt *WalkOpt, fn filepath.WalkFunc) err matched := false partial := true for _, pattern := range includePatterns { - if ok, p := prefix.Match(pattern, path); ok { + if ok, p := prefix.Match(pattern, path, false); ok { matched = true if !p { partial = false From accc0fae8749370e0467e19c6e57d8b0a650d189 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 28 Apr 2021 15:06:03 -0700 Subject: [PATCH 5/7] Cache whether parent dir entries have been copied yet --- copy/copy.go | 75 ++++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index c9dee91..65dad8c 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -227,6 +227,13 @@ type copier struct { xattrErrorHandler XAttrErrorHandler includePatterns []string excludePatternMatcher *fileutils.PatternMatcher + parentDirs []parentDir +} + +type parentDir struct { + srcPath string + dstPath string + copied bool } func newCopier(chown Chowner, tm *time.Time, mode *int, xeh XAttrErrorHandler, includePatterns, excludePatterns []string) (*copier, error) { @@ -411,50 +418,34 @@ func (c *copier) exclude(path string, fi os.FileInfo) (bool, error) { // Delayed creation of parent directories when a file or dir matches an include // pattern. func (c *copier) createParentDirs(src, srcComponents, target string, overwriteTargetMetadata bool) error { - if len(c.includePatterns) == 0 { - return nil - } - - count := strings.Count(srcComponents, string(filepath.Separator)) - if count != 0 { - srcPaths := []string{src} - targetPaths := []string{target} - for i := 0; i != count; i++ { - srcParentDir, _ := filepath.Split(srcPaths[len(srcPaths)-1]) - if len(srcParentDir) > 1 { - srcParentDir = strings.TrimSuffix(srcParentDir, string(filepath.Separator)) - } - srcPaths = append(srcPaths, srcParentDir) + for i, parentDir := range c.parentDirs { + if parentDir.copied { + continue + } - targetParentDir, _ := filepath.Split(targetPaths[len(targetPaths)-1]) - if len(targetParentDir) > 1 { - targetParentDir = strings.TrimSuffix(targetParentDir, string(filepath.Separator)) - } - targetPaths = append(targetPaths, targetParentDir) + fi, err := os.Stat(parentDir.srcPath) + if err != nil { + return errors.Wrapf(err, "failed to stat %s", src) + } + if !fi.IsDir() { + return errors.Errorf("%s is not a directory", parentDir.srcPath) } - for i := count; i > 0; i-- { - fi, err := os.Stat(srcPaths[i]) - if err != nil { - return errors.Wrapf(err, "failed to stat %s", src) - } - if !fi.IsDir() { - return errors.Errorf("%s is not a directory", srcPaths[i]) - } - created, err := copyDirectoryOnly(srcPaths[i], targetPaths[i], fi, overwriteTargetMetadata) - if err != nil { - return err + created, err := copyDirectoryOnly(parentDir.srcPath, parentDir.dstPath, fi, overwriteTargetMetadata) + if err != nil { + return err + } + if created { + if err := c.copyFileInfo(fi, parentDir.dstPath); err != nil { + return errors.Wrap(err, "failed to copy file info") } - if created { - if err := c.copyFileInfo(fi, targetPaths[i]); err != nil { - return errors.Wrap(err, "failed to copy file info") - } - if err := copyXAttrs(targetPaths[i], srcPaths[i], c.xattrErrorHandler); err != nil { - return errors.Wrap(err, "failed to copy xattrs") - } + if err := copyXAttrs(parentDir.dstPath, parentDir.srcPath, c.xattrErrorHandler); err != nil { + return errors.Wrap(err, "failed to copy xattrs") } } + + c.parentDirs[i].copied = true } return nil } @@ -478,6 +469,16 @@ func (c *copier) copyDirectory(ctx context.Context, src, srcComponents, dst stri } } + c.parentDirs = append(c.parentDirs, parentDir{ + srcPath: src, + dstPath: dst, + copied: skipIncludePatterns, + }) + + defer func() { + c.parentDirs = c.parentDirs[:len(c.parentDirs)-1] + }() + fis, err := ioutil.ReadDir(src) if err != nil { return false, errors.Wrapf(err, "failed to read %s", src) From 34d3aed2d0c4dd5dbd258c10aed1361c81329708 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 29 Apr 2021 13:06:16 -0700 Subject: [PATCH 6/7] Use named returns and add godoc comment for Match Signed-off-by: Aaron Lehmann --- prefix/match.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/prefix/match.go b/prefix/match.go index 8e263b8..57e745c 100644 --- a/prefix/match.go +++ b/prefix/match.go @@ -6,18 +6,21 @@ import ( "strings" ) -func Match(pattern, name string, slashSeparator bool) (bool, bool) { +// Match matches a path against a pattern. It returns m = true if the path +// matches the pattern, and partial = true if the pattern has more separators +// than the path and the common components match (for example, name = foo and +// pattern = foo/bar/*). slashSeparator determines whether the path and pattern +// are '/' delimited (true) or use the native path separator (false). +func Match(pattern, name string, slashSeparator bool) (m bool, partial bool) { separator := filepath.Separator if slashSeparator { separator = '/' } count := strings.Count(name, string(separator)) - partial := false if strings.Count(pattern, string(separator)) > count { pattern = trimUntilIndex(pattern, string(separator), count) partial = true } - var m bool if slashSeparator { m, _ = path.Match(pattern, name) } else { From 10412d7b66860b319c4f86633aa1f3e740994df8 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 29 Apr 2021 13:07:17 -0700 Subject: [PATCH 7/7] Code style review suggestion --- copy/copy.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 65dad8c..67d1ebf 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -396,23 +396,23 @@ func (c *copier) exclude(path string, fi os.FileInfo) (bool, error) { if err != nil { return false, errors.Wrap(err, "failed to match excludepatterns") } - if m { - if fi.IsDir() && c.excludePatternMatcher.Exclusions() { - dirSlash := path + string(filepath.Separator) - for _, pat := range c.excludePatternMatcher.Patterns() { - if !pat.Exclusion() { - continue - } - patStr := pat.String() + string(filepath.Separator) - if strings.HasPrefix(patStr, dirSlash) { - return false, nil - } + if !m { + return false, nil + } + + if fi.IsDir() && c.excludePatternMatcher.Exclusions() { + dirSlash := path + string(filepath.Separator) + for _, pat := range c.excludePatternMatcher.Patterns() { + if !pat.Exclusion() { + continue + } + patStr := pat.String() + string(filepath.Separator) + if strings.HasPrefix(patStr, dirSlash) { + return false, nil } } - return true, nil } - - return false, nil + return true, nil } // Delayed creation of parent directories when a file or dir matches an include