Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions scripts/normalize_cache_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
39 changes: 39 additions & 0 deletions tests/test_cache_names_are_normalised_after_fetching.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
69 changes: 69 additions & 0 deletions tests/test_no_case_conflicting_cache_pairs.py
Original file line number Diff line number Diff line change
@@ -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()))
)
Loading