mirror of
https://github.com/SCST-project/scst.git
synced 2026-10-22 06:02:33 +00:00
scripts: Add whole-file Checkpatch analysis
Expose make checkpatch through the common C source/header policy, including userspace and KUnit sources. DIFFBR selects whole changed files. Use the kernel's Checkpatch and companion data with --strict, without adding diagnostic exceptions or loading local configuration. Retain raw output and require native totals and a completion record for every selected file. Distinguish source findings from unreadable inputs, inconsistent reports and tool failures, with isolated fixtures. Keep native caches in a temporary directory and serialize only this analyzer's reports so it can run alongside the Kbuild analyzers. Make checkpatch_scan use this target's implementation and exit status instead of suppressing errors. Preserve the separate existing policy of patch/commit helpers and the kernel regression runner.
This commit is contained in:
Gleb Chesnokov
@@ -171,6 +171,7 @@ help:
|
||||
' sparse, smatch analyze configured modules through Kbuild' \
|
||||
' coccicheck run kernel Coccinelle report rules' \
|
||||
' cppcheck analyze fresh kernel and userspace build contexts' \
|
||||
' checkpatch check C source/header style, including KUnit' \
|
||||
' DIFFBR=commit compare with one commit; select relevant inputs' \
|
||||
' lint-test test shared analyzer tooling (no modules)' \
|
||||
' all make all' \
|
||||
@@ -280,6 +281,11 @@ coccicheck: export SCST_LINT_KDIR := $(KDIR)
|
||||
coccicheck:
|
||||
+@$(PYTHON) -B scripts/coccinelle/coccicheck.py
|
||||
|
||||
.PHONY: checkpatch
|
||||
checkpatch: export SCST_LINT_KDIR := $(KDIR)
|
||||
checkpatch:
|
||||
@$(PYTHON) -B scripts/style/checkpatch.py
|
||||
|
||||
.PHONY: cppcheck lint-userspace
|
||||
cppcheck: export SCST_LINT_KDIR := $(KDIR)
|
||||
cppcheck: export SCST_LINT_CC := $(CC)
|
||||
|
||||
+26
-6
@@ -15,13 +15,13 @@ direct callers of a relocated script must use its new directory below.
|
||||
| `sparse_smatch/` | Native Kbuild analyzers (`make sparse`, `make smatch`) and CHECK records. |
|
||||
| `coccinelle/` | Kernel semantic-patch report rules (`make coccicheck`). |
|
||||
| `cppcheck/` | Native analysis using captured compiler commands (`make cppcheck`). |
|
||||
| `style/` | Whole-file Checkpatch analysis (`make checkpatch`, `scripts/checkpatch_scan`). |
|
||||
| `tests/` | Isolated regression fixtures for the shared analyzer helpers (`make lint-test`). |
|
||||
|
||||
The existing `checkpatch`, `checkpatch_commits`, `checkpatch_diff`, and
|
||||
`checkpatch_scan` entry points remain directly under `scripts/`. The wrapper's
|
||||
diagnostic ignore list is in `checkpatch`; whole-tree scan exclusions are in
|
||||
`checkpatch_scan`. The kernel regression runner retains its separate checkpatch
|
||||
command and filtering; Sparse and Smatch use the dedicated Make targets below.
|
||||
The `checkpatch`, `checkpatch_commits` and `checkpatch_diff` helpers retain
|
||||
their existing patch/commit policy, including the ignored types in `checkpatch`.
|
||||
The kernel regression runner also retains its separate checkpatch filtering.
|
||||
`checkpatch_scan` now delegates to the unfiltered whole-file analyzer below.
|
||||
|
||||
Use Make targets from the repository root unless a component workflow says
|
||||
otherwise. Direct scripts retain their working-directory conventions:
|
||||
@@ -80,7 +80,8 @@ python3 -B scripts/analyzer_policy.py validate
|
||||
```
|
||||
|
||||
The Python commands also accept `--format=json` and a policy name: `common`,
|
||||
`sparse`, `smatch`, `coccicheck` or `cppcheck`. Use `make analyzer-policy ANALYZER=smatch` to see the
|
||||
`sparse`, `smatch`, `coccicheck`, `cppcheck` or `checkpatch`.
|
||||
Use `make analyzer-policy ANALYZER=smatch` to see the
|
||||
effective input scope and all diagnostic exceptions. A file inventory does
|
||||
not establish analysis or build coverage.
|
||||
|
||||
@@ -232,6 +233,25 @@ analysis. Source findings return 1 and incomplete analysis returns 2; Make
|
||||
maps both to 2. Full logs and the summary are in `.cache/lint/cppcheck.{log,json}`;
|
||||
the summary links to captured commands, native XML and checker coverage.
|
||||
|
||||
## Checkpatch
|
||||
|
||||
```sh
|
||||
make checkpatch KDIR=/path/to/kernel/build
|
||||
make checkpatch DIFFBR=master CHECKPATCH_PATH=/path/to/kernel/scripts
|
||||
```
|
||||
|
||||
The target checks whole C files and headers selected by the common policy,
|
||||
including userspace and KUnit. Diff mode selects whole changed files. It needs
|
||||
the kernel's `checkpatch.pl`, `spelling.txt` and `const_structs.checkpatch`,
|
||||
but no compilation or KUnit-enabled kernel. `CHECKPATCH_PATH` overrides `KDIR`.
|
||||
|
||||
Native `--strict` checks run without ignored types or project exceptions;
|
||||
local `.checkpatch.conf` files are not loaded. Source files are never rewritten.
|
||||
Findings return 1; missing per-file completion, inconsistent totals, stderr
|
||||
or tool failures return 2. Make maps both failures to 2. Complete output and
|
||||
coverage are in `.cache/lint/checkpatch.{log,json}`. `scripts/checkpatch_scan`
|
||||
uses the same policy and exit codes; legacy patch/commit helpers remain separate.
|
||||
|
||||
## Analyzer CI
|
||||
|
||||
The [workflow](../.github/workflows/analyzers.yml) runs on pushes to `master`
|
||||
|
||||
@@ -24,8 +24,8 @@ KBUILD_SCHEMA = {
|
||||
"kernel_exceptions": [{"header": str, "message": str, "reason": str}],
|
||||
}
|
||||
BASIC_SCHEMA = {"version": int, "notes": [str]}
|
||||
POLICIES = ("common", "sparse", "smatch", "coccicheck", "cppcheck")
|
||||
BASIC_POLICIES = ("coccicheck", "cppcheck")
|
||||
POLICIES = ("common", "sparse", "smatch", "coccicheck", "cppcheck", "checkpatch")
|
||||
BASIC_POLICIES = ("coccicheck", "cppcheck", "checkpatch")
|
||||
|
||||
|
||||
def relative_path(value: str, location: str):
|
||||
|
||||
+2
-20
@@ -2,23 +2,5 @@
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
rootdir="$(readlink -f "$(dirname "$0")/..")"
|
||||
scriptsdir="${rootdir}/scripts"
|
||||
outputfile="checkpatch.out"
|
||||
|
||||
mapfile -t src_files < <(
|
||||
"${scriptsdir}/source/list-source-files" "${rootdir}" | \
|
||||
grep -vE '^debian/|^fcst/linux-patches|patch$|pdf$|png$|^iscsi-scst/usr|^qla|^scripts/|^scstadmin/|^usr/|^www/' | \
|
||||
while read -r filename; do echo "${rootdir}/$filename"; done)
|
||||
|
||||
"${scriptsdir}/checkpatch" -f "${src_files[@]}" 1> "${outputfile}" || true
|
||||
|
||||
errors=$(grep -c '^ERROR' "${outputfile}" || true)
|
||||
warnings=$(grep -c '^WARNING' "${outputfile}" || true)
|
||||
checks=$(grep -c '^CHECK' "${outputfile}" || true)
|
||||
|
||||
echo "${errors} errors / ${warnings} warnings / ${checks} checks."
|
||||
grep -E '^WARNING|^ERROR|^CHECK' "${outputfile}" |
|
||||
sort |
|
||||
sed 's/^CHECK:CAMELCASE: Avoid CamelCase:.*/CHECK:CAMELCASE Avoid CamelCase/' |
|
||||
uniq -c
|
||||
scriptsdir="$(cd "$(dirname "$0")" && pwd)"
|
||||
exec "${PYTHON:-python3}" -B "$scriptsdir/style/checkpatch.py" "$@"
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
{
|
||||
"version": 1,
|
||||
"notes": [
|
||||
"Check whole C files and headers with native Checkpatch --strict.",
|
||||
"Include userspace and KUnit sources through the common input policy.",
|
||||
"No diagnostic exceptions, ignored types or local Checkpatch configuration.",
|
||||
"Legacy patch and commit helpers retain their separate existing policy."
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,134 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Check whole SCST C files with native Checkpatch and no diagnostic filters."""
|
||||
|
||||
import argparse
|
||||
from collections import Counter
|
||||
import fcntl
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
|
||||
sys.dont_write_bytecode = True
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
sys.path.insert(0, str(ROOT / "scripts"))
|
||||
from lint_common import copy_output, entrypoint, log_path, run, show_log # noqa: E402
|
||||
from lint_scope import select_sources # noqa: E402
|
||||
|
||||
TOTAL = re.compile(r"total: (\d+) errors, (\d+) warnings, (\d+) checks, \d+ lines checked")
|
||||
COMPLETE = re.compile(r"(.+) has (?:style problems, please review|"
|
||||
r"no obvious style problems and is ready for submission)\.")
|
||||
DIAGNOSTIC = re.compile(r"^(ERROR|WARNING|CHECK):([^:\n]+): ?(.*)$", re.MULTILINE)
|
||||
|
||||
|
||||
def checker_path():
|
||||
kernel = Path(os.environ.get("SCST_LINT_KDIR") or os.environ.get("KDIR") or
|
||||
f"/lib/modules/{os.environ.get('KVER') or os.uname().release}/build")
|
||||
directory = Path(os.environ.get("CHECKPATCH_PATH") or kernel / "scripts").resolve()
|
||||
for name in ("checkpatch.pl", "spelling.txt", "const_structs.checkpatch"):
|
||||
if not (directory / name).is_file():
|
||||
raise ValueError(f"missing Checkpatch tool/data: {directory / name}")
|
||||
return directory / "checkpatch.pl"
|
||||
|
||||
|
||||
def native_report(text, stderr, status, selected):
|
||||
"""Require matching native totals and completion for every requested file."""
|
||||
diagnostics = Counter(match[1] for match in DIAGNOSTIC.finditer(text))
|
||||
totals = [tuple(map(int, match.groups())) for line in text.splitlines()
|
||||
if (match := TOTAL.fullmatch(line))]
|
||||
completed = Counter(str(Path(match[1]).resolve()) for line in text.splitlines()
|
||||
if (match := COMPLETE.fullmatch(line)))
|
||||
expected = Counter(str(path.resolve()) for path in selected)
|
||||
failures = []
|
||||
if stderr:
|
||||
failures.append("native stderr: " + stderr.rstrip())
|
||||
if status not in (0, 1):
|
||||
failures.append(f"native exit status {status}")
|
||||
if len(totals) != len(selected):
|
||||
failures.append(f"expected {len(selected)} native totals, got {len(totals)}")
|
||||
for index, severity in enumerate(("ERROR", "WARNING", "CHECK")):
|
||||
if sum(row[index] for row in totals) != diagnostics[severity]:
|
||||
failures.append(f"native {severity} total does not match retained diagnostics")
|
||||
for name, count in (expected - completed).items():
|
||||
failures.append(f"missing {count} native completion record(s) for {name}")
|
||||
for name, count in (completed - expected).items():
|
||||
failures.append(f"unexpected {count} native completion record(s) for {name}")
|
||||
remaining = sum(diagnostics.values())
|
||||
if status in (0, 1) and status != int(bool(remaining)):
|
||||
failures.append("native exit status disagrees with diagnostic totals")
|
||||
return {"native_status": status, "counts": dict(diagnostics), "remaining": remaining,
|
||||
"failures": failures, "complete": not failures,
|
||||
"completed_files": sorted(completed)}
|
||||
|
||||
|
||||
def check(tool, selected, log):
|
||||
git_dir = subprocess.check_output(["git", "-C", str(ROOT), "rev-parse",
|
||||
"--absolute-git-dir"], text=True).strip()
|
||||
# Native Checkpatch writes/deletes CamelCase caches in cwd. Its own
|
||||
# configuration search must not import a user's --ignore or --fix flags.
|
||||
with tempfile.TemporaryDirectory(prefix="checkpatch-", dir=log.parent) as temporary:
|
||||
directory = Path(temporary)
|
||||
(directory / ".checkpatch.conf").write_text("")
|
||||
stdout = directory / "stdout"
|
||||
stderr = directory / "stderr"
|
||||
env = dict(os.environ, CHECKPATCH_CONFIG_DIR=str(directory),
|
||||
GIT_DIR=git_dir, GIT_WORK_TREE=str(ROOT))
|
||||
command = [str(tool), "--no-tree", "--root=" + str(ROOT), "--strict", "--show-types",
|
||||
"--color=never", "--no-fix", "--no-fix-inplace", "--file", *map(str, selected)]
|
||||
try:
|
||||
with stdout.open("wb") as output, stderr.open("wb") as errors:
|
||||
result = run(command, cwd=directory, env=env, stdout=output, stderr=errors,
|
||||
progress=f"Checkpatch ({len(selected)} files)")
|
||||
finally:
|
||||
with log.open("ab") as stream:
|
||||
for path in (stdout, stderr):
|
||||
if path.exists():
|
||||
copy_output(path, stream)
|
||||
return native_report(stdout.read_text(errors="replace"), stderr.read_text(errors="replace"),
|
||||
result.returncode, selected)
|
||||
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--base", default=os.environ.get("DIFFBR") or None)
|
||||
parser.add_argument("files", nargs="*", type=Path)
|
||||
args = parser.parse_args()
|
||||
with log_path(ROOT, "checkpatch.lock", truncate=False).open("a") as lock:
|
||||
# Serialize this report without blocking independent Kbuild analyzers.
|
||||
fcntl.flock(lock, fcntl.LOCK_EX)
|
||||
log = log_path(ROOT, "checkpatch.log")
|
||||
report = log.with_suffix(".json")
|
||||
summary = {"tool": "checkpatch", "run_id": os.environ.get("SCST_LINT_RUN_ID"),
|
||||
"outcome": "incomplete", "exit_code": 2, "remaining": 0,
|
||||
"checked_files": [], "selected_files": []}
|
||||
report.write_text(json.dumps(summary, indent=2) + "\n")
|
||||
show_log(log)
|
||||
try:
|
||||
selected = select_sources(ROOT, args.files, args.base, "checkpatch")
|
||||
summary["selected_files"] = [path.relative_to(ROOT).as_posix() for path in selected]
|
||||
if not selected:
|
||||
print("No eligible Checkpatch inputs; analysis skipped.")
|
||||
summary.update(outcome="skipped", exit_code=0)
|
||||
return 0
|
||||
summary.update(check(checker_path(), selected, log))
|
||||
if not summary["complete"]:
|
||||
print("Checkpatch analysis is incomplete:\n" + "\n".join(summary["failures"]),
|
||||
file=sys.stderr)
|
||||
return 2
|
||||
summary["checked_files"] = summary["selected_files"]
|
||||
status = int(bool(summary["remaining"]))
|
||||
summary.update(outcome="findings" if status else "clean", exit_code=status)
|
||||
print(f"Checkpatch: {len(selected)} files, {summary['remaining']} diagnostic(s).")
|
||||
return status
|
||||
finally:
|
||||
report.write_text(json.dumps(summary, indent=2) + "\n")
|
||||
if os.environ.get("LINT_DEFER_REPORTS") != "1":
|
||||
copy_output(log)
|
||||
show_log(log)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(entrypoint(main))
|
||||
@@ -157,7 +157,7 @@ class PolicyCliTests(unittest.TestCase):
|
||||
source = analyzer_policy.POLICY_PATH.parent.parent
|
||||
for name in ("analyzer_policy.py", "lint_scope.py", "policy/common.json",
|
||||
"policy/sparse.json", "policy/smatch.json", "policy/coccicheck.json",
|
||||
"policy/cppcheck.json"):
|
||||
"policy/cppcheck.json", "policy/checkpatch.json"):
|
||||
shutil.copyfile(source / name, self.scripts / name)
|
||||
|
||||
def run_cli(self, *args):
|
||||
@@ -233,7 +233,7 @@ class PolicyCliTests(unittest.TestCase):
|
||||
|
||||
def test_basic_analyzers_inherit_common_scope_without_exceptions(self):
|
||||
common = json.loads((self.scripts / "policy/common.json").read_text())
|
||||
for tool in ("coccicheck", "cppcheck"):
|
||||
for tool in ("coccicheck", "cppcheck", "checkpatch"):
|
||||
with self.subTest(tool=tool):
|
||||
result = self.run_cli("show", tool, "--format=json")
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
"""Checkpatch protocol fixtures without a kernel build or source rewrites."""
|
||||
|
||||
from pathlib import Path
|
||||
import sys
|
||||
import unittest
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
|
||||
from style.checkpatch import native_report
|
||||
|
||||
|
||||
class CheckpatchTests(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.first = Path("/tmp/scst/first.c")
|
||||
self.second = Path("/tmp/scst/second.c")
|
||||
self.clean = ("total: 0 errors, 0 warnings, 0 checks, 1 lines checked\n"
|
||||
f"{self.first} has no obvious style problems and is ready for submission.\n")
|
||||
self.warning = ("WARNING:SPDX_LICENSE_TAG: Missing SPDX tag\n"
|
||||
"total: 0 errors, 1 warnings, 0 checks, 1 lines checked\n"
|
||||
f"{self.first} has style problems, please review.\n")
|
||||
|
||||
def test_completed_clean_file(self):
|
||||
report = native_report(self.clean, "", 0, [self.first])
|
||||
self.assertTrue(report["complete"])
|
||||
self.assertEqual(report["remaining"], 0)
|
||||
|
||||
def test_source_warning_is_retained(self):
|
||||
report = native_report(self.warning, "", 1, [self.first])
|
||||
self.assertTrue(report["complete"])
|
||||
self.assertEqual(report["remaining"], 1)
|
||||
self.assertEqual(report["counts"], {"WARNING": 1})
|
||||
|
||||
def test_unreadable_second_file_cannot_hide_behind_first_file(self):
|
||||
for status, text in ((0, self.clean), (1, self.warning)):
|
||||
with self.subTest(status=status):
|
||||
report = native_report(text, "diff: second.c: Permission denied", status,
|
||||
[self.first, self.second])
|
||||
self.assertFalse(report["complete"])
|
||||
self.assertTrue(any("native stderr" in item for item in report["failures"]))
|
||||
self.assertTrue(any("completion" in item for item in report["failures"]))
|
||||
|
||||
def test_missing_duplicate_and_unknown_completions_fail(self):
|
||||
for text, files in ((self.clean, [self.first, self.second]),
|
||||
(self.clean + self.clean, [self.first, self.second]),
|
||||
(self.clean, [self.second]), ("", [self.first])):
|
||||
with self.subTest(text=text, files=files):
|
||||
self.assertFalse(native_report(text, "", 0, files)["complete"])
|
||||
|
||||
def test_native_failure_and_unaccounted_diagnostics_fail(self):
|
||||
for text, stderr, status in ((self.clean, "Perl failed", 0), (self.clean, "", 2),
|
||||
(self.warning, "", 0), (self.clean, "", 1),
|
||||
(self.warning.replace("1 warnings", "0 warnings"), "", 1),
|
||||
(self.warning.split("\n", 1)[1], "", 1)):
|
||||
with self.subTest(text=text, stderr=stderr, status=status):
|
||||
self.assertFalse(native_report(text, stderr, status, [self.first])["complete"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user