Thirteen agents triaging src/labels and src/disc died on a saturated
machine, leaving 5,836 insertions across 28 files uncommitted in a
worktree. Recovered by 3-way apply onto twelve commits of drift; zero
conflicts. The diff was archived to freemkv-private first, because a
worktree is not a backup and this one had already nearly been lost.
One production change, and it is the right one: mpls_universal::parse
read every playlist off the disc AND converted the entries to labels in
a single function, so the conversion — stream-type mapping, dedup key,
the dense global counters — could only be reached through a synthetic
UDF image. Extracted to build_labels(&[Playlist]), which unit tests can
drive from already-parsed values. Behaviour-preserving: same iteration
order, same skip-on-error.
Two collisions resolved by hand:
A second mod pass_progress_tests, written independently against the
same survivors as the one committed in c610285. Kept mine — it covers
the distinct-counters case and the Progress blanket impl, which theirs
does not — but theirs had three clamp tests mine lacked: good_pct,
bad_pct and pending_pct also clamp an overshoot, and I had only tested
that for work_pct. Merged those in as one test and proved each of the
three clamps load-bearing by removing them individually.
An unused_parens warning in a new fixture.
Method note, recorded because it cost real time: git apply --3way
STAGES its result, so `git diff` reads empty and the tree looks
untouched. I nearly concluded the patch had silently failed. Worse, the
first attempt piped through `head -20`, so `echo exit=$?` reported
head's status rather than git's — the same mistake this audit has
already documented once. Check the real exit status, and check
--cached, not just the working tree.
Mutation testing left both unconstrained.
sector/mod.rs — set_speed on the Box<dyn> and &mut dyn forwarding impls
could be replaced with an empty body and nothing failed. This one hides
better than the read methods because the trait's own default body is
already a no-op, so a forwarder that swallowed the call is
indistinguishable from a source with no speed control. Consequence is a
silently absent value, not a wrong one: the recovery path lowers read
speed through a damaged region, and a swallowed call leaves the drive
at full speed while the caller believes it slowed down. Routed through
a generic S: SectorSource bound, since a direct call on a &mut dyn
receiver auto-derefs to the vtable and never enters the forwarding body.
progress.rs — 42 survivors. All four percentage accessors could return
a constant, read the wrong byte counter, or have their divide-by-zero
guard inverted. Added exact-value tests (25%, not 'some percentage'),
both sides of each guard, the overshoot clamp, and one test setting all
three disc counters to distinct values at once — without it, a swapped
field still passes every single-counter test.
The Progress blanket impl for closures could return a constant true.
That return value is the cancellation signal, so a constant-true body
makes every closure-based consumer uncancellable.
Each mutation applied, observed red, reverted.
The Windows UI needs current winsafe, whose real minimum is 1.89 (its manifest
under-declares 1.87 while it uses NonNull::from_ref). Rather than stop at the
minimum, this goes to current stable and fixes what that costs.
The counter-intuitive result: 1.97 is CHEAPER than 1.89. libfreemkv had 54
clippy errors at 1.89 and 6 at 1.97, because clippy tightened the noisy
collapsible_if lint in between. Stopping at the minimum would have been the most
expensive choice available.
Roughly 47 lints across the eight repos, the large majority auto-fixed:
libfreemkv 6, freemkv-engine 14, bdemu 8, freemkv-keysources 7, autorip 6,
freemkv-unlock 3, freemkv-i18n 3. The hand-fixed ones are a descending sort to
sort_by_key(Reverse), four manual checked-division sites, a loop counter replaced
by enumerate, and a loop whose first let-else became a while-let.
Worth recording for whoever bumps next: clippy is MSRV-AWARE. Those 54 lints only
appear once the crate DECLARES 1.89 or later, because let-chains become
available. A bare `cargo +1.89 clippy` against a manifest still pinned at 1.87
reports clean and is meaningless — gate with the real precommit script, which is
also the only thing that covers build scripts.
The pin still sits below the Mac default, so it keeps doing its job: catching
lint drift locally before CI sees it.
Disc::sweep, Disc::patch, Disc::copy, SweepOptions and PatchOptions have
zero occurrences in src/ — recovery moved to freemkv-engine — but they were
still documented in 30 places across README.md, TROUBLESHOOTING.md, six
files under docs/, seven src/ doc comments and a Cargo.toml comment.
README.md is the crate's GitHub front page and carried a full multi-pass
code example that cannot compile.
Two of the src/ references were intra-doc LINKS to deleted items
([`disc::Disc::copy`], [`disc::Disc::patch`] in scsi/mod.rs). They produced
no warning on a normal `cargo doc` only because they sit on pub(crate)
items; `--document-private-items` reports both, and they are gone now.
The README example is deleted rather than rewritten against the engine's
API: libfreemkv documenting a downstream crate's API on its own front page
is the drift that produced this, and it cannot even depend on it. The src/
references become plain code spans naming freemkv_engine::recovery::* —
deliberately not links, for the same reason.
docs/rip-recovery.md was 202 lines about relocated code. It now documents
only what this crate owns — Drive::read, SenseFamily, DiscStream's adaptive
batch halving — plus the read-path design constraints, which belong with the
code that enforces them, and points at freemkv-engine/src/recovery/ for the
strategy. api-design.md's module tree is regenerated from the real src/disc/
and src/drive/ layouts instead of hand-patched; it had listed sweep.rs,
patch.rs, mapfile.rs and read_error.rs, none of which exist.
Three stale facts surfaced while rewriting and are corrected: the read
timeouts are 10 s / 60 s, not the documented 1.5 s / 30 s; Drive::reset and
SgIoTransport::reset no longer exist at all, so "no SCSI reset from any read
path" is now stated as the stronger fact it has become; and verify_title,
listed as a progress-emitting operation, was removed entirely.
CHANGELOG.md keeps its references — those are the historical record of the
releases that shipped the API.
Add LocatedRange + LocatedProgress to the progress contract and a 'located'
field on PassProgress, populated by the sweep + patch emitters from the
in-memory bad-range set + title. Move the range->chapter/time annotation
(locate_ranges, range_chapter, byte_offset_in_title) into the library so a
client renders the disc map + at-risk movie time straight from PassProgress
and never reads the mapfile itself — if the mapfile becomes a mapdb, this type
and its producer change, clients don't.
PassProgress is no longer Copy (located carries a Vec); it's built once per
throttled emission and passed by reference. Non-locating phases (verify,
extract) emit LocatedProgress::default(). Adds consts::MILLIS_PER_SEC.
Consumer-side wiring (autorip drops Mapfile::load) follows.
Add PassProgress::bytes_retryable_total (NonTrimmed/NonScraped — failed
and awaiting retry), distinct from bytes_pending_total which also folds
in not-yet-attempted (NonTried) bytes. Set it at every construction site
(Sweep from the snapshot, Patch from stats, 0 for sequential/placeholder
paths). The disc-level 'lost' display in the CLI can now use
unreadable+retryable instead of unreadable+pending, so a healthy
in-progress rip no longer reports its unread remainder as lost.
Library-wide review-and-fix pass: tightened AACS keydb/handshake/variant
handling and trailing-partial-unit policy, corrected MPLS mark offset and
added UDF allocation bounds, hardened the mux/codec framing and M2TS paths,
guarded SCSI READ CAPACITY short transfers and unified error mapping, added
overflow guards on untrusted disc input, and made prefetch shutdown
deterministic. Release profile now builds with thin LTO + single codegen unit.
- Adaptive probe algorithm in Disc::copy skip_on_error mode: after 4
consecutive errors, probe 1 sector at 256x batch (8 MB) ahead. If
good, zero-fill gap, mark NonTrimmed, jump. Clears bad zones in
seconds instead of hours.
- NOT_READY sense key (0x02) now retries up to 3x with 3s pause before
marking NonTrimmed. BU40N returns NOT READY for bad sectors, not
MEDIUM ERROR.
- PassProgress struct gains bytes_bad_total field for consumer-side
bad/retryable byte counts.
- Mapfile header version string fix: no longer duplicates 'libfreemkv v'
prefix on each write.
- Structured sense_key/asc/ascq logging in copy error path.
Pre-0.13.16 the rip API leaked internal mapfile concepts (pos,
bytes_good, work_done, bytes_pending, Finished/NonTrimmed) into per-pass
positional callbacks Fn(u64, u64, u64). Consumers reinvented the math
each time, and v0.13.15's UI bug surfaced exactly because of this —
autorip's web JS computed pct from bytes_good while the backend
computed from pos, silent drift, frozen UI bar.
This release replaces both Disc::copy::on_progress and
Disc::patch::on_progress callbacks with a single Progress trait +
PassProgress struct (new progress module).
pub struct PassProgress {
pub kind: PassKind, // Sweep | Trim {reverse} | Scrape {reverse} | Mux
pub work_done: u64,
pub work_total: u64,
pub bytes_good_total: u64,
pub bytes_total_disc: u64,
}
pub trait Progress {
fn report(&self, p: &PassProgress);
}
impl<F: Fn(&PassProgress)> Progress for F { ... } // closures work directly
CopyOptions::on_progress and PatchOptions::on_progress are renamed to
progress: Option<&dyn Progress>. Closure callers update trivially via
the blanket impl; struct callers gain a clean named-field shape with no
positional-arg confusion.
PassKind carries the semantic (sweep vs trim vs scrape vs mux) so
consumers can label phases without reinventing detection logic.
Disc::patch reports Trim {reverse} for retry passes with block_sectors
>= 2 and Scrape {reverse} when block_sectors == 1. Direction comes
through reverse: bool. Mux variant is reserved for v0.13.17 when the
mux pipeline emits progress.
Tests + clippy clean across all 4 crates.
Matthew Jackson