From 78d97a1c309fa31be8bc7603e59432343cd91b4f Mon Sep 17 00:00:00 2001 From: Gleb Chesnokov Date: Mon, 19 Oct 2026 21:59:22 +0300 Subject: [PATCH] .github/workflows: Run Checkpatch and SCST style analysis Add a parallel make checkpatch job for pushes to master and manual runs. Fail analyzer jobs on remaining source findings with status 1; keep tool, build and coverage failures separate with status 2. Validate the current report, Make status and native coverage before classifying results. Checkpatch needs its script and data, not a prepared KUnit kernel. Update the isolated CI report fixtures. Run SCST style fixtures and make scs_lint in a separate job without a kernel build, alongside the native analyzers. Pin uv and its setup action; the linter supplies its pinned parser dependencies. Retain the pull-request Checkpatch workflow for patch and commit checks. Remove the standalone push Checkpatch workflow, whose role is now covered by the common static-analyzer workflow. Rename Tooling to Analyzer script tests and clarify that analyzer jobs include KUnit sources without executing KUnit. Update Checkout and Upload Artifact to v7 to replace their deprecated Node.js 20 runtimes, and align the CI documentation with the actual jobs. --- .github/workflows/analyzers.yml | 38 ++++++++++++++------ .github/workflows/checkpatch_push.yml | 32 ----------------- doc/CI.md | 13 +++---- scripts/README.md | 16 +++++---- scripts/ci/run-analyzer.py | 46 ++++++++++++++++-------- scripts/tests/test_analyzer_ci.py | 52 +++++++++++++++++++++------ 6 files changed, 117 insertions(+), 80 deletions(-) delete mode 100644 .github/workflows/checkpatch_push.yml diff --git a/.github/workflows/analyzers.yml b/.github/workflows/analyzers.yml index 8cec429a5..ddecb0f8e 100644 --- a/.github/workflows/analyzers.yml +++ b/.github/workflows/analyzers.yml @@ -13,20 +13,39 @@ defaults: shell: bash jobs: - tooling: - name: Tooling + script-tests: + name: Analyzer script tests runs-on: ubuntu-24.04 timeout-minutes: 10 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: persist-credentials: false - - name: Check analyzer tooling + - name: Run analyzer script tests run: make lint-test + scs-lint: + name: SCST code style + needs: script-tests + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - uses: astral-sh/setup-uv@1c37ad07a6a961277cf70c0d37d6f313000f5884 # v10.3.0 + with: + version: "0.13.1" + python-version: "3.12" + enable-cache: false + - name: Test SCST style rules + run: make scs-lint-test + - name: Run make scs_lint + run: make scs_lint + native: - name: ${{ matrix.analyzer }} with KUnit - needs: tooling + name: ${{ matrix.analyzer }} (includes KUnit sources) + needs: script-tests runs-on: ubuntu-24.04 timeout-minutes: 45 strategy: @@ -35,11 +54,12 @@ jobs: analyzer: - sparse - smatch + - checkpatch # Supported by the runner; enable after reviewing native coverage. # - coccicheck # - cppcheck steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: persist-credentials: false - name: Build pinned analyzer environment @@ -51,8 +71,6 @@ jobs: ANALYZER: ${{ matrix.analyzer }} run: | source scripts/ci/kernel.env - # Temporary rollout mode: only completed source findings are advisory. - # Tool, build and coverage failures still fail this job. docker run --rm --init --network=none --cap-drop=ALL \ --security-opt=no-new-privileges --user "$(id -u):$(id -g)" \ --env HOME=/tmp --env GITHUB_STEP_SUMMARY=/scst/.cache/analyzer-summary \ @@ -67,7 +85,7 @@ jobs: fi - name: Save complete analyzer evidence if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: ${{ matrix.analyzer }}-evidence retention-days: 7 diff --git a/.github/workflows/checkpatch_push.yml b/.github/workflows/checkpatch_push.yml deleted file mode 100644 index 7fe56f4e7..000000000 --- a/.github/workflows/checkpatch_push.yml +++ /dev/null @@ -1,32 +0,0 @@ -name: Checkpatch upon push - -on: - push: - branches: - - master - -jobs: - checkpatch: - name: ${{matrix.commit.message}} - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - commit: ${{github.event.commits}} - steps: - - name: Checkout code - uses: actions/checkout@main - with: - ref: ${{matrix.commit.id}} - fetch-depth: 2 - - - name: Download checkpatch.pl - run: | - curl -fsSL --retry 5 https://raw.githubusercontent.com/torvalds/linux/master/scripts/checkpatch.pl -o checkpatch.pl - curl -fsSL --retry 5 https://raw.githubusercontent.com/torvalds/linux/master/scripts/spelling.txt -o spelling.txt - curl -fsSL --retry 5 https://raw.githubusercontent.com/torvalds/linux/master/scripts/const_structs.checkpatch -o const_structs.checkpatch - chmod +x checkpatch.pl - - - name: Run checkpatch.pl - run: | - git format-patch -1 --stdout | CHECKPATCH_PATH=. ./scripts/checkpatch diff --git a/doc/CI.md b/doc/CI.md index 464d5b45b..161cc9568 100644 --- a/doc/CI.md +++ b/doc/CI.md @@ -5,11 +5,11 @@ Workflows live in [`.github/workflows`](../.github/workflows). PR checks target | Workflow | Purpose | Events | | --- | --- | --- | -| Checkpatch ([PR](../.github/workflows/checkpatch_pull.yml), [push](../.github/workflows/checkpatch_push.yml)) | Patch and commit style | PR, push | +| [PR Checkpatch](../.github/workflows/checkpatch_pull.yml) | Individual patch and commit style | PR | | [CI](../.github/workflows/ci.yml) | Clang build and DEB packaging | PR, push, manual | | [Packages](../.github/workflows/rpm.yml) | Ubuntu DEBs, RPMs and a published package snapshot | Push to master only | | [Regression](../.github/workflows/run_regression_tests.yaml) | Module and patched-kernel builds across the kernel matrix | PR, push | -| [Static analyzers](../.github/workflows/analyzers.yml) | Sparse/Smatch with KUnit sources, tooling checks and full reports | Push to master, manual | +| [Static analyzers](../.github/workflows/analyzers.yml) | Sparse, Smatch, whole-file Checkpatch and SCST code style, including KUnit sources; script tests | Push to master, manual | | [KUnit](../.github/workflows/kunit.yml) | Kernel unit tests in QEMU | PR, push, manual | | [Ginkgo](../.github/workflows/ginkgo.yml) | Local and TCP iSCSI component tests in QEMU | PR, push, manual | | [Coverity](../.github/workflows/coverity.yml) | Static analysis | Push, upstream repository only | @@ -21,7 +21,7 @@ Edit that config to change path rules; it is validated before selection. Unknown paths and changes to the config or selector run every check. Known unrelated changes skip expensive work; regression selection happens before creating its matrix jobs. Some documentation is a package or kernel-patch -input and still needs validation. Checkpatch always runs. Packages runs only +input and still needs validation. PR Checkpatch always runs. Packages runs only after a push to master; the separate Clang build still checks DEB packaging on PRs. Push and manual runs retain their full checks. Selection errors fail the workflow; skip reasons appear in the job summary. @@ -53,9 +53,10 @@ kernel diagnostics and incomplete reports fail the run. Package artifacts are attached to the Packages workflow; upstream master builds also update the `master-rpm` prerelease snapshot. -Analyzer jobs retain full reports in CI artifacts. See the -[analyzer guide](../scripts/README.md#analyzer-ci) for reproduction and the -temporary nonblocking policy for source findings. +Analyzer jobs fail on remaining findings and print them in job logs; container +jobs also retain reports as artifacts. The +[analyzer guide](../scripts/README.md#analyzer-ci) covers reproduction and +the distinction between findings and incomplete analysis. ## Local reproduction diff --git a/scripts/README.md b/scripts/README.md index ceeb084c6..e71e1cdd6 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -271,15 +271,17 @@ returns 2 (Make maps both to 2). Empty input is a skip. ## Analyzer CI The [workflow](../.github/workflows/analyzers.yml) runs on pushes to `master` -and manual dispatch. Separate jobs run `make sparse` and `make smatch` with -`KUNIT_TEST=y`. Reports in `.cache/lint/` are kept for seven days. Use the -commands above to run either analyzer locally. The runner and image also +and manual dispatch. Separate jobs run `make sparse`, `make smatch` and +`make checkpatch`, including KUnit sources. `Analyzer script tests` runs +`make lint-test`; it does not execute SCST tests. A parallel `SCST code style` +job runs `make scs_lint` without a kernel build. The container jobs retain +reports from `.cache/lint/` for seven days. The runner and image also support Coccinelle and Cppcheck; their matrix entries remain commented out. -CI temporarily reports completed source findings without blocking while fixes -land. Build/tool failures, incomplete analysis and missing KUnit coverage -still fail. This policy lives in `ci/run-analyzer.py`; local Make targets -always fail on findings. +CI fails on findings remaining after policy exceptions, just like local Make +targets. The native-analyzer runner returns 1 for completed analysis with +findings and 2 for build/tool failures, incomplete analysis or missing KUnit +coverage. PRs retain the separate patch/commit Checkpatch workflow. ## Dependencies and tooling tests diff --git a/scripts/ci/run-analyzer.py b/scripts/ci/run-analyzer.py index db325762c..0363481f5 100644 --- a/scripts/ci/run-analyzer.py +++ b/scripts/ci/run-analyzer.py @@ -17,12 +17,23 @@ ROOT = Path(__file__).resolve().parents[2] sys.path.insert(0, str(ROOT / "scripts")) from analyzer_policy import source_policy # noqa: E402 from lint_common import copy_output, entrypoint, run # noqa: E402 +from style.checkpatch import checker_path # noqa: E402 -TOOLS = {"sparse": "sparse", "smatch": "smatch", "coccicheck": "spatch", "cppcheck": "cppcheck"} +TOOLS = {"sparse": "sparse", "smatch": "smatch", "coccicheck": "spatch", "cppcheck": "cppcheck", + "checkpatch": "checkpatch.pl"} def complete_native_report(report): tool = report.get("tool") + if tool == "checkpatch": + checked = report.get("checked_files") + completed = report.get("completed_files") + return (report.get("complete") is True and not report.get("failures") and + report.get("native_status") == report.get("exit_code") and + isinstance(checked, list) and bool(checked) and + checked == report.get("selected_files") and + isinstance(completed, list) and + sorted(str((ROOT / path).resolve()) for path in checked) == sorted(completed)) if tool in {"sparse", "smatch"}: return (bool(report.get("translation_units")) and report.get("make_status") == 0 and report.get("errors") == 0 and not report.get("unchecked_files") and @@ -50,7 +61,7 @@ def digest(path): def job_status(make_status, report): - """Only a completed scan with source findings may become advisory.""" + """Return 0 for clean scans, 1 for findings and 2 for incomplete analysis.""" status = report.get("exit_code") if (report.get("tool") not in TOOLS or type(status) is not int or status not in (0, 1) or @@ -73,8 +84,7 @@ def job_status(make_status, report): "plus userspace where selected by policy.", file=sys.stderr) return 2 - # Temporary rollout policy: completed source findings are advisory. - return 0 + return status def main(): @@ -82,12 +92,14 @@ def main(): parser.add_argument("tool", choices=tuple(TOOLS)) args = parser.parse_args() kernel = Path(os.environ.get("SCST_LINT_KDIR") or os.environ.get("KDIR", "")) - if not (kernel / "Module.symvers").is_file(): - raise ValueError("Set KDIR to a prepared kernel build tree with Module.symvers") - config = (kernel / ".config").read_text() - if not any(line in {"CONFIG_KUNIT=y", "CONFIG_KUNIT=m"} for line in config.splitlines()): - raise ValueError("The CI analysis kernel must enable CONFIG_KUNIT") - executable = shutil.which(TOOLS[args.tool]) + config_path = kernel / ".config" + config = config_path.read_text() if config_path.is_file() else "" + if args.tool != "checkpatch": + if not (kernel / "Module.symvers").is_file(): + raise ValueError("Set KDIR to a prepared kernel build tree with Module.symvers") + if not any(line in {"CONFIG_KUNIT=y", "CONFIG_KUNIT=m"} for line in config.splitlines()): + raise ValueError("The CI analysis kernel must enable CONFIG_KUNIT") + executable = str(checker_path()) if args.tool == "checkpatch" else shutil.which(TOOLS[args.tool]) if not executable: raise ValueError(f"{args.tool} is not installed") directory = ROOT / ".cache/lint" @@ -95,12 +107,18 @@ def main(): provenance = Path("/usr/local/share/scst-ci") / (args.tool + "-source.txt") environment = { "commit": subprocess.check_output(["git", "rev-parse", "HEAD"], cwd=ROOT, text=True).strip(), - "kernel": str(kernel.resolve()), "kernel_config_sha256": digest(kernel / ".config"), + "kernel": str(kernel.resolve()), + "kernel_config_sha256": digest(config_path) if config else None, "tool": executable, "tool_sha256": digest(Path(executable)), "version": subprocess.check_output([executable, "--version"], text=True).strip(), "source_provenance": provenance.read_text() if provenance.exists() else "unavailable", "kunit_test": "y", } + if args.tool == "checkpatch": + environment["version"] = next((line for line in environment["version"].splitlines() + if line.startswith("Version:")), environment["version"]) + environment["data_sha256"] = {name: digest(Path(executable).with_name(name)) + for name in ("spelling.txt", "const_structs.checkpatch")} environment_text = json.dumps(environment, indent=2) + "\n" (directory / f"{args.tool}.environment.json").write_text(environment_text) print(environment_text, flush=True) @@ -135,9 +153,9 @@ def main(): print(f"\nFull report: {log}", flush=True) copy_output(log, sys.stdout.buffer) status = job_status(result.returncode, report) - if status == 0 and report["exit_code"] == 1: - print(f"::warning::{args.tool}: {report['remaining']} source finding(s); " - "temporary report mode, analysis completed.") + if status == 1: + print(f"::error::{args.tool}: {report['remaining']} source finding(s); " + "analysis completed.") summary = (f"{args.tool}: outcome={report.get('outcome', 'missing')}, " f"remaining={report.get('remaining', 'unknown')}, " f"make exit={result.returncode}, job exit={status}\n") diff --git a/scripts/tests/test_analyzer_ci.py b/scripts/tests/test_analyzer_ci.py index 273e8a712..a36907c94 100644 --- a/scripts/tests/test_analyzer_ci.py +++ b/scripts/tests/test_analyzer_ci.py @@ -25,8 +25,8 @@ class AnalyzerCiTests(unittest.TestCase): # Exercise real GNU Make's mapping of recipe failures to status 2. # The fixture driver below produces reports without compiling SCST. (self.root / "Makefile").write_text( - "SHELL := /bin/sh\n.PHONY: sparse smatch coccicheck cppcheck\n" - "sparse smatch coccicheck cppcheck:\n" + "SHELL := /bin/sh\n.PHONY: sparse smatch coccicheck cppcheck checkpatch\n" + "sparse smatch coccicheck cppcheck checkpatch:\n" f"\t@{shlex.quote(sys.executable)} -B scripts/sparse_smatch/smp.py $@\n" "\t@exit $(FIXTURE_MAKE_TAIL_EXIT)\n") for name in ("git", "sparse", "smatch", "spatch", "cppcheck"): @@ -37,11 +37,18 @@ class AnalyzerCiTests(unittest.TestCase): kernel.mkdir() (kernel / "Module.symvers").touch() (kernel / ".config").write_text("CONFIG_KUNIT=y\n") + checker = kernel / "scripts/checkpatch.pl" + checker.parent.mkdir() + checker.write_text(f"#!{sys.executable}\nprint('Version: fixture-version')\n") + checker.chmod(0o755) + for name in ("spelling.txt", "const_structs.checkpatch"): + checker.with_name(name).touch() self.env = dict(os.environ, KDIR=str(kernel), SCST_LINT_KDIR=str(kernel), PATH=str(self.bin), MAKEFLAGS="", MFLAGS="", MAKEOVERRIDES="", GNUMAKEFLAGS="", FIXTURE_STATUS="0", FIXTURE_MAKE_TAIL_EXIT="0", FIXTURE_OUTCOME="clean", FIXTURE_COVERAGE="full") self.env.pop("GITHUB_STEP_SUMMARY", None) + self.env.pop("CHECKPATCH_PATH", None) driver = self.root / "scripts/sparse_smatch/smp.py" driver.write_text('''import json, os, sys from pathlib import Path @@ -59,7 +66,7 @@ if os.environ.get('FIXTURE_NO_REPORT'): checked = [p + '/fixture.c' for p in ('scst', 'iscsi-scst', 'qla2x00t-32gbit/qla2x00-target', 'srpt', 'fcst', 'scst_local')] checked += ['scst/src/kunit/fixture.c'] -if tool in ('coccicheck', 'cppcheck'): +if tool in ('coccicheck', 'cppcheck', 'checkpatch'): checked += ['usr/fileio/fixture.c', 'iscsi-scst/usr/fixture.c'] if os.environ['FIXTURE_COVERAGE'] != 'full': checked = checked[:1] @@ -72,9 +79,14 @@ report['rules'] = [dict(complete=True, native_status=0, failures=[], handled=che report['kernel_make_status'] = report['userspace_make_status'] = 0 report['profiles'] = [dict(profile=profile, complete=True, native_status=0, failures=[], translation_units=1, checked_files=checked) for profile in ('kernel', 'userspace')] +report.update(complete=True, native_status=status, failures=[], selected_files=checked, + completed_files=[str((root / path).resolve()) for path in checked]) if os.environ.get('FIXTURE_NATIVE_INCOMPLETE'): report['rules'][0]['complete'] = False report['profiles'][0]['complete'] = False + report['complete'] = False +if os.environ.get('FIXTURE_MISSING_COMPLETION'): + report['completed_files'].pop() if os.environ.get('FIXTURE_STALE'): report['run_id'] = 'previous run' (directory / (tool + '.json')).write_text(json.dumps(report)) @@ -88,19 +100,20 @@ sys.exit(status) tool], cwd=self.root, env=self.env, capture_output=True, text=True, timeout=10) - def test_only_completed_findings_can_be_advisory(self): - for tool in ("sparse", "smatch", "coccicheck", "cppcheck"): + def test_findings_fail_separately_from_incomplete_analysis(self): + for tool in ("sparse", "smatch", "coccicheck", "cppcheck", "checkpatch"): for status, outcome, expected in ( - (0, "clean", 0), (1, "findings", 0), (2, "incomplete", 2), + (0, "clean", 0), (1, "findings", 1), (2, "incomplete", 2), (1, "incomplete", 2), (0, "skipped", 2), (0, "findings", 2)): with self.subTest(tool=tool, status=status, outcome=outcome): self.env.update(FIXTURE_STATUS=str(status), FIXTURE_OUTCOME=outcome) result = self.run_ci(tool) self.assertEqual(result.returncode, expected, result.stdout + result.stderr) self.assertEqual(result.stdout.count("raw diagnostic fixture"), 1) - self.assertEqual("::warning::" in result.stdout, - outcome == "findings" and expected == 0) + self.assertEqual("::error::" in result.stdout, expected == 1) + self.assertNotIn("::warning::", result.stdout) self.assertIn(f"make exit={2 if status else 0}", result.stdout) + self.assertIn(f"job exit={expected}", result.stdout) def test_make_failure_after_clean_analysis_cannot_pass(self): self.env["FIXTURE_MAKE_TAIL_EXIT"] = "7" @@ -108,13 +121,30 @@ sys.exit(status) self.assertEqual(result.returncode, 2, result.stdout + result.stderr) self.assertIn("make exit=2, job exit=2", result.stdout) - def test_basic_analyzers_cannot_downgrade_incomplete_native_reports(self): + def test_basic_analyzers_cannot_report_incomplete_scans_as_findings(self): self.env.update(FIXTURE_STATUS="1", FIXTURE_OUTCOME="findings", FIXTURE_NATIVE_INCOMPLETE="1") - for tool in ("coccicheck", "cppcheck"): + for tool in ("coccicheck", "cppcheck", "checkpatch"): with self.subTest(tool=tool): result = self.run_ci(tool) self.assertEqual(result.returncode, 2, result.stdout + result.stderr) - self.assertNotIn("::warning::", result.stdout) + self.assertNotIn("::error::", result.stdout) + + def test_checkpatch_requires_every_native_file_completion(self): + self.env.update(FIXTURE_STATUS="1", FIXTURE_OUTCOME="findings", + FIXTURE_MISSING_COMPLETION="1") + result = self.run_ci("checkpatch") + self.assertEqual(result.returncode, 2, result.stdout + result.stderr) + self.assertNotIn("::error::", result.stdout) + + def test_checkpatch_needs_tool_data_but_not_a_prepared_kunit_kernel(self): + (self.root / "kernel/.config").unlink() + (self.root / "kernel/Module.symvers").unlink() + result = self.run_ci("checkpatch") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + (self.root / "kernel/scripts/spelling.txt").unlink() + result = self.run_ci("checkpatch") + self.assertEqual(result.returncode, 2, result.stdout + result.stderr) + self.assertIn("missing Checkpatch tool/data", result.stderr) def test_make_failure_without_a_new_report_cannot_reuse_previous_results(self): self.assertEqual(self.run_ci().returncode, 0)