fix: cost the device prefix exactly rather than approximately
Build and publish container / build (pull_request) Successful in 3m54s
Build and publish container / build (pull_request) Successful in 3m54s
The budget subtracted the prefix length plus two, on the assumption of a leading and a trailing slash. That is right for /Music and wrong for an empty prefix, where there is only one slash -- losing a character at the card root, which is exactly where the longest paths sit. Computed from the prefix as it will actually appear instead: /Music/ costs seven characters and gives a mirror-relative budget of 253, the root costs one and gives 259. Worth being exact about because the reverse error is worse. A checker comparing mirror-relative paths against the flat 260 passes everything between 253 and 260, and those are precisely the paths closest to the edge.
This commit is contained in:
@@ -241,7 +241,16 @@ run would move rather than what it would delete.
|
|||||||
Rockbox's `MAX_PATH` is 260, from `firmware/include/fs_defines.h`, and it bounds
|
Rockbox's `MAX_PATH` is 260, from `firmware/include/fs_defines.h`, and it bounds
|
||||||
the path *as the device sees it*. The directory the mirror is copied into comes
|
the path *as the device sees it*. The directory the mirror is copied into comes
|
||||||
out of the same budget, so `--device-prefix` (default `/Music`) is subtracted
|
out of the same budget, so `--device-prefix` (default `/Music`) is subtracted
|
||||||
from `--max-path` to get what a mirror-relative path may spend.
|
from `--max-path` to get what a mirror-relative path may spend:
|
||||||
|
|
||||||
|
| Destination on the device | Mirror-relative budget |
|
||||||
|
| ------------------------- | ---------------------- |
|
||||||
|
| `/Music/` | 253 |
|
||||||
|
| the card root | 259 |
|
||||||
|
|
||||||
|
Worth being exact about, because a checker that measures mirror-relative paths
|
||||||
|
against the flat 260 quietly passes everything from 253 to 260 — and those are
|
||||||
|
the paths most likely to be near the edge in the first place.
|
||||||
|
|
||||||
Over-budget paths are shortened from the **deepest component outward**: the
|
Over-budget paths are shortened from the **deepest component outward**: the
|
||||||
track name carries the least navigational value and the artist directory the
|
track name carries the least navigational value and the artist directory the
|
||||||
|
|||||||
+13
-1
@@ -167,6 +167,18 @@ def fat32_safe(component):
|
|||||||
return cleaned or "_"
|
return cleaned or "_"
|
||||||
|
|
||||||
|
|
||||||
|
def device_prefix_length(prefix):
|
||||||
|
"""Return the on-device prefix as it will actually appear, with slashes.
|
||||||
|
|
||||||
|
"/Music" costs seven characters -- the leading slash, the name, and the
|
||||||
|
separator before the mirror's own path -- while an empty prefix costs one.
|
||||||
|
Approximating that loses a character at the root, which is precisely where
|
||||||
|
the longest paths are.
|
||||||
|
"""
|
||||||
|
cleaned = prefix.strip("/")
|
||||||
|
return f"/{cleaned}/" if cleaned else "/"
|
||||||
|
|
||||||
|
|
||||||
def shorten_component(component, budget):
|
def shorten_component(component, budget):
|
||||||
"""Return a component of at most `budget` characters, cut from the middle.
|
"""Return a component of at most `budget` characters, cut from the middle.
|
||||||
|
|
||||||
@@ -783,7 +795,7 @@ def main(argv=None):
|
|||||||
|
|
||||||
# The device's limit covers the whole path it will see, so what the mirror
|
# The device's limit covers the whole path it will see, so what the mirror
|
||||||
# may spend is that less the directory it gets copied into.
|
# may spend is that less the directory it gets copied into.
|
||||||
budget = max(0, args.max_path - len(args.device_prefix.strip("/")) - 2)
|
budget = max(0, args.max_path - len(device_prefix_length(args.device_prefix)))
|
||||||
if args.fat32_safe:
|
if args.fat32_safe:
|
||||||
logger.info(
|
logger.info(
|
||||||
"paths are limited to %d characters, from --max-path %d less the %r prefix",
|
"paths are limited to %d characters, from --max-path %d less the %r prefix",
|
||||||
|
|||||||
@@ -764,3 +764,25 @@ def test_a_long_title_keeps_both_ends():
|
|||||||
assert len(str(fitted)) <= 253
|
assert len(str(fitted)) <= 253
|
||||||
assert fitted.name.startswith("The Beatles - Abbey Road - 09 - The Long One")
|
assert fitted.name.startswith("The Beatles - Abbey Road - 09 - The Long One")
|
||||||
assert fitted.name.endswith("The End.mp3")
|
assert fitted.name.endswith("The End.mp3")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("prefix", "expected"),
|
||||||
|
[("/Music", 253), ("Music", 253), ("/Music/", 253), ("", 259), ("/", 259)],
|
||||||
|
)
|
||||||
|
def test_the_device_prefix_is_costed_exactly(prefix, expected):
|
||||||
|
"""A mirror-relative path of 253 characters becomes 260 on the device once
|
||||||
|
/Music/ is in front of it, which is the whole of the limit. Approximating
|
||||||
|
the prefix loses a character at the root, where the longest paths are."""
|
||||||
|
assert 260 - len(music_mirror.device_prefix_length(prefix)) == expected
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_budget_is_reported_so_it_can_be_checked(tmp_path, make_flac, caplog):
|
||||||
|
source = tmp_path / "src"
|
||||||
|
mirror = tmp_path / "dst"
|
||||||
|
make_flac(source / "a.flac")
|
||||||
|
|
||||||
|
with caplog.at_level("INFO"):
|
||||||
|
run(source, mirror, "--fat32-safe")
|
||||||
|
|
||||||
|
assert "limited to 253 characters" in caplog.text
|
||||||
|
|||||||
+13
-1
@@ -28,6 +28,18 @@ PATH_LIMIT = 260
|
|||||||
DEVICE_PREFIX = "/Music"
|
DEVICE_PREFIX = "/Music"
|
||||||
|
|
||||||
|
|
||||||
|
def device_prefix_length(prefix):
|
||||||
|
"""Return the on-device prefix as it will actually appear, with slashes.
|
||||||
|
|
||||||
|
"/Music" costs seven characters -- the leading slash, the name, and the
|
||||||
|
separator before the mirror's own path -- while an empty prefix costs one.
|
||||||
|
Approximating that loses a character at the root, which is precisely where
|
||||||
|
the longest paths are.
|
||||||
|
"""
|
||||||
|
cleaned = prefix.strip("/")
|
||||||
|
return f"/{cleaned}/" if cleaned else "/"
|
||||||
|
|
||||||
|
|
||||||
def problems_with(relative, budget=PATH_LIMIT):
|
def problems_with(relative, budget=PATH_LIMIT):
|
||||||
"""Return every reason this relative path is unfit for FAT32."""
|
"""Return every reason this relative path is unfit for FAT32."""
|
||||||
found = []
|
found = []
|
||||||
@@ -68,7 +80,7 @@ def main(argv=None):
|
|||||||
f" of the budget (default {DEVICE_PREFIX})",
|
f" of the budget (default {DEVICE_PREFIX})",
|
||||||
)
|
)
|
||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
budget = max(0, args.max_path - len(args.device_prefix.strip("/")) - 2)
|
budget = max(0, args.max_path - len(device_prefix_length(args.device_prefix)))
|
||||||
|
|
||||||
root = Path(args.root)
|
root = Path(args.root)
|
||||||
if not root.is_dir():
|
if not root.is_dir():
|
||||||
|
|||||||
Reference in New Issue
Block a user