From fb8484c8a3e1c9f9d41c336d40c5a6e14be7d61a Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Thu, 24 Sep 2026 09:37:53 +0900 Subject: [PATCH 1/5] Check license headers across the repository 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 --- .github/workflows/license-headers.yaml | 23 ++ AGENTS.md | 2 +- docs/contributing.md | 3 +- justfile | 3 +- scripts/check_license_headers.py | 228 ++++++++++++++++++++ scripts/tests/test_check_license_headers.py | 148 +++++++++++++ 6 files changed, 404 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/license-headers.yaml create mode 100644 scripts/check_license_headers.py create mode 100644 scripts/tests/test_check_license_headers.py diff --git a/.github/workflows/license-headers.yaml b/.github/workflows/license-headers.yaml new file mode 100644 index 00000000..d279d3fb --- /dev/null +++ b/.github/workflows/license-headers.yaml @@ -0,0 +1,23 @@ +# Copyright 2026 The PyAthena authors +# +# Licensed under the MIT License. +# See LICENSE or https://opensource.org/licenses/MIT. +# +# SPDX-License-Identifier: MIT + +name: License Headers + +on: + pull_request: + +permissions: + contents: read + +jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - run: python3 scripts/check_license_headers.py diff --git a/AGENTS.md b/AGENTS.md index 6b7b0e0a..97bac068 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,7 +47,7 @@ Edit the canonical files when maintaining these skills so both agents use the sa ```bash just format # Auto-fix formatting and imports -just lint # Python lint, format check, mypy, and CloudFormation validation +just lint # Python lint, format check, mypy, CloudFormation validation, and license headers ``` ### Testing diff --git a/docs/contributing.md b/docs/contributing.md index 095e835d..40b82d5c 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -118,11 +118,12 @@ For a new Python file first published in 2026, use: # SPDX-License-Identifier: MIT ``` -Place the header near the start of the file, before imports or module documentation, while keeping required shebangs, encoding declarations, and front matter in their required positions. +Place the header at the start of the file, before imports or module documentation, while keeping required shebangs, encoding declarations, and front matter in their required positions. Use the equivalent comment syntax for other formats, such as an HTML comment in Markdown. For files with YAML front matter, the header may be written as YAML comments inside that front matter. Use this placement for GitHub issue templates so the notice belongs to the template metadata rather than the issue body. For files that cannot contain comments, and for generated build artifacts or third-party assets, agree on an appropriate attribution location in the issue rather than inserting an invalid header. +`just lint` and the License Headers workflow check the header with `scripts/check_license_headers.py`, which lists the existing files without it. The short notice and license reference follow [Google's MIT header example](https://opensource.google/documentation/reference/releasing/licenses#mit-header), with an [SPDX identifier](https://spdx.org/licenses/MIT.html) added. diff --git a/justfile b/justfile index 94c5baf3..0b248e04 100644 --- a/justfile +++ b/justfile @@ -24,12 +24,13 @@ format: uvx ruff@{{RUFF_VERSION}} check --select I --fix . uvx ruff@{{RUFF_VERSION}} format . -# Lint, format check, mypy, and CloudFormation validation +# Lint, format check, mypy, CloudFormation validation, and license headers lint: uvx ruff@{{RUFF_VERSION}} check . uvx ruff@{{RUFF_VERSION}} format --check . uv run mypy . uv run cfn-lint cloudformation/*.yaml + uv run python scripts/check_license_headers.py # Run tests: just test (pyathena|sqla|sqla-async) test target="help": diff --git a/scripts/check_license_headers.py b/scripts/check_license_headers.py new file mode 100644 index 00000000..a214100f --- /dev/null +++ b/scripts/check_license_headers.py @@ -0,0 +1,228 @@ +# Copyright 2026 The PyAthena authors +# +# Licensed under the MIT License. +# See LICENSE or https://opensource.org/licenses/MIT. +# +# SPDX-License-Identifier: MIT + +"""Check that repository files carry the PyAthena license header.""" + +# Usage (from the repository root): +# uv run python scripts/check_license_headers.py +# +# Checks tracked and untracked, non-ignored files in the working tree against +# the header described in docs/contributing.md. Reports missing headers and +# stale UNHEADED_FILES entries; never modifies files. + +import re +import subprocess +import sys +from pathlib import Path + +HEADER_LINES = ( + r"Copyright \d{4} The PyAthena authors", + "", + re.escape("Licensed under the MIT License."), + re.escape("See LICENSE or https://opensource.org/licenses/MIT."), + "", + re.escape("SPDX-License-Identifier: MIT"), +) + + +def _block(opening: str | None, prefix: str, closing: str | None) -> re.Pattern[str]: + blank = re.escape(prefix.rstrip()) + lines = [f"{re.escape(prefix)}{line}" if line else blank for line in HEADER_LINES] + if opening is not None: + lines.insert(0, re.escape(opening)) + if closing is not None: + lines.append(re.escape(closing)) + return re.compile("\n".join(lines) + "(\n|$)") + + +HEADER_BLOCKS = ( + _block(None, "# ", None), + _block(None, "// ", None), + _block(""), + _block("..", " ", None), + _block("{#", "", "-#}"), + _block("/*", " * ", " */"), +) + +SHEBANG = re.compile(r"#!.*\n") +ENCODING = re.compile(r"#.*coding[:=][ \t]*[-\w.]+.*\n") +FRONT_MATTER = "---\n" + +# Formats without comment syntax or with generated content. +EXEMPT_SUFFIXES = frozenset({".csv", ".gz", ".json", ".lock", ".png", ".tsv"}) + +# Existing files without the header, classified in +# https://github.com/pyathena-dev/PyAthena/issues/790 and described in NOTICE. +# New files carry the header; remove entries whose files gain the header or +# are deleted. +UNHEADED_FILES = frozenset( + { + ".github/PULL_REQUEST_TEMPLATE.md", + "LICENSE", + "NOTICE", + "cloudformation/github_actions_oidc.yaml", + "docs/aio.md", + "docs/arrow.md", + "docs/conf.py", + "docs/cursor.md", + "docs/pandas.md", + "docs/polars.md", + "docs/s3fs.md", + "docs/sqlalchemy.md", + "docs/usage.md", + "pyathena/__init__.py", + "pyathena/aio/arrow/cursor.py", + "pyathena/aio/common.py", + "pyathena/aio/pandas/cursor.py", + "pyathena/aio/polars/cursor.py", + "pyathena/aio/result_set.py", + "pyathena/arrow/async_cursor.py", + "pyathena/arrow/converter.py", + "pyathena/arrow/cursor.py", + "pyathena/arrow/result_set.py", + "pyathena/arrow/util.py", + "pyathena/async_cursor.py", + "pyathena/common.py", + "pyathena/connection.py", + "pyathena/converter.py", + "pyathena/cursor.py", + "pyathena/filesystem/s3.py", + "pyathena/filesystem/s3_object.py", + "pyathena/formatter.py", + "pyathena/model.py", + "pyathena/pandas/__init__.py", + "pyathena/pandas/async_cursor.py", + "pyathena/pandas/converter.py", + "pyathena/pandas/cursor.py", + "pyathena/pandas/result_set.py", + "pyathena/pandas/util.py", + "pyathena/parser.py", + "pyathena/polars/__init__.py", + "pyathena/polars/async_cursor.py", + "pyathena/polars/cursor.py", + "pyathena/result_set.py", + "pyathena/s3fs/async_cursor.py", + "pyathena/s3fs/cursor.py", + "pyathena/sqlalchemy/array.py", + "pyathena/sqlalchemy/base.py", + "pyathena/sqlalchemy/compiler.py", + "pyathena/sqlalchemy/constants.py", + "pyathena/sqlalchemy/temporal.py", + "pyathena/sqlalchemy/types.py", + "pyathena/util.py", + "pyproject.toml", + "tests/__init__.py", + "tests/pyathena/aio/sqlalchemy/test_base.py", + "tests/pyathena/aio/test_cursor.py", + "tests/pyathena/arrow/test_async_cursor.py", + "tests/pyathena/conftest.py", + "tests/pyathena/filesystem/test_s3.py", + "tests/pyathena/filesystem/test_s3_async.py", + "tests/pyathena/pandas/test_async_cursor.py", + "tests/pyathena/pandas/test_cursor.py", + "tests/pyathena/pandas/test_util.py", + "tests/pyathena/polars/test_async_cursor.py", + "tests/pyathena/s3fs/test_cursor.py", + "tests/pyathena/sqlalchemy/test_array.py", + "tests/pyathena/sqlalchemy/test_base.py", + "tests/pyathena/sqlalchemy/test_temporal.py", + "tests/pyathena/sqlalchemy/test_types.py", + "tests/pyathena/test_async_cursor.py", + "tests/pyathena/test_converter.py", + "tests/pyathena/test_cursor.py", + "tests/pyathena/test_model.py", + "tests/pyathena/test_util.py", + "tests/resources/queries/create_table.sql.jinja2", + "tests/sqlalchemy/test_suite.py", + } +) + + +def has_license_header(text: str) -> bool: + """Return whether the header starts the file. + + The header may follow a shebang and an encoding declaration, or open YAML + front matter as comments. + """ + pos = 0 + if match := SHEBANG.match(text, pos): + pos = match.end() + if match := ENCODING.match(text, pos): + pos = match.end() + if pos == 0 and text.startswith(FRONT_MATTER): + pos = len(FRONT_MATTER) + return any(block.match(text, pos) for block in HEADER_BLOCKS) + + +def exemption_reason(root: Path, path: str) -> str | None: + """Return why a file needs no header, or None when it needs one.""" + file = root / path + if file.is_symlink(): + return "symbolic link" + if Path(path).suffix in EXEMPT_SUFFIXES: + return "data or generated file" + data = file.read_bytes() + if b"\0" in data: + return "binary file" + try: + text = data.decode("utf-8") + except UnicodeDecodeError: + return "binary file" + if not text.strip(): + return "empty file" + return None + + +def check(root: Path, paths: list[str]) -> list[str]: + """Return problems for the given repository-relative paths.""" + problems = [] + existing = {path for path in paths if (root / path).is_symlink() or (root / path).is_file()} + for path in sorted(existing): + file = root / path + reason = exemption_reason(root, path) + headed = reason is None and has_license_header(file.read_text(encoding="utf-8")) + if path in UNHEADED_FILES: + if reason is not None: + problems.append(f"{path}: listed in UNHEADED_FILES but exempt as {reason}") + elif headed: + problems.append(f"{path}: listed in UNHEADED_FILES but has the header") + elif reason is None and not headed: + problems.append(f"{path}: missing license header") + problems.extend( + f"{path}: listed in UNHEADED_FILES but not found" + for path in sorted(UNHEADED_FILES - existing) + ) + return problems + + +def repository_files(root: Path) -> list[str]: + """Return tracked and untracked, non-ignored files.""" + output = subprocess.run( + ["git", "ls-files", "--cached", "--others", "--exclude-standard", "-z"], + cwd=root, + check=True, + capture_output=True, + ).stdout + return [path for path in output.decode("utf-8").split("\0") if path] + + +def main() -> int: + root = Path( + subprocess.run( + ["git", "rev-parse", "--show-toplevel"], check=True, capture_output=True, text=True + ).stdout.strip() + ) + problems = check(root, repository_files(root)) + if not problems: + return 0 + sys.stderr.write("".join(f"{problem}\n" for problem in problems)) + sys.stderr.write("See docs/contributing.md for the header of new original files.\n") + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_check_license_headers.py b/scripts/tests/test_check_license_headers.py new file mode 100644 index 00000000..08753461 --- /dev/null +++ b/scripts/tests/test_check_license_headers.py @@ -0,0 +1,148 @@ +# Copyright 2026 The PyAthena authors +# +# Licensed under the MIT License. +# See LICENSE or https://opensource.org/licenses/MIT. +# +# SPDX-License-Identifier: MIT + +import subprocess + +import pytest + +from scripts.check_license_headers import ( + UNHEADED_FILES, + check, + exemption_reason, + has_license_header, + repository_files, +) + +LINES = [ + "Copyright 2026 The PyAthena authors", + "", + "Licensed under the MIT License.", + "See LICENSE or https://opensource.org/licenses/MIT.", + "", + "SPDX-License-Identifier: MIT", +] + + +def prefixed(prefix: str) -> str: + return "".join(f"{prefix}{line}".rstrip() + "\n" for line in LINES) + + +HASH = prefixed("# ") +BODY = "".join(f"{line}\n" for line in LINES) + + +class TestHasLicenseHeader: + @pytest.mark.parametrize( + "text", + [ + HASH + "\nimport os\n", + HASH, + "#!/usr/bin/env bash\n" + HASH + "\nset -eu\n", + "# -*- coding: utf-8 -*-\n" + HASH, + "#!/usr/bin/env python\n# -*- coding: utf-8 -*-\n" + HASH, + "---\n" + HASH + "name: Bug report\n---\n\nBody\n", + "\n\n# Title\n", + "..\n" + prefixed(" ") + "\n.. _label:\n", + "{#\n" + BODY + "-#}\n\nSELECT 1\n", + "/*\n" + prefixed(" * ") + " */\n", + prefixed("// ") + "\n{}\n", + HASH.replace("2026", "2017"), + ], + ) + def test_accepted(self, text): + assert has_license_header(text) + + @pytest.mark.parametrize( + "text", + [ + "import os\n", + "import os\n\n" + HASH, + HASH.replace("The PyAthena authors", "laughingman7743"), + HASH.replace("2026", "26"), + HASH.replace("SPDX-License-Identifier: MIT", "SPDX-License-Identifier: Apache-2.0"), + HASH.replace("#\n", "", 1), + ""), - _block("..", " ", None), - _block("{#", "", "-#}"), - _block("/*", " * ", " */"), -) +HASH = _block(None, "# ", None) +JINJA = _block("{#", "", "-#}") + +# Comment syntax by file suffix; other files use HASH. Add an entry for a new +# file format with another comment syntax. +SUFFIX_BLOCKS = { + ".css": _block("/*", " * ", " */"), + ".html": JINJA, + ".jinja2": JINJA, + ".jsonc": _block(None, "// ", None), + ".md": _block(""), + ".rst": _block("..", " ", None), +} SHEBANG = re.compile(r"#!.*\n") ENCODING = re.compile(r"#.*coding[:=][ \t]*[-\w.]+.*\n") FRONT_MATTER = "---\n" +FRONT_MATTER_END = re.compile(r"\n---[ \t]*(\n|$)") # Formats without comment syntax or with generated content. EXEMPT_SUFFIXES = frozenset({".csv", ".gz", ".json", ".lock", ".png", ".tsv"}) # Existing files without the header, classified in # https://github.com/pyathena-dev/PyAthena/issues/790 and described in NOTICE. -# New files carry the header; remove entries whose files gain the header or -# are deleted. +# New files carry the header; add an entry only as agreed in the issue that +# proposes the file, and remove entries whose files gain the header or are +# deleted. UNHEADED_FILES = frozenset( { ".github/PULL_REQUEST_TEMPLATE.md", @@ -143,20 +150,35 @@ def _block(opening: str | None, prefix: str, closing: str | None) -> re.Pattern[ ) -def has_license_header(text: str) -> bool: - """Return whether the header starts the file. +def _front_matter_header(text: str, block: re.Pattern[str]) -> bool: + if not text.startswith(FRONT_MATTER): + return False + pos = len(FRONT_MATTER) + if HASH.match(text, pos): + return True + if not (end := FRONT_MATTER_END.search(text, pos - 1)): + return False + while pos <= end.start(): + if (match := HASH.match(text, pos)) and match.end() <= end.start() + 1: + return True + pos = text.index("\n", pos) + 1 + return bool(block.match(text, end.end())) + + +def has_license_header(text: str, suffix: str) -> bool: + """Return whether the header starts a file with the given suffix. - The header may follow a shebang and an encoding declaration, or open YAML - front matter as comments. + The header may follow a shebang and an encoding declaration. In a file + with YAML front matter, it may be written as YAML comments inside the front + matter or follow it. """ pos = 0 if match := SHEBANG.match(text, pos): pos = match.end() if match := ENCODING.match(text, pos): pos = match.end() - if pos == 0 and text.startswith(FRONT_MATTER): - pos = len(FRONT_MATTER) - return any(block.match(text, pos) for block in HEADER_BLOCKS) + block = SUFFIX_BLOCKS.get(suffix, HASH) + return bool(block.match(text, pos)) or _front_matter_header(text, block) def exemption_reason(root: Path, path: str) -> str | None: @@ -185,7 +207,9 @@ def check(root: Path, paths: list[str]) -> list[str]: for path in sorted(existing): file = root / path reason = exemption_reason(root, path) - headed = reason is None and has_license_header(file.read_text(encoding="utf-8")) + headed = reason is None and has_license_header( + file.read_text(encoding="utf-8"), Path(path).suffix + ) if path in UNHEADED_FILES: if reason is not None: problems.append(f"{path}: listed in UNHEADED_FILES but exempt as {reason}") @@ -208,14 +232,14 @@ def repository_files(root: Path) -> list[str]: check=True, capture_output=True, ).stdout - return [path for path in output.decode("utf-8").split("\0") if path] + return [path for path in os.fsdecode(output).split("\0") if path] def main() -> int: root = Path( subprocess.run( ["git", "rev-parse", "--show-toplevel"], check=True, capture_output=True, text=True - ).stdout.strip() + ).stdout.rstrip("\n") ) problems = check(root, repository_files(root)) if not problems: diff --git a/scripts/tests/test_check_license_headers.py b/scripts/tests/test_check_license_headers.py index 08753461..1a795bc0 100644 --- a/scripts/tests/test_check_license_headers.py +++ b/scripts/tests/test_check_license_headers.py @@ -35,45 +35,58 @@ def prefixed(prefix: str) -> str: BODY = "".join(f"{line}\n" for line in LINES) +HTML = "\n" + + class TestHasLicenseHeader: @pytest.mark.parametrize( - "text", + ("suffix", "text"), [ - HASH + "\nimport os\n", - HASH, - "#!/usr/bin/env bash\n" + HASH + "\nset -eu\n", - "# -*- coding: utf-8 -*-\n" + HASH, - "#!/usr/bin/env python\n# -*- coding: utf-8 -*-\n" + HASH, - "---\n" + HASH + "name: Bug report\n---\n\nBody\n", - "\n\n# Title\n", - "..\n" + prefixed(" ") + "\n.. _label:\n", - "{#\n" + BODY + "-#}\n\nSELECT 1\n", - "/*\n" + prefixed(" * ") + " */\n", - prefixed("// ") + "\n{}\n", - HASH.replace("2026", "2017"), + (".py", HASH + "\nimport os\n"), + (".py", HASH), + ("", HASH), + (".toml", HASH.replace("2026", "2017")), + (".sh", "#!/usr/bin/env bash\n" + HASH + "\nset -eu\n"), + (".py", "# -*- coding: utf-8 -*-\n" + HASH), + (".py", "#!/usr/bin/env python\n# -*- coding: utf-8 -*-\n" + HASH), + (".yaml", "---\n" + HASH + "key: value\n"), + (".md", HTML + "\n# Title\n"), + (".md", "---\n" + HASH + "name: Bug report\n---\n\nBody\n"), + (".md", "---\nname: skill\n" + HASH + "description: x\n---\n"), + (".md", "---\nname: skill\n---\n" + HTML), + (".rst", "..\n" + prefixed(" ") + "\n.. _label:\n"), + (".jinja2", "{#\n" + BODY + "-#}\n\nSELECT 1\n"), + (".html", "{#\n" + BODY + "-#}\n"), + (".css", "/*\n" + prefixed(" * ") + " */\n"), + (".jsonc", prefixed("// ") + "\n{}\n"), ], ) - def test_accepted(self, text): - assert has_license_header(text) + def test_accepted(self, suffix, text): + assert has_license_header(text, suffix) @pytest.mark.parametrize( - "text", + ("suffix", "text"), [ - "import os\n", - "import os\n\n" + HASH, - HASH.replace("The PyAthena authors", "laughingman7743"), - HASH.replace("2026", "26"), - HASH.replace("SPDX-License-Identifier: MIT", "SPDX-License-Identifier: Apache-2.0"), - HASH.replace("#\n", "", 1), - "") JINJA = _block("{#", "", "-#}") -# Comment syntax by file suffix; other files use HASH. Add an entry for a new +# Comment syntaxes by file suffix; other files use HASH. Add an entry for a new # file format with another comment syntax. SUFFIX_BLOCKS = { - ".css": _block("/*", " * ", " */"), - ".html": JINJA, - ".jinja2": JINJA, - ".jsonc": _block(None, "// ", None), - ".md": _block(""), - ".rst": _block("..", " ", None), + ".css": (_block("/*", " * ", " */"),), + ".html": (JINJA, HTML), + ".jinja2": (JINJA,), + ".jsonc": (_block(None, "// ", None),), + ".md": (HTML,), + ".rst": (_block("..", " ", None),), } SHEBANG = re.compile(r"#!.*\n") ENCODING = re.compile(r"#.*coding[:=][ \t]*[-\w.]+.*\n") +# YAML front matter in Markdown files; elsewhere, a leading YAML document marker. FRONT_MATTER = "---\n" FRONT_MATTER_END = re.compile(r"\n---[ \t]*(\n|$)") @@ -150,35 +152,39 @@ def _block(opening: str | None, prefix: str, closing: str | None) -> re.Pattern[ ) -def _front_matter_header(text: str, block: re.Pattern[str]) -> bool: +def _matches(blocks: tuple[re.Pattern[str], ...], text: str, pos: int, end: int) -> bool: + return any((match := block.match(text, pos)) and match.end() <= end for block in blocks) + + +def _front_matter_header(text: str, suffix: str, blocks: tuple[re.Pattern[str], ...]) -> bool: if not text.startswith(FRONT_MATTER): return False pos = len(FRONT_MATTER) - if HASH.match(text, pos): - return True + if suffix != ".md": + return _matches(HASH, text, pos, len(text)) if not (end := FRONT_MATTER_END.search(text, pos - 1)): return False while pos <= end.start(): - if (match := HASH.match(text, pos)) and match.end() <= end.start() + 1: + if _matches(HASH, text, pos, end.start() + 1): return True pos = text.index("\n", pos) + 1 - return bool(block.match(text, end.end())) + return _matches(blocks, text, end.end(), len(text)) def has_license_header(text: str, suffix: str) -> bool: """Return whether the header starts a file with the given suffix. - The header may follow a shebang and an encoding declaration. In a file - with YAML front matter, it may be written as YAML comments inside the front - matter or follow it. + The 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 + written as YAML comments inside the front matter or follow it. """ pos = 0 if match := SHEBANG.match(text, pos): pos = match.end() if match := ENCODING.match(text, pos): pos = match.end() - block = SUFFIX_BLOCKS.get(suffix, HASH) - return bool(block.match(text, pos)) or _front_matter_header(text, block) + blocks = SUFFIX_BLOCKS.get(suffix, HASH) + return _matches(blocks, text, pos, len(text)) or _front_matter_header(text, suffix, blocks) def exemption_reason(root: Path, path: str) -> str | None: @@ -236,11 +242,10 @@ def repository_files(root: Path) -> list[str]: def main() -> int: - root = Path( - subprocess.run( - ["git", "rev-parse", "--show-toplevel"], check=True, capture_output=True, text=True - ).stdout.rstrip("\n") - ) + output = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], check=True, capture_output=True + ).stdout + root = Path(os.fsdecode(output.removesuffix(b"\n"))) problems = check(root, repository_files(root)) if not problems: return 0 diff --git a/scripts/tests/test_check_license_headers.py b/scripts/tests/test_check_license_headers.py index 1a795bc0..93db62a5 100644 --- a/scripts/tests/test_check_license_headers.py +++ b/scripts/tests/test_check_license_headers.py @@ -57,6 +57,8 @@ class TestHasLicenseHeader: (".rst", "..\n" + prefixed(" ") + "\n.. _label:\n"), (".jinja2", "{#\n" + BODY + "-#}\n\nSELECT 1\n"), (".html", "{#\n" + BODY + "-#}\n"), + (".html", HTML + "

