mirror of
https://github.com/kuhyx/utils.git
synced 2026-11-10 19:07:17 +01:00
Make the allowlist inheritable, and cache the quarantine lookups
Two things that were fine in utils and would not have been in the next forty repos. **The allowlist is now fleet-wide plus per-repo.** `typescript-eslint` blocks TS 7 in EVERY repo that pins typescript -- eight of them and counting -- and a repo-only allowlist meant hand-copying one entry into eight files that then rot independently. That is the four-way fork of the 250-line cap, reproduced inside the mechanism built to avoid it. `~/utils/dependency-freshness.allowlist.yaml` is inherited by every repo; a repo entry for the same `ecosystem:package` overrides it. Inherited entries are exempt from the no-longer-needed check everywhere except the repo that owns the file. Without that carve-out a fleet-wide hold on typescript exits 2 in every Flutter app -- it excuses nothing there, which is the normal case, not rot -- and those repos become uncommittable. Confirmed against punchme, which now exits 0 while utils still reports the entry as in use. **`quarantine.py` bypassed the cache entirely.** It ran for every npm finding and fetched the FULL packument -- 7MB for react, 16MB for typescript -- with no TTL and no reuse, inside a pre-commit hook. One finding in utils, but the JS repos ahead have 16 to 37 each. Now cached under the normal 6h TTL, keyed `<name>@<dist-tag>`: the tag has to be part of the key rather than clamped afterwards, because clamping returns the ceiling and the ceiling is frequently the very release the quarantine is excluding. **Both cache and shared allowlist are now redirected by AUTOUSE fixtures.** A quarantine test had already seeded a fake answer for package "x" into the real ~/.cache/dep-freshness/registry.json, and the next run read its own pollution back and passed for the wrong reason. Removed the entry; the fixture makes it unrepeatable. 292 tests, 100% branch coverage on every module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRykabMwPgDzFHcARoqpaK
This commit is contained in:
parent
c4a5db8ebc
commit
1439c0f5c0
@ -114,5 +114,12 @@ PUB_SDK_PACKAGES: Final = frozenset({
|
||||
PUB_OVERRIDE_KEY: Final = "dependency_overrides"
|
||||
|
||||
ALLOWLIST_FILE: Final = "dependency-freshness.allowlist.yaml"
|
||||
|
||||
# A blocker like "typescript-eslint does not support TS 7" holds in EVERY repo
|
||||
# that pins typescript, so a repo-only allowlist would mean hand-copying one
|
||||
# entry into eight files that then all rot independently -- the four-way fork
|
||||
# of the 250-line cap, in the mechanism built to avoid it. The shared file in
|
||||
# ~/utils is inherited; a repo entry for the same package overrides it.
|
||||
SHARED_ALLOWLIST_ENV: Final = "DEP_FRESHNESS_SHARED_ALLOWLIST"
|
||||
ALLOWLIST_MAX_DAYS: Final = 90
|
||||
TRANSITIVE_PREFIX: Final = "transitive:"
|
||||
|
||||
@ -20,7 +20,13 @@ from pathlib import Path
|
||||
|
||||
import yaml
|
||||
|
||||
from dep_freshness._tables import ALLOWLIST_FILE, ALLOWLIST_MAX_DAYS
|
||||
import os
|
||||
|
||||
from dep_freshness._tables import (
|
||||
ALLOWLIST_FILE,
|
||||
ALLOWLIST_MAX_DAYS,
|
||||
SHARED_ALLOWLIST_ENV,
|
||||
)
|
||||
from dep_freshness.models import Exception_
|
||||
|
||||
REQUIRED = ("ecosystem", "package", "pinned", "reason", "blocked_by")
|
||||
@ -34,6 +40,18 @@ def path_for(root: Path) -> Path:
|
||||
return root / ALLOWLIST_FILE
|
||||
|
||||
|
||||
def shared_path() -> Path:
|
||||
"""The fleet-wide allowlist that every repo inherits.
|
||||
|
||||
Lives beside the gate it belongs to. Overridable so a test -- or a machine
|
||||
that keeps utils somewhere else -- is not forced to touch the real one.
|
||||
"""
|
||||
override = os.environ.get(SHARED_ALLOWLIST_ENV)
|
||||
if override:
|
||||
return Path(override).expanduser()
|
||||
return Path(__file__).resolve().parent.parent / ALLOWLIST_FILE
|
||||
|
||||
|
||||
def _parse_date(raw: object, where: str) -> date:
|
||||
if isinstance(raw, date) and not isinstance(raw, datetime):
|
||||
return raw
|
||||
@ -96,8 +114,21 @@ def check_expiry(entry: Exception_, today: date | None = None) -> None:
|
||||
|
||||
|
||||
def load(root: Path) -> list[Exception_]:
|
||||
"""Every exception declared for `root`; [] when there is no allowlist."""
|
||||
source = path_for(root)
|
||||
"""Every exception in force for `root`: the shared set, then the repo's.
|
||||
|
||||
A repo entry for the same `ecosystem:package` replaces the shared one, so a
|
||||
repo can always be stricter or state its own reason -- but nobody has to
|
||||
copy a fleet-wide blocker into forty files to be able to commit.
|
||||
"""
|
||||
entries = {
|
||||
f"{e.ecosystem}:{e.package}": e for e in _load_file(shared_path())
|
||||
}
|
||||
for entry in _load_file(path_for(root)):
|
||||
entries[f"{entry.ecosystem}:{entry.package}"] = entry
|
||||
return list(entries.values())
|
||||
|
||||
|
||||
def _load_file(source: Path) -> list[Exception_]:
|
||||
if not source.is_file():
|
||||
return []
|
||||
try:
|
||||
|
||||
@ -21,7 +21,7 @@ import sys
|
||||
|
||||
from dep_freshness import report
|
||||
from dep_freshness._tables import NPM
|
||||
from dep_freshness.allowlist import AllowlistError, load
|
||||
from dep_freshness.allowlist import AllowlistError, load, shared_path
|
||||
from dep_freshness.discover import find_manifests, is_manifest, parse_manifest
|
||||
from dep_freshness.evaluate import judge
|
||||
from dep_freshness.models import Finding, Severity
|
||||
@ -43,12 +43,19 @@ def repo_root(start: Path) -> Path:
|
||||
return Path(result.stdout.strip() or start)
|
||||
|
||||
|
||||
def _excuse(findings: list[Finding], entries) -> tuple[list[Finding], list]:
|
||||
def _excuse(
|
||||
findings: list[Finding], entries, shared: Path | None = None
|
||||
) -> tuple[list[Finding], list]:
|
||||
"""Split findings into (still failing, excused), and flag dead entries.
|
||||
|
||||
A `transitive:` entry clears itself the moment its dependency is no longer
|
||||
stale — that is what makes it a predicate rather than a date. An entry with
|
||||
nothing left to excuse is allowlist rot and exits 2.
|
||||
|
||||
Inherited entries are exempt from that rot check everywhere except the
|
||||
repo that owns the shared file. A fleet-wide hold on `typescript`
|
||||
legitimately excuses nothing in a Flutter app, and failing there would
|
||||
make every repo without that dependency uncommittable.
|
||||
"""
|
||||
by_label = {f"{e.ecosystem}:{e.package}": e for e in entries}
|
||||
used: set[str] = set()
|
||||
@ -64,7 +71,11 @@ def _excuse(findings: list[Finding], entries) -> tuple[list[Finding], list]:
|
||||
finding.dep, finding.severity, finding.latest, finding.detail,
|
||||
excused=entry.reason,
|
||||
))
|
||||
dead = [e for e in entries if f"{e.ecosystem}:{e.package}" not in used]
|
||||
dead = [
|
||||
e for e in entries
|
||||
if f"{e.ecosystem}:{e.package}" not in used
|
||||
and (shared is None or e.source != shared)
|
||||
]
|
||||
return failing, dead
|
||||
|
||||
|
||||
@ -149,7 +160,10 @@ def main(argv: list[str] | None = None) -> int:
|
||||
|
||||
resolver = Resolver(refresh=args.refresh)
|
||||
findings = collect(_targets(args, root), resolver)
|
||||
failing, dead = _excuse(findings, entries)
|
||||
shared = shared_path()
|
||||
failing, dead = _excuse(
|
||||
findings, entries, shared=None if root == shared.parent else shared
|
||||
)
|
||||
|
||||
still_blocking = {f.label: True for f in findings}
|
||||
report.exceptions_block(entries, still_blocking)
|
||||
|
||||
@ -17,9 +17,14 @@ from __future__ import annotations
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from dep_freshness._tables import NPM_API, NPM_QUARANTINE_HOURS
|
||||
from dep_freshness.cache import Cache
|
||||
from dep_freshness.registries.http import Offline, get_json
|
||||
from dep_freshness.versions import newest_stable, parse
|
||||
|
||||
# Its own cache namespace: the answer here is narrower than `npm/<name>` and
|
||||
# the two must not overwrite each other.
|
||||
CACHE_ECOSYSTEM = "npm-installable"
|
||||
|
||||
|
||||
def cutoff(now: datetime | None = None) -> datetime:
|
||||
"""Publishes at or after this instant are still quarantined."""
|
||||
@ -35,7 +40,10 @@ def _parse(stamp: str) -> datetime | None:
|
||||
|
||||
|
||||
def installable_latest(
|
||||
name: str, ceiling: str, now: datetime | None = None
|
||||
name: str,
|
||||
ceiling: str,
|
||||
now: datetime | None = None,
|
||||
cache: Cache | None = None,
|
||||
) -> str | None:
|
||||
"""Newest stable release published before the quarantine cutoff.
|
||||
|
||||
@ -50,11 +58,25 @@ def installable_latest(
|
||||
registry has no `time` map), which callers must treat as "no change" --
|
||||
narrowing on a guess would hide a genuinely stale dependency.
|
||||
"""
|
||||
# Cached under the normal TTL. Without this every gate run in a JS-heavy
|
||||
# repo re-downloads tens of full packuments, inside a pre-commit hook --
|
||||
# the exact cost the 6h cache exists to prevent.
|
||||
#
|
||||
# The ceiling is part of the KEY, not applied afterwards: clamping a cached
|
||||
# answer to the ceiling would return the ceiling itself, and the ceiling is
|
||||
# frequently the very release the quarantine is excluding. `dist-tags.latest`
|
||||
# is stable within a TTL, so hits still land across manifests.
|
||||
store = cache if cache is not None else Cache()
|
||||
key = f"{name}@{ceiling}"
|
||||
entry = store.get(CACHE_ECOSYSTEM, key)
|
||||
if entry is not None and entry.fresh(CACHE_ECOSYSTEM):
|
||||
return entry.version
|
||||
|
||||
url = NPM_API.format(name=name.replace("/", "%2f"))
|
||||
try:
|
||||
payload = get_json(url)
|
||||
except Offline:
|
||||
return None
|
||||
return entry.version if entry is not None else None # stale beats none
|
||||
if not payload:
|
||||
return None
|
||||
times = payload.get("time")
|
||||
@ -73,4 +95,7 @@ def installable_latest(
|
||||
if top is not None and (here is None or here > top):
|
||||
continue
|
||||
old_enough.append(version)
|
||||
return newest_stable(old_enough)
|
||||
answer = newest_stable(old_enough)
|
||||
store.put(CACHE_ECOSYSTEM, key, answer)
|
||||
store.save()
|
||||
return answer
|
||||
|
||||
@ -46,9 +46,35 @@ def no_network(monkeypatch):
|
||||
http.force_offline(False)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@pytest.fixture(autouse=True)
|
||||
def no_shared_allowlist(tmp_path, monkeypatch) -> Path:
|
||||
"""Point the fleet-wide allowlist at a path that does not exist.
|
||||
|
||||
AUTOUSE for the same reason as `cache_dir`: repos inherit the real
|
||||
~/utils/dependency-freshness.allowlist.yaml, so without this every test
|
||||
silently reads whatever is currently excused fleet-wide and starts
|
||||
depending on it.
|
||||
"""
|
||||
# In its OWN directory, not tmp_path: the `repo` fixture is tmp_path, and
|
||||
# the gate exempts inherited entries from the rot check everywhere except
|
||||
# the repo that owns the shared file. Putting both in one directory makes
|
||||
# every test look like it is running inside utils.
|
||||
target = tmp_path / "shared" / "shared-allowlist.yaml"
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
monkeypatch.setenv("DEP_FRESHNESS_SHARED_ALLOWLIST", str(target))
|
||||
return target
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def cache_dir(tmp_path, monkeypatch) -> Path:
|
||||
"""Point the on-disk registry snapshot at a scratch directory."""
|
||||
"""Point the on-disk registry snapshot at a scratch directory.
|
||||
|
||||
AUTOUSE, and not negotiable: any code path that constructs a bare `Cache()`
|
||||
writes to the real ~/.cache/dep-freshness/registry.json otherwise. That
|
||||
already happened once -- a quarantine test seeded a fake answer for package
|
||||
"x" into the live cache, and the next run of the same test read its own
|
||||
pollution back and passed for the wrong reason.
|
||||
"""
|
||||
target = tmp_path / "cache"
|
||||
monkeypatch.setenv("DEP_FRESHNESS_CACHE", str(target))
|
||||
return target
|
||||
|
||||
@ -152,3 +152,46 @@ def test_unreadable_yaml_is_an_error(tmp_path):
|
||||
_allowlist(tmp_path, "exceptions: [\n")
|
||||
with pytest.raises(AllowlistError, match="unreadable"):
|
||||
load(tmp_path)
|
||||
|
||||
|
||||
def test_the_shared_allowlist_is_inherited(tmp_path, no_shared_allowlist):
|
||||
"""A fleet-wide blocker must not have to be copied into forty repos."""
|
||||
no_shared_allowlist.write_text(TRANSITIVE, encoding="utf-8")
|
||||
entries = load(tmp_path)
|
||||
assert [(e.ecosystem, e.package) for e in entries] == [
|
||||
("pub", "plugin_platform_interface")
|
||||
]
|
||||
|
||||
|
||||
def test_a_repo_entry_overrides_the_shared_one(tmp_path, no_shared_allowlist):
|
||||
no_shared_allowlist.write_text(TRANSITIVE, encoding="utf-8")
|
||||
_allowlist(tmp_path, TRANSITIVE.replace(
|
||||
'reason: "firebase_core 4.2.0 constrains this below latest"',
|
||||
'reason: "this repo has its own reason"'))
|
||||
entries = load(tmp_path)
|
||||
assert len(entries) == 1
|
||||
assert entries[0].reason == "this repo has its own reason"
|
||||
|
||||
|
||||
def test_shared_and_repo_entries_for_different_packages_both_apply(
|
||||
tmp_path, no_shared_allowlist
|
||||
):
|
||||
no_shared_allowlist.write_text(TRANSITIVE, encoding="utf-8")
|
||||
_allowlist(tmp_path, _discretionary(30))
|
||||
assert {e.package for e in load(tmp_path)} == {
|
||||
"plugin_platform_interface", "abandoned"
|
||||
}
|
||||
|
||||
|
||||
def test_a_malformed_shared_allowlist_is_an_error(tmp_path, no_shared_allowlist):
|
||||
no_shared_allowlist.write_text("exceptions:\n - just-a-string\n",
|
||||
encoding="utf-8")
|
||||
with pytest.raises(AllowlistError, match="must be a mapping"):
|
||||
load(tmp_path)
|
||||
|
||||
|
||||
def test_the_shared_path_defaults_to_the_gate_repo(monkeypatch):
|
||||
from dep_freshness.allowlist import shared_path
|
||||
monkeypatch.delenv("DEP_FRESHNESS_SHARED_ALLOWLIST", raising=False)
|
||||
assert shared_path().name == NAME
|
||||
assert shared_path().parent.name == "utils"
|
||||
|
||||
@ -186,3 +186,43 @@ def _offline(_ecosystem, _name):
|
||||
|
||||
def test_the_cache_directory_is_honoured(cache_dir):
|
||||
assert os.environ["DEP_FRESHNESS_CACHE"] == str(cache_dir)
|
||||
|
||||
|
||||
SHARED = """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: some_other_package
|
||||
pinned: "1.0.0"
|
||||
reason: "a fleet-wide hold on something this repo does not use"
|
||||
blocked_by: "transitive:whatever@1.0.0"
|
||||
"""
|
||||
|
||||
|
||||
def test_an_inherited_entry_this_repo_does_not_need_is_not_rot(
|
||||
repo, run, no_shared_allowlist
|
||||
):
|
||||
"""A fleet-wide hold on typescript excuses nothing in a Flutter app.
|
||||
|
||||
Treating that as a dead entry would make every repo without the dependency
|
||||
uncommittable -- the shared allowlist would be unusable for the exact case
|
||||
it exists for.
|
||||
"""
|
||||
no_shared_allowlist.write_text(SHARED, encoding="utf-8")
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
assert run("--all") == 0
|
||||
|
||||
|
||||
def test_a_repo_local_entry_with_nothing_to_excuse_is_still_rot(
|
||||
repo, run, no_shared_allowlist
|
||||
):
|
||||
no_shared_allowlist.write_text(SHARED, encoding="utf-8")
|
||||
write(repo, "pubspec.yaml", CURRENT_PUBSPEC)
|
||||
write(repo, ALLOWLIST, """\
|
||||
exceptions:
|
||||
- ecosystem: pub
|
||||
package: http
|
||||
pinned: "1.5.0"
|
||||
reason: "this repo's own stale entry"
|
||||
blocked_by: "transitive:x@1.0.0"
|
||||
""")
|
||||
assert run("--all") == 2
|
||||
|
||||
@ -166,3 +166,52 @@ def test_a_repo_pinned_to_the_newest_installable_version_passes(
|
||||
'{"dependencies": {"@testing-library/react": "16.3.2"}}')
|
||||
monkeypatch.chdir(repo)
|
||||
assert check.main(["--all"]) == 0
|
||||
|
||||
|
||||
def test_the_answer_is_cached_so_a_second_run_makes_no_request(packument,
|
||||
cache_dir):
|
||||
"""A 7MB packument per npm finding, per run, inside a pre-commit hook is
|
||||
exactly what the 6h cache exists to prevent."""
|
||||
from dep_freshness.cache import Cache
|
||||
|
||||
store = Cache(cache_dir / "registry.json")
|
||||
packument(PACKUMENT)
|
||||
assert quarantine.installable_latest("x", "16.3.3", now=NOW,
|
||||
cache=store) == "16.3.2"
|
||||
|
||||
packument(None) # any further fetch would now answer "no such package"
|
||||
assert quarantine.installable_latest("x", "16.3.3", now=NOW,
|
||||
cache=store) == "16.3.2"
|
||||
|
||||
|
||||
def test_a_different_dist_tag_is_a_different_cache_entry(packument, cache_dir):
|
||||
"""The ceiling is part of the key: clamping a cached answer to it would
|
||||
return the ceiling, which is often the release being excluded."""
|
||||
from dep_freshness.cache import Cache
|
||||
|
||||
store = Cache(cache_dir / "registry.json")
|
||||
packument(PACKUMENT)
|
||||
assert quarantine.installable_latest("x", "16.3.2", now=NOW,
|
||||
cache=store) == "16.3.2"
|
||||
assert store.get(quarantine.CACHE_ECOSYSTEM, "x@16.3.3") is None
|
||||
|
||||
|
||||
def test_a_stale_cache_entry_is_served_when_the_registry_is_down(packument,
|
||||
cache_dir):
|
||||
from dep_freshness.cache import Cache
|
||||
from dep_freshness.registries.http import Offline
|
||||
|
||||
store = Cache(cache_dir / "registry.json")
|
||||
store.put(quarantine.CACHE_ECOSYSTEM, "x@16.3.3", "16.3.2")
|
||||
store._data[f"{quarantine.CACHE_ECOSYSTEM}/x@16.3.3"]["fetched_at"] = 0.0
|
||||
|
||||
def down(*_a, **_k):
|
||||
raise Offline("no network")
|
||||
|
||||
monkey = down
|
||||
import dep_freshness.quarantine as q
|
||||
original, q.get_json = q.get_json, monkey
|
||||
try:
|
||||
assert q.installable_latest("x", "16.3.3", cache=store) == "16.3.2"
|
||||
finally:
|
||||
q.get_json = original
|
||||
|
||||
@ -1,5 +1,11 @@
|
||||
# Dependencies deliberately held below latest stable, and why.
|
||||
#
|
||||
# THIS IS THE FLEET-WIDE FILE. Every repo inherits it; a repo's own
|
||||
# dependency-freshness.allowlist.yaml adds to it, and an entry there for the
|
||||
# same ecosystem:package overrides the one here. A blocker like "typescript-eslint
|
||||
# does not support TS 7" holds in every repo that pins typescript, and copying
|
||||
# it into eight of them would give eight entries that rot independently.
|
||||
#
|
||||
# Two entry classes, and the distinction is load-bearing:
|
||||
#
|
||||
# blocked_by: transitive:<pkg>@<ver>
|
||||
@ -24,6 +30,10 @@ exceptions:
|
||||
typescript-eslint 8.68.0 -- itself the latest release -- declares
|
||||
`typescript: ">=4.8.4 <6.1.0"` and throws "typescript-eslint does not
|
||||
support TS 7.0" when eslint loads its config. No 9.x exists. Taking TS 7
|
||||
would mean dropping every type-aware lint rule from ts_core and web_ui
|
||||
until upstream catches up, so the compiler waits for the linter.
|
||||
would mean dropping every type-aware lint rule from every repo that pins
|
||||
it, so the compiler waits for the linter.
|
||||
Held fleet-wide: this blocks ts_core, web_ui, konbini-67, iron-and-anvil,
|
||||
awesome-mcp-explorer, europe-county-map and every other repo pinning
|
||||
typescript, which is exactly why the entry lives here and not in eight
|
||||
separate files.
|
||||
blocked_by: "transitive:typescript-eslint@8.68.0"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user