diff --git a/REPORT.md b/REPORT.md index 67159623..11818e27 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,4 +1,16 @@ # REPORT +## [2026-09-18] 「조용한 실패」를 잡는 검사기에 «조용히 통과하는 길»이 있었다 (rustjava-merge-dropped-symbols-checker-swallows-git-failures) +- 무엇을: `scripts/check-merge-dropped-symbols.py` 의 `run()` 이 git 실패를 `None` 으로 삼키고 호출부가 전부 `(… or "")` 로 받아 ★**「git 이 못 답했다」가 「없다고 답했다」로 접혔다** ⇒ `✓ (0 file(s) examined)` · **rc=0**. +- ★**재현은 합성이 아니라 «진짜 얕은 클론»이다**(`--depth 10`): **전 rc=0** 에 `✓ 97660921 (0 …)` · `✓ 56bb54fa (0 …)` ↔ ★**완전 클론에서 `56bb54fa` 는 examined «20»** 이다. ⇒ 20개를 보던 머지가 0으로 접히고 run 전체가 green 이었다. **후 rc=2** `cannot measure: shallow clone: …(git fetch --unshallow)`. +- ★**preflight 만이 아니라 raise 경로도 쟀다** — 범위 오류 · 루프 «안» diff 실패 · 비-git 디렉터리 **전부 rc=2** 이고 문면에 git stderr 를 그대로 싣는다. +- ★★**급소 = «실패»와 «빈 결과»를 가르는 것**: `run()` 호출부 **8곳** 중 ★**`git show :` 한 자리만 «실패가 답»**이다(경로가 그 트리에 없는 것은 **정상**). ⇒ rc 로는 못 가르므로 ★**환경 자체를 preflight 로 배제**하고 그 뒤의 `show` 실패는 **부재로만** 읽는다(전제를 주석에 명기). +- ★**ⓒ CI 는 이미 `fetch-depth: 0`** 이다(`merge_drops` job · 주석에 이유까지) ⇒ ★**이 변경이 전 PR 을 막지 않는다**(해당 job 의 얕은 클론 빈도 **0**). ★그래서 처방이 「객체를 먼저 받는 것」이 아니라 **「rc 를 올리는 것」**으로 정해졌다. +- ★**양방향**: ⒜실패 → **rc=2 「못 쟀다」**(`✓` 금지) ⒝★**정상인데 0** — `97660921`(`.md`/`.json` 만 바뀐 머지 · 완전 클론에서도 진짜 0) → **여전히 `✓` rc=0** ⒞**탐지 회귀 0** — `e53b2142` **6** · `514d5b08` **6** · `56bb54fa` examined **20** 불변. +- ★**잃는 것**: 손으로 얕은 트리에서 돌리던 사람은 **이제 빨강**을 본다(그 초록이 거짓이었다) · 표시용·면제용 호출까지 일괄 raise 라 ★**더 자주 멈춘다**(안전한 예외가 `show` 하나뿐임을 표로 못박은 대가) · ★**preflight 는 «얕음»만 본다** — 부분 클론·손상 객체는 **여전히 `show` 의 부재로 읽힐 수 있다**(닫은 것은 가장 흔한 한 갈래) · F8 의 「0」이 정말 이 경로였는지는 **증명 못 한다**(가설과 정합할 뿐). +- ★**범위**: 판정 술어·필터 폭·`PATTERNS` **무접촉** · **`.rs` 0건** · 새 검사기·새 워크플로 **0** · 종료코드는 **이미 있던 `2`** 를 쓴다. +- 검증: 검사기 자기 실행 `✓ 430fef8a (11 file(s) examined)` rc=0 · `check-worklog-json` rc=0 · `check-dod-ci-parity` rc=0(명령 7개) · `cargo fmt` rc=0. +- ★**이 PR 은 #71 에 «쌓여» 있다** — 검사기가 `origin/main` 에 **아직 없다**(PR #71 브랜치에만 있다) ⇒ base = `feat/rustjava-merge-drop-check`. ★게이트③ 계약 5 대로 **#71 머지 회차가 먼저 base 를 `main` 으로 재지정**해야 한다. +- ★후속 추천: ⑴**부분 클론도 preflight 로 막을 것인가**(S · 남은 한 갈래) ⑵`show` 의 두 실패를 **stderr 문면으로 가를 것인가**(S · git 판올림에 약해 이번엔 환경 배제를 골랐다). 상세 = `docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.md`. ## [2026-09-18] 검사기의 «거짓 초록» 둘과 «거짓 빨강» 하나 — 게이트² 반려 승계 (rustjava-lock-every-named-exception-class-is-loadable-fix) - 무엇을: PR #72 의 검사기 결함 **3건** 정정. ★**베이스라인 0 인 검사기라 «거짓 초록 = 검사기 부재»** 다. ★런타임 클래스 추가 **0** · `loader.rs` `protos` **무접촉** · `jvm.rs` **무접촉**. - ★**F1(거짓 초록)** 짧은 이름 충돌 — `Formatter`(`java/util` ↔ `java/util/logging`) · `JarURLConnection`(`java/net` ↔ `org/rustjava/net`) **2쌍 실재**. ★재현: 한쪽 등재를 지우고 그 이름을 `exception(` 에 넣으면 **전 `✓ … 263 loadable` rc=0**(거짓 초록) → **후 rc=1**. 둘째 쌍도 동일. ★처방 = 키를 **(모듈, 타입, 함수)** 로. diff --git a/STATE.md b/STATE.md index 5e4beb94..e21fd100 100644 --- a/STATE.md +++ b/STATE.md @@ -7,6 +7,14 @@ (둘 다 이것보다 오래됐고 MERGEABLE/CONFLICTING 처분이 이미 걸려 있다). 겹침은 전부 **append 형 합집합**이라 해소는 기계적이다) ## 완료 +- [rustjava-merge-dropped-symbols-checker-swallows-git-failures] ★★**「조용한 실패」 검사기에 «조용히 통과하는 길»이 있었다 — 닫았다.** + ★**재현 = 진짜 얕은 클론**(`--depth 10`): 전 **rc=0** `✓ 56bb54fa (0 file(s) examined)` ↔ ★완전 클론에선 **examined 20** ⇒ 20→0 으로 접히고 green. 후 **rc=2 `cannot measure: shallow clone…`**. + ★raise 경로도 쟀다 — 범위 오류·루프 내 diff 실패·비-git **전부 rc=2**(git stderr 동봉). + ★★**급소**: `run()` 호출부 8곳 중 ★**`git show :` 하나만 «실패가 답»**(경로 부재는 정상) ⇒ rc 로 못 가르니 **환경을 preflight 로 배제**. + ★**ⓒ CI 는 이미 `fetch-depth: 0`** ⇒ 전 PR 을 막지 않는다(그 job 의 얕은 클론 빈도 **0**) — 그래서 처방이 «rc 올리기»로 정해졌다. + ★**양방향**: 실패→rc=2 · ★**정상 0**(`97660921`)→**여전히 ✓ rc=0** · 탐지 회귀 0(`e53b2142` 6 · `514d5b08` 6 · `56bb54fa` 20). + ★**잃는 것**: 얕은 트리 수동 실행은 이제 빨강 · 표시/면제 호출까지 일괄 raise 라 **더 자주 멈춘다** · ★preflight 는 «얕음»만 본다(부분 클론은 남았다) · F8 「0」의 인과는 **미증명**. + ★`.rs` 0건 · 술어·필터 폭 무접촉 · 종료코드는 기존 `2` 재사용. ★**PR 은 #71 에 쌓여 있다**(검사기가 main 에 없다) — 계약 5 재지정 필요. - [rustjava-lock-every-named-exception-class-is-loadable-fix] ★★**검사기의 «거짓 초록» 2건 + «거짓 빨강» 1건 정정**(게이트² 반려 승계 · PR #72). ★**F1** 짧은 이름 충돌(`Formatter`·`JarURLConnection` **2쌍**) ⇒ 한쪽 등재를 지워도 **전 rc=0(거짓 초록)** → **후 rc=1**. 키를 **(모듈,타입,함수)** 로. ★**F2** 줄 단위 스캔이 다중 줄 호출을 못 봄 ⇒ **전 rc=0(안 보임) → 후 rc=1**. ★**846 − 34 = 812** 로 검수자 수와의 차이를 설명했다. diff --git a/docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.json b/docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.json new file mode 100644 index 00000000..73dc958d --- /dev/null +++ b/docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.json @@ -0,0 +1,37 @@ +{ + "date": "2026-09-18", + "taskId": "rustjava-merge-dropped-symbols-checker-swallows-git-failures", + "summary": "The check that exists to catch a silent loss had a silent pass in it: run() returned None on a failing git, every caller wrote `run(...) or \"\"`, and the result was `✓ (0 file(s) examined)` with rc 0. git calls now raise, main() reports `cannot measure: …` with rc 2, and the one call where a non-zero exit is a real answer keeps tolerating it behind a preflight. The predicate for what counts as a dropped symbol is untouched.", + "changes": [ + "scripts/check-merge-dropped-symbols.py — new CannotMeasure exception; run() raises unless absence_is_an_answer=True; preflight() rejects a shallow clone; main() catches in two places and returns 2; the exit-code docstring says 'could not measure' rather than 'could not run'.", + "Six call sites lost their `or \"\"` fallback (parents, merge-base, two diffs, the trailer message, the display subject). symbols()'s `git show` is the single tolerated failure." + ], + "verification": [ + "REPRODUCED IN A REAL ENVIRONMENT, not a mock: cloning this repository at --depth 10 and running the pre-change script over one merge range printed `✓ 97660921 (0 file(s) examined)` and `✓ 56bb54fa (0 file(s) examined)` and exited 0. In a full clone the same merges examine 0 and 20 files respectively — so a merge that reads 20 files collapsed to 0 and the run stayed green.", + "AFTER, same shallow clone, same range: rc=2 with `cannot measure: shallow clone: a merge's second parent is not here, so every read of it would look empty. Fetch the full history (git fetch --unshallow) and run again.`", + "THE RAISE PATH WAS CHECKED SEPARATELY FROM THE PREFLIGHT, all rc=2, all carrying git's own stderr: a bad range (rev-list exits 128), a failure inside the per-merge loop (diff forced to fail), and running outside a git repository.", + "CALL-SITE CENSUS: 8 uses of run(). Seven must raise. One must not — `git show :` also exits non-zero when the path is simply not in that tree, which is ordinary and means 'no definitions here'. rc alone cannot separate that from a missing object, so the ambiguity is removed by ruling out the environment in preflight() rather than by reading stderr text.", + "CI ALREADY FETCHES FULL HISTORY: the merge_drops job pins fetch-depth: 0 with a comment saying why. rc!=0 fails that job because the script is its last step. So raising on failure does not red existing PRs — the frequency of a shallow checkout in that job is 0. That measurement is what decided the prescription (raise) over the alternative the ticket named (fetch the objects first): CI already fetches them.", + "BIDIRECTIONAL: (a) failure -> rc=2 and a 'cannot measure' line, never ✓. (b) a genuinely empty merge, 97660921, whose changed files are all .md/.json so nothing is readable by the patterns — still `✓ (0 file(s) examined)` rc=0. (c) detection unchanged: e53b2142 findings 6, 514d5b08 findings 6, 56bb54fa examined 20, 430fef8a examined 11, all identical to before.", + "cargo untouched: git diff --numstat is one file, scripts/check-merge-dropped-symbols.py 68/15, and .rs files changed = 0. check-worklog-json rc=0, check-dod-ci-parity rc=0 (7 commands), cargo fmt rc=0, and the checker's own run over origin/main..HEAD is `✓ 430fef8a (11 file(s) examined)` rc=0." + ], + "issues": [ + "Anyone who ran this by hand in a shallow tree now sees red where they saw green. The green was false, and the message says how to fix it, but it looks like a new breakage to whoever meets it first.", + "Display-only and exemption-only calls (the subject line, the trailer message) were raised too, so the check can now stop for reasons that used to degrade quietly. That was chosen over per-site judgement because such judgement goes stale; the one safe exception is pinned in a table in the worklog.", + "preflight() only rules out shallowness. A partial clone (--filter=blob:none) or a corrupt object can still make `git show` fail in a way that reads as absence. The other call sites raise, so the remaining hole is show-only — narrower than before but not closed.", + "Whether F8's original '0' really came through this path is not proven; there is no execution record from that round. The behaviour is consistent with it, which is all that can be said.", + "THIS PR IS STACKED ON PR #71. scripts/check-merge-dropped-symbols.py does not exist on origin/main yet — it only exists on PR #71's branch — so this work cannot be cut from main. The base is feat/rustjava-merge-drop-check, which makes this a child PR: gate3 contract 5 requires the round that merges #71 to re-parent it to main first, or GitHub closes it when the base branch is deleted." + ], + "adoptedProposals": [], + "proposals": [ + { + "title": "Decide whether a partial clone should also be refused up front", + "plainSummary": "The check now refuses to run in a shallow clone, but a clone fetched without file contents can still make it look like nothing is there.", + "userBenefit": "The same false green this round removed can still happen in a clone made with --filter, which is the shape CI systems increasingly default to.", + "why": "preflight() tests one thing: is the repository shallow. That covers the common case and was measured. A partial clone is different — the commits are all present, so every other git call succeeds, but `git show :` cannot fetch the blob and fails in exactly the way that now means 'the path is not in this tree'. The remaining hole is that one call, and it is the one call this round deliberately left tolerant.", + "tradeoff": "Refusing partial clones outright would block a legitimate and cheap way to run CI, and the check may work fine there if the blobs get fetched on demand — which depends on the remote's configuration rather than ours. Detecting it properly means asking git about promisor remotes, which is a smaller, quieter API than --is-shallow-repository and one more thing to keep working.", + "effort": "S", + "target": "scripts/check-merge-dropped-symbols.py" + } + ] +} diff --git a/docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.md b/docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.md new file mode 100644 index 00000000..7bb79c8c --- /dev/null +++ b/docs/worklog/2026-09-18-merge-drops-no-silent-git-failure.md @@ -0,0 +1,100 @@ +# 「조용한 실패」를 잡는 검사기에 «조용히 통과하는 길»이 있었다 + +## ⓐ 재현 — ★**합성이 아니라 «진짜 얕은 클론»에서 났다** + +`scripts/check-merge-dropped-symbols.py` 의 `run()` 이 git 실패를 `None` 으로 삼키고, +호출부가 전부 `(run(...) or "")` 로 받아 ★**「git 이 못 답했다」가 「git 이 «없다»고 답했다」로 접혔다.** + +이 저장소를 **깊이 10** 으로 얕게 클론해 그대로 돌렸다(`git clone --depth 10`): +``` +--- 전(착지 전 판본) rc=0 + ✓ 97660921 (0 file(s) examined) + ✓ 56bb54fa (0 file(s) examined) +11 merge(s) in …: 0 definition(s) dropped without a trailer +--- 후(이 회차) rc=2 +cannot measure: shallow clone: a merge's second parent is not here, so every read of it would +look empty. Fetch the full history (git fetch --unshallow) and run again. +``` +★**대조 — 같은 머지를 «완전 클론»에서 재면**: `56bb54fa` 는 **examined 20**, `430fef8a` 는 **11** 이다. +⇒ ★**20개를 보던 머지가 0개로 접히고 그 run 전체가 green 이었다.** 검사기가 막으려는 실패 양식 그 자체다. + +★**preflight 만이 아니라 «raise 경로»도 확인했다**(셋 다 rc=2 · 문면에 git stderr 를 그대로 싣는다): +``` +잘못된 범위 → cannot measure: git rev-list --merges no-such-ref..HEAD exited 128: fatal: ambiguous argument… +루프 «안» diff 실패 → cannot measure: git diff exited 128: … ← 머지별 검사 도중 +git 아닌 디렉터리 → cannot measure: git rev-parse --is-shallow-repository exited 128: fatal: not a git repository +``` + +## ⓑ 호출부 구분표 — ★**급소는 «실패»와 «빈 결과»를 가르는 것이다** + +`run()` 호출부는 **8곳**이고, ★**「실패 시 옳은 동작」이 한 곳만 다르다**: + +| 자리 | 호출 | 실패의 뜻 | 옳은 동작 | 처분 | +|---|---|---|---|---| +| `symbols()` | `show :` | ★**둘이 섞인다** — ⑴그 트리에 그 경로가 없다(**정상**) ⑵객체가 없다(**실패**) | ⑴은 `set()`, ⑵는 실패 | ★**`absence_is_an_answer=True`** + **preflight 로 ⑵를 배제** | +| `excused()` | `log -1 --format=%B` | trailer 를 못 읽는다 ⇒ 면제가 «덜» 적용돼 false red | 실패 | raise | +| `check()` | `rev-list --parents -n 1` | 부모를 못 읽는다 ⇒ **「머지가 아니다」로 오독** → 조용한 통과 | 실패 | raise | +| `check()` | `merge-base` | base 를 못 얻는다 ⇒ `[],0` → 조용한 통과 | 실패 | raise | +| `check()` ×2 | `diff --name-only` | 변경 파일 목록이 빈다 ⇒ **examined 0** → 조용한 통과 | 실패 | raise | +| `main()` | `rev-list --merges` | 범위를 못 읽는다 | 실패 | raise | +| `main()` | `log -1 --format=%s` | 제목을 못 읽는다(표시용) | 실패 | raise | + +★★**`show` 만 «실패가 답»인 이유**: `git show :` 는 **경로가 그 트리에 없을 때도** 실패한다. +그리고 그 경우는 **흔하고 정상**이다(머지가 «새로 추가한» 파일은 `theirs` 에 없다). +⇒ rc 만으로는 못 가른다 ⇒ ★**「객체가 없을 수 있는 환경」 자체를 preflight 로 먼저 배제**하고, +그 뒤의 `show` 실패는 **부재로만** 읽는다. ★그 전제를 코드 주석에 적었다(다음 사람이 preflight 를 지우면 이 가정이 깨진다). + +## ⓒ CI — ★**이미 `fetch-depth: 0` 이다. 그래서 이 변경이 PR 을 막지 않는다** + +`.github/workflows/rust.yml` 의 `merge_drops` job: +```yaml + # … fetch-depth 0 because the check needs the merge commits and their parents, + # which a shallow checkout does not have. One runner, not the matrix. + merge_drops: + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + - run: python3 scripts/check-merge-dropped-symbols.py +``` +★**rc≠0 은 그 step 을 실패시키고 job 이 빨개진다**(마지막 step 이라 job rc 가 곧 스크립트 rc). +★**그리고 얕은 클론은 그 job 에서 «구조적으로 일어나지 않는다»** — 다른 job 들은 기본(얕음)이지만 이 검사기를 돌리지 않는다. +⇒ ★**「즉시 전 PR 을 막는다」는 일어나지 않는다.** 계약 2 가 「재라」고 한 그 수를 재서 적는다: **해당 job 의 얕은 클론 빈도 = 0**. +★이 사실 때문에 처방이 «객체를 먼저 받는 것»이 아니라 **«rc 를 올리는 것»** 으로 정해졌다 — CI 는 이미 받고 있고, +남은 위험은 **사람이 손으로 돌리는 얕은 트리**뿐이며 거기서는 «빨강 + 처방 문면»이 옳다. + +## 무엇을 바꿨나 — 로직은 «실패 처리»만 + +- `CannotMeasure` 예외 신설 · `run()` 이 **raise**(단 `absence_is_an_answer=True` 한 자리만 `None`). +- `preflight()` — 얕은 클론이면 즉시 `CannotMeasure`. +- `main()` 이 두 자리에서 잡아 ★**`cannot measure: …` + rc=2**. +- 종료코드 계약은 ★**이미 있던 `2`** 를 쓴다(「could not run」 → 「could not measure」로 문면만 정확히). +★**판정 술어(무엇을 dropped symbol 로 보는가)·필터 폭(narrow/wide)·PATTERNS 무접촉.** + +## 양방향 + +| 축 | 결과 | +|---|---| +| ⒜ **git 실패** | 얕은 클론 **rc=0 `✓ (0 file(s) examined)` → rc=2 「cannot measure」** · 범위 오류/루프 내 실패/비-git **전부 rc=2** | +| ⒝ **정상인데 examined=0** | `97660921`(`.md`/`.json` 만 바뀐 머지 · ★완전 클론에서도 진짜 0) → ★**여전히 `✓` rc=0** | +| ⒞ **정상 탐지 회귀 0** | `e53b2142` **findings 6** · `514d5b08` **6** · `56bb54fa` examined **20** · `430fef8a` **11** — 전부 변경 전과 동일 | +★⒝ 가 없으면 이 변경은 「검사기를 더 시끄럽게 한 것」과 구별되지 않는다. ★그 사례를 **합성하지 않고 생산 데이터에서** 골랐다. + +## 잃는 것 — 「없다」로 적지 않는다 + +- ★**손으로 얕은 트리에서 돌리던 사람은 이제 빨강을 본다.** 그 전에는 초록이었다 — ★**그 초록이 거짓이었다**는 것이 이 회차의 요지이고, + 처방 문면(`git fetch --unshallow`)을 메시지에 넣었다. ★그래도 **처음 겪는 사람에게는 «새로 깨진 것»으로 보인다.** +- ★**`excused()`·`log --format=%s` 처럼 «표시용·면제용» 호출까지 raise 로 올렸다** — 그쪽 실패는 원래 false red 나 빈 제목으로 끝났다. + ⇒ ★**더 자주 멈출 수 있다.** 일괄로 올린 이유는 「어느 실패가 안전한가」를 자리마다 판단하면 그 판단이 낡기 때문이고, + 안전한 예외는 **`show` 한 곳뿐**임을 표로 못박았다. ★그 선택의 대가는 «과민»이다. +- ★★**preflight 는 «얕음»만 본다** — 객체가 빠지는 다른 경로(부분 클론 `--filter=blob:none` · 손상된 객체)는 + **여전히 `show` 의 «부재»로 읽힐 수 있다.** ⇒ ★**이 회차가 닫은 것은 «가장 흔한 한 갈래»이지 전부가 아니다.** + 그 경우 `diff`·`rev-list` 쪽은 raise 로 잡히지만, **`show` 만 실패하는 형상은 남는다.** +- ★**F8 의 「0」이 «정말 이 경로로» 났는지는 증명하지 못한다** — 그 회차의 실행 기록이 없다. ★가설과 정합할 뿐이다. + +## 후속 추천 + +⑴★**부분 클론·손상 객체까지 preflight 로 덮을 것인가**(S) — 위 「잃는 것」 셋째. `--filter` 로 받은 트리에서 + `show` 가 부재와 구별되지 않는다. ★비용은 preflight 가 무거워지는 것이고, 이득은 **남은 한 갈래**다. +⑵★**`show` 의 두 실패를 stderr 로 가를 것인가**(S) — `fatal: path '…' does not exist` ↔ 객체 부재는 문면이 다르다. + ★문면 의존이라 git 판올림에 약하다 — 그래서 이번에는 **환경을 배제하는 쪽**을 골랐다. 그 판단을 다시 볼 자리다. diff --git a/scripts/check-merge-dropped-symbols.py b/scripts/check-merge-dropped-symbols.py index 287ca449..2ceada16 100755 --- a/scripts/check-merge-dropped-symbols.py +++ b/scripts/check-merge-dropped-symbols.py @@ -61,7 +61,13 @@ class described next. On time: the measurement noise is larger than the differen Exit: 0 nothing dropped, or everything dropped is accounted for by a trailer 1 something was dropped and not accounted for - 2 could not run (bad range, no git) + 2 could not measure -- a git call failed, or the clone is shallow + +★ 2 is not a softer 0. Every git call here raises rather than returning nothing, because the one +failure this check must never produce is a green one: in a shallow clone the second parent's objects +are absent, every read of it comes back empty, and the old code printed `✓ (0 file(s) examined)` and +exited 0. Measured on this repository at depth 10: merges that examine 20 files in a full clone +examined 0 and the whole run was green. A check for a silent loss had a silent pass in it. `-s ours` merges -- "record these upstream cuts as ancestors" -- drop the whole other side on purpose, and one trailer per name would mean hundreds. They use the wildcard form instead: @@ -102,9 +108,44 @@ class described next. On time: the measurement noise is larger than the differen } -def run(*args): +class CannotMeasure(RuntimeError): + """git could not answer, so this run knows nothing -- as opposed to knowing there is nothing.""" + + +def run(*args, absence_is_an_answer=False): + """git's stdout. + + ★ A failing git raises. It used to return None, and every caller wrote `run(...) or ""`, which + turned "git could not tell me" into "git told me nothing": an empty diff, no parents, no + symbols -- and then `✓ (0 file(s) examined)` with rc 0. A check that exists to catch a silent + loss had a silent pass in it, in the one environment (an incomplete clone) where it matters. + + `absence_is_an_answer=True` is for the one call where a non-zero exit is a real answer: + `git show :` fails when the path is simply not in that tree, which is ordinary. The + preflight in main() rules out the other reason that call can fail, so once it has run, a + failure there means absence and nothing else. Every other call site raises. + """ result = subprocess.run(["git", *args], capture_output=True, text=True) - return result.stdout if result.returncode == 0 else None + if result.returncode == 0: + return result.stdout + if absence_is_an_answer: + return None + raise CannotMeasure(f"git {' '.join(args)} exited {result.returncode}: {result.stderr.strip() or '(no stderr)'}") + + +def preflight(): + """What has to be true before a failing `git show` can be read as "the path is not there". + + Only one thing, and it is the environment this check is blind in: a shallow clone does not hold + the objects a merge's second parent needs, so every read of it fails and every failure used to + read as "empty". Measured on this repository at depth 10: merges that examine 20 files in a full + clone examined 0 and the run was green. + """ + if (run("rev-parse", "--is-shallow-repository") or "").strip() == "true": + raise CannotMeasure( + "shallow clone: a merge's second parent is not here, so every read of it would look empty. " + "Fetch the full history (git fetch --unshallow) and run again." + ) def symbols(rev, path): @@ -114,7 +155,9 @@ def symbols(rev, path): break else: return None - blob = run("show", f"{rev}:{path}") + # The only tolerated failure in the file: a path that is not in this tree is a real answer, + # and it means "no definitions here". preflight() has already ruled out the other reason. + blob = run("show", f"{rev}:{path}", absence_is_an_answer=True) if blob is None: return set() found = set() @@ -129,7 +172,7 @@ def symbols(rev, path): def excused(merge): """Names the merge commit itself says were dropped on purpose, with a reason.""" - message = run("log", "-1", "--format=%B", merge) or "" + message = run("log", "-1", "--format=%B", merge) names = set() for line in message.split("\n"): match = re.match(r"^Dropped-from-theirs:\s*(.+?)\s+--\s+(\S.*)$", line.strip()) @@ -145,13 +188,15 @@ def excuses_everything(accounted): def check(merge): """(findings, files_examined) for one merge commit. findings is a list of (path, name).""" - parents = (run("rev-list", "--parents", "-n", "1", merge) or "").split() + parents = run("rev-list", "--parents", "-n", "1", merge).split() if len(parents) < 3: return [], 0 # not a merge: a squash or an ordinary commit has nothing to compare ours, theirs = parents[1], parents[2] - base = (run("merge-base", ours, theirs) or "").strip() + base = run("merge-base", ours, theirs).strip() if not base: - return [], 0 + # git succeeded and still named no base: unrelated histories. That is a fact, not a failure, + # but there is nothing to compare against, so say so rather than pass. + raise CannotMeasure(f"{merge[:8]}: its parents share no merge base, so there is nothing to diff") # Both what the merged-in branch touched and what the merge itself touched. Restricting this to # the first set was the original shape and it was wrong: a resolution can revert a file the other # branch never touched -- "fixed the conflict in A and put B back" -- and that is this check's @@ -160,8 +205,8 @@ def check(merge): # examines 13 and names the same four definitions the first incident dropped. The narrow filter # was not blind -- it read four files and still missed it, because none of the four was where # the loss landed. The cost is real and is recorded in Scope above. - changed = set((run("diff", "--name-only", base, theirs) or "").split("\n")) | set( - (run("diff", "--name-only", base, merge) or "").split("\n") + changed = set(run("diff", "--name-only", base, theirs).split("\n")) | set( + run("diff", "--name-only", base, merge).split("\n") ) findings = [] examined = 0 @@ -181,16 +226,24 @@ def check(merge): def main(): rev_range = sys.argv[1] if len(sys.argv) > 1 else "origin/main..HEAD" - merges = run("rev-list", "--merges", rev_range) - if merges is None: - print(f"cannot read {rev_range}", file=sys.stderr) + try: + preflight() + merges = run("rev-list", "--merges", rev_range) + except CannotMeasure as failure: + # ★ Not `✓`, and not rc 0. "I could not look" is its own outcome, and the whole point of + # this file is that it must not be spelled the same way as "I looked and it was clean". + print(f"cannot measure: {failure}", file=sys.stderr) return 2 merges = [m for m in merges.split("\n") if m] total = 0 for merge in merges: - findings, examined = check(merge) - subject = (run("log", "-1", "--format=%s", merge) or "").strip() + try: + findings, examined = check(merge) + subject = run("log", "-1", "--format=%s", merge).strip() + except CannotMeasure as failure: + print(f"cannot measure: {failure}", file=sys.stderr) + return 2 if findings: print(f"✗ {merge[:8]} {subject[:60]}") for path, name in findings: