Merge pull request #1876 from dmcgowan/fs-include-parent-directories

Emit unmodified change events for directories

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
+		f1, f2         *currentPath
-		rmdir  string
+		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 {
+			if emit {
-				return err
+				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
+}

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),
+	}
+}

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(