fix: report renames honestly in a dry run
Build and publish container / build (pull_request) Successful in 1m38s

--dry-run described a FAT32 rename as an encode. process() skipped the rename
whenever dry_run was set, then found no file at the target and fell through to
the encode path, so a preview of enabling --fat32-safe announced a full
re-encode of every track whose name held a reserved character. The real run
moves those files in a moment. A preview that inverts the cost of the thing
being previewed is worse than no preview at all.

Prune compounded it. With nothing renamed, the pre-sanitisation files are still
on disk, and they were reported as orphans due for deletion -- so the same dry
run claimed the library would be re-encoded and the originals thrown away,
neither of which is true.

Renames are now their own outcome: reported as "would rename" in a dry run,
counted separately from encodes in the pass summary, and excluded from the
orphan list when a dry run leaves them in place.
This commit is contained in:
Emma Thorpe
2026-08-25 11:34:42 +01:00
parent 1b0097f910
commit 3f50577de6
3 changed files with 86 additions and 6 deletions
+24 -6
View File
@@ -357,7 +357,7 @@ def copy(source, mirror, dry_run):
return Result("copied", mirror)
def adopt_existing(source, mirror, previous):
def adopt_existing(source, mirror, previous, dry_run=False):
"""Move an already-encoded file to its new name. Returns whether it moved.
Turning on FAT32-safe naming changes the path of every track whose name
@@ -367,6 +367,9 @@ def adopt_existing(source, mirror, previous):
"""
if previous == mirror or not previous.is_file() or not is_current(source, previous):
return False
if dry_run:
logger.info("would rename %s -> %s", previous.name, mirror.name)
return True
mirror.parent.mkdir(parents=True, exist_ok=True)
os.replace(previous, mirror)
logger.info("renamed %s -> %s", previous.name, mirror.name)
@@ -375,8 +378,12 @@ def adopt_existing(source, mirror, previous):
def process(source, mirror, quality_args, dry_run, previous=None):
"""Bring one source file's mirror entry up to date."""
if previous is not None and not dry_run and not mirror.exists():
adopt_existing(source, mirror, previous)
# Counted separately from an encode, and reported in a dry run, because the
# difference between moving a file and re-encoding it is the difference
# between a minute and an afternoon.
if previous is not None and not mirror.exists():
if adopt_existing(source, mirror, previous, dry_run):
return Result("renamed", mirror)
if is_current(source, mirror):
# A mirror written before this bit was set has a correct mtime, so
# nothing else in the pass would ever revisit it. Top it up here
@@ -483,7 +490,7 @@ def run_once(
"""
started = time.monotonic()
logger.info("pass starting with %d concurrent encoders", jobs)
counts = {"encoded": 0, "copied": 0, "skipped": 0, "failed": 0}
counts = {"encoded": 0, "copied": 0, "renamed": 0, "skipped": 0, "failed": 0}
failures = []
work = plan(scan_root, source_root, mirror_root, safe)
@@ -506,16 +513,27 @@ def run_once(
if result.action == "failed":
failures.append(result)
removed = prune(mirror_root, set(work), dry_run) if do_prune else 0
expected = set(work)
if safe and dry_run:
# Nothing was actually renamed, so the pre-sanitisation files are still
# on disk. They are not orphans -- they are the files a real run would
# move -- and reporting them for deletion would misrepresent the pass
# twice over.
expected |= {
mirror_path_for(source, source_root, mirror_root) for source in work.values()
}
removed = prune(mirror_root, expected, dry_run) if do_prune else 0
for failure in failures:
logger.error("failed: %s: %s", failure.path, failure.error)
logger.info(
"pass complete in %.1fs: %d encoded, %d copied, %d up to date, %d removed, %d failed",
"pass complete in %.1fs: %d encoded, %d copied, %d renamed, %d up to date,"
" %d removed, %d failed",
time.monotonic() - started,
counts["encoded"],
counts["copied"],
counts["renamed"],
counts["skipped"],
removed,
counts["failed"],