\n"), + (".md", "---\n---\n" + HTML), (".css", "/*\n" + prefixed(" * ") + " */\n"), (".jsonc", prefixed("// ") + "\n{}\n"), ], @@ -83,6 +85,9 @@ def test_accepted(self, suffix, text): (".md", "---\ntitle: x\n---\n\nText\n\n" + HTML), (".jinja2", "{#\n" + BODY + "#}\n"), (".rst", HASH), + (".yaml", "---\nkey: value\n---\n" + HASH), + (".yaml", "---\nkey: value\n" + HASH), + (".md", "---\n" + HASH), ], ) def test_rejected(self, suffix, text): From 026587bc98289fdd66ade30d40104102c780066f Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Thu, 24 Sep 2026 21:45:56 +0900 Subject: [PATCH 5/5] Move license header exemptions to a TOML config and run the check via 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 --- .github/workflows/license-headers.yaml | 8 +- docs/contributing.md | 2 +- justfile | 5 +- pyproject.toml | 1 + scripts/check_license_headers.py | 150 ++++++-------------- scripts/config/license_headers.toml | 96 +++++++++++++ scripts/tests/test_check_license_headers.py | 56 ++++++-- uv.lock | 2 + 8 files changed, 199 insertions(+), 121 deletions(-) create mode 100644 scripts/config/license_headers.toml diff --git a/.github/workflows/license-headers.yaml b/.github/workflows/license-headers.yaml index d279d3fb..0f33db74 100644 --- a/.github/workflows/license-headers.yaml +++ b/.github/workflows/license-headers.yaml @@ -20,4 +20,10 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false - - run: python3 scripts/check_license_headers.py + - uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + with: + enable-cache: true + - uses: taiki-e/install-action@7a79fe8c3a13344501c80d99cae481c1c9085912 # v2.81.10 + with: + tool: just + - run: just license-headers diff --git a/docs/contributing.md b/docs/contributing.md index f8845ea5..b3d4524b 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -124,7 +124,7 @@ For files with YAML front matter, the header may be written as YAML comments ins Use this placement for GitHub issue templates so the notice belongs to the template metadata rather than the issue body. For files that cannot contain comments, and for generated build artifacts or third-party assets, agree on an appropriate attribution location in the issue rather than inserting an invalid header. `just lint` and the License Headers workflow check the header with `scripts/check_license_headers.py`. -The checker exempts symbolic links and empty, binary, data, and lock files, and lists the other files without the header. +The checker exempts symbolic links and empty and binary files, and `scripts/config/license_headers.toml` lists the exempt file suffixes, such as data and lock files, and the other files without the header. A new file that is not exempt and has no header, such as third-party material, is added to that list as agreed in the issue. The short notice and license reference follow [Google's MIT header example](https://opensource.google/documentation/reference/releasing/licenses#mit-header), with an [SPDX identifier](https://spdx.org/licenses/MIT.html) added. diff --git a/justfile b/justfile index 0b248e04..b72f5911 100644 --- a/justfile +++ b/justfile @@ -25,11 +25,14 @@ format: uvx ruff@{{RUFF_VERSION}} format . # Lint, format check, mypy, CloudFormation validation, and license headers -lint: +lint: license-headers uvx ruff@{{RUFF_VERSION}} check . uvx ruff@{{RUFF_VERSION}} format --check . uv run mypy . uv run cfn-lint cloudformation/*.yaml + +# Check license headers +license-headers: uv run python scripts/check_license_headers.py # Run tests: just test (pyathena|sqla|sqla-async) diff --git a/pyproject.toml b/pyproject.toml index 3cc73add..92548c9b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,6 +91,7 @@ dev = [ "sphinx-design", "types-python-dateutil", "cfn-lint>=1", + "tomli>=2.0.0; python_version<'3.11'", ] [build-system] diff --git a/scripts/check_license_headers.py b/scripts/check_license_headers.py index c1fd0bbc..1fb05d57 100644 --- a/scripts/check_license_headers.py +++ b/scripts/check_license_headers.py @@ -7,19 +7,25 @@ """Check that repository files carry the PyAthena license header.""" -# Usage (from the repository root): -# uv run python scripts/check_license_headers.py +# Usage: just license-headers (also run by just lint) # # Checks tracked and untracked, non-ignored files in the working tree against -# the header described in docs/contributing.md. Reports missing headers and -# stale UNHEADED_FILES entries; never modifies files. +# the header described in docs/contributing.md, with the exemptions in +# scripts/config/license_headers.toml. Reports missing headers and stale +# unheaded-files entries; never modifies files. import os import re import subprocess import sys +from dataclasses import dataclass from pathlib import Path +if sys.version_info >= (3, 11): + import tomllib +else: + import tomli as tomllib + HEADER_LINES = ( r"Copyright \d{4} The PyAthena authors", "", @@ -61,95 +67,33 @@ def _block(opening: str | None, prefix: str, closing: str | None) -> re.Pattern[ FRONT_MATTER = "---\n" FRONT_MATTER_END = re.compile(r"\n---[ \t]*(\n|$)") -# Formats without comment syntax or with generated content. -EXEMPT_SUFFIXES = frozenset({".csv", ".gz", ".json", ".lock", ".png", ".tsv"}) - -# Existing files without the header, classified in -# https://github.com/pyathena-dev/PyAthena/issues/790 and described in NOTICE. -# New files carry the header; add an entry only as agreed in the issue that -# proposes the file, and remove entries whose files gain the header or are -# deleted. -UNHEADED_FILES = frozenset( - { - ".github/PULL_REQUEST_TEMPLATE.md", - "LICENSE", - "NOTICE", - "cloudformation/github_actions_oidc.yaml", - "docs/aio.md", - "docs/arrow.md", - "docs/conf.py", - "docs/cursor.md", - "docs/pandas.md", - "docs/polars.md", - "docs/s3fs.md", - "docs/sqlalchemy.md", - "docs/usage.md", - "pyathena/__init__.py", - "pyathena/aio/arrow/cursor.py", - "pyathena/aio/common.py", - "pyathena/aio/pandas/cursor.py", - "pyathena/aio/polars/cursor.py", - "pyathena/aio/result_set.py", - "pyathena/arrow/async_cursor.py", - "pyathena/arrow/converter.py", - "pyathena/arrow/cursor.py", - "pyathena/arrow/result_set.py", - "pyathena/arrow/util.py", - "pyathena/async_cursor.py", - "pyathena/common.py", - "pyathena/connection.py", - "pyathena/converter.py", - "pyathena/cursor.py", - "pyathena/filesystem/s3.py", - "pyathena/filesystem/s3_object.py", - "pyathena/formatter.py", - "pyathena/model.py", - "pyathena/pandas/__init__.py", - "pyathena/pandas/async_cursor.py", - "pyathena/pandas/converter.py", - "pyathena/pandas/cursor.py", - "pyathena/pandas/result_set.py", - "pyathena/pandas/util.py", - "pyathena/parser.py", - "pyathena/polars/__init__.py", - "pyathena/polars/async_cursor.py", - "pyathena/polars/cursor.py", - "pyathena/result_set.py", - "pyathena/s3fs/async_cursor.py", - "pyathena/s3fs/cursor.py", - "pyathena/sqlalchemy/array.py", - "pyathena/sqlalchemy/base.py", - "pyathena/sqlalchemy/compiler.py", - "pyathena/sqlalchemy/constants.py", - "pyathena/sqlalchemy/temporal.py", - "pyathena/sqlalchemy/types.py", - "pyathena/util.py", - "pyproject.toml", - "tests/__init__.py", - "tests/pyathena/aio/sqlalchemy/test_base.py", - "tests/pyathena/aio/test_cursor.py", - "tests/pyathena/arrow/test_async_cursor.py", - "tests/pyathena/conftest.py", - "tests/pyathena/filesystem/test_s3.py", - "tests/pyathena/filesystem/test_s3_async.py", - "tests/pyathena/pandas/test_async_cursor.py", - "tests/pyathena/pandas/test_cursor.py", - "tests/pyathena/pandas/test_util.py", - "tests/pyathena/polars/test_async_cursor.py", - "tests/pyathena/s3fs/test_cursor.py", - "tests/pyathena/sqlalchemy/test_array.py", - "tests/pyathena/sqlalchemy/test_base.py", - "tests/pyathena/sqlalchemy/test_temporal.py", - "tests/pyathena/sqlalchemy/test_types.py", - "tests/pyathena/test_async_cursor.py", - "tests/pyathena/test_converter.py", - "tests/pyathena/test_cursor.py", - "tests/pyathena/test_model.py", - "tests/pyathena/test_util.py", - "tests/resources/queries/create_table.sql.jinja2", - "tests/sqlalchemy/test_suite.py", - } -) +CONFIG = "scripts/config/license_headers.toml" + + +@dataclass(frozen=True) +class Config: + """Exemptions read from CONFIG.""" + + exempt_suffixes: frozenset[str] + unheaded_files: frozenset[str] + + +def load_config(file: Path) -> Config: + """Read the exemptions, rejecting unknown keys and duplicate entries.""" + with file.open("rb") as f: + data = tomllib.load(f) + keys = {"exempt-suffixes", "unheaded-files"} + if set(data) != keys: + raise ValueError(f"{file}: expected keys {sorted(keys)}, found {sorted(data)}") + values = {} + for key in sorted(keys): + items = data[key] + if not isinstance(items, list) or not all(isinstance(item, str) for item in items): + raise ValueError(f"{file}: {key} must be a list of strings") + if len(set(items)) != len(items): + raise ValueError(f"{file}: {key} has duplicate entries") + values[key] = frozenset(items) + return Config(values["exempt-suffixes"], values["unheaded-files"]) def _matches(blocks: tuple[re.Pattern[str], ...], text: str, pos: int, end: int) -> bool: @@ -187,12 +131,12 @@ def has_license_header(text: str, suffix: str) -> bool: return _matches(blocks, text, pos, len(text)) or _front_matter_header(text, suffix, blocks) -def exemption_reason(root: Path, path: str) -> str | None: +def exemption_reason(root: Path, path: str, exempt_suffixes: frozenset[str]) -> str | None: """Return why a file needs no header, or None when it needs one.""" file = root / path if file.is_symlink(): return "symbolic link" - if Path(path).suffix in EXEMPT_SUFFIXES: + if Path(path).suffix in exempt_suffixes: return "data or generated file" data = file.read_bytes() if b"\0" in data: @@ -206,26 +150,26 @@ def exemption_reason(root: Path, path: str) -> str | None: return None -def check(root: Path, paths: list[str]) -> list[str]: +def check(root: Path, paths: list[str], config: Config) -> list[str]: """Return problems for the given repository-relative paths.""" problems = [] existing = {path for path in paths if (root / path).is_symlink() or (root / path).is_file()} for path in sorted(existing): file = root / path - reason = exemption_reason(root, path) + reason = exemption_reason(root, path, config.exempt_suffixes) headed = reason is None and has_license_header( file.read_text(encoding="utf-8"), Path(path).suffix ) - if path in UNHEADED_FILES: + if path in config.unheaded_files: if reason is not None: - problems.append(f"{path}: listed in UNHEADED_FILES but exempt as {reason}") + problems.append(f"{path}: listed as unheaded in {CONFIG} but exempt as {reason}") elif headed: - problems.append(f"{path}: listed in UNHEADED_FILES but has the header") + problems.append(f"{path}: listed as unheaded in {CONFIG} but has the header") elif reason is None and not headed: problems.append(f"{path}: missing license header") problems.extend( - f"{path}: listed in UNHEADED_FILES but not found" - for path in sorted(UNHEADED_FILES - existing) + f"{path}: listed as unheaded in {CONFIG} but not found" + for path in sorted(config.unheaded_files - existing) ) return problems @@ -246,7 +190,7 @@ def main() -> int: ["git", "rev-parse", "--show-toplevel"], check=True, capture_output=True ).stdout root = Path(os.fsdecode(output.removesuffix(b"\n"))) - problems = check(root, repository_files(root)) + problems = check(root, repository_files(root), load_config(root / CONFIG)) if not problems: return 0 sys.stderr.write("".join(f"{problem}\n" for problem in problems)) diff --git a/scripts/config/license_headers.toml b/scripts/config/license_headers.toml new file mode 100644 index 00000000..0467a903 --- /dev/null +++ b/scripts/config/license_headers.toml @@ -0,0 +1,96 @@ +# Copyright 2026 The PyAthena authors +# +# Licensed under the MIT License. +# See LICENSE or https://opensource.org/licenses/MIT. +# +# SPDX-License-Identifier: MIT + +# Configuration for scripts/check_license_headers.py. + +# Formats without comment syntax or with generated content. +exempt-suffixes = [".csv", ".gz", ".json", ".lock", ".png", ".tsv"] + +# Existing files without the header, classified in +# https://github.com/pyathena-dev/PyAthena/issues/790 and described in NOTICE. +# New files carry the header; add an entry only as agreed in the issue that +# proposes the file, and remove entries whose files gain the header or are +# deleted. +unheaded-files = [ + ".github/PULL_REQUEST_TEMPLATE.md", + "LICENSE", + "NOTICE", + "cloudformation/github_actions_oidc.yaml", + "docs/aio.md", + "docs/arrow.md", + "docs/conf.py", + "docs/cursor.md", + "docs/pandas.md", + "docs/polars.md", + "docs/s3fs.md", + "docs/sqlalchemy.md", + "docs/usage.md", + "pyathena/__init__.py", + "pyathena/aio/arrow/cursor.py", + "pyathena/aio/common.py", + "pyathena/aio/pandas/cursor.py", + "pyathena/aio/polars/cursor.py", + "pyathena/aio/result_set.py", + "pyathena/arrow/async_cursor.py", + "pyathena/arrow/converter.py", + "pyathena/arrow/cursor.py", + "pyathena/arrow/result_set.py", + "pyathena/arrow/util.py", + "pyathena/async_cursor.py", + "pyathena/common.py", + "pyathena/connection.py", + "pyathena/converter.py", + "pyathena/cursor.py", + "pyathena/filesystem/s3.py", + "pyathena/filesystem/s3_object.py", + "pyathena/formatter.py", + "pyathena/model.py", + "pyathena/pandas/__init__.py", + "pyathena/pandas/async_cursor.py", + "pyathena/pandas/converter.py", + "pyathena/pandas/cursor.py", + "pyathena/pandas/result_set.py", + "pyathena/pandas/util.py", + "pyathena/parser.py", + "pyathena/polars/__init__.py", + "pyathena/polars/async_cursor.py", + "pyathena/polars/cursor.py", + "pyathena/result_set.py", + "pyathena/s3fs/async_cursor.py", + "pyathena/s3fs/cursor.py", + "pyathena/sqlalchemy/array.py", + "pyathena/sqlalchemy/base.py", + "pyathena/sqlalchemy/compiler.py", + "pyathena/sqlalchemy/constants.py", + "pyathena/sqlalchemy/temporal.py", + "pyathena/sqlalchemy/types.py", + "pyathena/util.py", + "pyproject.toml", + "tests/__init__.py", + "tests/pyathena/aio/sqlalchemy/test_base.py", + "tests/pyathena/aio/test_cursor.py", + "tests/pyathena/arrow/test_async_cursor.py", + "tests/pyathena/conftest.py", + "tests/pyathena/filesystem/test_s3.py", + "tests/pyathena/filesystem/test_s3_async.py", + "tests/pyathena/pandas/test_async_cursor.py", + "tests/pyathena/pandas/test_cursor.py", + "tests/pyathena/pandas/test_util.py", + "tests/pyathena/polars/test_async_cursor.py", + "tests/pyathena/s3fs/test_cursor.py", + "tests/pyathena/sqlalchemy/test_array.py", + "tests/pyathena/sqlalchemy/test_base.py", + "tests/pyathena/sqlalchemy/test_temporal.py", + "tests/pyathena/sqlalchemy/test_types.py", + "tests/pyathena/test_async_cursor.py", + "tests/pyathena/test_converter.py", + "tests/pyathena/test_cursor.py", + "tests/pyathena/test_model.py", + "tests/pyathena/test_util.py", + "tests/resources/queries/create_table.sql.jinja2", + "tests/sqlalchemy/test_suite.py", +] diff --git a/scripts/tests/test_check_license_headers.py b/scripts/tests/test_check_license_headers.py index 93db62a5..4bcdcda8 100644 --- a/scripts/tests/test_check_license_headers.py +++ b/scripts/tests/test_check_license_headers.py @@ -6,17 +6,22 @@ # SPDX-License-Identifier: MIT import subprocess +from pathlib import Path import pytest from scripts.check_license_headers import ( - UNHEADED_FILES, + CONFIG, + Config, check, exemption_reason, has_license_header, + load_config, repository_files, ) +SUFFIXES = frozenset({".csv", ".json", ".lock", ".tsv"}) + LINES = [ "Copyright 2026 The PyAthena authors", "", @@ -111,48 +116,69 @@ class TestExemptionReason: ) def test_file(self, tmp_path, name, content, expected): (tmp_path / name).write_bytes(content) - assert exemption_reason(tmp_path, name) == expected + assert exemption_reason(tmp_path, name, SUFFIXES) == expected def test_symlink(self, tmp_path): (tmp_path / "target.md").write_text("text\n") (tmp_path / "link.md").symlink_to("target.md") - assert exemption_reason(tmp_path, "link.md") == "symbolic link" + assert exemption_reason(tmp_path, "link.md", SUFFIXES) == "symbolic link" + + +class TestLoadConfig: + def test_repository_config(self): + config = load_config(Path(__file__).parents[2] / CONFIG) + assert ".lock" in config.exempt_suffixes + assert "LICENSE" in config.unheaded_files + + @pytest.mark.parametrize( + ("content", "message"), + [ + ("exempt-suffixes = []\n", "expected keys"), + ("exempt-suffixes = []\nunheaded-files = []\nextra = 1\n", "expected keys"), + ('exempt-suffixes = ".lock"\nunheaded-files = []\n', "list of strings"), + ('exempt-suffixes = []\nunheaded-files = ["a", "a"]\n', "duplicate entries"), + ], + ) + def test_invalid(self, tmp_path, content, message): + file = tmp_path / "config.toml" + file.write_text(content) + with pytest.raises(ValueError, match=message): + load_config(file) class TestCheck: + CONFIG = Config(exempt_suffixes=SUFFIXES, unheaded_files=frozenset({"LICENSE"})) + @pytest.fixture def root(self, tmp_path): - for path in UNHEADED_FILES: - file = tmp_path / path - file.parent.mkdir(parents=True, exist_ok=True) - file.write_text("content\n") + (tmp_path / "LICENSE").write_text("content\n") return tmp_path def test_clean(self, root): (root / "new.py").write_text(HASH) (root / "empty.py").write_text("") - assert check(root, [*UNHEADED_FILES, "new.py", "empty.py"]) == [] + assert check(root, ["LICENSE", "new.py", "empty.py"], self.CONFIG) == [] def test_missing_header(self, root): (root / "new.py").write_text("x = 1\n") - assert check(root, [*UNHEADED_FILES, "new.py"]) == ["new.py: missing license header"] + assert check(root, ["LICENSE", "new.py"], self.CONFIG) == ["new.py: missing license header"] def test_listed_file_with_header(self, root): (root / "LICENSE").write_text(HASH) - assert check(root, list(UNHEADED_FILES)) == [ - "LICENSE: listed in UNHEADED_FILES but has the header" + assert check(root, ["LICENSE"], self.CONFIG) == [ + f"LICENSE: listed as unheaded in {CONFIG} but has the header" ] def test_listed_file_exempt(self, root): (root / "LICENSE").write_text("") - assert check(root, list(UNHEADED_FILES)) == [ - "LICENSE: listed in UNHEADED_FILES but exempt as empty file" + assert check(root, ["LICENSE"], self.CONFIG) == [ + f"LICENSE: listed as unheaded in {CONFIG} but exempt as empty file" ] def test_listed_file_not_found(self, root): (root / "LICENSE").unlink() - assert check(root, list(UNHEADED_FILES)) == [ - "LICENSE: listed in UNHEADED_FILES but not found" + assert check(root, ["LICENSE"], self.CONFIG) == [ + f"LICENSE: listed as unheaded in {CONFIG} but not found" ] diff --git a/uv.lock b/uv.lock index 8052c20c..c45abd4b 100644 --- a/uv.lock +++ b/uv.lock @@ -1125,6 +1125,7 @@ dev = [ { name = "sphinx-multiversion" }, { name = "sphinxext-opengraph" }, { name = "sqlalchemy", extra = ["asyncio"] }, + { name = "tomli", marker = "python_full_version < '3.11'" }, { name = "types-python-dateutil" }, ] @@ -1170,6 +1171,7 @@ dev = [ { name = "sphinx-multiversion" }, { name = "sphinxext-opengraph" }, { name = "sqlalchemy", extras = ["asyncio"], specifier = ">=1.0.0" }, + { name = "tomli", marker = "python_full_version < '3.11'", specifier = ">=2.0.0" }, { name = "types-python-dateutil" }, ]