Fix Google Drive save sync reporting success while uploading nothing - #153
Open
ReemX wants to merge 1 commit into
Open
Fix Google Drive save sync reporting success while uploading nothing#153ReemX wants to merge 1 commit into
ReemX wants to merge 1 commit into
Conversation
Three problems made the background save watcher report success every cycle while nothing reached Drive. Ludusavi manifest never loaded. _manifest_path() looked in sff/cloud/data/manifest.yaml but the file ships in sff/data/, and none of the three specs bundled sff/data at all, so it was missing from installed builds either way. get_save_paths() returned an empty list for every app id, which meant the custom save-path half of scan_all_save_locations() found nothing and only Steam userdata got backed up. Games saving outside the Steam remote folder were never included. Fixed the path and added sff/data to the three specs; the manifest now indexes 48525 steam id blocks instead of zero. Drive change detection compared file size only. Save files rewritten in place at a constant length were treated as unchanged forever, so the copy on Drive quietly went stale. The backup metadata JSON hit this every time, since backed_up_at is a fixed-length timestamp, which froze the "last backup" date users see in the restore list. _list_folder_index now pulls md5Checksum and _upload_file_smart compares hashes, falling back to size when Drive has no checksum. Metadata writes bypass the check entirely. Upload failures were invisible. backup_save_location_gdrive discarded upload_folder's return value and returned True regardless, and the watcher passed log_func=None so every [FAIL] line was dropped. An entry whose uploads all failed still counted as backed up. Failures now propagate, a partial upload no longer stamps a fresh backup time, and the watcher logs what went wrong per entry. Also gave each worker thread its own Drive service instead of building 56 of them on the submitting thread, and let the folder cache actually accumulate rather than handing every worker a throwaway copy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-backup to Google Drive logged a successful sync every 10 minutes for two days straight while nothing new actually landed on Drive. Three separate bugs stack up here; the last one is what hides the other two.
Reproduced from a 6.6.6 debug log: 312 watcher cycles, every one of them reporting
Save watcher (gdrive_api): backed up 56 entries, each finishing 11-15 seconds after firing. 56 save folders cannot upload in 11 seconds — that is a list-and-skip pass moving zero bytes.1. The Ludusavi manifest is never loaded
_manifest_path()resolved tosff/cloud/data/manifest.yaml, but the manifest ships insff/data/manifest.yaml(as the README says). On top of that, none ofbuild_sff.spec,build_sff_gui.spec,build_sff_linux.specbundledsff/data, so the file is absent from installed builds regardless —findover an installed 6.6.6 turns up nomanifest.yamlanywhere.Consequence:
get_save_paths()returns[]for every app id, the Ludusavi block inscan_all_save_locations()(cloud_saves.py:843) contributes nothing, and only Steam userdata plus emulator roots get backed up. Any game that saves under Documents, AppData or its own install folder is silently skipped, which is exactly the case the feature was added for.Fixed the path and added
sff/datato all three specs. The index goes from 0 to 48525 steam id blocks; Lies of P now resolves toLiesofP/Saved/SaveGames/*as intended.2. Drive sync decides "unchanged" from file size alone
_upload_file_smart:Name plus byte size matching means skip — no mtime, no checksum. A save file rewritten in place at a constant length (fixed-width slots, container formats) is treated as unchanged forever and the Drive copy goes stale without a word. Worth noting the local provider gets this right (
_copy_source_to_destchecks size and mtime) and rclone handles it itself; the Drive path is the only one comparing size alone.It also breaks the timestamp users read to judge whether backups are running.
_entry_metawritesbacked_up_atasdatetime.now().isoformat(timespec="seconds"), always 19 characters, sosteamidra_meta.jsonis byte-identical in length every run, gets skipped, and the restore list keeps showing whenever the first backup happened._list_folder_indexnow requestsmd5Checksumand_upload_file_smartcompares hashes, falling back to size only when Drive returns no checksum. Metadata writes passforce=Trueand skip the comparison outright.3. Failures cannot be observed
backup_save_location_gdrivethrew awayupload_folder's return value and returnedTrueunconditionally, and the watcher called it withlog_func=None, sologwaslambda m: Noneand every[FAIL]line went nowhere. An entry whose uploads all failed with 403s still counted toward "backed up N entries". There is no way to tell a working sync from a completely broken one.Failures now propagate, a partial upload no longer stamps a fresh
backed_up_atover content that is not on Drive, and the watcher logs the per-entry failure lines.Also in the watcher:
get_service()was called once per entry inside the submitting thread (56 Drive clients built serially before any work started), and each worker gotdict(folder_cache)from an empty dict that was never written back, so the cache never accumulated. Now one service per worker thread and a shared cache under a lock.Testing
_manifest_path()resolves and indexes the real 18 MB manifest; spot-checked resolution for Lies of P (app 1627720)._upload_file_smartcovered against a stubbed Drive client: same size with different content uploads (the case that regressed), identical content skips,force=Trueuploads anyway, missing checksum falls back to the size comparison, unknown name creates._list_folder_indexexists outsidegoogle_drive.py.I have not rebuilt the installer, so the spec change is verified by reading only — worth a check that
sff/data/manifest.yamllands in_internal/sff/data/in the next build, since the 18 MB manifest does add to the package size.