diff --git a/scripts/normalize_cache_names.py b/scripts/normalize_cache_names.py index 535310e7..1b713c19 100644 --- a/scripts/normalize_cache_names.py +++ b/scripts/normalize_cache_names.py @@ -138,6 +138,21 @@ def main(argv: list[str] | None = None) -> int: # A conflict is left for a human: both casings exist as distinct files, and # choosing between two fetches of the same reference is not this script's # call. + # + # Said loudly, because the recipes that run this DISCARD its exit code on + # purpose -- normalising must not turn a failing validation green (#697) -- + # so a conflict would otherwise be one line in the middle of a long + # validation log (#706). The committed tree is separately gated by + # tests/test_no_case_conflicting_cache_pairs.py, which reads git's index + # rather than the filesystem so the clash is visible on macOS too. + if conflicts: + print( + f"\n⚠️ {conflicts} cache name conflict(s): both casings exist as " + f"distinct files, so on a case-sensitive filesystem at most one is " + f"reachable. Nothing was renamed for those; a human has to choose " + f"which fetch to keep (#706).", + file=sys.stderr, + ) return 1 if conflicts else 0 diff --git a/tests/test_cache_names_are_normalised_after_fetching.py b/tests/test_cache_names_are_normalised_after_fetching.py index 3ad59813..2985f6ac 100644 --- a/tests/test_cache_names_are_normalised_after_fetching.py +++ b/tests/test_cache_names_are_normalised_after_fetching.py @@ -224,3 +224,42 @@ def test_the_check_can_actually_fail(tmp_path): "survived, so `test_a_leftover_temporary_is_never_clobbered` would pass " "with or without the guard it is meant to defend" ) + + +def test_a_conflict_is_announced_where_a_reader_will_see_it(tmp_path, capsys, monkeypatch): + """A `[conflict]` must not be one line in the middle of a validation log. + + The recipes discard this script's exit code on purpose -- normalising must + not turn a failing validation green (#697) -- so the exit code cannot be the + signal. The banner is (#706). + + Driven by stubbing `rename`, because the real conflict needs two files whose + names differ only by case and macOS cannot hold both; the filesystem, not + the code, is what makes it unreachable here. + """ + module = _script() + (tmp_path / "DOI_10.9999_clash.md").write_text("from the fetcher", encoding="utf-8") + + def _always_conflicts(current, wanted): + return f"[conflict] {current.name}: {wanted.name} already exists and differs" + + monkeypatch.setitem(module, "rename", _always_conflicts) + + code = module["main"](["--cache-dir", str(tmp_path)]) + + captured = capsys.readouterr() + assert code == 1, "a conflict must still be a non-zero exit from the script itself" + assert "cache name conflict" in captured.err, captured.err + assert "#706" in captured.err, captured.err + + +def test_no_conflict_means_no_banner(tmp_path, capsys): + """The other direction, so the test above cannot pass vacuously.""" + module = _script() + (tmp_path / "DOI_10.9999_fine.md").write_text("cached", encoding="utf-8") + + code = module["main"](["--cache-dir", str(tmp_path)]) + + captured = capsys.readouterr() + assert code == 0, captured.err + assert "cache name conflict" not in captured.err, captured.err diff --git a/tests/test_no_case_conflicting_cache_pairs.py b/tests/test_no_case_conflicting_cache_pairs.py new file mode 100644 index 00000000..70cc4074 --- /dev/null +++ b/tests/test_no_case_conflicting_cache_pairs.py @@ -0,0 +1,69 @@ +"""Two cache files differing only by prefix case cannot both be reachable (#706). + +`scripts/normalize_cache_names.py` renames `DOI_x.md` to `doi_x.md`, and when +BOTH already exist as distinct tracked files it reports a `[conflict]` and +refuses — choosing between two fetches of the same reference is not a script's +call. But the justfile recipes discard the normaliser's exit code (deliberately: +normalising must not turn a failing validation green), so that conflict is one +printed line and nothing else. + +**Why the existing case-exactness test does not close this.** +`test_reference_cache_names_are_case_exact.py` reads +`os.listdir(REFERENCES_CACHE)` — the filesystem. macOS cannot hold `DOI_x.md` +and `doi_x.md` at once, so on a developer machine the pair is invisible and the +divergence only appears on Linux. That is #690's lesson stated as a test: a +green local run is not a green CI run. + +Reading git's index instead makes the check case-exact on every machine, because +git records the name it was told regardless of what the filesystem will store. +""" + +from __future__ import annotations + +import collections +import pathlib +import subprocess + +REPO = pathlib.Path(__file__).parent.parent + + +def _tracked_cache_names() -> list[str]: + """Cache filenames as GIT records them, not as the filesystem reports them.""" + result = subprocess.run( + ["git", "ls-files", "-z", "--", "references_cache/"], + cwd=REPO, + capture_output=True, + text=True, + timeout=120, + check=True, + ) + # -z, because a path containing a space or a newline would otherwise split + # into several bogus entries -- the whitespace-splitting trap. + return [entry.rsplit("/", 1)[-1] for entry in result.stdout.split("\0") if entry] + + +def test_there_are_tracked_cache_files_to_check(): + """A finder that found nothing would make the check below vacuous.""" + names = _tracked_cache_names() + assert len(names) >= 400, f"only {len(names)} tracked cache files; the listing broke" + + +def test_no_two_tracked_caches_differ_only_by_case(): + """One reference must not have two files whose names differ only in case. + + On a case-sensitive filesystem both exist and only one is reachable, so the + other is a fetch nothing can read -- and which one wins depends on which + casing the citation happens to use. + """ + by_folded = collections.defaultdict(list) + for name in _tracked_cache_names(): + by_folded[name.casefold()].append(name) + + clashes = {folded: sorted(names) for folded, names in by_folded.items() if len(set(names)) > 1} + assert clashes == {}, ( + "these cache files differ only by case, so on a case-sensitive " + "filesystem both exist and at most one is reachable (#706). The " + "normaliser reports this as a [conflict] and refuses to choose; a human " + "has to decide which fetch to keep:\n " + + "\n ".join(f"{folded}: {names}" for folded, names in sorted(clashes.items())) + )