Open the database read-only for report and trees (closes #8) #72

Merged
clawbot merged 1 commits from issue-8-read-only-report into next 2026-10-03 16:30:20 +02:00
7 changed files with 197 additions and 34 deletions
+22 -10
View File
@@ -152,16 +152,28 @@ All three subcommands operate on a single SQLite database file:
use. `report` and `trees` require an existing database; a missing use. `report` and `trees` require an existing database; a missing
database file is a fatal error (exit 1) telling the user to run database file is a fatal error (exit 1) telling the user to run
`scan` first. `scan` first.
- The database uses WAL journal mode and a busy timeout, so running a - While `scan` runs, the database is in WAL journal mode with a busy
report while a cron `scan` is in progress is safe. The filesystem timeout, so running a report while a cron `scan` is in progress is
is authoritative; the database is an eventually-consistent safe. The filesystem is authoritative; the database is an
reflection of it. Hashed records are committed in batched eventually-consistent reflection of it. Hashed records are
transactions while the scan is still running (keeping the WAL committed in batched transactions while the scan is still running
small and letting concurrent reports observe progress), so a (keeping the WAL small and letting concurrent reports observe
report may see a scan's changes partially applied, and a scan progress), so a report may see a scan's changes partially applied,
that dies partway leaves a valid database holding everything and a scan that dies partway leaves a valid database holding
hashed so far; the next scan skips those records and converges everything hashed so far; the next scan skips those records and
toward the filesystem. converges toward the filesystem.
- `scan` switches the database back to rollback-journal mode when it
closes it, so between scans the database file alone holds the whole
database. Each switch needs the database to itself: a `scan` that
starts while a report is still reading waits for it up to the
10-second busy timeout, then fails; a `scan` that ends while a
report has the database open warns and leaves the database in WAL
mode until the next scan.
- `report` and `trees` open the database read-only and need only read
access to the database file, and no write access to its directory.
While the database is in WAL mode they also read the `-wal` and
`-shm` files beside it, which SQLite creates with the database
file's permissions.
- Schema (`PRAGMA user_version` is the schema version, currently 1; a - Schema (`PRAGMA user_version` is the schema version, currently 1; a
database with any other version is a fatal error): database with any other version is a fatal error):
+4
View File
@@ -29,6 +29,10 @@
# Completed Steps # Completed Steps
- `report` and `trees` open the database read-only, and `scan` leaves it
out of WAL mode, so reading needs only read access (2026-10-03, closes
https://git.eeqj.de/sneak/sfdupes/issues/8)
- escape tabs, newlines, carriage returns and backslashes in report, - escape tabs, newlines, carriage returns and backslashes in report,
trees and warning paths; the root directory's path is `/` trees and warning paths; the root directory's path is `/`
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7) (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7)
+35 -9
View File
@@ -74,16 +74,24 @@ func databasePath() string {
return defaultDatabasePath return defaultDatabasePath
} }
// openDB opens the SQLite database at path with WAL journaling and a // scanParams are the connection parameters for scan: read-write, with
// busy timeout, so a report can run while a cron scan is in progress. // WAL journaling and a busy timeout, so a report can run while a cron
// It does not create or verify the schema. // scan is in progress. closeScanDatabase leaves WAL mode again.
func openDB(path string) (*sql.DB, error) { const scanParams = "_pragma=busy_timeout(10000)" +
dsn := "file:" + path +
"?_pragma=busy_timeout(10000)" +
"&_pragma=journal_mode(WAL)" + "&_pragma=journal_mode(WAL)" +
"&_pragma=synchronous(NORMAL)" "&_pragma=synchronous(NORMAL)"
db, err := sql.Open("sqlite", dsn) // reportParams are the connection parameters for report and trees:
// read-only, with the same busy timeout. They set no journal mode,
// because setting one is a write.
const reportParams = "mode=ro" +
"&_pragma=busy_timeout(10000)" +
"&_pragma=query_only(1)"
// openDB opens the SQLite database at path with the connection
// parameters params. It does not create or verify the schema.
func openDB(path, params string) (*sql.DB, error) {
db, err := sql.Open("sqlite", "file:"+path+"?"+params)
if err != nil { if err != nil {
return nil, fmt.Errorf("open database %s: %w", path, err) return nil, fmt.Errorf("open database %s: %w", path, err)
} }
@@ -104,7 +112,7 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
return nil, fmt.Errorf("create database directory: %w", err) return nil, fmt.Errorf("create database directory: %w", err)
} }
db, err := openDB(path) db, err := openDB(path, scanParams)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -119,6 +127,24 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
return db, nil return db, nil
} }
// closeScanDatabase switches the database at path from WAL back to
// rollback-journal mode and closes it. Out of WAL mode the database
// file alone holds the whole database, so a reader needs no -wal or
// -shm file beside it, nor write access to create them. The switch
// fails while a report has the database open; the database then stays
// in WAL mode, still readable, until a later scan closes it.
func closeScanDatabase(ctx context.Context, db *sql.DB, path string) {
// Runs on the way out of a cancelled scan too.
_, err := db.ExecContext(context.WithoutCancel(ctx),
"PRAGMA journal_mode = DELETE")
if err != nil {
fmt.Fprintf(os.Stderr, "scan: database %s left in WAL mode: %v\n",
path, err)
}
_ = db.Close()
}
// openReportDatabase opens an existing database for the report and // openReportDatabase opens an existing database for the report and
// trees subcommands. A missing database file is an error directing the // trees subcommands. A missing database file is an error directing the
// user to run scan first; the schema version must match exactly. // user to run scan first; the schema version must match exactly.
@@ -134,7 +160,7 @@ func openReportDatabase(ctx context.Context,
return nil, fmt.Errorf("database: %w", err) return nil, fmt.Errorf("database: %w", err)
} }
db, err := openDB(path) db, err := openDB(path, reportParams)
if err != nil { if err != nil {
return nil, err return nil, err
} }
+44
View File
@@ -5,6 +5,7 @@ import (
"database/sql" "database/sql"
"errors" "errors"
"fmt" "fmt"
"os"
"path/filepath" "path/filepath"
"slices" "slices"
"strings" "strings"
@@ -130,6 +131,49 @@ func TestOpenReportDatabaseOK(t *testing.T) {
_ = db.Close() _ = db.Close()
} }
func TestCloseScanDatabaseWhileReportOpen(t *testing.T) {
t.Parallel()
// A report holding the database open stops scan from taking it out
// of WAL mode. The -wal and -shm files must then stay beside it, so
// that a later report still needs only read access.
path := testDBPath(t)
scanDB, err := openScanDatabase(t.Context(), path)
if err != nil {
t.Fatal(err)
}
reportDB, err := openReportDatabase(t.Context(), path)
if err != nil {
t.Fatal(err)
}
closeScanDatabase(t.Context(), scanDB, path)
_ = reportDB.Close()
_, err = os.Stat(path + "-wal")
if err != nil {
t.Fatalf("no -wal left: the switch out of WAL mode was not "+
"stopped: %v", err)
}
makeReadOnly(t, path)
reportDB, err = openReportDatabase(t.Context(), path)
if err != nil {
t.Fatalf("openReportDatabase: %v", err)
}
defer func() { _ = reportDB.Close() }()
_, err = loadFileRows(t.Context(), reportDB)
if err != nil {
t.Fatalf("loadFileRows: %v", err)
}
}
func TestApplyChangesRoundTrip(t *testing.T) { func TestApplyChangesRoundTrip(t *testing.T) {
t.Parallel() t.Parallel()
+83 -7
View File
@@ -44,6 +44,44 @@ func assertNoSidecars(t *testing.T, path string) {
} }
} }
// makeReadOnly takes write permission away from the database at path,
// from any WAL sidecar beside it, and from their directory, as for a
// user reading a database that a root cron scan keeps. Root ignores
// file permissions, so it skips the test when run as root.
func makeReadOnly(t *testing.T, path string) {
t.Helper()
if os.Geteuid() == 0 {
t.Skip("root ignores file permissions")
}
err := os.Chmod(path, 0o400)
if err != nil {
t.Fatal(err)
}
for _, suffix := range walSuffixes {
err = os.Chmod(path+suffix, 0o400)
if err != nil && !errors.Is(err, fs.ErrNotExist) {
t.Fatal(err)
}
}
dir := filepath.Dir(path)
//nolint:gosec // reaching the database needs the search bit
err = os.Chmod(dir, 0o500)
if err != nil {
t.Fatal(err)
}
// Runs before t.TempDir's own cleanup, which must delete the files.
t.Cleanup(func() {
//nolint:gosec // removing the directory needs its search bit back
_ = os.Chmod(dir, 0o700)
})
}
// captureStdout redirects os.Stdout to a file for the rest of the test // captureStdout redirects os.Stdout to a file for the rest of the test
// and returns a function reading back everything written to it. Only // and returns a function reading back everything written to it. Only
// machine-readable data belongs on stdout (README design goal 4), so // machine-readable data belongs on stdout (README design goal 4), so
@@ -91,13 +129,13 @@ func captureStdout(t *testing.T) func() string {
// brokenDatabase writes a database that opens cleanly and passes the // brokenDatabase writes a database that opens cleanly and passes the
// schema-version check but has no files table, so the first query // schema-version check but has no files table, so the first query
// fails with the database already open: a fatal error on a path that // fails with the database already open: a fatal error on a path that
// owns an open database. // owns an open database. It closes the database the way scan does.
func brokenDatabase(t *testing.T) string { func brokenDatabase(t *testing.T) string {
t.Helper() t.Helper()
path := testDBPath(t) path := testDBPath(t)
db, err := openDB(path) db, err := openDB(path, scanParams)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -108,10 +146,7 @@ func brokenDatabase(t *testing.T) string {
t.Fatal(err) t.Fatal(err)
} }
err = db.Close() closeScanDatabase(t.Context(), db, path)
if err != nil {
t.Fatal(err)
}
return path return path
} }
@@ -144,7 +179,10 @@ func TestOpenDatabaseKeepsWALWhileOpen(t *testing.T) {
func TestRunFatalAfterOpenClosesDatabase(t *testing.T) { func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
// Every subcommand that owns an open database must close it when // Every subcommand that owns an open database must close it when
// it fails: no os.Exit between the open and the return. // it fails: no os.Exit between the open and the return. The
// sidecar check is evidence of the close only for scan: report and
// trees only read a database that is out of WAL mode, which leaves
// nothing on disk whether they close it or not.
cases := map[string][]string{ cases := map[string][]string{
cmdScan: {cmdScan}, cmdScan: {cmdScan},
cmdReport: {cmdReport}, cmdReport: {cmdReport},
@@ -395,3 +433,41 @@ func TestRunTreesSucceeds(t *testing.T) {
assertNoSidecars(t, path) assertNoSidecars(t, path)
} }
func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
// README §Database: report and trees need only read access to the
// database file. With its directory read-only as well, SQLite
// cannot create any file beside it.
path := testDBPath(t)
t.Setenv(databaseEnv, path)
dupes := scanFixture(t)
assertNoSidecars(t, path)
makeReadOnly(t, path)
cases := map[string]string{
cmdReport: "first\tdupe\tsize\n" +
dupes[0] + "\t" + dupes[1] + "\t300\n",
cmdTrees: "first\tdupe\tfiles\tsize\n" +
filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) +
"\t1\t300\n",
}
for name, want := range cases {
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run([]string{name}, &stderr)
if code != exitOK {
t.Errorf("run(%s) = %d, want %d; stderr: %s",
name, code, exitOK, stderr.String())
continue
}
if got := stdout(); got != want {
t.Errorf("%s stdout = %q, want %q", name, got, want)
}
}
}
+3 -3
View File
@@ -31,9 +31,9 @@ type scanRec struct {
// loadRecords opens the database and reads every file record for the // loadRecords opens the database and reads every file record for the
// report and trees subcommands. Any database problem — including a // report and trees subcommands. Any database problem — including a
// missing database — is fatal. The error is returned rather than // missing database — is fatal. The error is returned rather than
// exiting, so that the deferred close — which checkpoints the SQLite // exiting, so that the deferred close always runs; the database is
// WAL — always runs; the database is closed before the caller formats // closed before the caller formats its output, so it stays closed even
// its output, so it stays closed even if that output fails. // if that output fails.
func loadRecords(ctx context.Context) ([]scanRec, error) { func loadRecords(ctx context.Context) ([]scanRec, error) {
dbPath := databasePath() dbPath := databasePath()
+4 -3
View File
@@ -87,8 +87,9 @@ type fileMeta struct {
// hash only when its size, head, and tail match another file's. Flag // hash only when its size, head, and tail match another file's. Flag
// parsing and the at-least-one-operand check are done by cobra. Errors // parsing and the at-least-one-operand check are done by cobra. Errors
// are returned rather than exiting, so that the deferred close — which // are returned rather than exiting, so that the deferred close — which
// checkpoints the SQLite WAL — always runs. Cancelling ctx unwinds the // takes the database out of WAL mode — always runs. Cancelling ctx
// worker pools and aborts the scan with the context's error. // unwinds the worker pools and aborts the scan with the context's
// error.
func runScan(ctx context.Context, roots []string, workers int, func runScan(ctx context.Context, roots []string, workers int,
oneFS bool, oneFS bool,
) error { ) error {
@@ -108,7 +109,7 @@ func runScan(ctx context.Context, roots []string, workers int,
return err return err
} }
defer func() { _ = db.Close() }() defer closeScanDatabase(ctx, db, dbPath)
st, err := syncScan(ctx, db, roots, workers, oneFS) st, err := syncScan(ctx, db, roots, workers, oneFS)
if err != nil { if err != nil {