mtkennerly / ludusavi

Backup tool for PC game saves
MIT License
2.81k stars 61 forks source link

Relative paths are no longer evaluated properly #367

Closed Starz0r closed 4 months ago

Starz0r commented 4 months ago

Ludusavi version

v0.24.3

Operating system

Windows

Installation method

Other

Description

On Ludusavi 0.24.x, relative paths are no longer evaluating properly. In Ludusavi 0.22.x, I could run this ludusavi backup --api --force --compression zstd --differential-limit 10 --format zip --full-limit 20 --path .\foobar\, without an error, but after upgrading it's clear something has changed.

I seem to have traced it back to when StrictPath was reworked using git blame, but I'm unsure if that is the cause. Here's what I do know:

  1. Relative paths no longer actually evaluate to a canonical directory. When typing .\foobar\ into the terminal, it gets corrected to .\\foobar\\, which is obvious, it has to escape the slash escape sequences. Perhaps this should just auto-expand to the actual location? (C:\Users\Starz0r\GameSaveBackups\foobar)?

  2. The wrong error is brought up to the user. If you run the command you'll get something like Error: Unable to prepare backup target (either creating or emptying the folder). If you have the folder open in your file browser, try closing it: ./foobar, which doesn't make sense when I'm trying to do a (differential) backup?

  3. Originally I thought the error callsite appeared to be at: https://github.com/mtkennerly/ludusavi/blob/ae6cc0f74fc475084aec0e0153fc976d036b415d/src/scan.rs#L811 but it might be more likely that the error callsite is actually at: https://github.com/mtkennerly/ludusavi/blob/ae6cc0f74fc475084aec0e0153fc976d036b415d/src/path.rs#L178

  4. You can still workaround this issue by providing the actual full path, like so: ludusavi backup --api --force --compression zstd --differential-limit 10 --format zip --full-limit 20 --path C:\Users\Starz0r\GameSaveBackups\foobar. Proving that it's short-circuiting on interpreting the relative path.

Logs

[2024-07-11T17:50:08.137Z] ERROR [ludusavi::scan] Failed to prepare backup target: StrictPath { raw: ".\\foobar\\", basis: None } | Custom { kind: Other, error: "Cannot interpret path: StrictPath { raw: \".\\\\foobar\\\\\", basis: None }" }
mtkennerly commented 4 months ago

Thanks for the detailed report! I'll get this one fixed.

I seem to have traced it back to when StrictPath was reworked using git blame, but I'm unsure if that is the cause.

Yeah, part of the revamp was to treat relative paths as an error, but that wasn't meant to include the paths passed on the CLI.

Starz0r commented 4 months ago

Thanks, appreciate it!