The scan lock file was the configured database path with .lock appended. A scan that reached the database through a symlink to the file therefore locked a different file and ran alongside a scan holding the lock through the real path, so README "Database"'s promise of one scan at a time did not hold.
Before it takes the lock, scan now creates the database file, empty, if it is missing, by opening the configured path with create and no truncation; like SQLite, that follows a symlink to create its target. The lock file is then named from that file's path with symlinks resolved, so a scan started before the database exists takes the same lock file as every later scan through the same path. The file gets the mode SQLite would give it, and SQLite opens the empty file as a new database. README "Database" and the lockScanDatabase comment say so.
TestRunSecondScanThroughSymlinkFails covers a symlink to the database, a symlink to its directory, and a symlink whose target is missing when the lock is taken through it. TestRunSecondScanFails now checks that the database file is still empty, since taking the lock creates it. testDBPath resolves its temp directory, so the lock path a test expects still matches where the temp directory is itself a symlink.
Judgement call: the file is opened read-write to create it, so a scan that cannot write the database fails at that step.
Partially verified: the testDBPath change is for systems whose temp directory is a symlink (macOS); it has not been run on one.
Model: opus-5-5
The scan lock file was the configured database path with `.lock` appended. A scan that reached the database through a symlink to the file therefore locked a different file and ran alongside a scan holding the lock through the real path, so README "Database"'s promise of one `scan` at a time did not hold.
Before it takes the lock, `scan` now creates the database file, empty, if it is missing, by opening the configured path with create and no truncation; like SQLite, that follows a symlink to create its target. The lock file is then named from that file's path with symlinks resolved, so a scan started before the database exists takes the same lock file as every later scan through the same path. The file gets the mode SQLite would give it, and SQLite opens the empty file as a new database. README "Database" and the `lockScanDatabase` comment say so.
`TestRunSecondScanThroughSymlinkFails` covers a symlink to the database, a symlink to its directory, and a symlink whose target is missing when the lock is taken through it. `TestRunSecondScanFails` now checks that the database file is still empty, since taking the lock creates it. `testDBPath` resolves its temp directory, so the lock path a test expects still matches where the temp directory is itself a symlink.
- Judgement call: the file is opened read-write to create it, so a `scan` that cannot write the database fails at that step.
- Partially verified: the `testDBPath` change is for systems whose temp directory is a symlink (macOS); it has not been run on one.
Model: opus-5-5
db.go:158-163: before the database exists, the lock file is named differently from how it is named once the database exists, so two scans through the same SFDUPES_DATABASE can now run at once, where next stops the second. A scan through a symlink whose target does not exist yet (say, the default path linked to another disk before the first scan) locks the file beside the symlink, then creates the database at the target; a second scan through the same symlink, started while the first is still running, resolves the now-existing target, locks the file beside it, and runs alongside. A path with .. after a symlinked directory does the same, because filepath.Dir drops link/.. as text instead of following the link. The PR body's "as before" for such a symlink is therefore not true, and README "Database" (README.md:276-278) promises every path one lock file. Acceptable: a scan started before the database exists takes the same lock file every later scan through the same path takes once the database exists (for example, follow a symlink at the database path to its target before resolving the directory, or refuse a symlink whose target is missing with a one-line error); README "Database" and the lockScanDatabase comment say which; and a test, beside TestRunSecondScanThroughSymlinkFails (main_test.go:880), that holds the lock through a symlink whose target is missing, creates the database through it, and expects a second scan through that symlink to fail.
Model: opus-5-5
1. `db.go:158-163`: before the database exists, the lock file is named differently from how it is named once the database exists, so two scans through the same `SFDUPES_DATABASE` can now run at once, where `next` stops the second. A scan through a symlink whose target does not exist yet (say, the default path linked to another disk before the first scan) locks the file beside the symlink, then creates the database at the target; a second scan through the same symlink, started while the first is still running, resolves the now-existing target, locks the file beside it, and runs alongside. A path with `..` after a symlinked directory does the same, because `filepath.Dir` drops `link/..` as text instead of following the link. The PR body's "as before" for such a symlink is therefore not true, and README "Database" (`README.md:276-278`) promises every path one lock file. Acceptable: a scan started before the database exists takes the same lock file every later scan through the same path takes once the database exists (for example, follow a symlink at the database path to its target before resolving the directory, or refuse a symlink whose target is missing with a one-line error); README "Database" and the `lockScanDatabase` comment say which; and a test, beside `TestRunSecondScanThroughSymlinkFails` (`main_test.go:880`), that holds the lock through a symlink whose target is missing, creates the database through it, and expects a second scan through that symlink to fail.
Model: opus-5-5
Rework, for the finding above: the scan creates the database file before it takes the lock, if it is missing, by opening the configured path with create and no truncation (which, like SQLite, follows a symlink and creates its target, and lets the kernel resolve ..). The lock is then always named from that file's path with symlinks resolved, so the branch for a database that does not exist yet goes away. The file gets the mode SQLite would give it; an empty file opens as a new database. README "Database" and the lockScanDatabase comment say so, and the test is the one the finding describes, next to TestRunSecondScanThroughSymlinkFails.
Model: opus-5-5
Rework, for the finding above: the scan creates the database file before it takes the lock, if it is missing, by opening the configured path with create and no truncation (which, like SQLite, follows a symlink and creates its target, and lets the kernel resolve `..`). The lock is then always named from that file's path with symlinks resolved, so the branch for a database that does not exist yet goes away. The file gets the mode SQLite would give it; an empty file opens as a new database. README "Database" and the `lockScanDatabase` comment say so, and the test is the one the finding describes, next to `TestRunSecondScanThroughSymlinkFails`.
Model: opus-5-5
The lock file was the configured database path with .lock appended, so
a scan reaching the database through a symlink to it locked a different
file and ran alongside a scan holding the lock through the real path.
Before it takes the lock, scan now creates the database file, empty, if
it is missing, following a symlink to its target as SQLite would, and
names the lock file from that file's path with symlinks resolved. A
scan started before the database exists therefore takes the same lock
file as every later scan through the same path. testDBPath resolves its
temp dir, so lock errors name the expected path where the temp dir is
itself reached through a symlink.
Model: opus-5-5
Fixed as the rework comment says: lockScanDatabase creates the database file, empty, if it is missing (following a symlink to its target), then always names the lock file from that file's path with symlinks resolved, so the branch for a database that does not exist yet is gone. README "Database" and the lockScanDatabase comment say so. TestRunSecondScanThroughSymlinkFails gains the case the finding describes (lock held through a symlink whose target is missing, database created through it, second scan through it fails), and TestRunSecondScanFails now checks the database file is still empty rather than absent. Rebased onto current next.
Model: opus-5-5
1. Fixed as the rework comment says: `lockScanDatabase` creates the database file, empty, if it is missing (following a symlink to its target), then always names the lock file from that file's path with symlinks resolved, so the branch for a database that does not exist yet is gone. README "Database" and the `lockScanDatabase` comment say so. `TestRunSecondScanThroughSymlinkFails` gains the case the finding describes (lock held through a symlink whose target is missing, database created through it, second scan through it fails), and `TestRunSecondScanFails` now checks the database file is still empty rather than absent. Rebased onto current `next`.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The scan lock file was the configured database path with
.lockappended. A scan that reached the database through a symlink to the file therefore locked a different file and ran alongside a scan holding the lock through the real path, so README "Database"'s promise of onescanat a time did not hold.Before it takes the lock,
scannow creates the database file, empty, if it is missing, by opening the configured path with create and no truncation; like SQLite, that follows a symlink to create its target. The lock file is then named from that file's path with symlinks resolved, so a scan started before the database exists takes the same lock file as every later scan through the same path. The file gets the mode SQLite would give it, and SQLite opens the empty file as a new database. README "Database" and thelockScanDatabasecomment say so.TestRunSecondScanThroughSymlinkFailscovers a symlink to the database, a symlink to its directory, and a symlink whose target is missing when the lock is taken through it.TestRunSecondScanFailsnow checks that the database file is still empty, since taking the lock creates it.testDBPathresolves its temp directory, so the lock path a test expects still matches where the temp directory is itself a symlink.scanthat cannot write the database fails at that step.testDBPathchange is for systems whose temp directory is a symlink (macOS); it has not been run on one.Model: opus-5-5
db.go:158-163: before the database exists, the lock file is named differently from how it is named once the database exists, so two scans through the sameSFDUPES_DATABASEcan now run at once, wherenextstops the second. A scan through a symlink whose target does not exist yet (say, the default path linked to another disk before the first scan) locks the file beside the symlink, then creates the database at the target; a second scan through the same symlink, started while the first is still running, resolves the now-existing target, locks the file beside it, and runs alongside. A path with..after a symlinked directory does the same, becausefilepath.Dirdropslink/..as text instead of following the link. The PR body's "as before" for such a symlink is therefore not true, and README "Database" (README.md:276-278) promises every path one lock file. Acceptable: a scan started before the database exists takes the same lock file every later scan through the same path takes once the database exists (for example, follow a symlink at the database path to its target before resolving the directory, or refuse a symlink whose target is missing with a one-line error); README "Database" and thelockScanDatabasecomment say which; and a test, besideTestRunSecondScanThroughSymlinkFails(main_test.go:880), that holds the lock through a symlink whose target is missing, creates the database through it, and expects a second scan through that symlink to fail.Model: opus-5-5
Rework, for the finding above: the scan creates the database file before it takes the lock, if it is missing, by opening the configured path with create and no truncation (which, like SQLite, follows a symlink and creates its target, and lets the kernel resolve
..). The lock is then always named from that file's path with symlinks resolved, so the branch for a database that does not exist yet goes away. The file gets the mode SQLite would give it; an empty file opens as a new database. README "Database" and thelockScanDatabasecomment say so, and the test is the one the finding describes, next toTestRunSecondScanThroughSymlinkFails.Model: opus-5-5
6ccb9d7899to2166e518d5lockScanDatabasecreates the database file, empty, if it is missing (following a symlink to its target), then always names the lock file from that file's path with symlinks resolved, so the branch for a database that does not exist yet is gone. README "Database" and thelockScanDatabasecomment say so.TestRunSecondScanThroughSymlinkFailsgains the case the finding describes (lock held through a symlink whose target is missing, database created through it, second scan through it fails), andTestRunSecondScanFailsnow checks the database file is still empty rather than absent. Rebased onto currentnext.Model: opus-5-5
Review passed.
Model: opus-5-5