diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 4e56533..d30babe 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -4,86 +4,76 @@ on: pull_request: branches: [main] types: [opened, edited, synchronize, reopened] + paths: + - 'awesome-privacy.yml' + - '.github/README.md' permissions: contents: read pull-requests: read jobs: - detect-changes: - name: Detect changes + pr-compliance: + name: PR Compliance runs-on: ubuntu-latest - outputs: - yaml_changed: ${{ steps.changes.outputs.yaml_changed }} - non_yaml_changed: ${{ steps.changes.outputs.non_yaml_changed }} steps: - uses: actions/checkout@v4 - run: git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} - uses: actions/setup-python@v5 with: python-version: "3.12" - - name: Detect changed files - id: changes - run: python lib/checks/detect-changes.py --base-ref ${{ github.event.pull_request.base.sha }} - - name: Non-YAML changes warning - if: steps.changes.outputs.non_yaml_changed == 'true' - run: python lib/checks/warn-non-yaml.py --base-ref ${{ github.event.pull_request.base.sha }} - - pr-meta: - name: PR metadata - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: "3.12" + - name: Check README edits + id: readme + continue-on-error: true + run: python lib/checks/check-readme-edits.py --base-ref ${{ github.event.pull_request.base.sha }} - name: Check PR metadata + id: meta env: PR_TITLE: ${{ github.event.pull_request.title }} PR_BODY: ${{ github.event.pull_request.body }} PR_DRAFT: ${{ github.event.pull_request.draft }} + README_FAILED: ${{ steps.readme.outcome == 'failure' && 'true' || 'false' }} run: python lib/checks/check-pr-meta.py - name: Upload findings if: always() uses: actions/upload-artifact@v4 with: - name: findings-meta - path: /tmp/findings-meta.json + name: findings-compliance + path: /tmp/findings-compliance.json if-no-files-found: ignore - - file-checks: - name: File checks - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - run: git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} - - uses: actions/setup-python@v5 - with: - python-version: "3.12" - - name: Check for direct README edits - run: python lib/checks/check-readme-edits.py --base-ref ${{ github.event.pull_request.base.sha }} + - name: Fail if critical + if: steps.readme.outcome == 'failure' || steps.meta.outcome == 'failure' + run: exit 1 data-validation: - name: Data validation - needs: detect-changes - if: needs.detect-changes.outputs.yaml_changed == 'true' + name: Data Validation runs-on: ubuntu-latest + outputs: + yaml_changed: ${{ steps.changes.outputs.yaml_changed }} steps: - uses: actions/checkout@v4 - run: git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} - uses: actions/setup-python@v5 with: python-version: "3.12" - - run: pip install -q -r lib/requirements.txt + - name: Detect changes + id: changes + run: python lib/checks/detect-changes.py --base-ref ${{ github.event.pull_request.base.sha }} + - name: Install dependencies + if: steps.changes.outputs.yaml_changed == 'true' + run: pip install -q -r lib/requirements.txt - name: Schema validation + if: steps.changes.outputs.yaml_changed == 'true' id: schema continue-on-error: true run: make validate - name: YAML diff + if: steps.changes.outputs.yaml_changed == 'true' id: diff continue-on-error: true run: python lib/checks/check-yaml-diff.py --base-ref ${{ github.event.pull_request.base.sha }} - name: Check additions + if: steps.changes.outputs.yaml_changed == 'true' env: SCHEMA_OUTCOME: ${{ steps.schema.outcome }} run: python lib/checks/check-additions.py @@ -101,14 +91,14 @@ jobs: name: findings-data path: /tmp/findings-data.json if-no-files-found: ignore - - name: Fail if critical checks failed - if: steps.schema.outcome == 'failure' || steps.diff.outcome == 'failure' + - name: Fail if critical + if: steps.changes.outputs.yaml_changed == 'true' && (steps.schema.outcome == 'failure' || steps.diff.outcome == 'failure') run: exit 1 - project-checks: - name: Project checks + submission-eligibility: + name: Submission Eligibility needs: data-validation - if: "!cancelled() && needs.data-validation.result != 'skipped'" + if: "!cancelled() && needs.data-validation.outputs.yaml_changed == 'true'" runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -138,7 +128,7 @@ jobs: summary: name: Summary if: always() - needs: [detect-changes, pr-meta, file-checks, data-validation, project-checks] + needs: [pr-compliance, data-validation, submission-eligibility] runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -157,7 +147,6 @@ jobs: PR_USER: ${{ github.event.pull_request.user.login }} PR_NUMBER: ${{ github.event.pull_request.number }} RUN_ID: ${{ github.run_id }} - README_FAILED: ${{ needs.file-checks.result == 'failure' && 'true' || 'false' }} run: python lib/checks/format-comment.py - name: Upload PR metadata if: always() diff --git a/lib/checks/check-links.py b/lib/checks/check-links.py deleted file mode 100644 index 293fa24..0000000 --- a/lib/checks/check-links.py +++ /dev/null @@ -1,211 +0,0 @@ -""" -Validates URLs and metadata for added/modified services in a PR. -Reads the diff JSON produced by check-yaml-diff.py. -All checks are warnings only -- this script never fails (exit 0). -""" - -import argparse -import json -import os -import sys - -import requests -import yaml - -# Paths (relative to project root) -PROJECT_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -DATA_PATH = os.path.join(PROJECT_ROOT, "awesome-privacy.yml") - -# Exit codes -EXIT_PASS = 0 -EXIT_RUNTIME_ERROR = 2 - -# ANSI color helpers -_use_color = sys.stderr.isatty() and not os.environ.get("NO_COLOR") -red = (lambda s: f"\033[31m{s}\033[0m") if _use_color else (lambda s: s) -green = (lambda s: f"\033[32m{s}\033[0m") if _use_color else (lambda s: s) -yellow = (lambda s: f"\033[33m{s}\033[0m") if _use_color else (lambda s: s) - -TIMEOUT = 10 -USER_AGENT = "awesome-privacy-ci/1.0" -DESC_MIN_LEN = 50 -DESC_MAX_LEN = 250 - - -def check_url(url, label): - """Check if a URL is reachable. Returns (ok, message).""" - try: - resp = requests.head( - url, - timeout=TIMEOUT, - allow_redirects=True, - headers={"User-Agent": USER_AGENT}, - ) - if resp.status_code >= 400: - # Retry with GET -- some servers reject HEAD - resp = requests.get( - url, - timeout=TIMEOUT, - allow_redirects=True, - headers={"User-Agent": USER_AGENT}, - stream=True, - ) - resp.close() - if resp.status_code >= 400: - return False, f"{label}: HTTP {resp.status_code} for {url}" - return True, None - except requests.RequestException as e: - return False, f"{label}: Connection error for {url} ({type(e).__name__})" - - -def check_service(service_data, service_name, category, section): - """Run all checks on a single service. Returns list of warning strings.""" - warnings = [] - name_prefix = f"{category} > {section} > {service_name}" - - # Check url - url = service_data.get("url") - if url: - ok, msg = check_url(url, "url") - if not ok: - warnings.append(f"{name_prefix}: {msg}") - else: - warnings.append(f"{name_prefix}: missing required field 'url'") - - # Check icon - icon = service_data.get("icon") - if icon: - ok, msg = check_url(icon, "icon") - if not ok: - warnings.append(f"{name_prefix}: {msg}") - else: - warnings.append(f"{name_prefix}: missing 'icon' field (recommended by contributing guide)") - - # Check iosApp - ios_app = service_data.get("iosApp") - if ios_app: - ok, msg = check_url(ios_app, "iosApp") - if not ok: - warnings.append(f"{name_prefix}: {msg}") - - # Check github - github = service_data.get("github") - if github: - github_url = f"https://github.com/{github}" - ok, msg = check_url(github_url, "github") - if not ok: - warnings.append(f"{name_prefix}: {msg}") - - # Check description length - desc = service_data.get("description", "") - desc_stripped = desc.strip() - desc_len = len(desc_stripped) - if desc_len < DESC_MIN_LEN: - warnings.append(f"{name_prefix}: description too short ({desc_len} chars, minimum {DESC_MIN_LEN})") - elif desc_len > DESC_MAX_LEN: - warnings.append(f"{name_prefix}: description too long ({desc_len} chars, maximum {DESC_MAX_LEN})") - - return warnings - - -def find_service_in_head(category, section, service_name): - """Look up a service in the head YAML by category/section/service name.""" - try: - with open(DATA_PATH, "r") as f: - data = yaml.safe_load(f) - for cat in data.get("categories", []): - if cat.get("name") == category: - for sec in cat.get("sections", []): - if sec.get("name") == section: - for svc in sec.get("services", []): - if svc.get("name") == service_name: - return svc - except Exception: - pass - return None - - -def write_step_summary(all_warnings, services_checked): - """Write a Markdown summary to $GITHUB_STEP_SUMMARY.""" - summary_file = os.environ.get("GITHUB_STEP_SUMMARY") - if not summary_file: - return - - lines = ["## Link Validation\n"] - - if not services_checked: - lines.append("No services to check.\n") - elif not all_warnings: - lines.append(f"All checks passed for {services_checked} service(s).\n") - else: - lines.append(f"Checked {services_checked} service(s), found {len(all_warnings)} warning(s):\n") - lines.append("| Warning |") - lines.append("|---------|") - for w in all_warnings: - escaped = w.replace("|", "\\|") - lines.append(f"| {escaped} |") - lines.append("") - lines.append("> **Note:** Link warnings are informational only and do not fail the check. " - "URLs may be temporarily down or block automated requests.\n") - - with open(summary_file, "a") as f: - f.write("\n".join(lines) + "\n") - - -def main(): - parser = argparse.ArgumentParser(description="Validate links for added/modified services") - parser.add_argument("--diff-json", required=True, help="Path to the diff JSON file") - args = parser.parse_args() - - # Load diff - try: - with open(args.diff_json, "r") as f: - diff = json.load(f) - except (FileNotFoundError, json.JSONDecodeError) as e: - print(red(f"Failed to load diff JSON: {e}"), file=sys.stderr) - sys.exit(EXIT_RUNTIME_ERROR) - - all_warnings = [] - services_checked = 0 - - # Check added services - for svc in diff.get("services", {}).get("added", []): - services_checked += 1 - warnings = check_service( - svc.get("fields", {}), - svc["service"], - svc["category"], - svc["section"], - ) - all_warnings.extend(warnings) - - # Check modified services -- only if they have URL-related field changes - for svc in diff.get("services", {}).get("modified", []): - url_fields = {"url", "icon", "iosApp", "github", "description"} - changed = set(svc.get("changed_fields", [])) - if changed & url_fields: - services_checked += 1 - head_svc = find_service_in_head(svc["category"], svc["section"], svc["service"]) - if head_svc: - warnings = check_service( - head_svc, - svc["service"], - svc["category"], - svc["section"], - ) - all_warnings.extend(warnings) - - # Print results - if all_warnings: - print(yellow(f"Link validation: {len(all_warnings)} warning(s)"), file=sys.stderr) - for w in all_warnings: - print(f" {yellow('WARNING')} {w}", file=sys.stderr) - else: - print(green(f"Link validation passed. {services_checked} service(s) checked.")) - - write_step_summary(all_warnings, services_checked) - sys.exit(EXIT_PASS) - - -if __name__ == "__main__": - main() diff --git a/lib/checks/check-pr-meta.py b/lib/checks/check-pr-meta.py index f63f561..aa1379e 100644 --- a/lib/checks/check-pr-meta.py +++ b/lib/checks/check-pr-meta.py @@ -5,7 +5,7 @@ import os import re import sys -FINDINGS_PATH = "/tmp/findings-meta.json" +FINDINGS_PATH = "/tmp/findings-compliance.json" BAD_TITLES = {"update readme.md", "update awesome-privacy.yml"} @@ -27,6 +27,11 @@ CHECKBOX_MSG = ( " to confirm that you've read the contributing guidelines, checked your submission," " indicated your affiliation and agree to follow our CoC" ) +README_MSG = ( + "Do not edit the README directly. This file is auto-generated from the" + " content in `awesome-privacy.yml`, and so your changes will be overridden!" + " Instead, only modify the YAML file, and be sure to follow our Contributing Guidelines." +) def extract_section(body, header): @@ -78,6 +83,13 @@ def check_checkboxes(body): return None +def check_readme(readme_failed): + """Return a finding if the README check reported a failure.""" + if readme_failed == "true": + return README_MSG + return None + + def write_findings(findings): """Write the findings list to the output JSON file.""" with open(FINDINGS_PATH, "w") as f: @@ -91,6 +103,7 @@ def main(): title = os.environ.get("PR_TITLE", "") body = os.environ.get("PR_BODY", "") draft = os.environ.get("PR_DRAFT", "false") + readme_failed = os.environ.get("README_FAILED", "false") finding = check_title(title) if finding: @@ -111,6 +124,10 @@ def main(): finding = check_checkboxes(body) if finding: findings.append(finding) + + finding = check_readme(readme_failed) + if finding: + findings.append(finding) except Exception: pass diff --git a/lib/checks/check-template.py b/lib/checks/check-template.py deleted file mode 100644 index b53671d..0000000 --- a/lib/checks/check-template.py +++ /dev/null @@ -1,206 +0,0 @@ -""" -Checks PR body against the pull request template. -Reads the PR body from the PR_BODY environment variable (avoids shell injection). -Exits with code 1 for severe violations (empty body, missing required sections). -""" - -import os -import re -import sys - -# Exit codes -EXIT_PASS = 0 -EXIT_FAIL = 1 -EXIT_RUNTIME_ERROR = 2 - -# Warnings that should cause a hard failure -CRITICAL_WARNINGS = { - "PR body is empty or not provided", - "Type section is missing", - "Type section is empty", - "Changes section is missing", - "Changes section is empty", - "Checklist section is missing", - "Checklist section does not contain any checkbox items", -} - -# ANSI color helpers -_use_color = sys.stderr.isatty() and not os.environ.get("NO_COLOR") -red = (lambda s: f"\033[31m{s}\033[0m") if _use_color else (lambda s: s) -green = (lambda s: f"\033[32m{s}\033[0m") if _use_color else (lambda s: s) -yellow = (lambda s: f"\033[33m{s}\033[0m") if _use_color else (lambda s: s) - -# Valid PR types from the template -VALID_TYPES = {"Addition", "Amendment", "Removal", "Spelling or Grammar", "Website Update", "Misc"} - -# Raw template text that indicates an unfilled section -RAW_TYPE_LINE = "Addition / Amendment / Removal / Spelling or Grammar / Website Update / Misc" - - -def strip_html_comments(text): - """Remove comments from text.""" - return re.sub(r"", "", text, flags=re.DOTALL).strip() - - -def extract_section(body, header): - """Extract content between a ### header and the next --- or ### header.""" - pattern = rf"###\s*{re.escape(header)}\s*\n(.*?)(?=\n---|\n###|\Z)" - match = re.search(pattern, body, re.DOTALL) - if match: - return match.group(1) - return None - - -def check_type_section(content): - """Check the Type section. Returns list of warning strings.""" - warnings = [] - - if content is None: - warnings.append("Type section is missing") - return warnings - - cleaned = strip_html_comments(content).strip() - - if not cleaned: - warnings.append("Type section is empty") - return warnings - - # Check for raw template text (unchanged) - if RAW_TYPE_LINE in cleaned: - warnings.append("Type section appears unchanged from the template -- please select one type") - return warnings - - # Check how many valid types are present - found_types = [t for t in VALID_TYPES if t in cleaned] - - if len(found_types) == 0: - warnings.append(f"Type section does not contain a recognized type. Expected one of: {', '.join(sorted(VALID_TYPES))}") - elif len(found_types) > 1: - warnings.append(f"Type section contains multiple types ({', '.join(found_types)}) -- please select only one") - - return warnings - - -def check_text_section(content, section_name): - """Check a text section (Changes, Supporting Material, Affiliation). Returns list of warnings.""" - warnings = [] - - if content is None: - warnings.append(f"{section_name} section is missing") - return warnings - - cleaned = strip_html_comments(content).strip() - - if not cleaned: - warnings.append(f"{section_name} section is empty") - - return warnings - - -def check_checklist(content): - """Check the Checklist section. Returns list of warnings.""" - warnings = [] - - if content is None: - warnings.append("Checklist section is missing") - return warnings - - checked = re.findall(r"- \[x\]", content, re.IGNORECASE) - unchecked = re.findall(r"- \[ \]", content) - - total = len(checked) + len(unchecked) - - if total == 0: - warnings.append("Checklist section does not contain any checkbox items") - return warnings - - if unchecked: - warnings.append(f"Checklist has {len(unchecked)} unchecked item(s) out of {total}") - - return warnings - - -def has_critical_warnings(warnings): - """Return True if any warning is a critical (hard-fail) violation.""" - return any(w in CRITICAL_WARNINGS for w in warnings) - - -def write_step_summary(all_warnings): - """Write a Markdown summary to $GITHUB_STEP_SUMMARY.""" - summary_file = os.environ.get("GITHUB_STEP_SUMMARY") - if not summary_file: - return - - lines = ["## PR Template Check\n"] - - if not all_warnings: - lines.append("All template checks passed.\n") - else: - critical = has_critical_warnings(all_warnings) - lines.append(f"Found {len(all_warnings)} warning(s):\n") - for w in all_warnings: - lines.append(f"- {w}") - lines.append("") - if critical: - lines.append("> **Error:** One or more required sections are missing or empty. " - "Please fill out the PR template before this check can pass.\n") - else: - lines.append("> **Note:** Template warnings are informational and do not fail the check. " - "Reviewers will verify compliance.\n") - - with open(summary_file, "a") as f: - f.write("\n".join(lines) + "\n") - - -def main(): - pr_body = os.environ.get("PR_BODY") - - if pr_body is None or pr_body.strip() == "": - all_warnings = [ - "PR body is empty or not provided", - "Type section is missing", - "Changes section is missing", - "Supporting Material section is missing", - "Affiliation section is missing", - "Checklist section is missing", - ] - print(yellow(f"PR template check: {len(all_warnings)} warning(s)"), file=sys.stderr) - for w in all_warnings: - print(f" {yellow('WARNING')} {w}", file=sys.stderr) - write_step_summary(all_warnings) - sys.exit(EXIT_FAIL) - - all_warnings = [] - - # Check each section - type_content = extract_section(pr_body, "Type") - all_warnings.extend(check_type_section(type_content)) - - changes_content = extract_section(pr_body, "Changes") - all_warnings.extend(check_text_section(changes_content, "Changes")) - - supporting_content = extract_section(pr_body, "Supporting Material") - all_warnings.extend(check_text_section(supporting_content, "Supporting Material")) - - affiliation_content = extract_section(pr_body, "Affiliation") - all_warnings.extend(check_text_section(affiliation_content, "Affiliation")) - - checklist_content = extract_section(pr_body, "Checklist") - all_warnings.extend(check_checklist(checklist_content)) - - # Print results - if all_warnings: - print(yellow(f"PR template check: {len(all_warnings)} warning(s)"), file=sys.stderr) - for w in all_warnings: - print(f" {yellow('WARNING')} {w}", file=sys.stderr) - else: - print(green("PR template check passed.")) - - write_step_summary(all_warnings) - if has_critical_warnings(all_warnings): - sys.exit(EXIT_FAIL) - sys.exit(EXIT_PASS) - - -if __name__ == "__main__": - main() diff --git a/lib/checks/check-yaml-diff.py b/lib/checks/check-yaml-diff.py index 2931e68..44a5285 100644 --- a/lib/checks/check-yaml-diff.py +++ b/lib/checks/check-yaml-diff.py @@ -1,7 +1,5 @@ -""" -Analyzes the diff between base and head versions of awesome-privacy.yml. -Enforces the single-entry rule: only one service addition/amendment/removal per PR. -Outputs a JSON diff to /tmp/pr-diff.json and writes a step summary. +"""Analyzes the diff between base and head versions of awesome-privacy.yml. +Enforces the single-entry rule and outputs a JSON diff to /tmp/pr-diff.json. """ import argparse @@ -12,22 +10,18 @@ import sys import yaml -# Paths (relative to project root) PROJECT_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) DATA_PATH = os.path.join(PROJECT_ROOT, "awesome-privacy.yml") DIFF_OUTPUT_PATH = "/tmp/pr-diff.json" -# Exit codes EXIT_PASS = 0 EXIT_RULE_VIOLATION = 1 EXIT_RUNTIME_ERROR = 2 -# ANSI color helpers _use_color = sys.stderr.isatty() and not os.environ.get("NO_COLOR") red = (lambda s: f"\033[31m{s}\033[0m") if _use_color else (lambda s: s) green = (lambda s: f"\033[32m{s}\033[0m") if _use_color else (lambda s: s) yellow = (lambda s: f"\033[33m{s}\033[0m") if _use_color else (lambda s: s) -dim = (lambda s: f"\033[2m{s}\033[0m") if _use_color else (lambda s: s) def load_base_yaml(base_ref): @@ -35,13 +29,11 @@ def load_base_yaml(base_ref): try: result = subprocess.run( ["git", "show", f"{base_ref}:awesome-privacy.yml"], - capture_output=True, text=True, check=True, - cwd=PROJECT_ROOT, + capture_output=True, text=True, check=True, cwd=PROJECT_ROOT, ) return yaml.safe_load(result.stdout) except subprocess.CalledProcessError: - # File doesn't exist in base (completely new file) - print(yellow("Warning: awesome-privacy.yml not found in base ref, treating as empty"), file=sys.stderr) + print(yellow("awesome-privacy.yml not found in base ref, treating as empty"), file=sys.stderr) return {"categories": []} except yaml.YAMLError as e: print(red(f"Failed to parse base YAML: {e}"), file=sys.stderr) @@ -51,165 +43,45 @@ def load_base_yaml(base_ref): def load_head_yaml(): """Load the YAML from the current working tree.""" try: - with open(DATA_PATH, "r") as f: + with open(DATA_PATH) as f: return yaml.safe_load(f) - except FileNotFoundError: - print(red(f"File not found: {DATA_PATH}"), file=sys.stderr) - sys.exit(EXIT_RUNTIME_ERROR) - except yaml.YAMLError as e: - print(red(f"Failed to parse head YAML: {e}"), file=sys.stderr) + except (FileNotFoundError, yaml.YAMLError) as e: + print(red(f"Failed to load head YAML: {e}"), file=sys.stderr) sys.exit(EXIT_RUNTIME_ERROR) -def build_service_index(data): - """Build a dict keyed by (category, section, service_name) -> service dict.""" +def build_index(data, depth): + """Build a keyed index at the given depth (3=services, 2=sections, 1=categories).""" index = {} for cat in data.get("categories", []): - cat_name = cat.get("name", "") + cn = cat.get("name", "") + if depth == 1: + index[cn] = {k: v for k, v in cat.items() if k != "sections"} + continue for sec in cat.get("sections", []): - sec_name = sec.get("name", "") + sn = sec.get("name", "") + if depth == 2: + index[(cn, sn)] = {k: v for k, v in sec.items() if k != "services"} + continue for svc in sec.get("services", []): - svc_name = svc.get("name", "") - key = (cat_name, sec_name, svc_name) - index[key] = svc + index[(cn, sn, svc.get("name", ""))] = svc return index -def build_section_index(data): - """Build a dict keyed by (category, section) -> section metadata (excluding services).""" - index = {} - for cat in data.get("categories", []): - cat_name = cat.get("name", "") - for sec in cat.get("sections", []): - sec_name = sec.get("name", "") - key = (cat_name, sec_name) - meta = {k: v for k, v in sec.items() if k != "services"} - index[key] = meta - return index - - -def build_category_index(data): - """Build a dict keyed by category_name -> category metadata (excluding sections).""" - index = {} - for cat in data.get("categories", []): - cat_name = cat.get("name", "") - meta = {k: v for k, v in cat.items() if k != "sections"} - index[cat_name] = meta - return index - - -def diff_services(base_data, head_data): - """Find added, removed, and modified services.""" - base_idx = build_service_index(base_data) - head_idx = build_service_index(head_data) - - base_keys = set(base_idx.keys()) - head_keys = set(head_idx.keys()) - - added = [] - for key in sorted(head_keys - base_keys): - added.append({ - "category": key[0], - "section": key[1], - "service": key[2], - "fields": head_idx[key], - }) - - removed = [] - for key in sorted(base_keys - head_keys): - removed.append({ - "category": key[0], - "section": key[1], - "service": key[2], - }) - +def diff_index(base_idx, head_idx): + """Return (added_keys, removed_keys, modified_keys_with_changed_fields).""" + base_keys, head_keys = set(base_idx), set(head_idx) + added = sorted(head_keys - base_keys) + removed = sorted(base_keys - head_keys) modified = [] for key in sorted(base_keys & head_keys): - base_svc = base_idx[key] - head_svc = head_idx[key] - if base_svc != head_svc: - changed_fields = [] - all_fields = set(base_svc.keys()) | set(head_svc.keys()) - for field in sorted(all_fields): - old_val = base_svc.get(field) - new_val = head_svc.get(field) - if old_val != new_val: - changed_fields.append(field) - modified.append({ - "category": key[0], - "section": key[1], - "service": key[2], - "changed_fields": changed_fields, - }) - + if base_idx[key] != head_idx[key]: + all_fields = set(base_idx[key]) | set(head_idx[key]) + changed = sorted(f for f in all_fields if base_idx[key].get(f) != head_idx[key].get(f)) + modified.append((key, changed)) return added, removed, modified -def diff_sections(base_data, head_data): - """Find section-level metadata changes (intro, wordOfWarning, etc.).""" - base_idx = build_section_index(base_data) - head_idx = build_section_index(head_data) - - base_keys = set(base_idx.keys()) - head_keys = set(head_idx.keys()) - - changes = [] - - # New sections - for key in sorted(head_keys - base_keys): - changes.append({ - "category": key[0], - "section": key[1], - "change_type": "added_section", - }) - - # Removed sections - for key in sorted(base_keys - head_keys): - changes.append({ - "category": key[0], - "section": key[1], - "change_type": "removed_section", - }) - - # Modified section metadata - for key in sorted(base_keys & head_keys): - base_meta = base_idx[key] - head_meta = head_idx[key] - if base_meta != head_meta: - changed_fields = [] - all_fields = set(base_meta.keys()) | set(head_meta.keys()) - for field in sorted(all_fields): - if base_meta.get(field) != head_meta.get(field): - changed_fields.append(field) - changes.append({ - "category": key[0], - "section": key[1], - "change_type": "modified_section_metadata", - "changed_fields": changed_fields, - }) - - return changes - - -def diff_categories(base_data, head_data): - """Find structural category changes.""" - base_idx = build_category_index(base_data) - head_idx = build_category_index(head_data) - - base_keys = set(base_idx.keys()) - head_keys = set(head_idx.keys()) - - changes = [] - - for name in sorted(head_keys - base_keys): - changes.append({"category": name, "change_type": "added_category"}) - - for name in sorted(base_keys - head_keys): - changes.append({"category": name, "change_type": "removed_category"}) - - return changes - - def write_github_output(name, value): """Write a value to $GITHUB_OUTPUT.""" output_file = os.environ.get("GITHUB_OUTPUT") @@ -218,123 +90,113 @@ def write_github_output(name, value): f.write(f"{name}={value}\n") +def fmt_path(key): + """Format a tuple key as a readable path.""" + return " → ".join(key) if isinstance(key, tuple) else key + + def write_step_summary(diff_result): - """Write a Markdown summary to $GITHUB_STEP_SUMMARY.""" + """Write a bullet-point Markdown summary to $GITHUB_STEP_SUMMARY.""" summary_file = os.environ.get("GITHUB_STEP_SUMMARY") if not summary_file: return lines = ["## YAML Diff Analysis\n"] + bullets = [] - added = diff_result["services"]["added"] - removed = diff_result["services"]["removed"] - modified = diff_result["services"]["modified"] - section_changes = diff_result["sections"] - category_changes = diff_result["categories"] + for svc in diff_result["services"]["added"]: + bullets.append(f"- Added **{svc['service']}** in {svc['category']} → {svc['section']}") + for svc in diff_result["services"]["removed"]: + bullets.append(f"- Removed **{svc['service']}** from {svc['category']} → {svc['section']}") + for svc in diff_result["services"]["modified"]: + fields = ", ".join(f"`{f}`" for f in svc["changed_fields"]) + bullets.append(f"- Modified {fields} in {svc['category']} → {svc['section']} → {svc['service']}") + for change in diff_result["sections"]: + ct = change["change_type"] + path = f"{change['category']} → {change['section']}" + if ct == "added_section": + bullets.append(f"- Added section **{change['section']}** in {change['category']}") + elif ct == "removed_section": + bullets.append(f"- Removed section **{change['section']}** from {change['category']}") + else: + fields = ", ".join(f"`{f}`" for f in change.get("changed_fields", [])) + bullets.append(f"- Modified section metadata ({fields}) in {path}") + for change in diff_result["categories"]: + if change["change_type"] == "added_category": + bullets.append(f"- Added category **{change['category']}**") + else: + bullets.append(f"- Removed category **{change['category']}**") - if not added and not removed and not modified and not section_changes and not category_changes: - lines.append("No changes detected in `awesome-privacy.yml`.\n") + if bullets: + lines.extend(bullets) else: - lines.append("| Type | Category | Section | Service | Details |") - lines.append("|------|----------|---------|---------|---------|") - - for svc in added: - lines.append(f"| Added | {svc['category']} | {svc['section']} | {svc['service']} | New service |") - - for svc in removed: - lines.append(f"| Removed | {svc['category']} | {svc['section']} | {svc['service']} | Service removed |") - - for svc in modified: - fields = ", ".join(svc["changed_fields"]) - lines.append(f"| Modified | {svc['category']} | {svc['section']} | {svc['service']} | Changed: {fields} |") - - for change in section_changes: - detail = change["change_type"].replace("_", " ").title() - fields = ", ".join(change.get("changed_fields", [])) - if fields: - detail += f" ({fields})" - lines.append(f"| Section | {change['category']} | {change['section']} | - | {detail} |") - - for change in category_changes: - detail = change["change_type"].replace("_", " ").title() - lines.append(f"| Category | {change['category']} | - | - | {detail} |") - - lines.append("") + lines.append("No changes detected in `awesome-privacy.yml`.") with open(summary_file, "a") as f: f.write("\n".join(lines) + "\n") def main(): - parser = argparse.ArgumentParser(description="Analyze YAML diff for PR checks") - parser.add_argument("--base-ref", required=True, help="Base git ref (SHA or branch) to diff against") + parser = argparse.ArgumentParser() + parser.add_argument("--base-ref", required=True) args = parser.parse_args() - # Load both versions - base_data = load_base_yaml(args.base_ref) - head_data = load_head_yaml() + base = load_base_yaml(args.base_ref) + head = load_head_yaml() - # Compute diffs - added, removed, modified = diff_services(base_data, head_data) - section_changes = diff_sections(base_data, head_data) - category_changes = diff_categories(base_data, head_data) + svc_added, svc_removed, svc_modified = diff_index( + build_index(base, 3), build_index(head, 3), + ) + sec_added, sec_removed, sec_modified = diff_index( + build_index(base, 2), build_index(head, 2), + ) + cat_added, cat_removed, _ = diff_index( + build_index(base, 1), build_index(head, 1), + ) + + added = [{"category": k[0], "section": k[1], "service": k[2], + "fields": build_index(head, 3)[k]} for k in svc_added] + removed = [{"category": k[0], "section": k[1], "service": k[2]} for k in svc_removed] + modified = [{"category": k[0], "section": k[1], "service": k[2], + "changed_fields": cf} for k, cf in svc_modified] + + sections = [] + for k in sec_added: + sections.append({"category": k[0], "section": k[1], "change_type": "added_section"}) + for k in sec_removed: + sections.append({"category": k[0], "section": k[1], "change_type": "removed_section"}) + for k, cf in sec_modified: + sections.append({"category": k[0], "section": k[1], + "change_type": "modified_section_metadata", "changed_fields": cf}) + + categories = [] + for k in cat_added: + categories.append({"category": k, "change_type": "added_category"}) + for k in cat_removed: + categories.append({"category": k, "change_type": "removed_category"}) - # Build result diff_result = { - "services": { - "added": added, - "removed": removed, - "modified": modified, - }, - "sections": section_changes, - "categories": category_changes, + "services": {"added": added, "removed": removed, "modified": modified}, + "sections": sections, + "categories": categories, } - # Write diff JSON with open(DIFF_OUTPUT_PATH, "w") as f: json.dump(diff_result, f, indent=2) - print(f"Diff written to {DIFF_OUTPUT_PATH}") - # Determine if there are service-level changes - has_service_changes = bool(added or removed or modified) - write_github_output("has_service_changes", str(has_service_changes).lower()) - - # Write step summary + write_github_output("has_service_changes", str(bool(added or removed or modified)).lower()) write_step_summary(diff_result) - # Enforce single-entry rule - service_change_count = len(added) + len(removed) + len(modified) - - if service_change_count > 1: - print(red("Single-entry rule violation: PRs must contain only one service change."), file=sys.stderr) - print(red(f"Found {service_change_count} service-level changes:"), file=sys.stderr) - for svc in added: - print(f" + Added: {svc['category']} > {svc['section']} > {svc['service']}", file=sys.stderr) - for svc in removed: - print(f" - Removed: {svc['category']} > {svc['section']} > {svc['service']}", file=sys.stderr) - for svc in modified: - fields = ", ".join(svc["changed_fields"]) - print(f" ~ Modified: {svc['category']} > {svc['section']} > {svc['service']} ({fields})", file=sys.stderr) + svc_count = len(added) + len(removed) + len(modified) + if svc_count > 1: + print(red(f"Single-entry rule violation: {svc_count} service changes found."), file=sys.stderr) + sys.exit(EXIT_RULE_VIOLATION) + if svc_count == 0 and len(sections) > 1: + print(red(f"Single-entry rule violation: {len(sections)} section changes found."), file=sys.stderr) sys.exit(EXIT_RULE_VIOLATION) - # If no service changes, check section-level changes - if service_change_count == 0 and len(section_changes) > 1: - print(red("Single-entry rule violation: PRs must contain only one section-level change."), file=sys.stderr) - print(red(f"Found {len(section_changes)} section-level changes:"), file=sys.stderr) - for change in section_changes: - detail = change["change_type"].replace("_", " ") - fields = change.get("changed_fields", []) - extra = f" ({', '.join(fields)})" if fields else "" - print(f" ~ {change['category']} > {change['section']}: {detail}{extra}", file=sys.stderr) - sys.exit(EXIT_RULE_VIOLATION) - - # Summary - total = service_change_count + len(section_changes) + len(category_changes) - if total == 0: - print(green("No changes detected in awesome-privacy.yml")) - else: - print(green(f"Single-entry rule passed. {service_change_count} service change(s), " - f"{len(section_changes)} section change(s), {len(category_changes)} category change(s).")) + print(green(f"Single-entry rule passed. {svc_count} service, " + f"{len(sections)} section, {len(categories)} category change(s).")) sys.exit(EXIT_PASS) diff --git a/lib/checks/detect-changes.py b/lib/checks/detect-changes.py index 9b87e78..9fa591d 100644 --- a/lib/checks/detect-changes.py +++ b/lib/checks/detect-changes.py @@ -1,6 +1,6 @@ """ Detects which files changed between the PR base and HEAD. -Sets GitHub Actions outputs: yaml_changed, non_yaml_changed. +Sets GitHub Actions output: yaml_changed. """ import argparse @@ -38,12 +38,8 @@ def main(): print(f" {f}") yaml_changed = YAML_FILE in changed_files - non_yaml_changed = any(f != YAML_FILE for f in changed_files) - write_github_output("yaml_changed", str(yaml_changed).lower()) - write_github_output("non_yaml_changed", str(non_yaml_changed).lower()) - - print(f"yaml_changed={yaml_changed}, non_yaml_changed={non_yaml_changed}") + print(f"yaml_changed={yaml_changed}") if __name__ == "__main__": diff --git a/lib/checks/format-comment.py b/lib/checks/format-comment.py index e81497a..d2789b1 100644 --- a/lib/checks/format-comment.py +++ b/lib/checks/format-comment.py @@ -7,12 +7,6 @@ import sys ARTIFACTS_DIR = "/tmp/artifacts" OUTPUT_DIR = "/tmp/pr-meta" -README_MSG = ( - "Do not edit the README directly. This file is auto-generated from the" - " content in `awesome-privacy.yml`, and so your changes will be overridden!" - " Instead, only modify the YAML file, and be sure to follow our Contributing Guidelines." -) - CONTRIBUTING = "https://github.com/Lissy93/awesome-privacy/blob/main/.github/CONTRIBUTING.md" COMMENT_TEMPLATE = """ @@ -38,12 +32,10 @@ def load_findings(filename): return [] -def collect_findings(readme_failed): - """Gather all findings in display order: meta, readme, data, project.""" +def collect_findings(): + """Gather all findings in display order: compliance, data, project.""" all_findings = [] - all_findings.extend(load_findings("findings-meta.json")) - if readme_failed: - all_findings.append(README_MSG) + all_findings.extend(load_findings("findings-compliance.json")) all_findings.extend(load_findings("findings-data.json")) all_findings.extend(load_findings("findings-project.json")) return all_findings @@ -78,7 +70,6 @@ def main(): user = os.environ.get("PR_USER", "contributor") pr_number = os.environ.get("PR_NUMBER", "") run_id = os.environ.get("RUN_ID", "") - readme_failed = os.environ.get("README_FAILED", "false") == "true" os.makedirs(OUTPUT_DIR, exist_ok=True) @@ -89,7 +80,7 @@ def main(): with open(os.path.join(OUTPUT_DIR, "run-id.txt"), "w") as f: f.write(run_id) - findings = collect_findings(readme_failed) + findings = collect_findings() write_step_summary(findings) if findings: diff --git a/lib/checks/warn-non-yaml.py b/lib/checks/warn-non-yaml.py deleted file mode 100644 index e244dbc..0000000 --- a/lib/checks/warn-non-yaml.py +++ /dev/null @@ -1,58 +0,0 @@ -""" -Warns when a PR modifies files other than awesome-privacy.yml. -This is expected for Website Update or Misc PRs, but may need extra review. -""" - -import argparse -import os -import subprocess -import sys - -PROJECT_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) - -YAML_FILE = "awesome-privacy.yml" - -# ANSI color helpers -_use_color = sys.stderr.isatty() and not os.environ.get("NO_COLOR") -yellow = (lambda s: f"\033[33m{s}\033[0m") if _use_color else (lambda s: s) - - -def main(): - parser = argparse.ArgumentParser(description="Warn about non-YAML file changes") - parser.add_argument("--base-ref", required=True, help="Base git ref to diff against") - args = parser.parse_args() - - result = subprocess.run( - ["git", "diff", "--name-only", f"{args.base_ref}..HEAD"], - capture_output=True, text=True, check=True, - cwd=PROJECT_ROOT, - ) - - changed_files = [f for f in result.stdout.strip().splitlines() if f] - non_yaml = [f for f in changed_files if f != YAML_FILE] - - if not non_yaml: - return - - print(yellow("This PR modifies files other than awesome-privacy.yml:"), file=sys.stderr) - for f in non_yaml: - print(f" {f}", file=sys.stderr) - - # Write step summary - summary_file = os.environ.get("GITHUB_STEP_SUMMARY") - if summary_file: - lines = [ - "## Non-YAML Changes Warning\n", - "This PR modifies files other than `awesome-privacy.yml`:\n", - ] - for f in non_yaml: - lines.append(f"- `{f}`") - lines.append("") - lines.append("> **Note:** Most PRs should only modify `awesome-privacy.yml`. " - "Non-YAML changes may require additional review.\n") - with open(summary_file, "a") as f: - f.write("\n".join(lines) + "\n") - - -if __name__ == "__main__": - main()