Open the database read-only for report and trees (closes #8) #72
@@ -152,16 +152,28 @@ All three subcommands operate on a single SQLite database file:
|
||||
use. `report` and `trees` require an existing database; a missing
|
||||
database file is a fatal error (exit 1) telling the user to run
|
||||
`scan` first.
|
||||
- The database uses WAL journal mode and a busy timeout, so running a
|
||||
report while a cron `scan` is in progress is safe. The filesystem
|
||||
is authoritative; the database is an eventually-consistent
|
||||
reflection of it. Hashed records are committed in batched
|
||||
transactions while the scan is still running (keeping the WAL
|
||||
small and letting concurrent reports observe progress), so a
|
||||
report may see a scan's changes partially applied, and a scan
|
||||
that dies partway leaves a valid database holding everything
|
||||
hashed so far; the next scan skips those records and converges
|
||||
toward the filesystem.
|
||||
- While `scan` runs, the database is in WAL journal mode with a busy
|
||||
timeout, so running a report while a cron `scan` is in progress is
|
||||
safe. The filesystem is authoritative; the database is an
|
||||
eventually-consistent reflection of it. Hashed records are
|
||||
committed in batched transactions while the scan is still running
|
||||
(keeping the WAL small and letting concurrent reports observe
|
||||
progress), so a report may see a scan's changes partially applied,
|
||||
and a scan that dies partway leaves a valid database holding
|
||||
everything hashed so far; the next scan skips those records and
|
||||
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
|
||||
database with any other version is a fatal error):
|
||||
|
||||
|
||||
@@ -29,6 +29,10 @@
|
||||
|
||||
# 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,
|
||||
trees and warning paths; the root directory's path is `/`
|
||||
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7)
|
||||
|
||||
@@ -74,16 +74,24 @@ func databasePath() string {
|
||||
return defaultDatabasePath
|
||||
}
|
||||
|
||||
// openDB opens the SQLite database at path with WAL journaling and a
|
||||
// busy timeout, so a report can run while a cron scan is in progress.
|
||||
// It does not create or verify the schema.
|
||||
func openDB(path string) (*sql.DB, error) {
|
||||
dsn := "file:" + path +
|
||||
"?_pragma=busy_timeout(10000)" +
|
||||
// scanParams are the connection parameters for scan: read-write, with
|
||||
// WAL journaling and a busy timeout, so a report can run while a cron
|
||||
// scan is in progress. closeScanDatabase leaves WAL mode again.
|
||||
const scanParams = "_pragma=busy_timeout(10000)" +
|
||||
"&_pragma=journal_mode(WAL)" +
|
||||
"&_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 {
|
||||
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)
|
||||
}
|
||||
|
||||
db, err := openDB(path)
|
||||
db, err := openDB(path, scanParams)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -119,6 +127,24 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
|
||||
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
|
||||
// trees subcommands. A missing database file is an error directing the
|
||||
// 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)
|
||||
}
|
||||
|
||||
db, err := openDB(path)
|
||||
db, err := openDB(path, reportParams)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
+44
@@ -5,6 +5,7 @@ import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strings"
|
||||
@@ -130,6 +131,49 @@ func TestOpenReportDatabaseOK(t *testing.T) {
|
||||
_ = 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) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
+83
-7
@@ -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
|
||||
// and returns a function reading back everything written to it. Only
|
||||
// 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
|
||||
// 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
|
||||
// owns an open database.
|
||||
// owns an open database. It closes the database the way scan does.
|
||||
func brokenDatabase(t *testing.T) string {
|
||||
t.Helper()
|
||||
|
||||
path := testDBPath(t)
|
||||
|
||||
db, err := openDB(path)
|
||||
db, err := openDB(path, scanParams)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -108,10 +146,7 @@ func brokenDatabase(t *testing.T) string {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
err = db.Close()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
closeScanDatabase(t.Context(), db, path)
|
||||
|
||||
return path
|
||||
}
|
||||
@@ -144,7 +179,10 @@ func TestOpenDatabaseKeepsWALWhileOpen(t *testing.T) {
|
||||
|
||||
func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
|
||||
// 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{
|
||||
cmdScan: {cmdScan},
|
||||
cmdReport: {cmdReport},
|
||||
@@ -395,3 +433,41 @@ func TestRunTreesSucceeds(t *testing.T) {
|
||||
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,9 +31,9 @@ type scanRec struct {
|
||||
// loadRecords opens the database and reads every file record for the
|
||||
// report and trees subcommands. Any database problem — including a
|
||||
// missing database — is fatal. The error is returned rather than
|
||||
// exiting, so that the deferred close — which checkpoints the SQLite
|
||||
// WAL — always runs; the database is closed before the caller formats
|
||||
// its output, so it stays closed even if that output fails.
|
||||
// exiting, so that the deferred close always runs; the database is
|
||||
// closed before the caller formats its output, so it stays closed even
|
||||
// if that output fails.
|
||||
func loadRecords(ctx context.Context) ([]scanRec, error) {
|
||||
dbPath := databasePath()
|
||||
|
||||
|
||||
@@ -87,8 +87,9 @@ type fileMeta struct {
|
||||
// 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
|
||||
// are returned rather than exiting, so that the deferred close — which
|
||||
// checkpoints the SQLite WAL — always runs. Cancelling ctx unwinds the
|
||||
// worker pools and aborts the scan with the context's error.
|
||||
// takes the database out of WAL mode — always runs. Cancelling ctx
|
||||
// unwinds the worker pools and aborts the scan with the context's
|
||||
// error.
|
||||
func runScan(ctx context.Context, roots []string, workers int,
|
||||
oneFS bool,
|
||||
) error {
|
||||
@@ -108,7 +109,7 @@ func runScan(ctx context.Context, roots []string, workers int,
|
||||
return err
|
||||
}
|
||||
|
||||
defer func() { _ = db.Close() }()
|
||||
defer closeScanDatabase(ctx, db, dbPath)
|
||||
|
||||
st, err := syncScan(ctx, db, roots, workers, oneFS)
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user