From ba10e497b931449f1e8ecf689a67b4c373e6ab85 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 4 Dec 2017 18:15:16 -0800 Subject: [PATCH] Emit unmodified change events for directories When a file changes, emit events for the seen parent directories to ensure that those parents are included in change streams such as archives. The parents are needed accurately recreate diff representation apart from a parent (such as overlay and aufs graph drivers in Docker). Signed-off-by: Derek McGowan --- fs/diff.go | 83 +++++++++++++++++++++++++++++++++++++++--- fs/diff_test.go | 64 +++++++++++++++++++++++++++++++- fs/fstest/testsuite.go | 23 ++++++++++++ 3 files changed, 163 insertions(+), 7 deletions(-) diff --git a/fs/diff.go b/fs/diff.go index 9073d0d92..3a53f4215 100644 --- a/fs/diff.go +++ b/fs/diff.go @@ -222,8 +222,10 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err c1 = make(chan *currentPath) c2 = make(chan *currentPath) - f1, f2 *currentPath - rmdir string + f1, f2 *currentPath + rmdir string + lastEmittedDir = string(filepath.Separator) + parents []os.FileInfo ) g.Go(func() error { defer close(c1) @@ -258,7 +260,10 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err continue } - var f os.FileInfo + var ( + f os.FileInfo + emit = true + ) k, p := pathChange(f1, f2) switch k { case ChangeKindAdd: @@ -294,13 +299,35 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err f2 = nil if same { if !isLinked(f) { - continue + emit = false } k = ChangeKindUnmodified } } - if err := changeFn(k, p, f, nil); err != nil { - return err + if emit { + emittedDir, emitParents := commonParents(lastEmittedDir, p, parents) + for _, pf := range emitParents { + p := filepath.Join(emittedDir, pf.Name()) + if err := changeFn(ChangeKindUnmodified, p, pf, nil); err != nil { + return err + } + emittedDir = p + } + + if err := changeFn(k, p, f, nil); err != nil { + return err + } + + if f != nil && f.IsDir() { + lastEmittedDir = p + } else { + lastEmittedDir = emittedDir + } + + parents = parents[:0] + } else if f.IsDir() { + lastEmittedDir, parents = commonParents(lastEmittedDir, p, parents) + parents = append(parents, f) } } return nil @@ -308,3 +335,47 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err return g.Wait() } + +func commonParents(base, updated string, dirs []os.FileInfo) (string, []os.FileInfo) { + if basePrefix := makePrefix(base); strings.HasPrefix(updated, basePrefix) { + var ( + parents []os.FileInfo + last = base + ) + for _, d := range dirs { + next := filepath.Join(last, d.Name()) + if strings.HasPrefix(updated, makePrefix(last)) { + parents = append(parents, d) + last = next + } else { + break + } + } + return base, parents + } + + baseS := strings.Split(base, string(filepath.Separator)) + updatedS := strings.Split(updated, string(filepath.Separator)) + commonS := []string{string(filepath.Separator)} + + min := len(baseS) + if len(updatedS) < min { + min = len(updatedS) + } + for i := 0; i < min; i++ { + if baseS[i] == updatedS[i] { + commonS = append(commonS, baseS[i]) + } else { + break + } + } + + return filepath.Join(commonS...), []os.FileInfo{} +} + +func makePrefix(d string) string { + if d == "" || d[len(d)-1] != filepath.Separator { + return d + string(filepath.Separator) + } + return d +} diff --git a/fs/diff_test.go b/fs/diff_test.go index 506e7d867..126ba6a28 100644 --- a/fs/diff_test.go +++ b/fs/diff_test.go @@ -44,6 +44,7 @@ func TestSimpleDiff(t *testing.T) { fstest.Remove("/etc/unexpected"), ) diff := []TestChange{ + Unchanged("/etc"), Modify("/etc/hosts"), Modify("/etc/profile"), Delete("/etc/unexpected"), @@ -70,6 +71,7 @@ func TestDirectoryReplace(t *testing.T) { fstest.CreateFile("/dir1/f2", []byte("Now file"), 0666), ) diff := []TestChange{ + Unchanged("/dir1"), Add("/dir1/f11"), Modify("/dir1/f2"), } @@ -132,10 +134,13 @@ func TestParentDirectoryPermission(t *testing.T) { fstest.CreateFile("/dir3/f", []byte("irrelevant"), 0644), ) diff := []TestChange{ + Unchanged("/dir1"), Add("/dir1/d"), Add("/dir1/d/f"), Add("/dir1/f"), + Unchanged("/dir2"), Add("/dir2/f"), + Unchanged("/dir3"), Add("/dir3/f"), } @@ -184,6 +189,56 @@ func TestUpdateWithSameTime(t *testing.T) { } } +func TestUnchangedParent(t *testing.T) { + skipDiffTestOnWindows(t) + l1 := fstest.Apply( + fstest.CreateDir("/dir1", 0755), + fstest.CreateDir("/dir3", 0755), + fstest.CreateDir("/dir3/a", 0755), + fstest.CreateDir("/dir3/a/e", 0755), + fstest.CreateDir("/dir3/z", 0755), + ) + l2 := fstest.Apply( + fstest.CreateDir("/dir1/a", 0755), + fstest.CreateFile("/dir1/a/b", []byte("irrelevant"), 0644), + fstest.CreateDir("/dir1/a/e", 0755), + fstest.CreateFile("/dir1/a/e/f", []byte("irrelevant"), 0644), + fstest.CreateFile("/dir1/f", []byte("irrelevant"), 0644), + fstest.CreateDir("/dir2", 0755), + fstest.CreateFile("/dir2/f", []byte("irrelevant"), 0644), + fstest.CreateFile("/dir3/a/b", []byte("irrelevant"), 0644), + fstest.CreateDir("/dir3/a/c", 0755), + fstest.CreateDir("/dir3/a/e/i", 0755), + fstest.CreateFile("/dir3/a/e/i/f", []byte("irrelevant"), 0644), + fstest.CreateFile("/dir3/f", []byte("irrelevant"), 0644), + fstest.CreateFile("/dir3/z/f", []byte("irrelevant"), 0644), + ) + diff := []TestChange{ + Unchanged("/dir1"), + Add("/dir1/a"), + Add("/dir1/a/b"), + Add("/dir1/a/e"), + Add("/dir1/a/e/f"), + Add("/dir1/f"), + Add("/dir2"), + Add("/dir2/f"), + Unchanged("/dir3"), + Unchanged("/dir3/a"), + Add("/dir3/a/b"), + Add("/dir3/a/c"), + Unchanged("/dir3/a/e"), + Add("/dir3/a/e/i"), + Add("/dir3/a/e/i/f"), + Add("/dir3/f"), + Unchanged("/dir3/z"), + Add("/dir3/z/f"), + } + + if err := testDiffWithBase(l1, l2, diff); err != nil { + t.Fatalf("Failed diff with base: %+v", err) + } +} + // buildkit#172 func TestLchtimes(t *testing.T) { skipDiffTestOnWindows(t) @@ -347,7 +402,7 @@ func diffString(c1, c2 []TestChange) string { func changesString(c []TestChange) string { strs := make([]string, len(c)) for i := range c { - strs[i] = fmt.Sprintf("\t%s\t%s", c[i].Kind, c[i].Path) + strs[i] = fmt.Sprintf("\t%-10s\t%s", c[i].Kind, c[i].Path) } return strings.Join(strs, "\n") } @@ -372,3 +427,10 @@ func Modify(p string) TestChange { Path: filepath.FromSlash(p), } } + +func Unchanged(p string) TestChange { + return TestChange{ + Kind: ChangeKindUnmodified, + Path: filepath.FromSlash(p), + } +} diff --git a/fs/fstest/testsuite.go b/fs/fstest/testsuite.go index 546be89d1..a26e8bbd8 100644 --- a/fs/fstest/testsuite.go +++ b/fs/fstest/testsuite.go @@ -19,6 +19,7 @@ func FSSuite(t *testing.T, a TestApplier) { t.Run("Deletion", makeTest(t, a, deletionTest)) t.Run("Update", makeTest(t, a, updateTest)) t.Run("DirectoryPermission", makeTest(t, a, directoryPermissionsTest)) + t.Run("ParentDirectoryPermission", makeTest(t, a, parentDirectoryPermissionsTest)) t.Run("HardlinkUnmodified", makeTest(t, a, hardlinkUnmodified)) t.Run("HardlinkBeforeUnmodified", makeTest(t, a, hardlinkBeforeUnmodified)) t.Run("HardlinkBeforeModified", makeTest(t, a, hardlinkBeforeModified)) @@ -159,6 +160,28 @@ var ( ), } + // parentDirectoryPermissionsTest covers directory permissions for updated + // files + parentDirectoryPermissionsTest = []Applier{ + Apply( + CreateDir("/d1", 0700), + CreateDir("/d1/a", 0700), + CreateDir("/d1/a/b", 0700), + CreateDir("/d1/a/b/c", 0700), + CreateFile("/d1/a/b/f", []byte("content1"), 0644), + CreateDir("/d2", 0751), + CreateDir("/d2/a/b", 0751), + CreateDir("/d2/a/b/c", 0751), + CreateFile("/d2/a/b/f", []byte("content1"), 0644), + ), + Apply( + CreateFile("/d1/a/b/f", []byte("content1"), 0644), + Chmod("/d1/a/b/c", 0700), + CreateFile("/d2/a/b/f", []byte("content2"), 0644), + Chmod("/d2/a/b/c", 0751), + ), + } + hardlinkUnmodified = []Applier{ baseApplier, Apply(