ARCHITECTURE.md

@@ -359,7 +359,7 @@ CreateSnapshot(opts)
 
 ## Deduplication Strategy
 
-1. **File-level**: Files unchanged since last backup are skipped (metadata comparison: size, mtime, mode, uid, gid), unless the file lists a chunk that no uploaded blob holds; such a file is re-chunked
+1. **File-level**: Files and symlinks unchanged since last backup are skipped (metadata comparison: size, mtime, mode, uid, gid, and a symlink's target), unless the file lists a chunk that no uploaded blob holds; such a file is re-chunked
 
 2. **Chunk-level**: Chunks are content-addressed by SHA256 hash. If a chunk hash already exists in the database, the chunk data is not re-uploaded.
 

internal/snapshot/progress_test.go

@@ -78,8 +78,7 @@ func TestProgressPercentWithSecondPathSmaller(t *testing.T) {
 
 	stats := progress.GetStats()
 
-	// Directories count toward the total size but produce no chunks, so
-	// the percentage ends just under 100%.
+	// With both paths counted, the percentage must not exceed 100%.
 	percent := float64(stats.BytesProcessed.Load()) /
 		float64(stats.TotalSize.Load()) * 100
 	if percent > 100 {

internal/snapshot/scanner.go

@@ -284,6 +284,11 @@ func (s *Scanner) Scan(
 		}
 	}
 
+	err = s.recordDirectories(ctx, scanResult.Directories)
+	if err != nil {
+		return nil, err
+	}
+
 	// Summarize scan phase results and update progress
 	s.summarizeScanPhase(result, filesToProcess)
 
@@ -855,14 +860,15 @@ func (s *Scanner) batchInsertFileData(
 // ScanPhaseResult contains the results of the scan phase
 type ScanPhaseResult struct {
 	FilesToProcess   []*FileToProcess
-	UnchangedFileIDs []types.FileID // IDs of unchanged files to associate with snapshot
+	UnchangedFileIDs []types.FileID   // IDs of unchanged files to associate with snapshot
+	Directories      []*FileToProcess // Every directory found, to record
 }
 
 // scanPhase performs the initial directory scan to identify files to process
 // It uses the pre-loaded knownFiles map for fast change detection without DB
 // queries. It also populates existingFiles map for deletion detection.
-// Returns files needing processing and IDs of unchanged files for snapshot
-// association.
+// Returns files needing processing, IDs of unchanged files for snapshot
+// association, and the directories found.
 func (s *Scanner) scanPhase(
 	ctx context.Context,
 	path string,
@@ -870,8 +876,10 @@ func (s *Scanner) scanPhase(
 	existingFiles map[string]struct{},
 	knownFiles map[string]*database.File,
 ) (*ScanPhaseResult, error) {
-	// Use known file count as estimate for progress (accurate for subsequent backups)
-	estimatedTotal := int64(len(knownFiles))
+	// Use known file count as estimate for progress (accurate for
+	// subsequent backups). The walk does not count directories, so the
+	// estimate leaves them out.
+	estimatedTotal := countNonDirectories(knownFiles)
 
 	collector := &scanCollector{}
 
@@ -904,7 +912,7 @@ func (s *Scanner) scanPhase(
 
 		// Handle symlinks and directories
 		handled, err := s.recordSpecialEntry(
-			filePath, info, existingFiles, collector, result)
+			filePath, info, existingFiles, knownFiles, collector, result)
 		if handled {
 			return err
 		}
@@ -918,9 +926,10 @@ func (s *Scanner) scanPhase(
 		existingFiles[filePath] = struct{}{}
 
 		// Check file against in-memory map (no DB query!)
-		file, needsProcessing := s.checkFileInMemory(filePath, info, knownFiles)
+		file := s.buildRegularFileEntry(filePath, info)
+		needsProcessing := s.checkFileInMemory(file, knownFiles)
 
-		filesScanned, changedCount := collector.recordRegularFile(
+		filesScanned, changedCount := collector.recordFile(
 			filePath, info, file, needsProcessing)
 
 		// Update result stats
@@ -944,15 +953,32 @@ func (s *Scanner) scanPhase(
 	return &ScanPhaseResult{
 		FilesToProcess:   collector.filesToProcess,
 		UnchangedFileIDs: collector.unchangedFileIDs,
+		Directories:      collector.directories,
 	}, nil
 }
 
+// countNonDirectories returns how many of files are not directories.
+func countNonDirectories(files map[string]*database.File) int64 {
+	var count int64
+
+	for _, f := range files {
+		if f.Mode&uint32(os.ModeDir) == 0 {
+			count++
+		}
+	}
+
+	return count
+}
+
 // recordSpecialEntry records symlinks and directories (which have no
-// data to chunk) and reports whether it handled the entry. For a symlink
-// whose target cannot be read it returns handleWalkError's result.
+// data to chunk) and reports whether it handled the entry. A symlink is
+// counted and checked for changes as a regular file is; a directory is
+// set aside to be recorded and is not counted. For a symlink whose
+// target cannot be read it returns handleWalkError's result.
 func (s *Scanner) recordSpecialEntry(
 	filePath string, info os.FileInfo,
 	existingFiles map[string]struct{},
+	knownFiles map[string]*database.File,
 	collector *scanCollector, result *ScanResult,
 ) (bool, error) {
 	// Handle symlinks
@@ -963,8 +989,9 @@ func (s *Scanner) recordSpecialEntry(
 		}
 
 		existingFiles[filePath] = struct{}{}
-		collector.addToProcess(filePath, info, file)
-		s.updateScanEntryStats(result, true, info)
+		needsProcessing := s.checkFileInMemory(file, knownFiles)
+		collector.recordFile(filePath, info, file, needsProcessing)
+		s.updateScanEntryStats(result, needsProcessing, info)
 
 		return true, nil
 	}
@@ -974,7 +1001,7 @@ func (s *Scanner) recordSpecialEntry(
 	if info.IsDir() {
 		file := s.buildDirectoryEntry(filePath, info)
 		existingFiles[filePath] = struct{}{}
-		collector.addToProcess(filePath, info, file)
+		collector.addDirectory(filePath, info, file)
 
 		return true, nil
 	}
@@ -1004,28 +1031,29 @@ type scanCollector struct {
 	mu               sync.Mutex
 	filesToProcess   []*FileToProcess
 	unchangedFileIDs []types.FileID // Just IDs - no new records needed
+	directories      []*FileToProcess
 	filesScanned     int64
 }
 
-// addToProcess queues a file entry for processing.
-func (c *scanCollector) addToProcess(
+// addDirectory sets a directory aside to be recorded. It is not counted
+// as a file.
+func (c *scanCollector) addDirectory(
 	path string, info os.FileInfo, file *database.File,
 ) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	c.filesToProcess = append(c.filesToProcess, &FileToProcess{
+	c.directories = append(c.directories, &FileToProcess{
 		Path:     path,
 		FileInfo: info,
 		File:     file,
 	})
-	c.filesScanned++
 }
 
-// recordRegularFile records a regular file as changed (to process) or
-// unchanged (snapshot association only) and returns the running scanned
-// and changed counts.
-func (c *scanCollector) recordRegularFile(
+// recordFile records a regular file or symlink as changed (to process)
+// or unchanged (snapshot association only) and returns the running
+// scanned and changed counts.
+func (c *scanCollector) recordFile(
 	path string, info os.FileInfo, file *database.File, needsProcessing bool,
 ) (int64, int) {
 	c.mu.Lock()
@@ -1128,7 +1156,28 @@ func (s *Scanner) printScanProgressLine(
 	}
 }
 
-// buildSymlinkEntry creates a File record for a symlink.
+// buildRegularFileEntry creates a File record for a regular file, with
+// no ID: checkFileInMemory sets it.
+func (s *Scanner) buildRegularFileEntry(path string, info os.FileInfo) *database.File {
+	var uid, gid uint32
+	if stat, ok := info.Sys().(*syscall.Stat_t); ok {
+		uid = stat.Uid
+		gid = stat.Gid
+	}
+
+	return &database.File{
+		Path:       types.FilePath(path),
+		SourcePath: types.SourcePath(s.currentSourcePath),
+		MTime:      info.ModTime(),
+		Size:       info.Size(),
+		Mode:       uint32(info.Mode()),
+		UID:        uid,
+		GID:        gid,
+	}
+}
+
+// buildSymlinkEntry creates a File record for a symlink, with no ID:
+// checkFileInMemory sets it.
 func (s *Scanner) buildSymlinkEntry(
 	path string, info os.FileInfo,
 ) (*database.File, error) {
@@ -1144,7 +1193,6 @@ func (s *Scanner) buildSymlinkEntry(
 	}
 
 	return &database.File{
-		ID:         types.NewFileID(),
 		Path:       types.FilePath(path),
 		SourcePath: types.SourcePath(s.currentSourcePath),
 		MTime:      info.ModTime(),
@@ -1176,6 +1224,22 @@ func (s *Scanner) buildDirectoryEntry(path string, info os.FileInfo) *database.F
 	}
 }
 
+// recordDirectories records every directory the walk found. A directory
+// has no data to back up, so it is not among the files counted and
+// backed up.
+func (s *Scanner) recordDirectories(
+	ctx context.Context, directories []*FileToProcess,
+) error {
+	for _, dir := range directories {
+		err := s.recordNonRegularFile(ctx, dir)
+		if err != nil {
+			return fmt.Errorf("recording directory %s: %w", dir.Path, err)
+		}
+	}
+
+	return nil
+}
+
 // recordNonRegularFile writes a symlink or directory entry to the database
 // and associates it with the current snapshot. No chunking is performed.
 func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess) error {
@@ -1189,62 +1253,36 @@ func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess)
 	})
 }
 
-// checkFileInMemory checks if a file needs processing using the in-memory map
+// checkFileInMemory reports whether a regular file or symlink needs
+// processing, and gives file the ID of its known record, or a new ID for
+// a new path.
 // No database access is performed - this is purely CPU/memory work
 func (s *Scanner) checkFileInMemory(
-	path string, info os.FileInfo, knownFiles map[string]*database.File,
-) (*database.File, bool) {
-	var uid, gid uint32
-	if stat, ok := info.Sys().(*syscall.Stat_t); ok {
-		uid = stat.Uid
-		gid = stat.Gid
-	}
-
-	// Check against in-memory map first to get existing ID if available
-	existingFile, exists := knownFiles[path]
-
-	// Create file record with ID set upfront
-	// For new files, generate UUID immediately so it's available for chunk associations
-	// For existing files, reuse the existing ID
-	var fileID types.FileID
-	if exists {
-		fileID = existingFile.ID
-	} else {
-		fileID = types.NewFileID()
-	}
-
-	file := &database.File{
-		ID:         fileID,
-		Path:       types.FilePath(path),
-		SourcePath: types.SourcePath(s.currentSourcePath),
-		MTime:      info.ModTime(),
-		Size:       info.Size(),
-		Mode:       uint32(info.Mode()),
-		UID:        uid,
-		GID:        gid,
-	}
+	file *database.File, knownFiles map[string]*database.File,
+) bool {
+	existingFile, exists := knownFiles[file.Path.String()]
 
 	// New file - needs processing
 	if !exists {
-		return file, true
+		file.ID = types.NewFileID()
+
+		return true
 	}
 
+	file.ID = existingFile.ID
+
 	// No uploaded blob holds one of its chunks (see loadFilesToRechunk)
-	if _, rechunk := s.filesToRechunk[fileID]; rechunk {
-		return file, true
+	if _, rechunk := s.filesToRechunk[file.ID]; rechunk {
+		return true
 	}
 
 	// Check if file has changed
-	if existingFile.Size != file.Size ||
+	return existingFile.Size != file.Size ||
 		!existingFile.MTime.Equal(file.MTime) ||
 		existingFile.Mode != file.Mode ||
 		existingFile.UID != file.UID ||
-		existingFile.GID != file.GID {
-		return file, true
-	}
-
-	// File unchanged
-	return file, false
+		existingFile.GID != file.GID ||
+		existingFile.LinkTarget != file.LinkTarget
 }
 
 // batchAddFilesToSnapshot adds existing file IDs to the snapshot association table
@@ -1371,17 +1409,16 @@ func (s *Scanner) processPhase(
 	return s.finalizeProcessPhase(ctx, result)
 }
 
-// processFileWithErrorHandling records a directory or symlink, or wraps
+// processFileWithErrorHandling records a symlink, or wraps
 // processFileStreaming for a regular file with error recovery for deleted
 // files and skip-errors mode. Returns (skipped, error).
 func (s *Scanner) processFileWithErrorHandling(
 	ctx context.Context, fileToProcess *FileToProcess, result *ScanResult,
 ) (bool, error) {
-	// A directory or symlink has no data to open or read; it is only
-	// recorded in the local index. An error recording it stops the run
-	// even under --skip-errors, or the snapshot would complete without it.
-	mode := os.FileMode(fileToProcess.File.Mode)
-	if mode&os.ModeSymlink != 0 || mode.IsDir() {
+	// A symlink has no data to open or read; it is only recorded in the
+	// local index. An error recording it stops the run even under
+	// --skip-errors, or the snapshot would complete without it.
+	if os.FileMode(fileToProcess.File.Mode)&os.ModeSymlink != 0 {
 		err := s.recordNonRegularFile(ctx, fileToProcess)
 		if err != nil {
 			return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)

internal/snapshot/scanner_estimate_test.go

@@ -0,0 +1,28 @@
+//nolint:testpackage // exercises the unexported countNonDirectories helper
+package snapshot
+
+import (
+	"os"
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/database"
+)
+
+// The walk counts regular files and symlinks, not directories, so the
+// estimate of its count made from the local index's records leaves the
+// directories out. See https://git.eeqj.de/sneak/vaultik/issues/305.
+func TestCountNonDirectories(t *testing.T) {
+	t.Parallel()
+
+	known := map[string]*database.File{
+		"/src":       {Mode: uint32(os.ModeDir | 0o755)},
+		"/src/sub":   {Mode: uint32(os.ModeDir | 0o755)},
+		"/src/a.txt": {Mode: 0o644},
+		"/src/link":  {Mode: uint32(os.ModeSymlink | 0o777), LinkTarget: "a.txt"},
+	}
+
+	got := countNonDirectories(known)
+	if got != 2 {
+		t.Errorf("countNonDirectories = %d, want 2", got)
+	}
+}

internal/vaultik/snapshot_summary_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 	"sneak.berlin/go/vaultik/internal/config"
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
@@ -188,6 +189,42 @@ func (e *summaryEnv) dataLine(total, backedUp int64) string {
 		e.v.UI.Size(total), e.v.UI.Size(backedUp))
 }
 
+// enumeratedLine returns the line printed when the walk of a source path
+// ends.
+func (e *summaryEnv) enumeratedLine(
+	files int, total int64, toBackUp int, toBackUpSize int64,
+) string {
+	return fmt.Sprintf(
+		"Enumerated %d snapshot source files (%s total), %d to back up (%s).",
+		files, e.v.UI.Size(total), toBackUp, e.v.UI.Size(toBackUpSize))
+}
+
+// snapshotPaths returns the paths the snapshot lists, sorted.
+func (e *summaryEnv) snapshotPaths(t *testing.T, snapshotID string) []string {
+	t.Helper()
+
+	rows, err := e.db.Conn().QueryContext(context.Background(), `
+		SELECT f.path FROM snapshot_files sf JOIN files f ON f.id = sf.file_id
+		WHERE sf.snapshot_id = ? ORDER BY f.path`, snapshotID)
+	require.NoError(t, err)
+
+	defer func() { _ = rows.Close() }()
+
+	var paths []string
+
+	for rows.Next() {
+		var path string
+
+		require.NoError(t, rows.Scan(&path))
+
+		paths = append(paths, path)
+	}
+
+	require.NoError(t, rows.Err())
+
+	return paths
+}
+
 // A first backup stores copy.bin's chunks while backing up a.bin, so
 // copy.bin's chunks are deduplicated within the run. Each file and byte
 // is still counted once.
@@ -248,6 +285,85 @@ func TestSnapshotSummaryIncrementalRunWithDeduplicatedChunks(t *testing.T) {
 	assert.Equal(t, uncompressed, snap.BlobUncompressedSize)
 }
 
+// A backup of a tree with directories and a symlink, none of which
+// changed since the last backup, backs up nothing, and the snapshot
+// still lists every one of them. Both sides of the line that ends the
+// walk count the same entries, the files and the symlink, and no
+// directory's size. See https://git.eeqj.de/sneak/vaultik/issues/305.
+func TestSnapshotSummaryUnchangedRunWithDirectoriesAndSymlink(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newSummaryEnv(t)
+
+	const linkTarget = "a.bin"
+
+	require.NoError(t, os.Symlink(linkTarget,
+		filepath.Join(filepath.Dir(env.aPath), "link")))
+
+	// A symlink's size is the length of its target.
+	total := env.totalSize + int64(len(linkTarget))
+
+	first := env.backUp(t, "first", false)
+	assert.Contains(t, first, env.enumeratedLine(4, total, 4, total))
+	assert.Contains(t, first, filesLine(4, 4, 0))
+
+	second := env.backUp(t, "second", false)
+	assert.Contains(t, second, env.enumeratedLine(4, total, 0, 0))
+	assert.Contains(t, second, filesLine(4, 0, 4))
+	assert.Contains(t, second, env.dataLine(total, 0))
+
+	assert.Equal(t,
+		env.snapshotPaths(t, env.snapshot(t, "first").ID.String()),
+		env.snapshotPaths(t, env.snapshot(t, "second").ID.String()))
+}
+
+// A directory whose mtime changed and a symlink whose target changed are
+// recorded again by the next backup, which counts only the symlink as
+// backed up.
+func TestSnapshotSummaryChangedDirectoryAndSymlink(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newSummaryEnv(t)
+
+	// The local index keeps paths with symlinks resolved.
+	dir, err := filepath.EvalSymlinks(filepath.Dir(env.aPath))
+	require.NoError(t, err)
+
+	// The new link gets the old one's mtime, and a target, which need not
+	// exist, of the old target's length, a symlink's size: only the
+	// target differs.
+	linkTime := unix.NsecToTimeval(
+		time.Date(2029, time.March, 4, 5, 6, 7, 0, time.UTC).UnixNano())
+	link := filepath.Join(dir, "link")
+	require.NoError(t, os.Symlink("a.bin", link))
+	require.NoError(t, unix.Lutimes(link, []unix.Timeval{linkTime, linkTime}))
+
+	env.backUp(t, "first", false)
+
+	require.NoError(t, os.Remove(link))
+	require.NoError(t, os.Symlink("b.bin", link))
+	require.NoError(t, unix.Lutimes(link, []unix.Timeval{linkTime, linkTime}))
+
+	mtime := time.Date(2030, time.January, 2, 3, 4, 5, 0, time.UTC)
+	require.NoError(t, os.Chtimes(dir, mtime, mtime))
+
+	assert.Contains(t, env.backUp(t, "second", false), filesLine(4, 1, 3))
+
+	ctx := context.Background()
+
+	dirRow, err := env.repos.Files.GetByPath(ctx, dir)
+	require.NoError(t, err)
+	require.NotNil(t, dirRow)
+	assert.Equal(t, mtime.Unix(), dirRow.MTime.Unix())
+
+	linkRow, err := env.repos.Files.GetByPath(ctx, link)
+	require.NoError(t, err)
+	require.NotNil(t, linkRow)
+	assert.Equal(t, "b.bin", linkRow.LinkTarget.String())
+}
+
 // Under --cron the progress reporter is off; the upload figures must
 // still reach the summary and the snapshots row. The snapshot has two
 // paths, each backed up by its own scan.