diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b5c49b..3ae6904 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ releases may contain breaking changes. - LIFT-folder handling: `RangesFile` (standalone `.lift-ranges` documents, same fidelity guarantees), automatic companion discovery/tracking on load (`Lexicon.ranges_files`), `save()` writes companions together, - `all_ranges()` merged view, `media_refs()` / `missing_media()` helpers, + `all_ranges()` merged view, `media_refs()` / `check_media()` helpers, build-from-scratch helpers `Lexicon.add_ranges_file()` / `RangesFile.add_range()` / `Range.add_element()` (`save()` writes and header-references a new companion beside the `.lift`); vendored @@ -71,7 +71,7 @@ releases may contain breaking changes. file, entry, and line it concerns. RELAX NG layer with two documented departures from strict validation (invalid `file://` hrefs downgraded to `uri-not-rfc` warnings; legal interleaving not falsely flagged); vendored - ranges schema over companions; and twelve semantic checks, one `Problem` + ranges schema over companions; and thirteen semantic checks, one `Problem` code each (with missing-id opt-in via `require_ids`). Every code is described in `docs/en/guides/validate.md`. - Canonical sort: `Lexicon.sort()` / `RangesFile.sort()` (entries by diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 02fa940..961e7a1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,10 +44,11 @@ The fidelity tests assert that saving writes back the **exact bytes** of `tests/corpus/`, or `tests/tools/xslt/`. Even a trailing-newline tweak breaks the suite. `.gitattributes` and `.editorconfig` list exceptions so git and editors leave these files alone — don't remove them. -- Adding a fixture requires an entry in `tests/corpus/PROVENANCE.md`: source - URL, commit SHA, fetch date, license. Hand-authored fixtures (e.g. under - `tests/corpus/negative/`) carry an XML comment documenting the defect and the - expected validator finding. +- Adding a fetched fixture requires an entry in `tests/corpus/PROVENANCE.md`: + source URL, commit SHA, fetch date, license. Hand-authored fixtures under + `tests/corpus/negative/` follow the rule in that file's `negative/` section + instead of getting an entry each, and carry an XML comment documenting the + defect and the expected validator finding. - The migrated `spec-examples/0.13/` files are generated by `tests/tools/migrate_corpus.py`; regenerate rather than edit. diff --git a/docs/en/guides/cli.md b/docs/en/guides/cli.md index 1c1e64d..b9c709e 100644 --- a/docs/en/guides/cli.md +++ b/docs/en/guides/cli.md @@ -9,14 +9,22 @@ sil-lift validate PATH [--format {text,json}] [--strict] [--no-check-media] [--a sil-lift stats PATH [--format {text,json}] entry/sense/language counts (streaming; any size) sil-lift sort PATH [-o OUT] canonically sorted, diff-ready copy (default: in place) -sil-lift check-media PATH missing and orphaned media report; exit 1 if missing +sil-lift check-media PATH missing, misspelled, and orphaned media report; exit 1 if missing or misspelled sil-lift export PATH [-o OUT] [--langs L] [--tsv] one row per leaf sense (subsenses flattened) to CSV/TSV (streaming) ``` -`--format json` writes a single JSON object to stdout (and nothing else) for CI/automation consumption; see the schema in the example below. `--strict` treats warnings as errors, exiting 1 if any are found — use it to gate a build on no warnings at all rather than on errors alone. `--no-check-media` skips the filesystem media-presence check (suppressing `missing-media` findings), which is useful when validating a freshly generated export whose audio/photo files live elsewhere rather than in the same folder. `--require-ids` additionally fails (a `missing-id` error) on any entry lacking a `guid` or sense lacking an `id` — stricter than LIFT, for workflows that re-import by a stable id. Passing `-` as the path reads the document from stdin (a piped document has no folder, so its companion `.lift-ranges` and media are not resolved). `stats` likewise takes `--format json`, emitting the counts as a single JSON object. +`validate`'s options: -`--allow CODE[,CODE...]` (repeatable) reports the given codes' findings but leaves them out of the pass/fail decision. It applies to errors and warnings alike, so an allowed warning does not trip `--strict` either. The summary line tallies allowed findings per code; the JSON summary's `allowed` gives their total. A code that never occurs is ignored, so an allow list keeps working after a check is retired. +- `--format json` writes a single JSON object to stdout (and nothing else) for CI/automation consumption; see the schema in the example below. +- `--strict` treats warnings as errors, exiting 1 if any are found — use it to gate a build on no warnings at all rather than on errors alone. +- `--no-check-media` skips the filesystem media-presence check, suppressing `missing-media` and `media-href-mismatch` findings. Useful when validating a freshly generated export whose audio/photo files live elsewhere rather than in the same folder. +- `--allow CODE[,CODE...]` (repeatable) reports the given codes' findings but leaves them out of the pass/fail decision. It applies to errors and warnings alike, so an allowed warning does not trip `--strict` either. The summary line tallies allowed findings per code; the JSON summary's `allowed` gives their total. A code that never occurs is ignored, so an allow list keeps working after a check is retired. +- `--require-ids` additionally fails (a `missing-id` error) on any entry lacking a `guid` or sense lacking an `id` — stricter than LIFT, for workflows that re-import by a stable id. + +Passing `-` as the path reads the document from stdin. A piped document has no folder, so its companion `.lift-ranges` and media are not resolved. + +`stats` likewise takes `--format json`, emitting the counts as a single JSON object. !!! note `validate`'s exit codes and `--format json` schema are a supported automation interface: both are covered by tests and change only under SemVer. @@ -73,4 +81,4 @@ $ sil-lift export dictionary.lift --langs en,fr -o dictionary.csv All output is UTF-8, on every platform and whether it goes to a console, a pipe, or a `>` redirect — never the locale encoding (cp1252 on Windows, ASCII under a C/POSIX locale), which cannot represent LIFT content. `sil-lift export dictionary.lift > dictionary.csv` therefore writes exactly the bytes `-o dictionary.csv` writes, CRLF row terminators included. -Exit codes: `0` success (warnings do not fail the run unless `--strict`), `1` findings (validation errors / missing media / warnings under `--strict`, not counting codes given to `--allow`), `2` an I/O failure at either end — input that cannot be read, or output that cannot be written (a reader like `head` closing the pipe, a full disk). +Exit codes: `0` success (warnings do not fail the run unless `--strict`), `1` findings (validation errors / missing or misspelled media / warnings under `--strict`, not counting codes given to `--allow`), `2` an I/O failure at either end — input that cannot be read, or output that cannot be written (a reader like `head` closing the pipe, a full disk). diff --git a/docs/en/guides/folder-media.md b/docs/en/guides/folder-media.md index 4e03341..145d8ea 100644 --- a/docs/en/guides/folder-media.md +++ b/docs/en/guides/folder-media.md @@ -41,11 +41,18 @@ Pass `resolve_ranges=False` to `load()` to skip companion discovery. for ref in lex.media_refs(): # every and print(ref.kind, ref.href, ref.entry_id) -lex.missing_media() # refs whose files don't exist +lex.check_media() # refs that don't resolve cleanly ``` Resolution follows the conventional layout: a relative href is checked as given (backslashes normalized — WeSay writes `pictures\photo with space.png`) and under `audio/` (for pronunciation media) or `pictures/` (for illustrations). Remote/absolute hrefs can't be checked and are skipped. +`check_media()` reports only the references that did not resolve cleanly, one `MediaResolution` each: + +- `status="missing"` — no file answered the href under any spelling. +- `status="mismatch"` — one did, but only under case folding or NFC; `found` names the file on disk. + +A href that spells its file exactly is not reported. Unlike a companion name, which folds silently, a media href is also read by whatever serves the folder afterwards, so the fold is reported as [`media-href-mismatch`](validate.md#problem-codes). + ## Other folder contents A LIFT folder often holds files sil-lift doesn't model — writing-system LDML under `WritingSystems/`, The Combine's speaker consent audio/image files under `consent/`, and the like; `load()`/`save()` leave these untouched, and [`Lexicon.save_zip()`](lift-export-interop.md) carries them through verbatim when packaging the folder. diff --git a/docs/en/guides/validate.md b/docs/en/guides/validate.md index 9d52ccb..71346d1 100644 --- a/docs/en/guides/validate.md +++ b/docs/en/guides/validate.md @@ -24,11 +24,11 @@ Each `Problem` carries `level` (`"error"`/`"warning"`), a stable `code`, `messag 1. **RELAX NG** against the LIFT 0.13 grammar (vendored from lift-standard — a byte-identical copy committed into this package). 2. **Ranges schema** — this project's `lift-ranges-0.13.rng` — over every tracked `.lift-ranges` companion, addressed to the companion rather than the `.lift`. -3. **Semantic checks** the grammar cannot express — twelve of them, one code each. +3. **Semantic checks** the grammar cannot express — thirteen of them, one code each. ## Problem codes -Every finding carries one of these, whichever layer produced it — `schema` and `uri-not-rfc` come from the schema layers, the other twelve are semantic checks. The strings are a supported interface; `--strict` promotes every warning to an error. `sil-lift validate --allow CODE` leaves a code out of the pass/fail decision. +Every finding carries one of these, whichever layer produced it — `schema` and `uri-not-rfc` come from the schema layers, the other thirteen are semantic checks. The strings are a supported interface; `--strict` promotes every warning to an error. `sil-lift validate --allow CODE` leaves a code out of the pass/fail decision. | code | level | what it flags | | ------------------------ | ------- | -------------------------------------------------------------------------- | @@ -38,6 +38,7 @@ Every finding carries one of these, whichever layer produced it — `schema` and | `duplicate-form-lang` | warning | two forms in one multitext sharing a language | | `duplicate-guid` | error | a guid reused among entries, or among one document's ranges/range-elements | | `form-missing-lang` | error | a `
` or `` without the `lang` the schema requires | +| `media-href-mismatch` | warning | a media href that reaches its file only under case folding or NFC | | `missing-id` | error | opt-in via `require_ids`: an entry without a guid, a sense without an id | | `missing-media` | warning | a referenced audio or picture file not on disk | | `normalization-mismatch` | warning | a name that reaches the id it refers to only under NFC | @@ -51,7 +52,14 @@ All three layers work from the document serialized as it stands, so one that can A companion name matching several files loads none of them: the ranges they define go absent until all but one is renamed or removed. -The three companion-folder codes (`ambiguous-ranges-file`, `dangling-ranges-href`, and `unreadable-ranges-file`) are reported only when companion discovery ran. Loading with `resolve_ranges=False` puts companions out of scope, so none of them is reported; attaching one afterwards with `add_ranges_file()` does not bring them back, since it never reads the folder these codes report on. `missing-media` is unaffected: media is never resolved into the model, so nothing was opted out of. +The three companion-folder codes (`ambiguous-ranges-file`, `dangling-ranges-href`, and `unreadable-ranges-file`) are reported only when companion discovery ran. Loading with `resolve_ranges=False` puts companions out of scope, so none of them is reported; attaching one afterwards with `add_ranges_file()` does not bring them back, since it never reads the folder these codes report on. `missing-media` and `media-href-mismatch` are unaffected: media is never resolved into the model, so nothing was opted out of. + +`missing-media` and `media-href-mismatch` divide the media check between them: the first means no file answered the href under any spelling, the second that one did but the href does not name it exactly. The second is the portability finding — the file is here, and a case-sensitive host serving this folder will not find it. + +Two rules govern what `media-href-mismatch` reports: + +- A misspelled *folder* is one rename however many references cross it, so it is reported once and carries no entry. A misspelled *filename* is reported per reference, addressed to the entry that wrote it. +- The conventional `audio/`/`pictures/` subfolder is sil-lift's own guess rather than something the document wrote, so a folder spelling it another way resolves without being reported. ## Real-world FieldWorks (FLEx) output diff --git a/src/sil_lift/__init__.py b/src/sil_lift/__init__.py index e8aaa4f..d86c110 100644 --- a/src/sil_lift/__init__.py +++ b/src/sil_lift/__init__.py @@ -21,6 +21,7 @@ GrammaticalInfo, Lexicon, MediaRef, + MediaResolution, Note, Pronunciation, RangesChanges, @@ -62,6 +63,7 @@ "LiftWriteError", "LiftWriter", "MediaRef", + "MediaResolution", "Multitext", "Note", "Problem", diff --git a/src/sil_lift/_cli.py b/src/sil_lift/_cli.py index c4da71e..a239abe 100644 --- a/src/sil_lift/_cli.py +++ b/src/sil_lift/_cli.py @@ -25,9 +25,9 @@ from ._canonical import canonicalize from ._errors import LiftError -from ._model import Lexicon, _normalize_href +from ._model import Lexicon, _folded_entries, _media_matches, _normalize_href from ._stream import open_reader -from ._validate import iter_problems +from ._validate import iter_problems, media_mismatch_groups if TYPE_CHECKING: from collections.abc import Sequence @@ -39,6 +39,9 @@ __all__ = ["main"] +#: What --no-check-media suppresses: everything the filesystem media check says. +_MEDIA_CODES = frozenset({"missing-media", "media-href-mismatch"}) + def _problem_json(problem: Problem) -> dict[str, object]: return { @@ -68,7 +71,7 @@ def _collect_problems(args: argparse.Namespace) -> list[Problem]: return [ problem for problem in problems - if not (args.no_check_media and problem.code == "missing-media") + if not (args.no_check_media and problem.code in _MEDIA_CODES) ] @@ -168,25 +171,42 @@ def _cmd_sort(args: argparse.Namespace) -> int: def _cmd_check_media(args: argparse.Namespace) -> int: lexicon = Lexicon.load(args.path) - missing = lexicon.missing_media() - for ref in missing: - owner = ref.entry_id or ref.entry_guid or "?" - print(f"missing {ref.kind:12s} {ref.href!r} (entry {owner})") - - referenced: set[Path] = set() base = lexicon.path.parent if lexicon.path is not None else Path(args.path).parent + resolutions = lexicon.check_media() + missing = [item for item in resolutions if item.status == "missing"] + for item in missing: + owner = item.ref.entry_id or item.ref.entry_guid or "?" + print(f"missing {item.ref.kind:12s} {item.ref.href!r} (entry {owner})") + directories, files = media_mismatch_groups(resolutions, base) + # Spellings differing only in normalization render identically. + for written, on_disk in directories: + print(f"mismatch {'folder':12s} {written!a} is {on_disk!a} on disk") + for item, written, on_disk in files: + owner = item.ref.entry_id or item.ref.entry_guid or "?" + print(f"mismatch {item.ref.kind:12s} {written!a} is {on_disk!a} on disk (entry {owner})") + + # The files the hrefs reach, under the folding check_media() uses, rather + # than the paths they spell: a file named inexactly is in use, not orphaned. + referenced: set[Path] = set() + listings: dict[Path, dict[str, list[Path]]] = {} for ref in lexicon.media_refs(): relative = _normalize_href(ref.href) - if relative is None: # remote/absolute hrefs can't confirm a local file + if relative is None or not relative.parts: # remote/absolute/empty: no local file continue - referenced.add((base / relative).resolve()) subfolder = "audio" if ref.kind == "media" else "pictures" - referenced.add((base / subfolder / relative).resolve()) + for candidate in (relative, Path(subfolder) / relative): + for match in _media_matches(base, candidate, listings): + referenced.add(match.resolve()) + media_folders = [ + path + for name in ("audio", "pictures") + for path in _folded_entries(base, listings).get(name, ()) + if path.is_dir() + ] orphans = [ file - for folder in ("audio", "pictures") - if (base / folder).is_dir() - for file in sorted((base / folder).rglob("*")) + for folder in media_folders + for file in sorted(folder.rglob("*")) if file.is_file() and file.resolve() not in referenced ] for file in orphans: @@ -196,8 +216,9 @@ def _cmd_check_media(args: argparse.Namespace) -> int: "note: WeSay-style audio writing systems reference files from form " "text, which this check does not follow" ) - print(f"{len(missing)} missing, {len(orphans)} orphaned") - return 1 if missing else 0 + mismatched = len(directories) + len(files) + print(f"{len(missing)} missing, {mismatched} mismatched, {len(orphans)} orphaned") + return 1 if missing or mismatched else 0 def _leaf_senses(entry: Entry) -> list[Sense]: @@ -355,7 +376,10 @@ def main(argv: Sequence[str] | None = None) -> int: validate.add_argument( "--no-check-media", action="store_true", - help="skip the filesystem media-presence check (suppresses missing-media findings)", + help=( + "skip the filesystem media-presence check " + "(suppresses missing-media and media-href-mismatch findings)" + ), ) validate.add_argument( "--allow", diff --git a/src/sil_lift/_model.py b/src/sil_lift/_model.py index dca3e53..a8da5b1 100644 --- a/src/sil_lift/_model.py +++ b/src/sil_lift/_model.py @@ -40,6 +40,7 @@ "GrammaticalInfo", "Lexicon", "MediaRef", + "MediaResolution", "Note", "Pronunciation", "RangesChanges", @@ -275,6 +276,21 @@ class MediaRef: sense_id: str | None = None # set for illustrations (they live on senses) +@dataclass(slots=True) +class MediaResolution: + """A media reference that did not resolve cleanly against the LIFT folder. + + ``missing`` means no file answered the href under any spelling; + ``mismatch`` means one did, but only under case folding or NFC, and + ``found`` is the file it named. A href that names its file exactly is not + reported at all -- see :meth:`Lexicon.check_media`. + """ + + ref: MediaRef + status: Literal["missing", "mismatch"] + found: Path | None = None + + @dataclass(slots=True) class RangesChanges: """What differs between a companion ``.lift-ranges`` and the file it was read from. @@ -541,6 +557,115 @@ def _existing_file(candidate: Path, listings: dict[Path, dict[str, list[Path]]]) return matches[0] if len(matches) == 1 else None +def _folded_entries( + folder: Path, listings: dict[Path, dict[str, list[Path]]] +) -> dict[str, list[Path]]: + """Everything in ``folder``, keyed by folded name; one read per folder. + + Unlike :func:`_folded_matches` this never probes an exact spelling first, + because an exact probe on a case-folding filesystem answers for a name it + cannot report: ``SDD.PNG`` stats true against an on-disk ``sdd.png`` and + the real spelling is never seen. Listing is also the cheaper half of the + trade, since a document's media probes the same few folders over and over. + """ + if folder not in listings: + entries: dict[str, list[Path]] = {} + try: + # Sorted, so which of several folding matches gets reported does + # not depend on the order a filesystem happens to list them in. + for path in sorted(folder.iterdir()): + entries.setdefault(_fold(path.name), []).append(path) + except OSError: + pass # missing or unreadable folder: nothing resolves out of it + listings[folder] = entries + return listings[folder] + + +def _media_matches( + base: Path, relative: Path, listings: dict[Path, dict[str, list[Path]]] +) -> list[Path]: + """Every file under ``base`` that ``relative`` names, folding each component. + + Component by component, because a folder spelled another way hides + everything under it: a Windows-authored ``Pictures\\`` read on a + case-sensitive filesystem would otherwise take every href across it with + it. + + A href reaching outside the folder (``..``) is probed as written and not + folded: which directory to search would be a guess. + """ + parts = relative.parts + if not parts: + return [] + if not _foldable(parts): + candidate = base / relative + try: + return [candidate] if candidate.is_file() else [] + except OSError: + return [] + current = [base] + for part in parts[:-1]: + current = [ + match + for folder in current + for match in _folded_entries(folder, listings).get(_fold(part), ()) + if _is_dir(match) + ] + if not current: + return [] + return [ + match + for folder in current + for match in _folded_entries(folder, listings).get(_fold(parts[-1]), ()) + if _is_file(match) + ] + + +def _is_file(path: Path) -> bool: + """``path.is_file()``, False where stat fails for any reason. + + Before Python 3.14 pathlib swallows only a few errnos, so a + ``PermissionError`` would otherwise escape a listable but unsearchable + folder. + """ + try: + return path.is_file() + except OSError: + return False + + +def _is_dir(path: Path) -> bool: + """``path.is_dir()``, False where stat fails for any reason.""" + try: + return path.is_dir() + except OSError: + return False + + +def _foldable(parts: tuple[str, ...]) -> bool: + """Whether every component of a relative path names something to look up.""" + return not any(part in (".", "..") for part in parts) + + +def _href_components(href: str, found: Path) -> list[tuple[str, str]]: + """The href's own components paired with the on-disk names they reached. + + Only what the href wrote: the conventional ``audio/``/``pictures/`` + subfolder is sil-lift's guess, so its spelling is nobody's defect and it is + left out of the comparison. + """ + relative = _normalize_href(href) + if relative is None or not _foldable(relative.parts): + return [] + written = relative.parts + return list(zip(written, found.parts[-len(written) :], strict=True)) + + +def _spelled_exactly(href: str, found: Path) -> bool: + """Whether ``href`` names ``found`` byte for byte, not merely under folding.""" + return all(written == on_disk for written, on_disk in _href_components(href, found)) + + class _Candidate(NamedTuple): """A place a companion may be found, and what sent :meth:`Lexicon.load` there.""" @@ -1060,9 +1185,9 @@ def iter_problems(self, *, require_ids: bool = False) -> Iterator[Problem]: only when companion discovery ran: a lexicon loaded with ``resolve_ranges=False`` put companions out of scope, so none is reported for it, and attaching one afterwards with - :meth:`add_ranges_file` does not reinstate them. ``missing-media`` is - unaffected: media is never resolved into the model, so nothing was - opted out of. + :meth:`add_ranges_file` does not reinstate them. ``missing-media`` and + ``media-href-mismatch`` are unaffected: media is never resolved into + the model, so nothing was opted out of. """ from ._validate import iter_lexicon_problems @@ -1126,26 +1251,43 @@ def media_refs(self) -> Iterator[MediaRef]: illustration.href, "illustration", entry.id, entry.guid, sense.id ) - def missing_media(self) -> list[MediaRef]: - """Media references whose files don't exist in the LIFT folder layout. + def check_media(self) -> list[MediaResolution]: + """Media references that don't resolve cleanly in the LIFT folder layout. A relative href is checked as given (backslashes normalized) and under the conventional subfolder (``audio/`` for media, ``pictures/`` for illustrations). Remote/absolute hrefs can't be checked and are skipped. + + A reference whose href names its file exactly is not reported. One that + reaches a file only under case folding or NFC is a ``mismatch``: the + file is here, but a case-sensitive host serving this folder will not + find it. One that reaches nothing is ``missing``. """ if self.path is None: return [] base = self.path.parent subfolder = {"media": "audio", "illustration": "pictures"} - missing = [] + listings: dict[Path, dict[str, list[Path]]] = {} + unresolved = [] for ref in self.media_refs(): relative = _normalize_href(ref.href) if relative is None: continue - candidates = [base / relative, base / subfolder[ref.kind] / relative] - if not any(candidate.is_file() for candidate in candidates): - missing.append(ref) - return missing + if not relative.parts: + # "" or ".": names no file, and appending it to the + # conventional subfolder would name the subfolder itself. + unresolved.append(MediaResolution(ref, "missing")) + continue + matches = [ + match + for candidate in (relative, Path(subfolder[ref.kind]) / relative) + for match in _media_matches(base, candidate, listings) + ] + if not matches: + unresolved.append(MediaResolution(ref, "missing")) + elif not any(_spelled_exactly(ref.href, match) for match in matches): + unresolved.append(MediaResolution(ref, "mismatch", matches[0])) + return unresolved def find(self, *, id: str | None = None, guid: str | None = None) -> Entry | None: """The first entry matching the given id and/or guid, or None. diff --git a/src/sil_lift/_validate.py b/src/sil_lift/_validate.py index eb7b9da..5af34bb 100644 --- a/src/sil_lift/_validate.py +++ b/src/sil_lift/_validate.py @@ -47,6 +47,7 @@ Lexicon, _existing_file, _folded_matches, + _href_components, _normalize_href, _ranges_candidates, _same_file, @@ -55,9 +56,10 @@ if TYPE_CHECKING: import os - from collections.abc import Collection, Iterator + from collections.abc import Collection, Iterable, Iterator from ._header import Range + from ._model import MediaResolution __all__ = ["Problem", "iter_problems", "validate_file"] @@ -321,6 +323,46 @@ def _form_shape_problems( ) +def media_mismatch_groups( + resolutions: Iterable[MediaResolution], base: Path +) -> tuple[list[tuple[str, str]], list[tuple[MediaResolution, str, str]]]: + """Media spelling mismatches split into folder findings and file findings. + + A folder the hrefs misspell is one rename however many references cross it, + so it is reported once and carries no entry; a misspelled filename is one + rename each, addressed to the entry that wrote it. Without the split, a + folder authored as ``Pictures\\`` would report once per media reference in + the document. + + A folder is identified by the file that reached it, not by what the href + spells. Names taken from the href alone cannot tell ``a/foo`` from + ``pictures/a/foo``: what separates them is the conventional subfolder, + which the lookup supplied and the document never wrote. Paths are reported + relative to ``base``, the folder holding the ``.lift``. + + Shared with the CLI's ``check-media``, which groups the same way. + """ + directories: dict[tuple[Path, str], tuple[str, str]] = {} + files: list[tuple[MediaResolution, str, str]] = [] + for resolution in resolutions: + if resolution.status != "mismatch" or resolution.found is None: + continue + components = _href_components(resolution.ref.href, resolution.found) + for index, (written, on_disk) in enumerate(components): + if written == on_disk: + continue + if index == len(components) - 1: + files.append((resolution, written, on_disk)) + continue + # As many levels above the file as there are components after this + # one -- the directory this component actually named. + folder = resolution.found.parents[len(components) - index - 2] + directories.setdefault( + (folder, written), (written, folder.relative_to(base).as_posix()) + ) + return list(directories.values()), files + + def _semantic_problems( lexicon: Lexicon, entry_lines: list[tuple[int | None, str | None, str | None]], @@ -673,13 +715,40 @@ def range_named(name: str) -> str | None: file=ranges_paths.get(range_id) or file, ) - # Missing media files. - for media_ref in lexicon.missing_media(): + # Media files that are absent, or here under another spelling. + resolutions = lexicon.check_media() + for resolution in resolutions: + if resolution.status != "missing": + continue yield Problem( "warning", "missing-media", - f"{media_ref.kind} file not found: {media_ref.href!r}", + f"{resolution.ref.kind} file not found: {resolution.ref.href!r}", + file=file, + entry_id=resolution.ref.entry_id, + guid=resolution.ref.entry_guid, + ) + # A pathless lexicon resolves no media at all, so there is nothing to group. + media_base = lexicon.path.parent if lexicon.path is not None else Path() + directories, files = media_mismatch_groups(resolutions, media_base) + # The two spellings can render identically, so name them by code point. + for written, on_disk in directories: + yield Problem( + "warning", + "media-href-mismatch", + f"media hrefs name folder {written!a}, which is {on_disk!a} on disk; they " + "match only under case folding or Unicode normalization, so a " + "case-sensitive host will not find any file under it", + file=file, + ) + for resolution, written, on_disk in files: + yield Problem( + "warning", + "media-href-mismatch", + f"{resolution.ref.kind} href names {written!a}, which is {on_disk!a} on disk; " + "they match only under case folding or Unicode normalization, so a " + "case-sensitive host will not find it", file=file, - entry_id=media_ref.entry_id, - guid=media_ref.entry_guid, + entry_id=resolution.ref.entry_id, + guid=resolution.ref.entry_guid, ) diff --git a/tests/corpus/PROVENANCE.md b/tests/corpus/PROVENANCE.md index 826dc91..b5f0558 100644 --- a/tests/corpus/PROVENANCE.md +++ b/tests/corpus/PROVENANCE.md @@ -71,7 +71,7 @@ that script; committed so tests don't depend on regeneration. `pictures/cultural law.png` (space in filename). The file has a UTF-8 BOM and tab-indented attribute-per-line formatting — a byte-fidelity edge case. Upstream `Moma.WeSayConfig` not taken (not LIFT). Primary fixture for - media_refs()/missing_media(). + `media_refs()`/`check_media()`. ## misc/sample.lift @@ -184,7 +184,7 @@ What did match only after normalizing is still reported, as a `normalization-mismatch` warning per range-element id: 2 parent links and 80 grammatical-info values reach 5 ids in Sango, one of them under both aliases of the part-of-speech list, so 6 warnings — and none in the other fixtures. -`negative/nfd-range-ids.lift` is the hand-authored version of the same shape. +`negative/nfd-range-ids/` is the hand-authored version of the same shape. ## generated/ — synthetic large files (not committed) @@ -193,15 +193,16 @@ git-ignored, regenerated on demand. ## negative/ — invalid fixtures (hand-authored) -Each file carries an XML comment documenting its defect and the expected -Problem code: `duplicate-guid`, `dangling-ref`, `range-parent`, -`undefined-range-value` (2 warnings \+ a clean control entry), -`duplicate-form-lang` (the Schematron-only rule), `schema-invalid` -(structural), `missing-media/` (a folder fixture), `flex-quirks` -(URI quirks that must yield warnings, never schema errors), and -`nfd-range-ids` (a `.lift` \+ `.lift-ranges` pair carrying FLEx's -historical normalization asymmetry: NFD ids, NFC references, and one -parent that dangles in every normalization). +One defect class per fixture, each carrying an XML comment that names the +defect and the Problem code it must yield. Some hold several instances of +their defect, and some a clean control entry beside them, pinning a check +against over-firing as well as under-firing. + +A defect contained in one document is a single `.lift`. A defect that needs +more than the document gets a subfolder named for it, holding the `.lift` and +everything its check reads. Any media files in those folders are empty +placeholders rather than fetched content: nothing ever reads their bytes. + `schema-invalid.lift` and `flex-quirks.lift` are raw-RNG-invalid (the latter only under libxml2's anyURI check) and appear in the corpus test's expected-invalid list. diff --git a/tests/corpus/negative/media-href-mismatch/Audio/word.wav b/tests/corpus/negative/media-href-mismatch/Audio/word.wav new file mode 100644 index 0000000..e69de29 diff --git a/tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift b/tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift new file mode 100644 index 0000000..8e1feb4 --- /dev/null +++ b/tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift @@ -0,0 +1,34 @@ + + + + +one + + + + + +
two
+ + + +
+ +
three
+ +
three
+ +
+ +three + +
+
diff --git a/tests/corpus/negative/media-href-mismatch/pictures/other.png b/tests/corpus/negative/media-href-mismatch/pictures/other.png new file mode 100644 index 0000000..e69de29 diff --git a/tests/corpus/negative/media-href-mismatch/pictures/sdd.png b/tests/corpus/negative/media-href-mismatch/pictures/sdd.png new file mode 100644 index 0000000..e69de29 diff --git a/tests/corpus/negative/nfd-range-ids.lift b/tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift similarity index 100% rename from tests/corpus/negative/nfd-range-ids.lift rename to tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift diff --git a/tests/corpus/negative/nfd-range-ids.lift-ranges b/tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift-ranges similarity index 100% rename from tests/corpus/negative/nfd-range-ids.lift-ranges rename to tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift-ranges diff --git a/tests/test_cli.py b/tests/test_cli.py index 8ac08d4..b8b46d1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -78,6 +78,54 @@ def test_validate_no_check_media(capsys: pytest.CaptureFixture[str]) -> None: assert "[missing-media]" not in capsys.readouterr().out +def test_validate_no_check_media_covers_spelling_findings( + capsys: pytest.CaptureFixture[str], +) -> None: + # The flag is for media that lives outside the folder; a spelling verdict + # on files it was told not to check would contradict that. + path = CORPUS_DIR / "negative" / "media-href-mismatch" / "media-href-mismatch.lift" + assert main(["validate", str(path)]) == 0 + assert "[media-href-mismatch]" in capsys.readouterr().out + assert main(["validate", str(path), "--no-check-media"]) == 0 + assert "[media-href-mismatch]" not in capsys.readouterr().out + + +def test_check_media_follows_a_dot_segment_href_to_its_file( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # "audio/sub/../one.wav" and the "audio/one.wav" the folder walk finds are + # one file: the href resolves, so nothing is missing and nothing orphaned. + (tmp_path / "audio" / "sub").mkdir(parents=True) + (tmp_path / "audio" / "one.wav").write_bytes(b"") + path = tmp_path / "dots.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + "
\n" + "
\n", + encoding="utf-8", + ) + assert main(["check-media", str(path)]) == 0 + assert "0 missing, 0 mismatched, 0 orphaned" in capsys.readouterr().out + + +def test_check_media_flags_mismatch_without_calling_the_file_orphaned( + capsys: pytest.CaptureFixture[str], +) -> None: + path = CORPUS_DIR / "negative" / "media-href-mismatch" / "media-href-mismatch.lift" + assert main(["check-media", str(path)]) == 1 + out = capsys.readouterr().out + assert "0 missing, 2 mismatched, 0 orphaned" in out + # The files are named inexactly, not unnamed: reporting them orphaned as + # well as unresolved is the second half of the same bug. + assert "no media/illustration references it" not in out + assert "'SDD.PNG' is 'sdd.png' on disk" in out + assert "'Pictures' is 'pictures' on disk" in out + + def test_validate_allow_excludes_a_warning_from_strict(capsys: pytest.CaptureFixture[str]) -> None: path = CORPUS_DIR / "negative" / "flex-quirks.lift" assert main(["validate", str(path), "--strict", "--allow", "uri-not-rfc"]) == 0 @@ -229,7 +277,7 @@ def test_check_media_absolute_href_does_not_mark_local_file_referenced( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: # An absolute href (FLEx-style dangling path) must not mark a folder file - # as referenced — mirrors missing_media(), which skips non-relative hrefs. + # as referenced — mirrors check_media(), which skips non-relative hrefs. audio = tmp_path / "audio" audio.mkdir() wav = audio / "one.wav" @@ -345,7 +393,8 @@ def test_export_filename_with_space(tmp_path: Path) -> None: def test_validate_text_output_survives_a_cp1252_stdout(monkeypatch: pytest.MonkeyPatch) -> None: raw = _redirect(monkeypatch, "stdout", "cp1252") - assert main(["validate", str(CORPUS_DIR / "negative" / "nfd-range-ids.lift")]) == 1 + path = CORPUS_DIR / "negative" / "nfd-range-ids" / "nfd-range-ids.lift" + assert main(["validate", str(path)]) == 1 sys.stdout.flush() out = raw.getvalue().decode("utf-8") assert unicodedata.normalize("NFD", "Órfão") in out # the id cp1252 cannot hold diff --git a/tests/test_ranges_folder.py b/tests/test_ranges_folder.py index 3a56d23..80b7c8f 100644 --- a/tests/test_ranges_folder.py +++ b/tests/test_ranges_folder.py @@ -199,31 +199,96 @@ def test_ranges_schema_is_loadable_and_spec_faithful() -> None: ) -def test_media_refs_and_missing_media_on_moma_folder() -> None: +def test_media_refs_and_check_media_on_moma_folder() -> None: lexicon = sil_lift.load(CORPUS_DIR / "folder" / "Moma" / "Moma.lift") refs = list(lexicon.media_refs()) assert {r.href for r in refs} == {"pictures\\cultural law.png", "pictures\\sdd.png"} assert all(r.kind == "illustration" for r in refs) assert all(r.entry_id for r in refs) - assert lexicon.missing_media() == [] + # Every href spells its file exactly, so nothing is reported at all. + assert lexicon.check_media() == [] -def test_missing_media_on_all_flex_fields() -> None: +def test_check_media_on_all_flex_fields() -> None: # The corpus deliberately omits the upstream filler media (PROVENANCE.md), # so these references must be reported missing. lexicon = sil_lift.load(CORPUS_DIR / "flex" / "AllFLExFields" / "AllFLExFields.lift") - missing = {(r.kind, r.href) for r in lexicon.missing_media()} + missing = {(r.ref.kind, r.ref.href) for r in lexicon.check_media() if r.status == "missing"} assert ("media", "Kalimba.mp3") in missing assert ("illustration", "Desert.jpg") in missing -def test_missing_media_flags_broken_ref(tmp_path: Path) -> None: +def test_check_media_flags_broken_ref(tmp_path: Path) -> None: src = CORPUS_DIR / "folder" / "Moma" shutil.copytree(src, tmp_path / "Moma") lexicon = sil_lift.load(tmp_path / "Moma" / "Moma.lift") (tmp_path / "Moma" / "pictures" / "sdd.png").unlink() - missing = lexicon.missing_media() - assert [r.href for r in missing] == ["pictures\\sdd.png"] + assert [(r.ref.href, r.status) for r in lexicon.check_media()] == [ + ("pictures\\sdd.png", "missing") + ] + + +def _write_lift_with_illustration(folder: Path, href: str) -> Path: + """A minimal loadable .lift whose one sense illustrates ``href``.""" + folder.mkdir(parents=True, exist_ok=True) + path = folder / "media.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + f'\n' + "
\n" + "
\n", + encoding="utf-8", + ) + return path + + +def test_check_media_reports_a_normalization_only_match(tmp_path: Path) -> None: + # Not a corpus fixture: every checkout filesystem preserves case in a + # filename, but HFS+ rewrites normalization, so a committed NFD/NFC pair + # has a premise the checkout itself could alter. + name_nfc = unicodedata.normalize("NFC", "café.png") + name_nfd = unicodedata.normalize("NFD", "café.png") + assert name_nfc != name_nfd + (tmp_path / "pictures").mkdir() + (tmp_path / "pictures" / name_nfd).write_bytes(b"") + path = _write_lift_with_illustration(tmp_path, f"pictures/{name_nfc}") + resolutions = sil_lift.load(path).check_media() + assert [r.status for r in resolutions] == ["mismatch"] + assert resolutions[0].found is not None + assert resolutions[0].found.name == name_nfd + + +def test_check_media_reports_a_href_that_several_files_fold_onto(tmp_path: Path) -> None: + if not _case_sensitive(tmp_path): + pytest.skip("needs a case-sensitive filesystem to hold both spellings") + (tmp_path / "pictures").mkdir() + for name in ("SDD.PNG", "sdd.png"): + (tmp_path / "pictures" / name).write_bytes(b"") + # Matching neither exactly is what makes it ambiguous; it is still one + # defect with one fix, so it reports as an ordinary mismatch. + path = _write_lift_with_illustration(tmp_path, "pictures/Sdd.png") + assert [r.status for r in sil_lift.load(path).check_media()] == ["mismatch"] + + +@pytest.mark.parametrize("href", ["", "."]) +def test_check_media_reports_an_empty_href_missing(tmp_path: Path, href: str) -> None: + # A file named like the conventional subfolder is what an empty href + # appended to it would reach. + (tmp_path / "pictures").write_bytes(b"") + path = _write_lift_with_illustration(tmp_path, href) + assert [r.status for r in sil_lift.load(path).check_media()] == ["missing"] + + +def test_check_media_resolves_an_exactly_spelled_href_above_the_folder(tmp_path: Path) -> None: + # ".." names no directory to search, so it is probed as written rather than + # folded -- and must keep resolving. + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "one.png").write_bytes(b"") + path = _write_lift_with_illustration(tmp_path / "lex", "../shared/one.png") + assert sil_lift.load(path).check_media() == [] def _write_case_variant_pair(folder: Path, lift_name: str, ranges_name: str) -> Path: diff --git a/tests/test_validate.py b/tests/test_validate.py index a307fa5..aa1bd0a 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -167,7 +167,7 @@ def test_header_range_id_reaches_a_companion_id_in_another_normalization() -> No def test_nfd_ids_warn_once_and_still_flag_the_real_dangling_parent(tmp_path: Path) -> None: - path = NEGATIVE_DIR / "nfd-range-ids.lift" + path = NEGATIVE_DIR / "nfd-range-ids" / "nfd-range-ids.lift" problems = problems_for(path) assert codes(problems) == {("error", "range-parent"), ("warning", "normalization-mismatch")} (dangling,) = [p for p in problems if p.code == "range-parent"] @@ -184,7 +184,7 @@ def test_nfd_ids_warn_once_and_still_flag_the_real_dangling_parent(tmp_path: Pat # Normalization belongs to the comparison only: the mixed forms survive. sil_lift.Lexicon.load(path).save(tmp_path / path.name) for name in (path.name, "nfd-range-ids.lift-ranges"): - assert (tmp_path / name).read_bytes() == (NEGATIVE_DIR / name).read_bytes(), name + assert (tmp_path / name).read_bytes() == (path.parent / name).read_bytes(), name def test_one_id_referenced_in_two_spellings_still_warns_once() -> None: @@ -342,6 +342,113 @@ def test_missing_media_folder_fixture() -> None: assert any("gone.png" in m for m in hrefs) +def test_media_href_mismatch_folder_fixture() -> None: + # No skipif: the whole point of resolving against the directory listing is + # that a case-folding host reaches the same verdict as a case-sensitive one. + problems = problems_for(NEGATIVE_DIR / "media-href-mismatch" / "media-href-mismatch.lift") + assert not [p for p in problems if p.code == "missing-media"], "every file is present" + mismatches = [p for p in problems if p.code == "media-href-mismatch"] + assert all(p.level == "warning" for p in mismatches) + # The misspelled folder is one rename, so it reports once and names no + # entry; the misspelled filename is addressed to the entry that wrote it. + folder = [p for p in mismatches if p.entry_id is None] + assert len(folder) == 1 + assert "'Pictures'" in folder[0].message and "'pictures'" in folder[0].message + files = [p for p in mismatches if p.entry_id is not None] + assert [(p.entry_id, "'SDD.PNG'" in p.message) for p in files] == [("one", True)] + + +def test_media_href_mismatch_reports_each_misspelled_folder_separately(tmp_path: Path) -> None: + # One misspelled component under two parents is two renames, so keying the + # finding on the component alone would undercount the work. + for parent in ("a", "b"): + (tmp_path / parent / "foo").mkdir(parents=True) + (tmp_path / parent / "foo" / f"{parent}.png").write_bytes(b"") + path = tmp_path / "two.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + '\n' + "
\n" + "
\n", + encoding="utf-8", + ) + problems = problems_for(path) + mismatches = [p for p in problems if p.code == "media-href-mismatch"] + assert len(mismatches) == 2 + assert any("'a/foo'" in p.message for p in mismatches) + assert any("'b/foo'" in p.message for p in mismatches) + + +def test_media_href_mismatch_tells_apart_two_folders_one_href_could_name( + tmp_path: Path, +) -> None: + # The conventional pictures/ lookup reaches a folder the direct one cannot, + # and both answer to "a/Foo": what the href writes cannot tell them apart, + # only the file each one reached. + (tmp_path / "a" / "foo").mkdir(parents=True) + (tmp_path / "pictures" / "a" / "foo").mkdir(parents=True) + (tmp_path / "a" / "foo" / "one.png").write_bytes(b"") + (tmp_path / "pictures" / "a" / "foo" / "two.png").write_bytes(b"") + path = tmp_path / "roots.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + '\n' + "
\n" + "
\n", + encoding="utf-8", + ) + mismatches = [p for p in problems_for(path) if p.code == "media-href-mismatch"] + assert sorted(p.message.split(";")[0] for p in mismatches) == [ + "media hrefs name folder 'Foo', which is 'a/foo' on disk", + "media hrefs name folder 'Foo', which is 'pictures/a/foo' on disk", + ] + + +def test_media_href_mismatch_reports_one_folder_once_however_hrefs_reach_it( + tmp_path: Path, +) -> None: + # One folder is one rename however inconsistently the hrefs spell the way + # down to it: 'A/Foo' and 'a/Foo' reach the same 'a/foo' and report once. + (tmp_path / "a" / "foo").mkdir(parents=True) + for name in ("one.png", "two.png"): + (tmp_path / "a" / "foo" / name).write_bytes(b"") + path = tmp_path / "nested.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + '\n' + "
\n" + "
\n", + encoding="utf-8", + ) + problems = problems_for(path) + mismatches = [p for p in problems if p.code == "media-href-mismatch"] + # Two real defects: the parent written 'A', and 'a/foo' written 'Foo'. + assert sorted(p.message.split(";")[0] for p in mismatches) == [ + "media hrefs name folder 'A', which is 'a' on disk", + "media hrefs name folder 'Foo', which is 'a/foo' on disk", + ] + + +def test_media_href_mismatch_leaves_the_conventional_subfolder_alone() -> None: + # Entry "three" writes a bare "word.wav" that only resolves because the + # guessed audio/ folds onto the on-disk Audio/. That component is sil-lift's + # own, so its spelling is nobody's defect and must never be reported. + lexicon = sil_lift.load(NEGATIVE_DIR / "media-href-mismatch" / "media-href-mismatch.lift") + assert [r.ref.entry_id for r in lexicon.check_media()] == ["one", "two"] + + def test_flex_uri_quirks_warn_but_never_error() -> None: problems = problems_for(NEGATIVE_DIR / "flex-quirks.lift") assert problems, "the quirky URIs must be reported"