fix: exFAT scan truncation and ScanStageProperties migration - #1381
Conversation
fastwalk only honours filepath.SkipDir for an entry it typed as a directory or a symlink. Returned for anything else it escapes readDir and aborts iteration of the whole containing directory, dropping every entry after it. On exFAT and FAT the dirent carries no symlink bit, so direntTypesReportSymlinks is false and entryIsSymlink finds links by lstat while fastwalk still sees a regular file. The symlink-excluded and symlink-alias branches then skipped the rest of the directory instead of the one entry. A MiSTer log carried 1450 truncated directories under _Arcade/_alternatives, each losing the alternative MRAs that followed the alias. skipEntry returns the sentinel only when fastwalk will act on it, and nil otherwise, which is the correct skip for an entry fastwalk never descends into. The existing walk_skipdir_test could not catch this: it runs on ext4, where the dirent is typed and fastwalk's own escape hatch applies.
The table and its index were only ever created because sqlClearScanStage failed on the missing table, logged "scan staging tables missing; recreating scratch schema" and called sqlEnsureScanStagingTables to repair it. Every fresh media.db took that error path on its first system. The repair path stays for copied and partially migrated databases; IF NOT EXISTS keeps it a no-op elsewhere. TestMigrations_BrowseSortCollationBumpsVersionWithoutTableWork used a bare goose.Down, which reverts whichever migration happens to be last, so it only tested the collation downgrade while that migration was newest. It now uses DownTo against the version before it.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe changes add a ChangesMedia database migrations
Media scanner traversal
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change prevents exFAT and FAT scans from stopping early after excluded symlink aliases and creates scan-staging schema during migration while preserving recovery behavior for incomplete databases. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant fastwalk
participant filesystem
participant skipEntry
fastwalk->>filesystem: Read directory entries
fastwalk->>skipEntry: Evaluate alias entry
skipEntry-->>fastwalk: Return nil for regular-file dirent
fastwalk->>filesystem: Continue reading sibling media
skipEntry-->>fastwalk: Return filepath.SkipDir for directory or symlink
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
GetFilesreturnedfilepath.SkipDir, which fastwalk only honours for an entry it typed as a directory or a symlink. Returned for anything else it escapesreadDirand aborts iteration of the whole containing directory, silently dropping every entry after the alias.direntTypesReportSymlinksis false andentryIsSymlinkfinds links by lstat while fastwalk still sees a regular file. A MiSTer log from a user carried 1450 truncated directories under_Arcade/_alternatives, each losing the alternative MRAs that followed the alias.skipEntrynow returns the sentinel only when fastwalk will act on it and nil otherwise, which is the correct skip for an entry fastwalk never descends into.walk_skipdir_testcould not catch this because it runs on ext4, where the dirent is typed and fastwalk's own escape hatch applies; the new cases cover the untyped entry and pin the upstream behaviour being worked around.ScanStagePropertiesand its index only ever existed becausesqlClearScanStagefailed on the missing table and calledsqlEnsureScanStagingTablesto repair the scratch schema, so every fresh media.db took that error path on its first system. They now have a migration. The repair path stays for copied and partially migrated databases, andIF NOT EXISTSkeeps it a no-op elsewhere.TestMigrations_BrowseSortCollationBumpsVersionWithoutTableWorkused a baregoose.Down, which reverts whichever migration happens to be last, so it only tested the collation downgrade while that migration was newest. It now usesDownToagainst the version before it.Summary by CodeRabbit
Bug Fixes
Tests