Check license headers across the repository - #808
Conversation
| return None | ||
|
|
||
|
|
||
| def check(root: Path, paths: list[str]) -> list[str]: |
There was a problem hiding this comment.
Self-review round one (implementation behavior): CLEAN
Base 82e88f4ebbd89a4dad8b3b57ac412012601f0c45, head f6db31e0f953a7ae178113e4348b5c3d65d6f95b.
Covered:
- header matching for all six comment syntaxes;
- shebang, encoding, and front-matter offsets;
- escaping of regex metacharacters in prefixes (
*,..); - the automatic exemptions (symlink, empty, NUL/invalid UTF-8, suffixes);
- all three stale-entry states, including a listed file deleted only from the working tree;
git ls-files --cached --others --exclude-standardwith the current.gitignore(.venv,.tox, caches,htmlcov,.envare ignored);- gitlinks and directories are skipped by
is_file(); - CRLF working trees:
Path.read_textuses universal newlines.
Tests exercise both acceptance and rejection, and each problem message.
Before the review, manually injected failures (an unheaded new file, a missing listed file) failed as expected, and the fixes for escaping and missing listed files were validated then.
| ## Validation | ||
|
|
||
| Run the Python and CloudFormation checks, ShellCheck, actionlint, and offline script tests from the repository root: | ||
| Run the Python and CloudFormation checks, the license header check, ShellCheck, actionlint, and offline script tests from the repository root: |
There was a problem hiding this comment.
Self-review round two (claims and operational behavior): FINDINGS, repaired in eaaab65
Base 82e88f4ebbd89a4dad8b3b57ac412012601f0c45, head f6db31e0f953a7ae178113e4348b5c3d65d6f95b.
Claims checked:
- The Test workflow skips fork PRs and docs/
**.md-only PRs: confirmed fromtest-suite.yaml(if:on the head repo) andtest.yaml(paths-ignore). - 77 listed files: confirmed with
len(UNHEADED_FILES). - 39 new tests and 68 script tests: confirmed from the pytest output.
- The contributing guide's placement wording matches the checker.
Findings and repairs:
- This README said all script tests use botocore stubs and did not mention the new check. Both are reworded here.
- The PR body said the tox runs execute
just lint. Only thepyathenaenv does (just test pyathenadepends onlint;sqladoes not). The PR body is corrected. - Operational effect: locally, untracked, non-ignored files are now checked by
just lint. This is stated in the PR body. - A new format with a different comment syntax needs a
HEADER_BLOCKSentry. A comment was added aboveHEADER_BLOCKS.
Revalidated at eaaab65 with just scripts (68 passed) and markdownlint.
|
|
||
| # Comment syntax by file suffix; other files use HASH. Add an entry for a new | ||
| # file format with another comment syntax. | ||
| SUFFIX_BLOCKS = { |
There was a problem hiding this comment.
Independent review (relayed): FINDINGS, repaired in 4bbc563
- Reviewer: OpenAI Codex CLI 0.156.0 (
codex exec --sandbox read-only, reasoning effort high), session01a0d0de-9035-71d3-a719-00186dc07d5e. - Scope: static review of
82e88f4ebbd89a4dad8b3b57ac412012601f0c45..eaaab658ee265a4f92084ca0c9310b8421c141e8in a detached snapshot, without the PR number, description, or prior findings. The reviewer confirmed that the 77-entryUNHEADED_FILESbaseline matches the current non-exempt unheaded files.
Findings and dispositions:
- There was no permitted path for a new third-party/generated file that cannot carry the header. Fixed: the
UNHEADED_FILEScomment anddocs/contributing.mdnow say an entry is added only as agreed in the issue. - Header syntax was accepted regardless of format, so six
#lines in Markdown passed while rendering as headings. Fixed:SUFFIX_BLOCKSselects each format's syntax, and tests reject#in.md,#in.rst, and HTML in.py. - A YAML-comment header after keys inside front matter was rejected, contrary to
docs/contributing.md. Fixed:_front_matter_headeraccepts it anywhere inside the front matter, or in the file's syntax directly after the closing---. - ASCII-only binary formats (for example PDF) weren't detected as binary. Rejected: no such file exists; a new binary format gets its suffix added when the file is proposed.
stdout.strip()broke repository paths ending in whitespace, and strict UTF-8 decoding broke non-UTF-8 file names. Fixed:rstrip("\n")andos.fsdecode, with a test for a non-ASCII name with a trailing space.
Both self-review perspectives were applied to the repair:
just scripts: 78 passed.- The checker exits 0 on the tree.
- The PR body is updated.
An independent follow-up review of eaaab65..4bbc563 is pending.
There was a problem hiding this comment.
Independent follow-up reviews (relayed)
Follow-up 1 — Codex CLI 0.156.0, read-only, effort high, session 01a0d0eb-0ee8-7101-b9a8-7a2d1c58dd73. Static review of eaaab65..4bbc563 at the snapshot 4bbc563ed4526a249e435c41d910d61233e7d59e. Result: FINDINGS, all fixed in fa99986594d7f00fbc82a7738880f1e171c5fc8d.
.htmlaccepted only Jinja comments..htmlnow also accepts HTML comments.- In multi-document YAML, the second
---was treated as the end of front matter, so a header only in a later document passed. Front-matter handling is now limited to.md; other files accept the header only directly after a leading document marker. Tests were added. - The root path was decoded as text with
rstrip("\n"). It is now decoded from bytes withremovesuffix(b"\n")andos.fsdecode. docs/contributing.mdconflicted with the automatic exemptions. The guide now names the exemptions and says only non-exempt unheaded files are listed.
Repair validation: just scripts (83 passed), the checker exits 0 on the tree, and markdownlint passes. The PR body is updated.
Follow-up 2 — same reviewer configuration, session 01a0d0f4-6a44-7d81-aeb4-2440328fc7ae, static review of 4bbc563..fa99986 at fa99986594d7f00fbc82a7738880f1e171c5fc8d. Result: CLEAN. All four findings are resolved, with no regression for current-tree formats or tests, and the docs match the behavior.
The two non-blocking suggestions are deferred:
- A test for repository roots containing a newline or non-UTF-8 bytes (the fix is one expression, and such roots are impractical).
- Stating that Markdown front matter needs a closing
---(front matter without one isn't front matter).
There was a problem hiding this comment.
Maintainer-requested change and independent follow-up 3 (relayed)
At the maintainer's request, 9315fb63f05c74431c5fa793176fe8cb80d6bd47:
- moves the exempt suffixes and the unheaded file list to
scripts/config/license_headers.toml, read withtomllib(tomlion Python 3.10, now an explicit dev dependency); - runs the checker through a
just license-headersrecipe thatlintdepends on. The License Headers workflow now sets up uv and just and runs the recipe.
Validation:
just scripts: 88 passed.- Python 3.10: the checker exits 0 and the 59 checker tests pass.
- Injected failures report the new messages.
Both self-review perspectives were applied, and the PR body and docs/contributing.md are updated.
Follow-up 3 — Codex CLI 0.156.0, read-only, effort high, session 01a0d374-3fd9-7531-8f64-dfd46ccebff3. Static review of fa99986..9315fb6 at 9315fb6. Result: CLEAN.
- The migrated entries and suffixes match the prior constants.
- Invalid, malformed, or missing config fails explicitly.
- The Python 3.10 path is covered by the marked
tomlidependency. just lintreaches the checker first.- The workflow actions are SHA-pinned with read-only permissions.
Non-blocking suggestion, deferred: pin an interpreter in the License Headers workflow. uv currently selects the runner's Python, which satisfies requires-python, and uv run installs the default dev dependencies.
There was a problem hiding this comment.
CI repair and independent follow-up 4 (relayed)
- CI failure at
9315fb6: the Benchmark toolingofflinejob failed inuv sync --project benchmarks --locked.benchmarks/uv.lockalso records the root project's dev group, so it needed the newtomlientry. - Repair:
1e9787a7210c455891013e3e182789c2b5f6e0e1regenerates it withjust benchmark lock.just benchmark testpasses (80 passed, 1 skipped), and the Benchmark tooling workflow now passes.
Follow-up 4 — Codex CLI 0.156.0, read-only, effort high, session 01a0d377-d150-79f3-9475-f7e15b1259ee. Static review of 9315fb6..1e9787a. Result: CLEAN.
- The change adds only the root dev dependency's metadata entry, matching
pyproject.tomland the root lock. - The benchmark project targets Python 3.12, so no resolved
tomlipackage entry is needed. - No other lock file, workflow, or recipe needs a matching change.
There was a problem hiding this comment.
Rebase onto master after #811 and #813
026587bc98289fdd66ade30d40104102c780066f, rebased from 1e9787a.
git range-diff 82e88f4..1e9787a origin/master..026587b:
- The five checker commits are identical (
=). - The benchmark-lock-only commit is dropped. Manage the benchmark project as a uv workspace member #813 removed
benchmarks/uv.lockand made benchmarks a member of the uv workspace, so the rootuv.locknow carries thetomlientry.
Upstream effects checked:
just license-headersrunsuv run python, which syncs only the root project.- Rerun tests once on Athena service-side query failures #811 changes only the test recipes.
Validation:
uv lock --checkpasses.just license-headersexits 0 on the rebased tree.just scripts: 88 passed.just benchmark test: 80 passed, 1 skipped.
No new independent review: the patches are unchanged.
Add scripts/check_license_headers.py, which reports files without the "The PyAthena authors" MIT header described in docs/contributing.md. It checks every tracked and untracked, non-ignored file, exempts symbolic links, empty, binary, and data files, and lists the existing files without the header explicitly. Listed files that gain the header, become exempt, or disappear are reported so the list only shrinks. Run the check from `just lint` and from a License Headers workflow on every pull request, including fork and documentation-only pull requests that the Test workflow skips. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Accept only the comment syntax of each file format, so that a Markdown file cannot pass with '#' lines that render as headings. In a file with YAML front matter, accept the header as YAML comments anywhere inside the front matter, or in the file's own syntax directly after it. Keep repository paths ending in whitespace and non-UTF-8 file names intact, and document that a new file without the header is listed only as agreed in its issue. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Treat a leading '---' in files other than Markdown as a YAML document marker only, so that a header in a later YAML document does not pass. Accept HTML comments as well as Jinja comments in .html files, decode the repository root from bytes, and describe the automatic exemptions next to the list of files without the header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… just Read the exempt suffixes and the files without the header from scripts/config/license_headers.toml, rejecting unknown keys, non-string lists, and duplicate entries. Python 3.10 reads it with tomli, now an explicit dev dependency. Add a license-headers recipe, run by lint and by the License Headers workflow, so that both use the project's Python through uv. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1e9787a to
026587b
Compare
WHAT
Add
scripts/check_license_headers.py, which reports files without theThe PyAthena authorsMIT header described indocs/contributing.md.Scope: every tracked and untracked, non-ignored file in the working tree (
git ls-files --cached --others --exclude-standard).Accepted header: the documented six lines with any four-digit year, at the start of the file, in the comment syntax of the file's format:
.md..comments for.rst{# -#}comments for.jinja2, and Jinja or HTML comments for.html/* */for.css//for.jsonc#for everything elseThe header may follow a shebang, an encoding declaration, or a leading YAML document marker. In a Markdown file with YAML front matter, it may be YAML comments anywhere inside the front matter, or an HTML comment directly after it.
Configuration:
scripts/config/license_headers.tomlholds the exemptions:exempt-suffixes:.csv,.tsv,.json,.gz,.png, and.lock.unheaded-files: the existing files without the header.Unknown keys, non-string lists, and duplicate entries are rejected. Python 3.10 reads the file with
tomli, now an explicit dev dependency for Python < 3.11 (already locked through pytest).Automatic exemptions: symbolic links, empty or whitespace-only files, binary files (NUL bytes or invalid UTF-8), and the configured suffixes.
Explicit exemptions:
unheaded-fileslists the 77 existing files without the header, as classified in Standardize license headers and document PyHive acknowledgements #790 and described inNOTICE. The checker also reports a listed file that gains the header, becomes exempt, or no longer exists. A new file without the header, such as third-party material, is added only as agreed in its issue;docs/contributing.mdsays so.Behavior: it prints problems and exits 1. It never modifies files.
The check runs in these places:
just license-headers, a new recipe (uv run python scripts/check_license_headers.py).just lintdepends on it, and so dojust scripts,just test pyathena, and thepyathenatox job of the Test workflow. Locally, an untracked, non-ignored file without the header also failsjust lint.just license-headersafter setting up uv and just. The Test workflow skips fork PRs and docs/**.md-only PRs, and new Markdown files are the most likely to miss the header. The job needs no dependencies and no AWS credentials.docs/contributing.mdnow says the header goes at the start of the file (the checker enforces this) and points to the checker.AGENTS.mdandscripts/README.mddescribe the extended checks. Another comment syntax needs an entry inSUFFIX_BLOCKS.WHY
Closes #806.
#806 left open whether to check only added files or the whole tree with an exception list. The maintainer chose the whole tree with an exception list, because the number of files without the header should not grow.
Compared with a diff-based check, it:
TEST
Tested at 026587b, rebased onto master after #811 and #813:
just scripts: passes (ruff, format check, mypy, cfn-lint, the new check, shellcheck, actionlint, and 88 script tests including 59 new ones).just license-headerson the current tree: exit 0.uv lock --check: passes with the uv workspace lock from Manage the benchmark project as a uv workspace member #813.just benchmark test: 80 passed, 1 skipped.uv run --python 3.10in a separate environment): the checker exits 0, and the 59 checker tests pass, which covers thetomlipath.missing license header; moving a listed file away reportslisted as unheaded in scripts/config/license_headers.toml but not found;just license-headersexits 1.#lines in Markdown), and a header after front matter ends in the wrong syntax or after content;No AWS resources are needed.
🤖 Generated with Claude Code