fix: group-readable mirror output, and atomic copies #4
@@ -51,9 +51,11 @@ The mtime is read _before_ encoding rather than after. A file still being
|
||||
written when the pass reaches it would otherwise be stamped with its final
|
||||
mtime while holding truncated audio, and never be revisited.
|
||||
|
||||
Encodes are written to a temporary file and renamed into place, so an
|
||||
interrupted run cannot leave a truncated MP3 that the next run mistakes for
|
||||
finished work. A lock file in the mirror root stops two passes overlapping.
|
||||
Both encodes and copies are written to a temporary file and renamed into place,
|
||||
so an interrupted run cannot leave a truncated MP3 that the next run mistakes
|
||||
for finished work. Copies need it as much as encodes do: the mtime comes across
|
||||
with the bytes, so a half-written copy would look current for ever. A lock file
|
||||
in the mirror root stops two passes overlapping.
|
||||
|
||||
### Permissions
|
||||
|
||||
|
||||
+15
-3
@@ -264,19 +264,31 @@ def encode(source, mirror, quality_args, dry_run):
|
||||
|
||||
|
||||
def copy(source, mirror, dry_run):
|
||||
"""Copy an already-MP3 source into the mirror."""
|
||||
"""Copy an already-MP3 source into the mirror, atomically."""
|
||||
if dry_run:
|
||||
logger.info("would copy %s", source)
|
||||
return Result("copied", mirror)
|
||||
|
||||
mirror.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Through a temporary file and a rename, for the same reason encodes go
|
||||
# that way, and a sharper one: copy2 reproduces the source's mtime as well
|
||||
# as its bytes, so a copy cut short by a full disk or a killed container
|
||||
# would leave a truncated MP3 that every later pass reads as current.
|
||||
handle, temporary = tempfile.mkstemp(dir=mirror.parent, suffix=".mp3.part")
|
||||
os.close(handle)
|
||||
temporary = Path(temporary)
|
||||
|
||||
try:
|
||||
shutil.copy2(source, mirror)
|
||||
shutil.copy2(source, temporary)
|
||||
# copy2 brings the source's mode with it, and the source library is not
|
||||
# ours to have permissions opinions about.
|
||||
make_group_readable(mirror)
|
||||
make_group_readable(temporary)
|
||||
os.replace(temporary, mirror)
|
||||
except OSError as error:
|
||||
return Result("failed", source, str(error))
|
||||
finally:
|
||||
temporary.unlink(missing_ok=True)
|
||||
|
||||
logger.info("copied %s", source)
|
||||
return Result("copied", mirror)
|
||||
|
||||
@@ -3,6 +3,7 @@ import shutil
|
||||
import stat
|
||||
import subprocess
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -154,6 +155,30 @@ def test_existing_mp3_is_copied_not_re_encoded(tmp_path, make_flac):
|
||||
assert (mirror / "b.mp3").read_bytes() == (source / "b.mp3").read_bytes()
|
||||
|
||||
|
||||
def test_interrupted_copy_leaves_nothing_behind(tmp_path, make_flac, monkeypatch):
|
||||
"""copy2 reproduces the source mtime, so a truncated copy left in the mirror
|
||||
would be read as current by every later pass."""
|
||||
source = tmp_path / "src"
|
||||
mirror = tmp_path / "dst"
|
||||
flac = make_flac(source / "a.flac")
|
||||
subprocess.run(
|
||||
["ffmpeg", "-loglevel", "error", "-y", "-i", str(flac), str(source / "b.mp3")],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
flac.unlink()
|
||||
|
||||
def truncated(src, destination, **kwargs):
|
||||
Path(destination).write_bytes(Path(src).read_bytes()[:64])
|
||||
raise OSError("no space left on device")
|
||||
|
||||
monkeypatch.setattr(music_mirror.shutil, "copy2", truncated)
|
||||
|
||||
assert run(source, mirror) == 1
|
||||
assert not (mirror / "b.mp3").exists()
|
||||
assert list(mirror.rglob("*.part")) == []
|
||||
|
||||
|
||||
def test_encoded_file_is_group_readable(tmp_path, make_flac, tight_umask):
|
||||
"""mkstemp creates 0600 whatever the umask, so the bit has to be added."""
|
||||
source = tmp_path / "src"
|
||||
|
||||
Reference in New Issue
Block a user