fix(targethasher): frame directory hashes for BUG-007

core/targethasher/sourcehasher.go

@@ -40,10 +40,16 @@ const (
 	_defaultSourceFileVisibility = "//visibility:private"
 )
 
-// cancelCheckInterval is the number of files hashed between cancellation
-// checks during a directory walk.
+// cancelCheckInterval is the number of directory entries processed between
+// cancellation checks during a directory walk.
 const cancelCheckInterval = 1024
 
+const (
+	directoryEntryMarker byte = iota
+	regularFileEntryMarker
+	symlinkEntryMarker
+)
+
 // SourceHasher provides hashes for source nodes in the target graph. These
 // can be calculated based on disk contents or form other sources such as a
 // vcs system.
@@ -146,30 +152,60 @@ func hashFile(path string) (hash.Hash, error) {
 	return hash, nil
 }
 
+// hashDir returns a deterministic hash of a directory tree by streaming
+// WalkDir's lexical traversal into the digest without buffering the tree. It
+// hashes each entry's local name and platform-independent kind together with
+// regular-file contents or symlink targets, while excluding checkout location
+// and filesystem mode bits.
 func hashDir(ctx context.Context, root string) (hash.Hash, error) {
 	if err := context.Cause(ctx); err != nil {
 		return nil, err
 	}
 
 	dirHash := newHash()
-	var fileCount int
-	walkDirFunc := func(path string, d fs.DirEntry, err error) error {
+	var entryCount int
+	walkDirFunc := func(path string, entry fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
+		if path == root {
+			return nil
+		}
+		if entryCount%cancelCheckInterval == 0 {
+			if err := context.Cause(ctx); err != nil {
+				return err
+			}
+		}
+		entryCount++
 
-		if d.Type().IsRegular() {
-			if fileCount%cancelCheckInterval == 0 {
-				if err := context.Cause(ctx); err != nil {
-					return err
-				}
+		switch {
+		case entry.Type()&fs.ModeSymlink != 0:
+			writeDirectoryEntryHeader(dirHash, symlinkEntryMarker, entry.Name())
+			target, err := os.Readlink(path)
+			if err != nil {
+				return err
+			}
+			target, err = stableSymlinkTarget(path, target)
+			if err != nil {
+				return err
+			}
+			writeFramedDirectoryBytes(dirHash, []byte(target))
+		case entry.IsDir():
+			writeDirectoryEntryHeader(dirHash, directoryEntryMarker, entry.Name())
+		default:
+			info, err := entry.Info()
+			if err != nil {
+				return err
 			}
-			fileCount++
+			if !info.Mode().IsRegular() {
+				return fmt.Errorf("unsupported directory entry %q with mode %s", path, info.Mode())
+			}
+			writeDirectoryEntryHeader(dirHash, regularFileEntryMarker, entry.Name())
 			fileHash, err := hashFile(path)
 			if err != nil {
 				return err
 			}
-			dirHash.Write(fileHash.Sum(nil))
+			writeDirectoryBytes(dirHash, fileHash.Sum(nil))
 		}
 		return nil
 	}
@@ -178,6 +214,39 @@ func hashDir(ctx context.Context, root string) (hash.Hash, error) {
 	return dirHash, err
 }
 
+func stableSymlinkTarget(path, target string) (string, error) {
+	if filepath.IsAbs(target) {
+		relativeTarget, err := filepath.Rel(filepath.Dir(path), target)
+		if err != nil {
+			return "", fmt.Errorf("make symlink target %q relative to %q: %w", target, path, err)
+		}
+		target = relativeTarget
+	}
+	return filepath.ToSlash(filepath.Clean(target)), nil
+}
+
+func writeDirectoryEntryHeader(h hash.Hash, marker byte, name string) {
+	writeDirectoryBytes(h, []byte{marker})
+	writeFramedDirectoryBytes(h, []byte(name))
+}
+
+func writeFramedDirectoryBytes(h hash.Hash, value []byte) {
+	writeDirectoryUint64(h, uint64(len(value)))
+	writeDirectoryBytes(h, value)
+}
+
+func writeDirectoryUint64(h hash.Hash, value uint64) {
+	var encoded [8]byte
+	binary.BigEndian.PutUint64(encoded[:], value)
+	writeDirectoryBytes(h, encoded[:])
+}
+
+func writeDirectoryBytes(h hash.Hash, value []byte) {
+	// hash.Hash.Write is specified to consume all bytes and never return an
+	// error, so there is no actionable error to propagate here.
+	_, _ = h.Write(value)
+}
+
 func filterVisibilityLabels(labels []string) (res []string) {
 	for _, v := range labels {
 		if v != _defaultSourceFileVisibility {

core/targethasher/sourcehasher_test.go

@@ -157,6 +157,111 @@ func TestDiskHashHelper_HashesDirectory(t *testing.T) {
 	assert.NotEmpty(t, got, "expected non-empty hash for directory")
 }
 
+func TestHashDir_RenameChangesHash(t *testing.T) {
+	before := t.TempDir()
+	after := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(before, "before.txt"), []byte("same contents"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(after, "after.txt"), []byte("same contents"), 0o644))
+
+	assert.NotEqual(t, hashDirectoryForTest(t, before), hashDirectoryForTest(t, after))
+}
+
+func TestHashDir_HashesSymlinkEntriesAndTargets(t *testing.T) {
+	root := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(root, "first.txt"), []byte("same contents"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(root, "second.txt"), []byte("same contents"), 0o644))
+
+	withoutSymlink := hashDirectoryForTest(t, root)
+	link := filepath.Join(root, "link")
+	if err := os.Symlink("first.txt", link); err != nil {
+		t.Skipf("symlinks are unavailable: %v", err)
+	}
+	firstTarget := hashDirectoryForTest(t, root)
+	assert.NotEqual(t, withoutSymlink, firstTarget)
+
+	require.NoError(t, os.Remove(link))
+	require.NoError(t, os.Symlink("second.txt", link))
+	secondTarget := hashDirectoryForTest(t, root)
+	assert.NotEqual(t, firstTarget, secondTarget)
+
+	require.NoError(t, os.Rename(link, filepath.Join(root, "renamed-link")))
+	assert.NotEqual(t, secondTarget, hashDirectoryForTest(t, root))
+}
+
+func TestHashDir_NormalizesAbsoluteSymlinkTargets(t *testing.T) {
+	left := t.TempDir()
+	right := t.TempDir()
+	writeDirectoryTestFile(t, left, "target.txt", "same contents")
+	writeDirectoryTestFile(t, right, "target.txt", "same contents")
+
+	if err := os.Symlink(filepath.Join(left, "target.txt"), filepath.Join(left, "link")); err != nil {
+		t.Skipf("symlinks are unavailable: %v", err)
+	}
+	require.NoError(t, os.Symlink(filepath.Join(right, "target.txt"), filepath.Join(right, "link")))
+
+	assert.Equal(t, hashDirectoryForTest(t, left), hashDirectoryForTest(t, right))
+}
+
+func TestHashDir_IsDeterministicAcrossCreationOrder(t *testing.T) {
+	left := t.TempDir()
+	right := t.TempDir()
+
+	for _, relativePath := range []string{"z.txt", filepath.Join("nested", "b.txt"), filepath.Join("nested", "a.txt")} {
+		writeDirectoryTestFile(t, left, relativePath, relativePath)
+	}
+	for _, relativePath := range []string{filepath.Join("nested", "a.txt"), filepath.Join("nested", "b.txt"), "z.txt"} {
+		writeDirectoryTestFile(t, right, relativePath, relativePath)
+	}
+
+	leftHash := hashDirectoryForTest(t, left)
+	assert.Equal(t, leftHash, hashDirectoryForTest(t, left))
+	assert.Equal(t, leftHash, hashDirectoryForTest(t, right))
+}
+
+func TestHashDir_DistinguishesDirectoryStructures(t *testing.T) {
+	t.Run("regular file from nested file", func(t *testing.T) {
+		fileTree := t.TempDir()
+		nestedTree := t.TempDir()
+		writeDirectoryTestFile(t, fileTree, "entry", "same contents")
+		writeDirectoryTestFile(t, nestedTree, filepath.Join("entry", "child"), "same contents")
+
+		assert.NotEqual(t, hashDirectoryForTest(t, fileTree), hashDirectoryForTest(t, nestedTree))
+	})
+
+	t.Run("empty directory from empty tree", func(t *testing.T) {
+		emptyTree := t.TempDir()
+		directoryTree := t.TempDir()
+		require.NoError(t, os.Mkdir(filepath.Join(directoryTree, "empty"), 0o755))
+
+		assert.NotEqual(t, hashDirectoryForTest(t, emptyTree), hashDirectoryForTest(t, directoryTree))
+	})
+}
+
+func TestHashDir_OmitsFileMode(t *testing.T) {
+	regularTree := t.TempDir()
+	executableTree := t.TempDir()
+	writeDirectoryTestFile(t, regularTree, "tool", "same contents")
+	writeDirectoryTestFile(t, executableTree, "tool", "same contents")
+	require.NoError(t, os.Chmod(filepath.Join(regularTree, "tool"), 0o644))
+	require.NoError(t, os.Chmod(filepath.Join(executableTree, "tool"), 0o755))
+
+	assert.Equal(t, hashDirectoryForTest(t, regularTree), hashDirectoryForTest(t, executableTree))
+}
+
+func hashDirectoryForTest(t *testing.T, root string) []byte {
+	t.Helper()
+	h, err := hashDir(context.Background(), root)
+	require.NoError(t, err)
+	return h.Sum(nil)
+}
+
+func writeDirectoryTestFile(t *testing.T, root, relativePath, contents string) {
+	t.Helper()
+	path := filepath.Join(root, relativePath)
+	require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755))
+	require.NoError(t, os.WriteFile(path, []byte(contents), 0o644))
+}
+
 func TestDiskHashHelper_MissingFileProducesDeterministicHash(t *testing.T) {
 	h := &diskHashHelper{
 		workspaceroot:   t.TempDir(),