From 021a28abf1934a6fb84a1ce19dbc162d447bee6c Mon Sep 17 00:00:00 2001 From: bong-water-water-bong <277547417+bong-water-water-bong@users.noreply.github.com> Date: Wed, 11 Nov 2026 09:40:55 -0400 Subject: [PATCH 1/2] copyright: --fix leaves a file its own --check rejects when the notice is already a two-liner Met while fixing a new file's header in #2177, and it is the kind of defect this tool exists to prevent, in the tool: $ printf '# Copyright 2026 bong-water-bong\n# SPDX-License-Identifier: Apache-2.0\n\nx = 1\n' > twoliner.py $ python3 tools/copyright.py --check run: python3 tools/copyright.py --fix missing copyright notice: twoliner.py $ python3 tools/copyright.py --fix added: twoliner.py $ python3 tools/copyright.py --check run: python3 tools/copyright.py --fix copyright notice duplicated inside the header window (delete the second copy): twoliner.py `--fix` reported success and left an invalid file. Neither mode says the state it produced is invalid; only a second `--check` does, and the instruction it then prints is the one thing `--fix` cannot do. The cause is a gap between two predicates that each look right alone. `has_notice()` wants the licence block, so a file carrying only the SPDX pair is "missing". `near_miss_holder()` filters what `add_notice()` drops before prepending the block, and it returns False for an *exact* holder **by design** -- an exact holder beside the canonical block is the *repaired* state (engine#1606). So the exact pair survived the prepend and became the second copy. The pair is not a second claim: the block `add_notice()` inserts contains both lines. Now `add_notice()` drops an in-window line that IS one of the canonical notice lines, so the merge is a merge. Two arms in tests/copyright_gate_test.sh, and they bite: against main's `tools/copyright.py` the second one fails with "window: 2 SPDX line(s)", which is exactly the duplicate. The first arm alone passes on old code -- the second `--check` there still finds a duplicate, so what actually fails to be one notice is that count, and that is what is asserted. Verified: tests/copyright_gate_test.sh PASS with the patch, FAIL (2 SPDX lines) without it; `tools/copyright.py --check` clean on this tree. The first push of this branch also carried four files staged for #2177's header check -- `git add` is needed for `tools/copyright.py` to see a new file, and the following `git commit` then took the whole index. Rebuilt from main with the two files this PR is about. --- tests/copyright_gate_test.sh | 15 ++++++++++++++- tools/copyright.py | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) mode change 100644 => 100755 tools/copyright.py diff --git a/tests/copyright_gate_test.sh b/tests/copyright_gate_test.sh index 2f02feedb..c64266992 100755 --- a/tests/copyright_gate_test.sh +++ b/tests/copyright_gate_test.sh @@ -137,5 +137,18 @@ check "near-miss beside an existing notice leaves exactly two copyright lines" \ check "and the near-miss spelling is the one that went" "[ $(grep -c 'bong-water-bong$' "$both") -eq 0 ]" "$(grep 'Copyright' "$both")" rm -f "$both"; git -C "$tmp" update-index --force-remove both.py 2>/dev/null +# 2. the two-liner without the licence block: --fix must leave ONE notice, not a copy of the pair beside the block +# (engine#2129 -- --fix used to prepend the block, keep the two-liner, and then fail its own --check with "duplicated +# inside the header window", so the file it had just written was invalid and neither mode said so). +printf '# Copyright 2026 bong-water-bong\n# SPDX-License-Identifier: Apache-2.0\n\nx = 1\n' > "$tmp/twoliner.py" +git -C "$tmp" add twoliner.py +( cd "$tmp" && python3 copyright.py --fix >/dev/null 2>&1 ) +check "--fix on a two-liner leaves a file that its own --check accepts" \ + "! ( cd \"$tmp\" && python3 copyright.py --check 2>&1 | grep -q duplicated )" \ + "$( cd "$tmp" && python3 copyright.py --check 2>&1 )" +check "...and exactly one SPDX line is left in the window" \ + "[ \$(head -20 \"$tmp/twoliner.py\" | grep -c 'SPDX-License-Identifier') -eq 1 ]" \ + "window: $(head -20 "$tmp/twoliner.py" | grep -c 'SPDX-License-Identifier') SPDX line(s)" + echo "$([ $fail -eq 0 ] && echo PASS || echo FAIL)" -exit $((fail)) +exit $((fail)) \ No newline at end of file diff --git a/tools/copyright.py b/tools/copyright.py old mode 100644 new mode 100755 index 705dc082c..880018ac1 --- a/tools/copyright.py +++ b/tools/copyright.py @@ -229,6 +229,21 @@ def near_miss_holder(line): return n >= HOLDER_NEAR_MISS_PREFIX +_CANON_LINES = {re.sub(r"^\W+", "", n).strip() for n in NOTICE} + + +def canonical_notice_line(line): + """True when the line IS one of the canonical notice lines, spelled exactly. + + engine#2129, met while fixing a new file's header: an SPDX two-liner without the licence block is "missing" to + has_notice(), so --fix prepends the block -- and the two-liner it just merged into that block stayed behind, so the + file --fix had just written failed --check with "duplicated inside the header window". near_miss_holder() cannot + cover it by construction: it returns False for an exact holder, because an exact holder next to the canonical block + is the *repaired* state. The block already contains both lines, so a copy inside the window is not a second claim. + """ + return re.sub(r"^\W+", "", line).strip() in _CANON_LINES + + def repair_notice(text, block): """The file already HAS the canonical notice, plus a near-miss of our holder: delete the misspelling. @@ -258,7 +273,7 @@ def add_notice(text, block): # block and leave the hand-written one in place, which is how a file ended up with two notices and a green # `--check` (engine#1606). Dual attribution with a different holder is untouched - only the window is read. head = lines[:NOTICE_WINDOW] - kept = [l for l in head if not near_miss_holder(l)] + kept = [l for l in head if not near_miss_holder(l) and not canonical_notice_line(l)] lines = kept + lines[NOTICE_WINDOW:] return "".join(lines[:insert_at]) + block + "".join(lines[insert_at:]) From ab644b232e94a914f604d7abffedec7366334979 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 13:22:19 -0400 Subject: [PATCH 2/2] copyright: --fix also repairs the block pasted twice, which it used to report as `repaired:` and leave (engine#2192) The two-liner case in the previous commit was one of two states in which `--fix` printed success and left a file its own `--check` rejects. The other is the block pasted twice: $ python3 copyright.py --check copyright notice duplicated inside the header window (delete the second copy): doubled.py $ python3 copyright.py --fix repaired: doubled.py $ python3 copyright.py --check copyright notice duplicated inside the header window (delete the second copy): doubled.py `repair_notice()` is the path `--fix` takes for a file that already has the canonical notice, and it existed for the near-miss-beside-canonical shape (engine#1606): it drops a typo'd holder and the SPDX line beneath it, keeps everything else, and falls back to `add_notice()` when the result no longer has a notice. A *second canonical* notice is neither of those things, so it passed through untouched -- and `has_notice()` was still true, so the fallback never ran either. It now keeps the first occurrence of each canonical notice line and drops later copies, which is what `--check`'s own instruction ("delete the second copy") asks for and what the word `repaired:` has always claimed. Dual attribution is untouched: `Copyright 2026 The 1bit-MONSTER Authors.` is not a canonical line, so it survives beside the block exactly as before. Two arms, and they bite: against the previous `repair_notice()` the SPDX count in the window is 2 and the arm fails ("...and exactly one SPDX line survives the repair"), so the whole gate test is FAIL without the fix and PASS with it. The first arm (that `--check` stops reporting a duplicate) passes on old code, which is worth knowing: it means the *count* is the assertion that has teeth here, not the absence of the word "duplicated" -- the same asymmetry as the two-liner case in the previous commit. --- tests/copyright_gate_test.sh | 19 +++++++++++++++++++ tools/copyright.py | 14 ++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/tests/copyright_gate_test.sh b/tests/copyright_gate_test.sh index c64266992..f73a8d4a2 100755 --- a/tests/copyright_gate_test.sh +++ b/tests/copyright_gate_test.sh @@ -53,6 +53,13 @@ git -C "$tmp" commit -qm base # captured into a variable, never piped: `gate | grep -q` would make grep close the pipe early and, with pipefail, # report the gate's SIGPIPE instead of its verdict +NOTICE_BLOCK=$(python3 - "$tmp/copyright.py" <<'PYN' +import sys +src = open(sys.argv[1]).read() +notice = src.split('NOTICE = """')[1].split('""".split')[0] +print("".join("# " + l + "\n" if l.strip() else "#\n" for l in notice.splitlines()), end="") +PYN +) gate() { ( cd "$tmp" && python3 copyright.py --check 2>&1 ); } gate_has() { case "$(gate)" in *"$1"*) return 0;; *) return 1;; esac; } @@ -150,5 +157,17 @@ check "...and exactly one SPDX line is left in the window" \ "[ \$(head -20 \"$tmp/twoliner.py\" | grep -c 'SPDX-License-Identifier') -eq 1 ]" \ "window: $(head -20 "$tmp/twoliner.py" | grep -c 'SPDX-License-Identifier') SPDX line(s)" +# 3. the block pasted twice: --fix must leave ONE notice. This is #2192's second shape -- `--fix` printed `repaired:` and +# left the copy in place, so the file it had just written still failed the very check that sent it there. +printf '%s%s\nx = 1\n' "$NOTICE_BLOCK" "$NOTICE_BLOCK" > "$tmp/doubled.py" +git -C "$tmp" add doubled.py +( cd "$tmp" && python3 copyright.py --fix >/dev/null 2>&1 ) +check "--fix on a doubled block leaves a file that its own --check accepts" \ + "! ( cd \"$tmp\" && python3 copyright.py --check 2>&1 | grep -q duplicated )" \ + "$( cd "$tmp" && python3 copyright.py --check 2>&1 )" +check "...and exactly one SPDX line survives the repair" \ + "[ \$(grep -c 'SPDX-License-Identifier' \"$tmp/doubled.py\") -eq 1 ]" \ + "$(grep -c 'SPDX-License-Identifier' "$tmp/doubled.py") SPDX line(s)" + echo "$([ $fail -eq 0 ] && echo PASS || echo FAIL)" exit $((fail)) \ No newline at end of file diff --git a/tools/copyright.py b/tools/copyright.py index 880018ac1..2e3888f6a 100755 --- a/tools/copyright.py +++ b/tools/copyright.py @@ -251,8 +251,13 @@ def repair_notice(text, block): and it needed its own path because it is neither "notice missing" (has_notice() is true, so --fix skipped the file) nor "duplicate canonical pair" (duplicated_notice() only reported it, even under --fix). Dropping the copyright line and the SPDX line directly beneath it keeps a near-miss pair from leaving an orphan. + + It also keeps the FIRST canonical notice and drops later copies of the same line, which is the doubled-block shape: + --fix used to answer `repaired:` for that state while `--check` went on reporting the duplicate, i.e. the one mode + that could fix it claimed to and did not. """ out, drop_next_spdx = [], False + seen_canonical = set() for line in text.splitlines(keepends=True): if near_miss_holder(line): drop_next_spdx = True @@ -261,6 +266,15 @@ def repair_notice(text, block): drop_next_spdx = False continue drop_next_spdx = False + # A SECOND canonical notice is the same defect as a near-miss beside the first: the block pasted twice. `--check` + # has always reported it (`duplicated_notice`); `--fix` used to print "repaired" and leave the copy in place, so the + # file it had just written still failed the very check that sent it there. The first occurrence is kept and later + # ones dropped, which is what "delete the second copy" means. + if canonical_notice_line(line): + key = re.sub(r"^\W+", "", line).strip() + if key in seen_canonical: + continue + seen_canonical.add(key) out.append(line) repaired = "".join(out) return repaired if has_notice(repaired) else add_notice(repaired, block)