From 7f967ca5667a4f37f88e9c8d09f9ee4d1201583e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=B6nke?= Date: Wed, 27 May 2026 07:36:50 +0000 Subject: [PATCH 1/2] ci: fail XML lint job on errors, port helper to Python The merged workflow reported success even when phases failed; add a Verdict step so the job goes red. Replace scripts/lint-xml.sh with scripts/lint_xml.py. --- .github/workflows/quality-assurance.yml | 31 ++-- scripts/lint-xml.sh | 99 ---------- scripts/lint_xml.py | 230 ++++++++++++++++++++++++ 3 files changed, 246 insertions(+), 114 deletions(-) delete mode 100755 scripts/lint-xml.sh create mode 100755 scripts/lint_xml.py diff --git a/.github/workflows/quality-assurance.yml b/.github/workflows/quality-assurance.yml index eadeadf..ee7e893 100644 --- a/.github/workflows/quality-assurance.yml +++ b/.github/workflows/quality-assurance.yml @@ -29,29 +29,30 @@ jobs: - name: Well-formedness id: wellformed continue-on-error: true - run: bash scripts/lint-xml.sh well-formed + run: python scripts/lint_xml.py well-formed - name: XSD schema validation id: schema continue-on-error: true - run: bash scripts/lint-xml.sh schema + run: python scripts/lint_xml.py schema - name: zpretty formatting id: format continue-on-error: true - run: bash scripts/lint-xml.sh format + run: python scripts/lint_xml.py format - - name: Summary + - name: Verdict if: always() + shell: bash run: | - { - echo "## XML Lint" - echo "" - echo "| Phase | Files | Failed | Outcome |" - echo "| --- | ---: | ---: | --- |" - echo "| Well-formedness | ${{ steps.wellformed.outputs.total }} | ${{ steps.wellformed.outputs.failed }} | ${{ steps.wellformed.outcome }} |" - echo "| Schema validation | ${{ steps.schema.outputs.total }} | ${{ steps.schema.outputs.failed }} | ${{ steps.schema.outcome }} |" - echo "| zpretty formatting | ${{ steps.format.outputs.total }} | ${{ steps.format.outputs.failed }} | ${{ steps.format.outcome }} |" - echo "" - echo "_Pre-existing failures are recorded but do not fail the workflow. They will be reduced incrementally._" - } >> "$GITHUB_STEP_SUMMARY" + fail=0 + for outcome in \ + "${{ steps.wellformed.outcome }}" \ + "${{ steps.schema.outcome }}" \ + "${{ steps.format.outcome }}"; do + [[ "$outcome" != "success" ]] && fail=1 + done + if (( fail )); then + echo "::error::One or more XML lint phases reported failures. See the job summary." + exit 1 + fi diff --git a/scripts/lint-xml.sh b/scripts/lint-xml.sh deleted file mode 100755 index 926fa5a..0000000 --- a/scripts/lint-xml.sh +++ /dev/null @@ -1,99 +0,0 @@ -#!/usr/bin/env bash -# XML quality checks for PenText. -# well-formed - xmllint --noout per file -# schema - XSD validation of files declaring xsi:noNamespaceSchemaLocation -# format - zpretty --check on every *.xml file -# No arg or `all` runs all three; exit non-zero if any phase has failures. -# -# When GITHUB_OUTPUT is set (inside a GitHub Actions step), each phase -# writes `total=` and `failed=` to it so the workflow summary can report -# counts. -set -uo pipefail - -phase="${1:-all}" - -mapfile -t xml_files < <( - find . -type f -name "*.xml" \ - -not -path "./target/*" \ - -not -path "./.git/*" \ - -not -path "./.github/*" \ - | sort -) - -emit_summary() { - local name="$1" total="$2" failed="$3" - echo "SUMMARY phase=$name total=$total failed=$failed" - if [[ -n "${GITHUB_OUTPUT:-}" ]]; then - { - echo "total=$total" - echo "failed=$failed" - } >> "$GITHUB_OUTPUT" - fi -} - -run_wellformed() { - echo "::group::Well-formedness (${#xml_files[@]} files)" - local failed=0 - for f in "${xml_files[@]}"; do - if ! xmllint --noout "$f"; then - failed=$((failed + 1)) - fi - done - echo "::endgroup::" - emit_summary well-formed "${#xml_files[@]}" "$failed" - [[ $failed -eq 0 ]] -} - -run_schema() { - echo "::group::XSD schema validation" - local fails=0 checked=0 - local dtd_dir - dtd_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/dtd" - for f in "${xml_files[@]}"; do - local xsd_rel xsd_name xsd_abs - xsd_rel=$(grep -m1 -oE 'xsi:noNamespaceSchemaLocation="[^"]+"' "$f" \ - | sed -E 's/.*="([^"]+)"/\1/' || true) - [[ -z "$xsd_rel" ]] && continue - xsd_name="$(basename "$xsd_rel")" - xsd_abs="$dtd_dir/$xsd_name" - if [[ ! -f "$xsd_abs" ]]; then - echo "WARN: schema not found in dtd/ for $f -> $xsd_name" >&2 - fails=$((fails + 1)) - continue - fi - checked=$((checked + 1)) - echo "-> $f (schema: $xsd_name)" - if ! xmllint --noout --xinclude --schema "$xsd_abs" "$f"; then - fails=$((fails + 1)) - fi - done - echo "::endgroup::" - emit_summary schema "$checked" "$fails" - [[ $fails -eq 0 ]] -} - -run_format() { - echo "::group::zpretty formatting (${#xml_files[@]} files)" - local out - out=$(zpretty --check "${xml_files[@]}" 2>&1 || true) - echo "$out" - local failed - failed=$(printf '%s\n' "$out" | grep -c "^This file would be rewritten:" || true) - echo "::endgroup::" - emit_summary format "${#xml_files[@]}" "$failed" - [[ $failed -eq 0 ]] -} - -case "$phase" in - well-formed) run_wellformed ;; - schema) run_schema ;; - format) run_format ;; - all) - rc=0 - run_wellformed || rc=1 - run_schema || rc=1 - run_format || rc=1 - exit $rc - ;; - *) echo "usage: $0 {well-formed|schema|format|all}" >&2; exit 2 ;; -esac diff --git a/scripts/lint_xml.py b/scripts/lint_xml.py new file mode 100755 index 0000000..ac2a10b --- /dev/null +++ b/scripts/lint_xml.py @@ -0,0 +1,230 @@ +#!/usr/bin/env python3 +"""XML quality checks for PenText. + +Phases: + well-formed - xmllint --noout per *.xml file + schema - XSD validation of files declaring + xsi:noNamespaceSchemaLocation, looked up by basename + in dtd/, run with xmllint --xinclude + format - zpretty --check on every *.xml file + +When GITHUB_OUTPUT / GITHUB_STEP_SUMMARY are set (running inside a +GitHub Actions step), the script appends machine-readable counts and +a markdown section per phase. The exit code reflects whether the +selected phase(s) passed. +""" + +from __future__ import annotations + +import argparse +import os +import re +import subprocess +import sys +from dataclasses import dataclass, field +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +DTD_DIR = REPO_ROOT / "dtd" +EXCLUDE_PARTS = {"target", ".git", ".github"} +SCHEMA_RE = re.compile(r'xsi:noNamespaceSchemaLocation="([^"]+)"') + +PHASE_TITLES = { + "well-formed": "Well-formedness", + "schema": "XSD schema validation", + "format": "zpretty formatting", +} + + +@dataclass +class Result: + phase: str + total: int = 0 + failures: list[str] = field(default_factory=list) + + @property + def failed(self) -> int: + return len(self.failures) + + @property + def ok(self) -> bool: + return self.failed == 0 + + +def find_xml_files(root: Path = REPO_ROOT) -> list[Path]: + files: list[Path] = [] + for p in sorted(root.rglob("*.xml")): + parts = p.relative_to(root).parts + if any(part in EXCLUDE_PARTS for part in parts): + continue + files.append(p) + return files + + +def rel(p: Path) -> str: + return str(p.relative_to(REPO_ROOT)) + + +def run_wellformed(files: list[Path]) -> Result: + res = Result("well-formed", total=len(files)) + for f in files: + proc = subprocess.run( + ["xmllint", "--noout", str(f)], + capture_output=True, + text=True, + ) + if proc.returncode != 0: + err = (proc.stderr or proc.stdout).strip() + print(err) + first = err.splitlines()[0] if err else "parse error" + res.failures.append(f"{rel(f)}: {first}") + return res + + +def read_schema_ref(xml_file: Path) -> str | None: + try: + head = xml_file.read_text(encoding="utf-8", errors="replace")[:4096] + except OSError: + return None + m = SCHEMA_RE.search(head) + return Path(m.group(1)).name if m else None + + +def run_schema(files: list[Path]) -> Result: + res = Result("schema") + for f in files: + xsd_name = read_schema_ref(f) + if not xsd_name: + continue + xsd_path = DTD_DIR / xsd_name + relpath = rel(f) + if not xsd_path.exists(): + msg = f"{relpath}: schema not found in dtd/ ({xsd_name})" + print(f"WARN: {msg}") + res.failures.append(msg) + continue + res.total += 1 + print(f"-> {relpath} (schema: {xsd_name})") + proc = subprocess.run( + [ + "xmllint", + "--noout", + "--xinclude", + "--schema", + str(xsd_path), + str(f), + ], + capture_output=True, + text=True, + ) + out = (proc.stderr + proc.stdout).strip() + if out: + print(out) + if proc.returncode != 0: + res.failures.append(f"{relpath}: validation failed") + return res + + +def run_format(files: list[Path]) -> Result: + res = Result("format", total=len(files)) + proc = subprocess.run( + ["zpretty", "--check", *[str(f) for f in files]], + capture_output=True, + text=True, + ) + output = proc.stdout + proc.stderr + if output: + sys.stdout.write(output if output.endswith("\n") else output + "\n") + prefix = "This file would be rewritten:" + for line in output.splitlines(): + if line.startswith(prefix): + path = line[len(prefix):].strip() + try: + path = rel(Path(path).resolve()) + except ValueError: + pass + res.failures.append(path) + return res + + +PHASES = { + "well-formed": run_wellformed, + "schema": run_schema, + "format": run_format, +} + + +def emit_output(result: Result) -> None: + path = os.environ.get("GITHUB_OUTPUT") + if not path: + return + with open(path, "a", encoding="utf-8") as fh: + fh.write(f"total={result.total}\n") + fh.write(f"failed={result.failed}\n") + + +def append_summary(result: Result) -> None: + path = os.environ.get("GITHUB_STEP_SUMMARY") + if not path: + return + title = PHASE_TITLES.get(result.phase, result.phase) + status = "passed" if result.ok else "failed" + lines = [ + f"### {title}: {status}", + "", + f"- Files checked: **{result.total}**", + f"- Failures: **{result.failed}**", + ] + if result.failures: + lines += [ + "", + f"
{result.failed} failing item(s)", + "", + "```", + *result.failures, + "```", + "", + "
", + ] + lines.append("") + with open(path, "a", encoding="utf-8") as fh: + fh.write("\n".join(lines) + "\n") + + +def run_phase(phase: str, files: list[Path]) -> int: + title = PHASE_TITLES[phase] + print(f"::group::{title}") + result = PHASES[phase](files) + print("::endgroup::") + print( + f"SUMMARY phase={result.phase} " + f"total={result.total} failed={result.failed}" + ) + emit_output(result) + append_summary(result) + return 0 if result.ok else 1 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "phase", + choices=[*PHASES, "all"], + nargs="?", + default="all", + ) + args = parser.parse_args() + + files = find_xml_files() + + if args.phase == "all": + rc = 0 + for name in PHASES: + rc |= run_phase(name, files) + return rc + + return run_phase(args.phase, files) + + +if __name__ == "__main__": + sys.exit(main()) From 60f521be5349f40534765f15f2984797384af086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=B6nke?= Date: Wed, 10 Jun 2026 12:59:34 +0000 Subject: [PATCH 2/2] ci: use the zpretty action (ROS build) for the format phase Replaces `pip install zpretty` with gronke/zpretty's composite action. --- .github/workflows/quality-assurance.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/quality-assurance.yml b/.github/workflows/quality-assurance.yml index ee7e893..744f596 100644 --- a/.github/workflows/quality-assurance.yml +++ b/.github/workflows/quality-assurance.yml @@ -15,16 +15,15 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Set up Python - uses: actions/setup-python@v6 + - name: Set up zpretty + uses: gronke/zpretty/.github/actions/zpretty@ros with: - python-version: "3.x" + spec: "git+https://github.com/gronke/zpretty@ros" - - name: Install xmllint and zpretty + - name: Install xmllint run: | sudo apt-get update sudo apt-get install -y --no-install-recommends libxml2-utils - pip install --no-cache-dir zpretty - name: Well-formedness id: wellformed