From ac76239ba3e89233569d050d2417d5f648c9d7c8 Mon Sep 17 00:00:00 2001 From: Yashasvi Nancherla Date: Mon, 23 Feb 2026 14:56:28 +0530 Subject: [PATCH 1/2] Removes use of github APIs Uses repo checkout to get base and head commit of PR to support deprecation of github token Signed-off-by: Yashasvi Nancherla --- README.md | 24 +++-- action.yml | 17 +++- check_commits.py | 190 ++++++++++++++++++------------------ tests/test_check_commits.py | 189 ++++++++++++++++++++--------------- 4 files changed, 238 insertions(+), 182 deletions(-) diff --git a/README.md b/README.md index 51f351a..9165ded 100644 --- a/README.md +++ b/README.md @@ -9,21 +9,29 @@ This GitHub Action enforces consistent commit message formatting for Qualcomm pr Create a new GitHub Actions workflow in your project, e.g. at .github/workflows/commit-check.yml name: Commit Msg Check Action - + on: pull_request: types: [opened, synchronize, reopened] - + jobs: check-commits: runs-on: ubuntu-latest - - steps: - - name: Run custom commit check - uses: qualcomm/commit-msg-check-action@v1.0.0 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + permissions: + contents: read + pull-requests: read + steps: + - name: Check out repository + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 + + - name: Commit Check + uses: qualcomm/commit-msg-check-action@v2.0.0 with: + base: ${{ github.event.pull_request.base.sha }} + head: ${{ github.event.pull_request.head.sha }} body-char-limit: 72 sub-char-limit: 50 check-blank-line: true diff --git a/action.yml b/action.yml index a0f960b..9312bd3 100644 --- a/action.yml +++ b/action.yml @@ -1,7 +1,15 @@ -name: "My Python Commit Check" +name: "Commit Msg Check" description: "Checks commit messages using a Python script" inputs: + base: + description: "Base SHA for the commit range" + required: true + + head: + description: "Head SHA for the commit range" + required: true + body-char-limit: required: false default: "72" @@ -14,7 +22,7 @@ inputs: check-blank-line: required: false - default: true + default: "true" description: "check for a blank line between commit title and body" runs: @@ -39,10 +47,11 @@ runs: - name: Check commit messages shell: bash + working-directory: ${{ github.workspace }} run: | python ${{ github.action_path }}/check_commits.py \ - --repo "${{ github.repository }}" \ - --pr-number "${{ github.event.pull_request.number }}" \ + --base "${{ inputs.base }}" \ + --head "${{ inputs.head }}" \ --body-limit "${{ inputs.body-char-limit }}" \ --sub-limit "${{ inputs.sub-char-limit }}" \ --check-blank-line "${{ inputs.check-blank-line }}" diff --git a/check_commits.py b/check_commits.py index 62b1be1..535d6d8 100644 --- a/check_commits.py +++ b/check_commits.py @@ -3,123 +3,129 @@ import os import sys -import requests import argparse - -api_base_url = os.getenv("GITHUB_API_URL") +import subprocess def parse_arguments(): parser = argparse.ArgumentParser( - description="Validate commit messages in a GitHub PR." + description="Validate commit messages using local Git history (no API/token)." + ) + parser.add_argument( + "--base", required=True, help="Base SHA for the range (base..head)" + ) + parser.add_argument( + "--head", required=True, help="Head SHA for the range (or single ref)" ) - parser.add_argument("--repo", required=True) - parser.add_argument("--pr-number", required=True) parser.add_argument("--body-limit", type=int, default=72) parser.add_argument("--sub-limit", type=int, default=50) parser.add_argument("--check-blank-line", type=str, default="true") return parser.parse_args() -def fetch_commits(args): - token = os.getenv("GITHUB_TOKEN") - if not token: - print("::error::No GITHUB_TOKEN found!") - sys.exit(1) - - url = f"{api_base_url}/repos/{args.repo}/pulls/{args.pr_number}/commits" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3+json", - } - response = requests.get(url, headers=headers) - - if response.status_code != 200: - print( - f"::error::Failed to fetch PR commits: {response.status_code} {response.text}" +def fetch_commits(base, head): + """ + Fetch commits between base..head from local repository. + Returns a list of dicts with 'sha' and 'message' keys. + """ + if not head: + print("::error::Tokenless mode requires --head (and usually --base).") + sys.exit(2) + + rev_range = f"{base}..{head}" if base else head + try: + shas = ( + subprocess.check_output( + ["git", "rev-list", "--no-merges", rev_range], text=True + ) + .strip() + .splitlines() ) - sys.exit(1) - - return response.json() - - -def validate_commit_message(commit, sub_char_limit, body_char_limit, check_blank_line): - sha = commit["sha"] - message = commit["commit"]["message"] - lines = message.splitlines() - n = len(lines) - - subject = lines[0] if n >= 1 else "" - body = [ - line.strip() - for line in lines[1:] - if line.strip() and not line.lower().startswith("signed-off-by") - ] - signed_off = lines[-1] if "signed-off-by" in lines[-1].lower() else "" - missing_sub_body_line = False - missing_body_sign_line = False - - if check_blank_line.lower() == "true": - if n > 1 and lines[1].strip() != "": - missing_sub_body_line = True - else: - body = [ - line.strip() - for line in lines[2:] - if line.strip() and not line.lower().startswith("signed-off-by") - ] - if signed_off and lines[-2].strip() != "": - missing_body_sign_line = True - + if not shas: + return [] + output = subprocess.check_output( + ["git", "show", "-s", "--format=%H%x00%B%x00"] + shas, + text=True + ) + commits = [] + parts = output.split('\x00') + for i in range(0, len(parts) - 1, 2): + sha = parts[i].strip() + message = parts[i + 1] if i + 1 < len(parts) else "" + if sha: + commits.append({"sha": sha, "message": message}) + return commits + + except subprocess.CalledProcessError as e: + print(f"::error::Failed to fetch commits with git: {e}") + sys.exit(2) + + +def validate_subject(subject, sub_char_limit): + """Validate the commit subject line.""" errors = [] if len(subject.strip()) == 0: errors.append("Commit message is missing subject!") if len(subject) > sub_char_limit: errors.append(f"Subject exceeds {sub_char_limit} characters!") + return errors + + +def validate_body(lines, n, body_char_limit, check_blank_line): + """Validate the commit body.""" + errors = [] + + body_index = 1 if check_blank_line.lower() == "true": - if missing_sub_body_line and subject and body: + # Check for blank line after subject + if n > 1 and lines[1].strip() != "": errors.append("Subject and body must be separated by a blank line") - if missing_body_sign_line and body and signed_off: - errors.append("Body and Signed-off-by must be separated by a blank line") + body_index = 2 + body = [ + line.strip() + for line in lines[body_index:n] + if line.strip() and not line.lower().startswith("signed-off-by") + ] if len(body) == 0: errors.append("Commit message is missing a body!") for line in body: if len(line) > body_char_limit: errors.append(f"Line exceeds {body_char_limit} characters: {line}") + return errors, body + + +def validate_signoff(lines, n, body, check_blank_line): + """Validate the Signed-off-by line.""" + errors = [] + + signed_off = lines[-1] if n > 0 and "signed-off-by" in lines[-1].lower() else "" + + if check_blank_line.lower() == "true" and body and signed_off: + if n >= 2 and lines[-2].strip() != "": + errors.append("Body and Signed-off-by must be separated by a blank line") + + return errors + + +def validate_commit_message(commit, sub_char_limit, body_char_limit, check_blank_line): + sha = commit["sha"] + message = commit["message"] + lines = message.splitlines() + n = len(lines) + subject = lines[0] if n >= 1 else "" + + errors = [] + subject_errors = validate_subject(subject, sub_char_limit) + body_errors, body = validate_body(lines, n, body_char_limit, check_blank_line) + signed_off_errors = validate_signoff(lines, n, body, check_blank_line) + + errors.extend(subject_errors + body_errors + signed_off_errors) + return sha, errors -def add_commit_comment(repo, sha, message): - token = os.getenv("GITHUB_TOKEN") - if not token: - return - url = f"{api_base_url}/repos/{repo}/commits/{sha}/comments" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3+json", - } - requests.post(url, headers=headers, json={"body": message}) - - -def set_commit_status(repo, sha, state, description): - token = os.getenv("GITHUB_TOKEN") - if not token: - return - url = f"{api_base_url}/repos/{repo}/statuses/{sha}" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3+json", - } - data = { - "state": state, - "description": description, - "context": "commit-message-check", - } - requests.post(url, headers=headers, json=data) - - -def process_commits(commits, repo, sub_limit, body_limit, check_blank_line): +def process_commits(commits, sub_limit, body_limit, check_blank_line): failed_count = 0 for commit in commits: sha, errors = validate_commit_message( @@ -130,20 +136,17 @@ def process_commits(commits, repo, sub_limit, body_limit, check_blank_line): failed_count += 1 for err in errors: print(f"::error:: {err}") - add_commit_comment(repo, sha, "\n".join(errors)) - set_commit_status(repo, sha, "failure", "Commit message validation failed") print("::endgroup::") else: print(f"✅ Commit {sha} passed all checks.") - set_commit_status(repo, sha, "success", "Commit message validation passed") return failed_count def main(): args = parse_arguments() - commits = fetch_commits(args) + commits = fetch_commits(args.base, args.head) failed_count = process_commits( - commits, args.repo, args.sub_limit, args.body_limit, args.check_blank_line + commits, args.sub_limit, args.body_limit, args.check_blank_line ) summary_path = os.getenv("GITHUB_STEP_SUMMARY") @@ -160,3 +163,4 @@ def main(): if __name__ == "__main__": main() + diff --git a/tests/test_check_commits.py b/tests/test_check_commits.py index 7f4d299..7476b13 100644 --- a/tests/test_check_commits.py +++ b/tests/test_check_commits.py @@ -1,112 +1,147 @@ # Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. # SPDX-License-Identifier: BSD-3-Clause-Clear +import os +import sys import unittest -from unittest.mock import patch, MagicMock from io import StringIO -import sys -import os +from unittest.mock import patch +from contextlib import redirect_stdout + sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) import check_commits class TestCheckCommits(unittest.TestCase): - def setUp(self): - self.repo = "test/repo" - self.sample_commit = { + self.valid_sample_commit = { "sha": "abc123", - "commit": { - "message": ( - "Valid subject\n\n" - "This is a valid description line.\n" - "It continues here.\n\n" - "Signed-off-by: Developer " - ) - } + "message": ( + "Valid subject\n\n" + "This is a valid description line.\n" + "It continues here.\n\n" + "Signed-off-by: Developer " + ), + } + self.invalid_sample_commit = { + "sha": "badcommit1234", + "message": ( + "Invalid subject which is definitely longer than 50 characters" + "This is a valid description line.\n" + "It continues here." + "Signed-off-by: Developer " + ), } - @patch('check_commits.parse_arguments') - def test_parse_arguments(self, mock_parse_args): - mock_args = MagicMock() - mock_args.repo = "test/repo" - mock_args.pr_number = "123" - mock_args.body_limit = 72 - mock_args.sub_limit = 50 - mock_args.check_blank_line = "true" - mock_parse_args.return_value = mock_args - - args = check_commits.parse_arguments() - - self.assertEqual(args.repo, "test/repo") - self.assertEqual(args.pr_number, "123") + def test_parse_arguments_with_base_head(self): + argv = [ + "check_commits.py", + "--base", + "1111111", + "--head", + "2222222", + "--body-limit", + "72", + "--sub-limit", + "50", + "--check-blank-line", + "true", + ] + with patch.object(sys, "argv", argv): + args = check_commits.parse_arguments() + self.assertEqual(args.base, "1111111") + self.assertEqual(args.head, "2222222") self.assertEqual(args.body_limit, 72) self.assertEqual(args.sub_limit, 50) self.assertEqual(args.check_blank_line, "true") - @patch('check_commits.requests.get') - @patch('os.getenv') - def test_fetch_commits_success(self, mock_getenv, mock_get): - mock_getenv.return_value = "fake_token" - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.json.return_value = [self.sample_commit] - mock_get.return_value = mock_response - - args = MagicMock() - args.repo = "test/repo" - args.pr_number = "123" - - result = check_commits.fetch_commits(args) - - self.assertEqual(result, [self.sample_commit]) - mock_get.assert_called_once() - def test_validate_commit_message_valid(self): sha, errors = check_commits.validate_commit_message( - self.sample_commit, 50, 72, check_blank_line="true" + self.valid_sample_commit, + sub_char_limit=50, + body_char_limit=72, + check_blank_line="true", ) self.assertEqual(sha, "abc123") self.assertEqual(errors, []) - @patch('sys.stdout', new_callable=StringIO) - def test_process_commits_all_valid(self, mock_stdout): - commits = [self.sample_commit] - failed_count = check_commits.process_commits( - commits, self.repo, 50, 72, check_blank_line="true" - ) - self.assertEqual(failed_count, 0) - self.assertIn("✅ Commit abc123 passed all checks", mock_stdout.getvalue()) - - def test_validate_commit_message_subject_too_long(self): + def test_validate_commit_message_subject_too_long_no_blank_check(self): commit = { "sha": "def456", - "commit": { - "message": ( - "This subject line is way too long and should definitely fail the check\n" - "Valid description line.\n\n" - "Signed-off-by: Developer " - ) - } + "message": ( + "This subject line is way too long and should definitely fail the check\n" + "Body line.\n\n" + "Signed-off-by: Developer " + ), } - sha, errors = check_commits.validate_commit_message(commit, 50, 72, check_blank_line="false") # check_blank_line is set to false + _sha, errors = check_commits.validate_commit_message( + commit, sub_char_limit=50, body_char_limit=72, check_blank_line="false" + ) self.assertIn("Subject exceeds 50 characters!", errors) - self.assertIsNot("Subject and description must be separated by a blank line",errors) - - def test_validate_commit_message_subject_too_long_and_check_blank_line(self): + self.assertTrue( + all( + "Subject and body must be separated by a blank line" not in e + for e in errors + ) + ) + + def test_validate_commit_message_subject_too_long_with_blank_check(self): commit = { "sha": "def456", - "commit": { - "message": ( - "This subject line is way too long and should definitely fail the check\n" - "Valid description line.\n\n" - "Signed-off-by: Developer " - ) - } + "message": ( + "This subject line is way too long and should definitely fail the check\n" + "Body line without blank separator\n\n" + "Signed-off-by: Developer " + ), } - sha, errors = check_commits.validate_commit_message(commit, 50, 72, check_blank_line="true") # check_blank_line is set to true + _sha, errors = check_commits.validate_commit_message( + commit, sub_char_limit=50, body_char_limit=72, check_blank_line="true" + ) self.assertIn("Subject exceeds 50 characters!", errors) - self.assertIn("Subject and body must be separated by a blank line",errors) + self.assertIn("Subject and body must be separated by a blank line", errors) + + def test_process_commits_all_valid_prints_and_returns_zero(self): + commits = [self.valid_sample_commit] + buf = StringIO() + with redirect_stdout(buf): + failed = check_commits.process_commits( + commits, sub_limit=50, body_limit=72, check_blank_line="true" + ) + out = buf.getvalue() + self.assertEqual(failed, 0) + self.assertIn("✅ Commit abc123 passed all checks.", out) + + def test_process_commits_mixed_failures(self): + bad_commit = { + "sha": "bad999", + "message": ( + "Bad subject with excessive length that violates the rule right away\n" + "Body line\n" + ), + } + commits = [self.valid_sample_commit, bad_commit] + buf = StringIO() + with redirect_stdout(buf): + failed = check_commits.process_commits( + commits, sub_limit=50, body_limit=72, check_blank_line="true" + ) + out = buf.getvalue() + self.assertEqual(failed, 1) + self.assertIn("❌ Errors in commit bad999", out) + self.assertIn("Subject exceeds 50 characters!", out) + + def test_commits_failures(self): + commits = [self.invalid_sample_commit] + buf = StringIO() + with redirect_stdout(buf): + failed = check_commits.process_commits( + commits, sub_limit=50, body_limit=72, check_blank_line="true" + ) + out = buf.getvalue() + self.assertEqual(failed, 1) + self.assertIn("❌ Errors in commit badcommit1234", out) + self.assertIn("Subject exceeds 50 characters!", out) + self.assertIn("Subject and body must be separated by a blank line", out) if __name__ == "__main__": From 439b72c8f9991dfdc4f91763c019f48d537e6aa5 Mon Sep 17 00:00:00 2001 From: Yashasvi Nancherla Date: Tue, 17 Mar 2026 16:18:17 +0530 Subject: [PATCH 2/2] reformats check_commits.py Signed-off-by: Yashasvi Nancherla --- check_commits.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/check_commits.py b/check_commits.py index 535d6d8..df7d3c2 100644 --- a/check_commits.py +++ b/check_commits.py @@ -44,11 +44,10 @@ def fetch_commits(base, head): if not shas: return [] output = subprocess.check_output( - ["git", "show", "-s", "--format=%H%x00%B%x00"] + shas, - text=True + ["git", "show", "-s", "--format=%H%x00%B%x00"] + shas, text=True ) commits = [] - parts = output.split('\x00') + parts = output.split("\x00") for i in range(0, len(parts) - 1, 2): sha = parts[i].strip() message = parts[i + 1] if i + 1 < len(parts) else "" @@ -163,4 +162,3 @@ def main(): if __name__ == "__main__": main() -