diff --git a/plans/pkh-lint.md b/plans/pkh-lint.md new file mode 100644 index 0000000..efbe18f --- /dev/null +++ b/plans/pkh-lint.md @@ -0,0 +1,555 @@ +# `pkh lint` — Package linting — Spec + +Status: **phase 1 (wrapper + native skeleton) implemented**; phases 2–4 proposal / discussion draft + +## Phase 1 implementation notes + +What landed (all of it behind `pkh lint`): + +- `src/lint/`: tag model, check trait + registry (`check.rs`), collectors + (`collect.rs`: source name from control/changelog, git dirty-state under + `debian/`), emitter + report (`emit.rs`), text/JSON rendering + exit-code + decision (`output.rs`), lintian wrapper (`wrapper.rs`), and the first + native check `pkh-git` (`checks/pkh.rs`, tag + `pkh-debian-changes-not-committed`). +- CLI: every option of the table below; `--list-tags` prints the native + catalog. Tests: parser goldens captured from real lintian 2.129 output + (source, binary, overridden, `--info` shapes), registry lint, merge dedup, + display/exit-code matrix, git-check fixtures. +- Measured deltas from the original draft, resolved during implementation: + - lintian accepts only built package files, never trees — the wrapper + prefers the `pkh build` output next to the tree when current + (freshness-guarded), else packs fresh fast (`-Zgzip -z1`); see + [Wrapper mode](#wrapper-mode-the-parity-layer). A tree with uncommitted + quilt changes makes dpkg-source refuse; the error points at committing + or `--native`. + - lintian's own exit code is 2 both for "fail-on met" and "runtime error", + so the wrapper never trusts it: the verdict comes from parsed findings, + and pkh keeps its 0 (clean) / 1 (fail-on met) / 2 (runtime error) contract. + - `--display-info` (lintian's `-I`) exists as its own flag: `--info` shows + explanations (lintian's `-i`), it does not raise the display level. +- Licensing outcome (see risks): pkh will be dual-licensed GPL-2+ / MIT; + any future lintian-derived files stay GPL-2+-only, file-level separated. + +## Problem + +pkh covers the package lifecycle (`pull`, `chlog`, `build`, `deb`, `put`) but +never validates the packaging itself: nothing catches a broken +`debian/control` stanza, an unparsable changelog version, a missing copyright +file, or a leftover `UNRELEASED` entry *before* a build or a PPA upload — +and when the build does fail, the error often surfaces late and far from the +cause. The standard tool, lintian, is a separate invocation with its own +conventions, its own lab setup, and no integration with pkh's workflow. + +The requirement is **feature parity with lintian**. Rewriting lintian +outright is a multi-year project; ignoring it leaves no parity at all. The +strategy below gets parity on day one (wrap lintian), then migrates checks +to a native Rust engine incrementally, with a differential harness proving +equivalence as checks move. + +## What lintian actually contains (measured, v2.129) + +Numbers from the installed package, because they drive every decision: + +| Component | Size | Declarative? | +|---|---|---| +| Tag catalog | **1,546** tag files (`Tag/Severity/Check/Explanation/See-Also`) | **Yes — already data** | +| Check logic | **346** Perl modules, **47,586** lines total | No — code | +| — small modules (280 of 346) | < 150 lines (≈40–60 of those are GPL headers/boilerplate; real logic often 10–100 lines) | Partly expressible as predicates | +| — heavy modules (18) | 400–1,089 lines (Debhelper, Scripts, Changelog, Dep5, Cruft, Menus, Debconf, InitD, Rules…) | No — mini-programs | +| Static data | **110** files, **2.2 MB** (spelling dictionaries, section lists, debhelper command tables…) | Yes | +| Infrastructure | unpackers, file index, ELF analysis (objdump integration), changelog/copyright parsers | No — code | + +The split that matters: **157 of the 346 checks** walk the unpacked binary +tree (`visit_installed_files`); the rest operate on source/control data. +Binary-tree checks are the majority of the long tail and need unpack +infrastructure before they can be native. + +## Goal + +`pkh lint [path]` is the single linting entry point of pkh: + +- **Day-one parity via wrapping**: by default, `pkh lint` runs the installed + lintian with mapped flags and merges its findings with pkh's own. Every + lintian check, tag, profile and override works through pkh. +- **A native Rust engine growing underneath**: pkh-native checks (git + workflow, PPA conventions — things lintian cannot know) plus checks + ported from lintian one batch at a time, each gated by a differential + harness against real lintian. Parity never regresses because the wrapper + keeps covering whatever is not yet native. +- **New checks are cheap to add**: one Rust file per check area, one + registration line, fixture tests — plus a data-driven escape hatch for + one-off, package-specific rules without writing Rust. +- **Lintian-compatible where it matters**: lintian's tag naming, override + file formats and names, output letter conventions and exit codes, so + muscle memory, docs, and in-tree overrides carry over. Where a check is + equivalent to a lintian check, we use lintian's tag name (this also makes + deduplication between wrapper and native findings trivial). + +### Non-goals + +- **No monolithic rewrite upfront.** Native coverage grows check by check + under the harness; the wrapper keeps full parity in the meantime. +- **No YAML as a check-logic language.** YAML carries the tag catalog, data + tables and a thin predicate layer; real check logic is Rust (see + [Why not YAML for check logic](#why-not-yaml-for-check-logic)). +- **No lintian profile files**, no Perl/Python plugin loading. +- **No auto-fixing** (`--fix` is a possible follow-up; checks report, they + don't edit). + +## Proposed UX + +``` +$ pkh lint # lints the source tree in the current directory +$ pkh lint ../hello_2.10/ # or an explicit path +$ pkh lint --json # machine-readable (CI, editor integrations) +$ pkh lint --native # skip the lintian wrapper, native engine only +``` + +Default mode: **wrapper + native** when lintian is installed (notice printed +if it is absent, native-only fallback); `--native` forces native only. +Native findings and parsed lintian findings are merged into one report; +duplicates are dropped by tag name (works because we mirror lintian's tag +names for equivalent checks). + +Typical output, lintian-shaped: + +``` +E: hello source: broken-debian-changelog-line line 12: unterminated line +W: hello source: maintainer-name-missing John +I: hello source: out-of-date-standards-version 4.5.0 (current is 4.7.2) +W: hello source: pkh-debian-changes-not-committed (native: uncommitted debian/ edits) +N: 2 overridden tags were shown as O: +``` + +Options (lintian names kept where equivalent; pkh common options where +relevant): + +| Option | Meaning | +|---|---| +| `[path]` | Source tree to lint (default: cwd) | +| `-d, --dist ` | Target distribution (debian/ubuntu); default: from changelog / host | +| `-s, --series ` | Target series; enables series-validity checks | +| `--native` | Native engine only, do not invoke lintian | +| `--repack` | Ignore existing pkh build output; pack the tree fresh for linting | +| `--fail-on ` | Severities that make the exit code 1, comma-separated (default: `error`) | +| `--info` / `-i` | Show long descriptions and references after each finding | +| `--display-info` / `-I` | Also display info-level tags | +| `--pedantic` | Also display pedantic tags | +| `--experimental` | Also display experimental tags | +| `--show-overrides` | Also display suppressed tags marked `O:` | +| `--suppress-tags ` | Ignore specific tag names for this run | +| `--check ` | Run only the named native checks (repeatable) | +| `--list-tags` | Print the merged tag catalog (name, severity, description) and exit | +| `--json` | Emit findings as JSON instead of text (same exit code) | +| `--color ` | `auto` (default: TTY without `NO_COLOR`), `always`, `never` | + +Exit codes, mirroring lintian: `0` no findings at/above `--fail-on`; `1` +findings at/above the threshold (in wrapper mode: the merge of lintian's +verdict and native findings); `2` runtime error. Default display level: +`error` + `warning` (deliberately quieter than lintian, which also shows +`I:`; info is one `-i` away). + +## Wrapper mode (the parity layer) + +`src/lint/wrapper.rs`: + +- **Requires the `lintian` binary** on PATH; without it, prints a notice and + runs native-only (exit code unaffected by the missing wrapper). +- **Artifact strategy — reuse pkh build output, never lint stale state.** + Lintian accepts only built package files, so the wrapper lints + `../_.dsc` (pkh build's own naming) when it matches the + current changelog entry and no tree content is newer (mtimes, skipping + `.git`/`.pc`). Current output → linted directly, no packing at all + (the fast path: 2.7 s vs 12.5 s on a 111 MB tree). Missing, stale, or + `--repack` → the tree is packed fresh with `dpkg-source -b` into a temp + dir, using weak compression (`-Zgzip -z1`) since the artifact is + ephemeral; quilt trees get their orig tarballs symlinked from the tree's + parent (dpkg-source searches cwd, which is the temp dir). Each run prints + an `N:` note stating which artifact was linted and why. +- **Flag mapping**: `--info`, `--display-info`, `--pedantic`, + `--experimental`, `--show-overrides`, `--suppress-tags` translate 1:1; + `path`, `--repack`, `--check` and `--json` are pkh-side. +- **Output parsing is load-bearing here** (unlike a convenience-only + bridge): lintian's line format (`X: :
`) is + parsed into pkh's `LintReport` model so both worlds merge, `--json` works + uniformly, and native checks can be deduplicated against lintian's. The + parser is tested against golden samples of real lintian output (the + format is stable and version-pinned by the installed lintian). +- **Exit code**: max(lintian's verdict, native findings at/above + `--fail-on`); lintian crash → exit 2 with raw output retained. +- The wrapper streams nothing to the terminal until merged: pkh renders the + unified report (same renderer in all modes). + +## Native engine + +New module `src/lint/`, library-first like every other subcommand; `main.rs` +only parses flags and picks a view. + +``` +src/lint/ + mod.rs engine entry point: run(options) -> LintReport + tag.rs Severity, Certainty, Tag metadata + check.rs Check trait, registry + collect.rs collectors producing LintData (see below) + emit.rs Emitter (dedup, override matching) + LintReport + output.rs text / JSON rendering, exit-code decision + overrides.rs lintian override file parsing and matching + wrapper.rs lintian invocation, flag mapping, output parsing, merge + checks/ + control.rs debian/control checks + changelog.rs debian/changelog checks + copyright.rs debian/copyright checks + source_format.rs debian/source/format checks + rules.rs debian/rules checks + watch.rs debian/watch checks + patches.rs debian/patches/ checks (DEP-3) + deps.rs dependency-structure checks (uses debian::deps) + pkh.rs pkh-native checks (git workflow, PPA conventions) +``` + +### Tag model + +```rust +pub enum Severity { Error, Warning, Info, Pedantic } +pub enum Certainty { Certain, Possible, WildGuess } + +pub struct Tag { + pub name: &'static str, // lintian-compatible kebab-case + pub severity: Severity, + pub certainty: Certainty, + pub experimental: bool, + pub description: &'static str, // shown by --info and --list-tags + pub references: &'static [&'static str], +} +``` + +Certainty exists so gates can be tuned later (`--fail-on error,certain` is +out of scope for v1, but the data is captured from day one). + +### Check trait and registry + +```rust +pub trait Check: Sync { + fn id(&self) -> &'static str; // "control", "changelog", ... + fn tags(&self) -> &'static [Tag]; + fn run(&self, data: &LintData, emit: &mut Emitter); +} +``` + +Checks are zero-sized structs registered in one explicit array in +`checks/mod.rs` (`pub static CHECKS: &[&dyn Check] = &[&control::Control, ...]`), +in line with the codebase's low-magic style (no proc macros, no `inventory` +crate). A unit test walks the registry and fails on duplicate tag names, +missing descriptions, or malformed tag casing — the registry cannot rot +silently. + +`Emitter` wraps the report and knows the tag registry; `emit.tag("name", +"detail")` looks up metadata once and applies overrides immediately, so +suppressed findings never reach the view. The engine shares the `report.rs` +ports philosophy: the emitter produces a `LintReport`; rendering +(text/JSON/quiet) is a separate concern. + +### Collectors: `LintData` + +One struct built per run, lazily where expensive (source trees are small; +lintian's on-disk lab is unnecessary at this stage): + +```rust +pub struct LintData<'a> { + pub root: &'a Path, + pub dist: Option, // resolved from --dist / changelog + pub series: Option, + pub control: Option, // debian::control + pub changelog: Option<...>, // debian::changelog + pub deps: Option, // debian::deps (Build-Depends etc.) + pub files: Vec, // walked tree: path, mode, size + pub source_format: Option, + pub copyright: Option, + pub patches: Vec, // series + DEP-3 headers + pub read(&self, rel: &Path) -> Option<&str>, // cached file content +} +``` + +Collector failures (e.g. unparsable control) are themselves findings: the +collector records a tag and downstream checks that need that data are +skipped — exactly lintian's behavior. + +### Overrides + +Parse the formats and file names lintian already defines, so existing and +shared trees work with both tools unchanged: + +- `debian/source.lintian-overrides` — applies to the source package +- `debian/.lintian-overrides` — applies to that binary package's + tags (accepted in v1, enforced when binary tags land) + +Syntax supported: `#` comments, bare `tag-name`, and `tag-name [restrictions]` +with arch/type restrictions parsed but honored only as far as native tags +go (source type only). Unknown tag names in override files emit an +`override-file-unknown-tag` info tag — cheap guard against typos (lintian +calls this out too). + +### Distro scoping (assessment + design) + +Ground truth from the installed lintian 2.129: lintian **is** distro-aware +via a vendor/profile mechanism, but Ubuntu's layer is thin. The +`ubuntu/main` profile only disables 14 Debian-specific tags (NMU +bookkeeping, `bugs-field-does-not-refer-to-debian-infrastructure`, upstart +leftovers, ...) and the `vendors/ubuntu/` data adds exactly one file +(`changes-file/known-dists`). On an Ubuntu host, lintian auto-selects the +ubuntu vendor via `/etc/dpkg/origins/default`. Consequences, visible on e.g. +`linux-riscv`: + +- `unknown-field Ubuntu-Compatible-Signing` fires even on Ubuntu hosts: + lintian's derivative-field allowlist mechanism exists + (`data/fields/derivative-fields`, format `field ~~ regex ~~ explanation`), + Ubuntu just never shipped its fields in `vendors/ubuntu/`. The proper fix + is upstreaming to lintian (bug/merge proposal against Ubuntu's lintian); + pkh should carry a stopgap suppression meanwhile. +- `malformed-debian-changelog-version 7.2.0-5.5.2 (for native)` and + `binary-nmu-debian-revision-in-source`: lintian's changelog logic rejects + native-format-with-revision, which is how Ubuntu kernel packages are + shaped by convention. Profiles can only disable whole tags, not reshape + logic, so this class can't be fixed by lintian data — it is exactly the + pkh-side scoping below (and eventually our own native checks, which know + the dist). +- Ordinary Ubuntu backport versions (`...~26.04` tildes) are fine: tilde is + valid dpkg version syntax, and the ubuntu profile already disables the + Debian upload-version bookkeeping checks. + +Design (phase 2, needs sign-off on the tag lists): + +- **Distro → wrapper profile**: done in phase 1 — `-d ubuntu`/`-d debian` + passes `--profile` to lintian so the target distro's rules apply even on + a foreign host (auto-detection already covers the same-host case). +- **Distro-level suppression data** (`data/lint.yml`, embedded): per-dist + lists of tags to downgrade or drop for wrapper-origin findings, e.g. + ubuntu: `unknown-field` once the field list is upstreamed — deliberately + kept minimal; suppression is policy, not taste. +- **Package-level suppression via `quirks.yml`** (existing package-keyed + pattern), e.g. for the kernel family: + ```yaml + linux-riscv: + lint: + suppressed_tags: [malformed-debian-changelog-version, + binary-nmu-debian-revision-in-source] + ``` + In-tree `debian/source.lintian-overrides` remains the package's own voice + (shared with lintian, already honored); quirks are pkh's voice for + conventions the ecosystem hasn't written down. + +## Checks — first native batch + +~20 tags across 7 check areas, chosen for what bites pkh users before an +upload. Severities below are the proposal; tuning happens in review. + +**control** (`debian/control`): +- `missing-package-field` (E) — required source fields absent +- `maintainer-name-missing` / `maintainer-email-missing` (W) — malformed + Maintainer/Uploaders +- `unknown-priority` (E), `unknown-section` (W) — validated against + `data/` lists (new small embedded data files) +- `description-too-short` (I) — Description under ~20 chars +- `missing-tests-control` (I) — no `Testsuite: autopkgtest` nor + `debian/tests/` +- `binary-field-mismatch` (W) — Architecture/Section inconsistencies between + stanzas + +**changelog** (uses `debian::changelog` + `debian::version`): +- `changelog-file-missing` (E) — no parsable `debian/changelog` +- `bad-urgent-value` (W) — urgency outside low/medium/high/critical +- `unreleased-changes-present` (W) — top entry is UNRELEASED +- `unknown-distribution` (W) — distribution not in the dist's series list + (reuses `distro_info` data!) nor a known alias +- `version-syntax-error` (E) — `DebianVersion::parse` failure +- `version-native-mismatch` (E) — native format with revision, or quilt + format with native version + +**copyright**: +- `missing-debian-copyright-file` (E) +- `copyright-not-machine-readable` (I) — no DEP-5 header +- `missing-copyright-license-paragraph` (W) — files referenced but no + License stanza + +**source_format**: `missing-debian-source-format` (W), +`non-3.0-source-format` (I) + +**rules**: `debian-rules-not-executable` (E), `debian-rules-missing` (E), +`debian-rules-no-required-target` (W) — conservative static scan, no make +invocation + +**pkh** (native only, the differentiator — lintian can't know these): +- `pkh-debian-changes-not-committed` (W) — git tree dirty in `debian/` + (pkh's whole flow is git-centric; catching "edited control, forgot to + commit" before `pkh put`) +- `pkh-new-template-drift` (I) — tree was scaffolded by `pkh new` and + template files have drifted from the current templates in `data/` + +Note: `pkh new`'s existing structural self-checks should eventually be +rebuilt on this engine so there is one definition of "a healthy tree". + +## Why not YAML for check logic + +The question "can the 1,500 checks live in YAML?" decomposes into three +different things, with three different answers: + +1. **Tag catalog: yes — and it is already data.** Lintian stores all 1,546 + tag descriptions as flat files (`Tag/Severity/Check/Explanation/See-Also`). + Converting them to a YAML/JSON catalog is a mechanical script job — no + agentic engineering required. The native registry should load or generate + from this catalog so native tags and wrapper-parsed tags share one + metadata source. +2. **Static data tables: yes.** The 2.2 MB under lintian's `data/` (spelling + dictionaries, section/priority lists, debhelper command tables) is + exactly the kind of content `data/*.yml` + `embed_data!` exists for. + License applies (see risks): copying these files makes pkh GPL-derived. +3. **Check logic: no.** Even the *smallest* lintian check is code — + `Files/LdSo.pm` is one regex on a filename plus one condition on the + package name, as a Perl method; most of the 280 small checks are similar + ("visit file X, apply conditions, emit tag"), and 18 checks are + 400–1,089-line mini-programs with loops, cross-file state, package + relation graphs and version arithmetic. A DSL expressive enough for + those is a scripting language with an interpreter you now have to + maintain, debug, and document — worse than Rust, with no compiler + safety net. YAML is a data format, and check logic is not data. + +Where YAML *does* fit in logic: a **thin predicate layer** for the simple +tail (field present/absent, file exists, glob match, regex on a field, +version comparison) — this is what `debian/pkh-lint.yml` (below) exposes +for user-specific rules. That covers a meaningful slice of the small +checks, but pursuing 100% parity through it would mean maintaining a +bespoke interpreter as big as the problem. + +**Agentic engineering changes the economics of porting, not the target +representation.** LLM agents are good at mechanical translation +(Perl check → Rust check) and at writing the differential tests; the +correctness gate is the harness below, not the faithfulness of the +translation. Porting is per-check, priority-ordered, and reviewable — +compatible with the wrapper keeping parity in the meantime. + +## Differential harness (the trust anchor for porting) + +Before any ported check ships: + +1. **Corpus**: a set of real source packages (the pkh pull flow already + fetches them) plus lintian's own test suite fixtures (in the lintian + source package). +2. **Record**: run real lintian over the corpus, store expected tag sets + per package. +3. **Diff**: `pkh lint --native` over the same corpus; report tag-set + deltas (missing / extra / severity-mismatch) per package. The CI gate + for ported checks is an empty delta on the covered corpus subset. +4. **Graduation**: a check is "native" once its delta is empty; the wrapper + continues covering everything else, and both-mode dedup by tag name + means graduation is invisible to users. + +## Declarative rules (user-specific, no Rust) + +The user-facing "add specific rules" path, mirroring the `quirks.yml` +philosophy: a small YAML file linted against the tree. + +`debian/pkh-lint.yml`: + +```yaml +rules: + - id: keep-changelog-synced + tag: pkh-changelog-sync-rule + severity: warning + when: + control-field-present: Vcs-Browser + expect: + file-present: debian/salsa-ci.yml + - id: no-embedded-js + tag: pkh-embedded-javascript + severity: error + expect: + no-file-matching: "*.min.js" +``` + +Phase 2; the predicates are the collector primitives (field present/absent, +file present/absent, glob, regex on a field). Custom `pkh-*` tags get +auto-generated catalog entries. This keeps the Rust registry for real +checks while giving package maintainers one-off rules. + +## Output + +- **Text**: lintian's ` : ` line format, + colors only when TTY (reuse the ui.rs conventions; `--info` appends + indented description + references). Summary line: `N: N tags (3 E, 5 W, + ...), M overridden`. +- **JSON** (`--json`): array of `{tag, severity, certainty, experimental, + check, message, overridden, file?, line?, origin: native|lintian}` plus a + summary object — schema-versioned for CI use. Works identically in + wrapper and native modes (wrapper findings are parsed into the same + model). +- **Exit**: 0/1/2 as above; `--fail-on` decides the threshold + (`error` default; `error,warning` for upload gating). + +## Testing + +- **Wrapper parser**: golden tests against captured output of real lintian + (several packages, all display levels, overridden/`O:` and `C:` lines). + Version the fixtures by lintian version. +- **Differential harness**: as above; the CI gate for native checks. +- **Fixture trees** (`tests/fixtures/lint//`): minimal source trees + exercising one tag each, with a manifest of expected tags; negative + fixtures (clean trees, zero tags) live next to positive ones — same + pattern `pkh new` verification uses. +- **Registry lint test**: duplicate/undocumented tag names fail the test + suite. +- **Override parser**: table-driven tests against lintian's documented + syntax, including the weird corners ([restrictions], inline comments). +- **Golden text output**: one snapshot test of the renderer so formatting + changes are deliberate. + +## Phases + +1. **Wrapper v1 (parity day one)** — `src/lint/` skeleton, lintian + invocation + flag mapping + output parsing, unified report + (text + JSON, exit codes), native engine skeleton with the pkh-native + checks (`pkh-*`), CLI wiring. Feature parity achieved here. +2. **Native core checks + harness** — collectors, first native batch + (control/changelog/copyright/source_format/rules), overrides, tag + catalog import from lintian's tag files, differential harness in CI, + both-mode dedup. `pkh new` self-checks migrate onto the engine. +3. **Porting track (agentic, incremental)** — batches of lintian checks + ported Perl→Rust, priority-ordered by pkh-user relevance (source checks + first); each batch gated by the harness. Predicate layer + + `debian/pkh-lint.yml`. Data-table import once licensing is decided. +4. **Binary artifacts + unpack infra** — lint `.changes` + built `.deb`s + (temp unpack): file index unlocks the 157 installed-files checks family + (manpages, md5sums, maintainer scripts, init/systemd units, scripts). + The heavy checks (Debhelper, Scripts, Dep5…) and their data land here, + batch by batch under the harness. `.lintian-overrides` enforcement; + optional `pkh deb --lint` and a `pkh put` upload gate. + +## Risks & open questions + +- **Licensing is a blocking decision for the porting track.** Lintian is + GPL-2+. Porting its check logic or copying its data files produces a + derivative work; pkh currently declares no license. Options: (a) pkh goes + GPL-2+ — porting and data import are clean; (b) pkh stays unlicensed/ + permissive — then only the wrapper plus independently-authored checks + (the `pkh-*` set) are possible, and the porting track is off the table. + The wrapper alone is always fine (subprocess boundary, no derivative). + This decision gates phase 3, not phases 1–2. +- **Wrapper parsing depends on lintian's output format.** Stable for years + and pinned to the installed version, but a lintian major change would + break wrapper mode; mitigated by golden fixtures and the native track + reducing dependency over time. +- **False positives on PPA workflows.** Lintian targets archive uploads; + some checks are noise for quick PPA builds. Mitigation: quiet default + display level, `--fail-on error` default, `pkh-*` severities tuned for + the pkh flow. +- **Porting velocity is the open question of phase 3.** The harness makes + each port verifiable, but 346 checks (and their data) is a campaign, not + a sprint; the wrapper exists precisely so this never blocks users. +- **Tag-name mirroring is a commitment.** Reusing lintian names buys + familiarity, shared override files, and trivial both-mode dedup — at the + cost of renames breaking overrides. Accepted; lintian tag names are + stable. +- **Overlap with `pkh new` self-checks** — migrating them onto the engine + needs a small "run subset of checks programmatically" API; the + library-first design allows it but it should be an explicit decision when + touched. diff --git a/src/lib.rs b/src/lib.rs index e022e15..5aa8cad 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,6 +21,8 @@ pub mod debian; pub mod distro_info; /// Launchpad integration: PPA upload targets and account discovery pub mod launchpad; +/// Lint a source tree: lintian wrapper for full parity plus pkh-native checks (`pkh lint`) +pub mod lint; /// Scaffold a new Debian source package (`pkh new`) pub mod new; /// Obtain information about one or multiple packages diff --git a/src/lint/check.rs b/src/lint/check.rs new file mode 100644 index 0000000..16c874b --- /dev/null +++ b/src/lint/check.rs @@ -0,0 +1,106 @@ +//! The check trait and the static check registry. +//! +//! Checks are zero-sized structs, one module per packaging area, registered +//! in one explicit array — reviewable, greppable, and free of proc macros. +//! A registry test fails the build on duplicate tag names, undocumented +//! tags or casing drift, so the catalog cannot rot silently. + +use crate::lint::collect::LintData; +use crate::lint::emit::Emitter; +use crate::lint::tag::Tag; + +/// A group of checks over one area of the packaging (control, changelog, +/// git workflow, ...). One check instance may emit any of the tags it +/// declares; the emitter resolves metadata and applies suppression. +pub trait Check: Sync { + /// Registry identifier, also the `--check` value (one word, e.g. `pkh-git`). + fn id(&self) -> &'static str; + + /// Static metadata of every tag this check may emit. + fn tags(&self) -> &'static [Tag]; + + /// Run against the collected package information, reporting findings + /// through `emit`. + fn run(&self, data: &LintData, emit: &mut Emitter); +} + +/// Every registered check, in catalog order. +pub static CHECKS: &[&dyn Check] = &[&super::checks::pkh::PkhGit as &dyn Check]; + +/// Look up a registered check by its id (`--check` value). +pub fn find_check(id: &str) -> Option<&'static dyn Check> { + CHECKS.iter().copied().find(|check| check.id() == id) +} + +/// Look up tag metadata by tag name across the whole registry. +pub fn find_tag(name: &str) -> Option<&'static Tag> { + CHECKS + .iter() + .flat_map(|check| check.tags()) + .find(|tag| tag.name == name) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn registry_tag_names_are_unique_and_well_formed() { + let mut seen = Vec::new(); + for tag in CHECKS.iter().flat_map(|check| check.tags()) { + assert!( + !seen.contains(&tag.name), + "duplicate tag name: {}", + tag.name + ); + seen.push(tag.name); + let valid = |c: char| c.is_ascii_lowercase() || c.is_ascii_digit() || "+.-".contains(c); + assert!( + tag.name.starts_with(valid) + && tag.name.chars().all(valid) + && !tag.name.ends_with('-'), + "tag name is not kebab-case: {}", + tag.name + ); + assert!( + !tag.description.trim().is_empty(), + "tag without a description: {}", + tag.name + ); + } + } + + #[test] + fn check_ids_are_unique_and_resolvable() { + let mut seen = Vec::new(); + for check in CHECKS { + assert!( + !seen.contains(&check.id()), + "duplicate check id: {}", + check.id() + ); + seen.push(check.id()); + assert!( + find_check(check.id()).is_some(), + "find_check cannot resolve its own registry: {}", + check.id() + ); + } + } + + #[test] + fn pkh_native_tags_are_namespaced() { + for check in CHECKS { + if check.id().starts_with("pkh-") { + for tag in check.tags() { + assert!( + tag.name.starts_with("pkh-"), + "pkh-native check '{}' emits non-namespaced tag '{}'", + check.id(), + tag.name + ); + } + } + } + } +} diff --git a/src/lint/checks/mod.rs b/src/lint/checks/mod.rs new file mode 100644 index 0000000..a5c846b --- /dev/null +++ b/src/lint/checks/mod.rs @@ -0,0 +1,4 @@ +//! Check areas; one module per area, registered in [`crate::lint::check::CHECKS`]. + +/// Pkh-native workflow checks (git-centric trees, PPA uploads). +pub mod pkh; diff --git a/src/lint/checks/pkh.rs b/src/lint/checks/pkh.rs new file mode 100644 index 0000000..d6d06fd --- /dev/null +++ b/src/lint/checks/pkh.rs @@ -0,0 +1,148 @@ +//! Pkh-native checks: workflow knowledge lintian cannot have, because it +//! lives in pkh's flows (git-centric trees, PPA uploads, scaffolding). + +use crate::lint::check::Check; +use crate::lint::collect::LintData; +use crate::lint::emit::Emitter; +use crate::lint::tag::{Certainty, Severity, Tag}; + +/// Metadata of every tag the pkh-native checks emit. +pub static TAGS: &[Tag] = &[Tag { + name: "pkh-debian-changes-not-committed", + severity: Severity::Warning, + certainty: Certainty::Certain, + experimental: false, + description: "The debian/ directory contains changes that are not committed to git. \ +pkh builds and uploads the tree as-is (pkh deb, pkh put); committing first keeps \ +the upload and the git history in sync.", + references: &[], +}]; + +/// Flags `debian/` content that exists in the tree but is not committed to +/// git. Skips trees outside any git repository: archive-pulled sources +/// legitimately have none. +pub struct PkhGit; + +impl Check for PkhGit { + fn id(&self) -> &'static str { + "pkh-git" + } + + fn tags(&self) -> &'static [Tag] { + TAGS + } + + fn run(&self, data: &LintData, emit: &mut Emitter) { + let Some(git) = &data.git else { + return; + }; + if git.dirty_debian.is_empty() { + return; + } + let examples: Vec<&str> = git + .dirty_debian + .iter() + .take(3) + .map(String::as_str) + .collect(); + let more = if git.dirty_debian.len() > examples.len() { + ", ..." + } else { + "" + }; + emit.tag( + "pkh-debian-changes-not-committed", + format!( + "{} uncommitted change(s) under debian/ (e.g. {}{more})", + git.dirty_debian.len(), + examples.join(", ") + ), + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + fn write(path: &Path, content: &str) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, content).unwrap(); + } + + fn commit_all(repo: &git2::Repository) { + let signature = git2::Signature::now("pkh test", "test@example.com").unwrap(); + let mut index = repo.index().unwrap(); + index + .add_all(["*"], git2::IndexAddOption::DEFAULT, None) + .unwrap(); + // write_tree alone does not persist the index; without this, a + // committed worktree still reads as index-deleted + untracked. + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_id).unwrap(); + repo.commit(Some("HEAD"), &signature, &signature, "init", &tree, &[]) + .unwrap(); + } + + #[test] + fn no_git_repository_is_no_finding() { + let dir = tempfile::tempdir().unwrap(); + write(&dir.path().join("debian/control"), "Source: hello\n"); + let data = LintData::collect(dir.path(), None, None); + assert!(data.git.is_none()); + + let mut findings = Vec::new(); + let mut emitter = Emitter::new(data.source_name.clone(), &mut findings); + PkhGit.run(&data, &mut emitter); + assert!(findings.is_empty()); + } + + #[test] + fn clean_worktree_is_no_finding() { + let dir = tempfile::tempdir().unwrap(); + write(&dir.path().join("debian/control"), "Source: hello\n"); + let repo = git2::Repository::init(dir.path()).unwrap(); + commit_all(&repo); + + let data = LintData::collect(dir.path(), None, None); + let mut findings = Vec::new(); + let mut emitter = Emitter::new(data.source_name.clone(), &mut findings); + PkhGit.run(&data, &mut emitter); + assert!(findings.is_empty()); + } + + #[test] + fn dirty_debian_tree_is_a_finding() { + let dir = tempfile::tempdir().unwrap(); + write(&dir.path().join("debian/control"), "Source: hello\n"); + write(&dir.path().join("hello.txt"), "upstream\n"); + let repo = git2::Repository::init(dir.path()).unwrap(); + commit_all(&repo); + + // A modified tracked file and a fresh untracked patch: both count. + write( + &dir.path().join("debian/control"), + "Source: hello\nDepends: x\n", + ); + write(&dir.path().join("debian/patches/new.patch"), "...\n"); + write(&dir.path().join("hello.txt"), "changed upstream\n"); + + let data = LintData::collect(dir.path(), None, None); + let mut findings = Vec::new(); + let mut emitter = Emitter::new(data.source_name.clone(), &mut findings); + PkhGit.run(&data, &mut emitter); + assert_eq!(findings.len(), 1); + let finding = &findings[0]; + assert_eq!(finding.tag_name, "pkh-debian-changes-not-committed"); + assert_eq!(finding.letter, 'W'); + assert!( + finding + .message + .starts_with("2 uncommitted change(s) under debian/") + ); + assert!(finding.message.contains("debian/control")); + assert!(finding.message.contains("debian/patches/new.patch")); + } +} diff --git a/src/lint/collect.rs b/src/lint/collect.rs new file mode 100644 index 0000000..ac60375 --- /dev/null +++ b/src/lint/collect.rs @@ -0,0 +1,98 @@ +//! Collectors: package information gathered once per run and shared by every +//! check, mirroring lintian's collection phase without the on-disk lab. +//! Source trees are small, so everything is computed eagerly except git +//! status, which only exists when the tree is a git worktree. + +use std::path::{Path, PathBuf}; + +use crate::debian::control::ControlInfo; + +/// Git worktree state relevant to pkh's git-centric workflow checks. +pub struct GitStatus { + /// Repo-relative paths under `debian/` with uncommitted content + /// (modified, staged or untracked), sorted. + pub dirty_debian: Vec, +} + +/// Everything the checks and the report renderer know about the linted tree. +pub struct LintData { + /// Root of the source tree being linted. + pub root: PathBuf, + /// Source package name, resolved from `debian/control`, else from the + /// changelog's first line, else the directory name. + pub source_name: String, + /// Target distribution (`--dist`); unresolved when None. + pub dist: Option, + /// Target series (`--series`); unresolved when None. + pub series: Option, + /// Git worktree state; None when the tree is not inside a git repository. + pub git: Option, +} + +impl LintData { + /// Collect information about the source tree at `root`. + pub fn collect(root: &Path, dist: Option<&str>, series: Option<&str>) -> LintData { + LintData { + root: root.to_path_buf(), + source_name: resolve_source_name(root), + dist: dist.map(str::to_string), + series: series.map(str::to_string), + git: collect_git(root), + } + } +} + +/// Source package name from `debian/control`, falling back to the changelog +/// header and then the directory name; the report needs a display name even +/// for broken trees. +fn resolve_source_name(root: &Path) -> String { + if let Ok(control) = ControlInfo::parse(&root.join("debian/control")) { + return control.source_name().to_string(); + } + if let Ok(changelog) = std::fs::read_to_string(root.join("debian/changelog")) + && let Some(first) = changelog.lines().next() + && let Some(name) = first.split(" (").next() + && !name.trim().is_empty() + { + return name.trim().to_string(); + } + root.file_name() + .map(|name| name.to_string_lossy().to_string()) + .unwrap_or_else(|| "package".to_string()) +} + +/// Dirty paths under `debian/` when the tree lives in a git worktree. +/// Untracked files count: a fresh patch nobody committed is exactly the +/// mistake the workflow checks exist to catch. None when there is no +/// repository (or git is somehow unusable here) — never a finding, since +/// archive-pulled trees legitimately have none. +fn collect_git(root: &Path) -> Option { + let repo = git2::Repository::discover(root).ok()?; + let workdir = repo.workdir()?; + // Statuses are workdir-relative; trees nested inside a repository only + // care about their own slice of it. + let prefix = root.strip_prefix(workdir).unwrap_or(Path::new("")); + let debian_dir = prefix.join("debian"); + + let mut options = git2::StatusOptions::new(); + options + .include_untracked(true) + .include_ignored(false) + .recurse_untracked_dirs(true); + let statuses = repo.statuses(Some(&mut options)).ok()?; + + let mut dirty_debian = Vec::new(); + for entry in statuses.iter() { + // CURRENT is the zero flag in libgit2, so any non-empty status is + // some kind of change (worktree or staged). + if entry.status().is_empty() { + continue; + } + let path = entry.path()?; + if Path::new(path).starts_with(&debian_dir) { + dirty_debian.push(path.to_string()); + } + } + dirty_debian.sort(); + Some(GitStatus { dirty_debian }) +} diff --git a/src/lint/emit.rs b/src/lint/emit.rs new file mode 100644 index 0000000..3f2f601 --- /dev/null +++ b/src/lint/emit.rs @@ -0,0 +1,118 @@ +//! Findings, the run report, and the emitter checks report through. +//! +//! The emitter is deliberately thin: checks name a tag and give a message; +//! metadata, output letter and explanations come from the registry, so +//! findings from the native engine and findings parsed from the wrapped +//! lintian share one shape and one rendering path. + +use crate::lint::check; +use crate::lint::tag::Tag; + +/// Where a finding comes from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Origin { + /// Emitted by pkh's native Rust checks. + Native, + /// Parsed from the wrapped lintian's output. + Lintian, +} + +/// One lint finding, whatever produced it. +#[derive(Debug, Clone)] +pub struct Finding { + /// Output letter (`E`, `W`, `I`, `P`, `X`, `O`, `C`): severity for + /// native findings, verbatim from the output line in wrapper mode. + pub letter: char, + /// Stable tag name (`missing-debian-copyright-file`). + pub tag_name: String, + /// Free-form details after the tag name; empty when the tag stands alone. + pub message: String, + /// Package the finding belongs to (`hello`). + pub package: String, + /// Processable type lintian displays after the package name (`source`, + /// `changes`, ...); None for binary findings, which lintian prints + /// without a type. + pub processable_type: Option, + /// Explanation lines shown by `--info`: the `N:` blocks lintian attaches + /// to the finding in wrapper mode, or the native tag's description and + /// references. + pub explanation: Vec, + /// Native or parsed-from-lintian. + pub origin: Origin, +} + +impl Finding { + /// Lowercase severity/classification name for this finding's letter, as + /// used in `--fail-on` values and JSON output. + pub fn severity_name(&self) -> &'static str { + match self.letter { + 'E' => "error", + 'W' => "warning", + 'I' => "info", + 'P' => "pedantic", + 'X' => "experimental", + 'O' => "overridden", + _ => "classification", + } + } +} + +/// The full result of one lint run: every finding, from every source. +pub struct LintReport { + /// All findings in emission order: the wrapper's first, then the native + /// ones (deduplicated by tag name against the wrapper's). + pub findings: Vec, + /// Display name of the linted source package. + pub source_name: String, + /// The wrapper was skipped because lintian is not installed; the + /// renderer prints a notice and the run is native-only. + pub wrapper_unavailable: bool, + /// Run-level notes (`N:` lines): how the linted artifact was obtained, + /// and similar context that is not a finding. + pub notes: Vec, +} + +/// Sink checks report findings through. Resolves tag metadata from the +/// registry so checks only ever name the tag they mean. +pub struct Emitter<'a> { + package: String, + findings: &'a mut Vec, +} + +impl<'a> Emitter<'a> { + /// Emitter for findings of `package` (e.g. the source package name), + /// appending into `findings`. + pub fn new(package: String, findings: &'a mut Vec) -> Emitter<'a> { + Emitter { package, findings } + } + + /// Emit `tag_name` with `message` as its detail line. Native findings + /// always report on the source package; an unknown tag name (a registry + /// bug) still produces a finding rather than panicking — the registry + /// test makes that path unreachable in practice. + pub fn tag(&mut self, tag_name: &str, message: impl Into) { + let (letter, explanation) = match check::find_tag(tag_name) { + Some(tag) => (tag.letter(), explanation_of(tag)), + None => ('E', Vec::new()), + }; + self.findings.push(Finding { + letter, + tag_name: tag_name.to_string(), + message: message.into(), + package: self.package.clone(), + processable_type: Some("source".to_string()), + explanation, + origin: Origin::Native, + }); + } +} + +/// The explanation `--info` shows for a native tag: its description plus any +/// references, prefixed like lintian's "Please refer to" pointers. +fn explanation_of(tag: &Tag) -> Vec { + let mut lines = vec![tag.description.to_string()]; + for reference in tag.references { + lines.push(format!("Please refer to {}", reference)); + } + lines +} diff --git a/src/lint/mod.rs b/src/lint/mod.rs new file mode 100644 index 0000000..75b150b --- /dev/null +++ b/src/lint/mod.rs @@ -0,0 +1,186 @@ +//! `pkh lint`: lint a Debian source tree, with lintian feature parity. +//! +//! The strategy is *wrap first, port second* (see `plans/pkh-lint.md`): the +//! [wrapper](wrapper) runs the installed lintian over an ephemeral source +//! package for day-one parity with every lintian check, while the native +//! engine (check registry, collectors, emitter) hosts pkh-specific workflow +//! checks (`pkh-*` tags) and grows ported lintian checks incrementally. +//! Both findings merge into one report — deduplicated by tag name, which is +//! why native checks mirror lintian's tag names for equivalent checks — +//! rendered as lintian-shaped text or JSON, with lintian's exit-code +//! contract (0 clean, 1 findings at/above `--fail-on`, 2 runtime error). + +pub mod check; +pub mod checks; +pub mod collect; +pub mod emit; +pub mod output; +pub mod tag; +pub mod wrapper; + +use std::collections::HashSet; +use std::path::PathBuf; + +use crate::lint::check::Check; +use crate::lint::collect::LintData; +use crate::lint::emit::{Finding, LintReport}; + +/// Knobs of one `pkh lint` run, built from the CLI in `main.rs`. +pub struct LintOptions { + /// Source tree to lint. + pub path: PathBuf, + /// Run the native engine only; never invoke the lintian wrapper. + pub native: bool, + /// Levels that make the exit code 1 (`--fail-on`, default: error). + pub fail_on: Vec, + /// Show tag explanations under each finding (`--info`, lintian's `-i`). + pub info: bool, + /// Also display info-level (`I:`) findings (lintian's `-I`). + pub display_info: bool, + /// Also display pedantic (`P:`) findings. + pub pedantic: bool, + /// Also display experimental (`X:`) findings. + pub experimental: bool, + /// Also display overridden (`O:`) findings. + pub show_overrides: bool, + /// Tag names to ignore for this run. + pub suppress_tags: Vec, + /// Run only these native checks (`--check`, repeatable). + pub only_checks: Vec, + /// Ignore any existing `pkh build` output and pack the tree fresh. + pub repack: bool, + /// Emit JSON instead of text. + pub json: bool, + /// Text colorization mode (`--color`, default auto: TTY without NO_COLOR). + pub color: output::ColorMode, + /// Target distribution (debian/ubuntu), when known. + pub dist: Option, + /// Target series, when known. + pub series: Option, +} + +/// Run one lint: collect package information, run the native checks, wrap +/// lintian (unless `--native`), and merge everything into one report. +pub fn run(options: &LintOptions) -> Result { + let data = LintData::collect( + &options.path, + options.dist.as_deref(), + options.series.as_deref(), + ); + let mut report = LintReport { + findings: Vec::new(), + source_name: data.source_name.clone(), + wrapper_unavailable: false, + notes: Vec::new(), + }; + + if !options.native { + match wrapper::run(&data.root, options)? { + Some(outcome) => { + report.findings.extend(outcome.findings); + report.notes = outcome.notes; + } + None => report.wrapper_unavailable = true, + } + } + + let selected: Vec<&'static dyn Check> = if options.only_checks.is_empty() { + check::CHECKS.to_vec() + } else { + options + .only_checks + .iter() + .map(|id| check::find_check(id).ok_or_else(|| format!("Unknown --check '{id}'"))) + .collect::>()? + }; + + let mut native = Vec::new(); + { + let mut emitter = emit::Emitter::new(report.source_name.clone(), &mut native); + for check in selected { + check.run(&data, &mut emitter); + } + } + report.findings.extend(merge(&report.findings, native)); + + if !options.suppress_tags.is_empty() { + report + .findings + .retain(|finding| !options.suppress_tags.contains(&finding.tag_name)); + } + Ok(report) +} + +/// Append `native` findings to `lintian`'s, dropping native duplicates by +/// tag name (the wrapper's verdict wins for tags both engines produce). +fn merge(lintian: &[Finding], native: Vec) -> Vec { + let known: HashSet<&str> = lintian.iter().map(|f| f.tag_name.as_str()).collect(); + native + .into_iter() + .filter(|finding| !known.contains(finding.tag_name.as_str())) + .collect() +} + +/// The `--list-tags` catalog: one line per registered native tag. +pub fn list_tags() -> String { + let mut out = String::new(); + for check in check::CHECKS { + for tag in check.tags() { + out.push_str(&format!( + "{} [{}] {}\n {}\n", + tag.letter(), + check.id(), + tag.name, + tag.description + )); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::lint::emit::Origin; + + fn finding(tag: &str, origin: Origin) -> Finding { + Finding { + letter: 'W', + tag_name: tag.to_string(), + message: String::new(), + package: "hello".to_string(), + processable_type: Some("source".to_string()), + explanation: Vec::new(), + origin, + } + } + + #[test] + fn merge_drops_native_duplicates_by_tag_name() { + let lintian = vec![finding("no-debian-copyright-in-source", Origin::Lintian)]; + let native = vec![ + finding("no-debian-copyright-in-source", Origin::Native), + finding("pkh-debian-changes-not-committed", Origin::Native), + ]; + let merged = merge(&lintian, native); + assert_eq!(merged.len(), 1); + assert_eq!(merged[0].tag_name, "pkh-debian-changes-not-committed"); + assert_eq!(merged[0].origin, Origin::Native); + } + + #[test] + fn native_tags_resolve_from_the_registry() { + let tag = check::find_tag("pkh-debian-changes-not-committed") + .expect("pkh check tags must resolve"); + assert_eq!(tag.letter(), 'W'); + } + + #[test] + fn unknown_native_tags_still_produce_findings() { + let mut sink = Vec::new(); + let mut emitter = emit::Emitter::new("hello".to_string(), &mut sink); + emitter.tag("no-such-tag-anywhere", "boom"); + assert_eq!(sink.len(), 1); + assert_eq!(sink[0].tag_name, "no-such-tag-anywhere"); + } +} diff --git a/src/lint/output.rs b/src/lint/output.rs new file mode 100644 index 0000000..12ee6d9 --- /dev/null +++ b/src/lint/output.rs @@ -0,0 +1,450 @@ +//! Rendering of a lint report (text and JSON) and the exit-code decision. +//! +//! Text output keeps lintian's line shape (`: : +//!
`) so findings read identically in both modes; JSON is the +//! schema-stable form for CI. Display filtering (what is shown) and the +//! `--fail-on` threshold (what makes the exit code 1) are independent, like +//! lintian's. Colorization happens here, at render time — findings are +//! captured plain (`--color never` is passed to lintian) and re-painted by +//! severity, so colors are a renderer concern that survives the switch from +//! wrapped lintian output to native checks. + +use crossterm::style::Stylize; +use serde_json::json; + +use crate::lint::LintOptions; +use crate::lint::emit::{Finding, LintReport, Origin}; + +/// When to colorize the text report. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum ColorMode { + /// Colorize when stdout is a terminal and `NO_COLOR` is unset. + #[default] + Auto, + /// Always colorize (piping, CI logs). + Always, + /// Never colorize. + Never, +} + +impl ColorMode { + /// Parse one `--color` keyword. + pub fn parse(word: &str) -> Option { + match word { + "auto" => Some(ColorMode::Auto), + "always" => Some(ColorMode::Always), + "never" => Some(ColorMode::Never), + _ => None, + } + } + + /// Whether the text renderer should emit ANSI colors. + fn should_color(self) -> bool { + match self { + ColorMode::Always => true, + ColorMode::Never => false, + ColorMode::Auto => { + std::io::IsTerminal::is_terminal(&std::io::stdout()) + && std::env::var_os("NO_COLOR").is_none() + } + } + } +} + +/// The output letter, painted with its severity color when `color` is set: +/// errors red (bold), warnings yellow, info cyan, pedantic/experimental +/// magenta, overridden green. Mirrors lintian's tty palette closely enough +/// for muscle memory. +fn paint_letter(letter: char, color: bool) -> String { + if !color { + return letter.to_string(); + } + match letter { + 'E' => "E".red().bold().to_string(), + 'W' => "W".yellow().to_string(), + 'I' => "I".cyan().to_string(), + 'P' => "P".magenta().to_string(), + 'X' => "X".magenta().to_string(), + 'O' => "O".green().to_string(), + _ => letter.to_string(), + } +} + +/// A named severity class, mirroring the values lintian's `--fail-on` +/// accepts. `Experimental` is the `X:` pseudo-level (the letter hides the +/// underlying severity) and `Overridden` lets gates count suppressed tags. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Level { + /// `E:` findings. + Error, + /// `W:` findings. + Warning, + /// `I:` findings. + Info, + /// `P:` findings. + Pedantic, + /// `X:` findings. + Experimental, + /// `O:` findings (only reported when overrides are shown). + Overridden, +} + +impl Level { + /// The output letter the level stands for. + pub fn letter(self) -> char { + match self { + Level::Error => 'E', + Level::Warning => 'W', + Level::Info => 'I', + Level::Pedantic => 'P', + Level::Experimental => 'X', + Level::Overridden => 'O', + } + } + + /// Parse one comma-separated `--fail-on` keyword. + pub fn parse(word: &str) -> Option { + match word.trim() { + "error" => Some(Level::Error), + "warning" => Some(Level::Warning), + "info" => Some(Level::Info), + "pedantic" => Some(Level::Pedantic), + "experimental" => Some(Level::Experimental), + "override" => Some(Level::Overridden), + _ => None, + } + } +} + +/// Parse the whole `--fail-on` value (comma-separated level names). +pub fn parse_fail_on(spec: &str) -> Result, String> { + let levels: Vec = spec + .split(',') + .filter(|word| !word.trim().is_empty()) + .map(Level::parse) + .collect::>() + .ok_or_else(|| { + format!( + "Invalid --fail-on value '{spec}': expected comma-separated \ + error, warning, info, pedantic, experimental or override" + ) + })?; + if levels.is_empty() { + return Err("Empty --fail-on value: expected at least one level".to_string()); + } + Ok(levels) +} + +/// The letter a finding displays as for display-level purposes: overridden +/// findings carry their `O:` letter, classification tags are JSON-only. +fn is_displayed(finding: &Finding, options: &LintOptions) -> bool { + match finding.letter { + 'E' | 'W' => true, + 'I' => options.display_info, + 'P' => options.pedantic, + 'X' => options.experimental, + 'O' => options.show_overrides, + _ => false, + } +} + +/// Exit code of the run: 1 when any finding (except classification tags) is +/// at a level listed in `--fail-on`, 0 otherwise. Runtime failures never +/// reach this function — the caller exits 2 directly. +pub fn exit_code(report: &LintReport, options: &LintOptions) -> i32 { + let failed = report + .findings + .iter() + .any(|finding| match level_of_letter(finding.letter) { + Some(level) => options.fail_on.contains(&level), + None => false, + }); + if failed { 1 } else { 0 } +} + +/// The level a finding's letter maps to for `--fail-on` purposes; None for +/// classification tags, which lintian also never fails on. +fn level_of_letter(letter: char) -> Option { + match letter { + 'E' => Some(Level::Error), + 'W' => Some(Level::Warning), + 'I' => Some(Level::Info), + 'P' => Some(Level::Pedantic), + 'X' => Some(Level::Experimental), + 'O' => Some(Level::Overridden), + _ => None, + } +} + +/// Render the report as lintian-shaped text: one `: : +///
` line per displayed finding, explanations under `--info`, then +/// an `N:` summary line. +pub fn render_text(report: &LintReport, options: &LintOptions) -> String { + let color = options.color.should_color(); + let mut out = String::new(); + if report.wrapper_unavailable { + out.push_str( + "N: lintian is not installed; showing pkh-native checks only \ + (install lintian for full check coverage)\n", + ); + } + for note in &report.notes { + out.push_str(&format!("N: {note}\n")); + } + + let mut shown_counts = [('E', 0), ('W', 0), ('I', 0), ('P', 0), ('X', 0), ('O', 0)]; + let mut shown = 0; + for finding in &report.findings { + if !is_displayed(finding, options) { + continue; + } + shown += 1; + if let Some((_, count)) = shown_counts + .iter_mut() + .find(|(letter, _)| *letter == finding.letter) + { + *count += 1; + } + out.push_str(&format!( + "{}: {}\n", + paint_letter(finding.letter, color), + line_subject(finding) + )); + if options.info { + for line in &finding.explanation { + if line.is_empty() { + out.push_str("N:\n"); + } else { + out.push_str(&format!("N: {}\n", line.trim_end())); + } + } + } + } + + let mut parts = Vec::new(); + for (letter, count) in shown_counts { + if count > 0 { + parts.push(format!("{} {}", count, paint_letter(letter, color))); + } + } + let hidden = report.findings.len() - shown; + if parts.is_empty() { + out.push_str(&format!("N: no displayed tags; {hidden} hidden\n")); + } else { + out.push_str(&format!( + "N: {shown} tag(s) shown ({}); {hidden} hidden\n", + parts.join(", ") + )); + } + out +} + +/// `hello source: tag details` — the part of a lintian line after the +/// letter, reproduced identically for both origins. +fn line_subject(finding: &Finding) -> String { + let mut line = String::from(&finding.package); + if let Some(ptype) = &finding.processable_type { + line.push(' '); + line.push_str(ptype); + } + line.push_str(": "); + line.push_str(&finding.tag_name); + if !finding.message.is_empty() { + line.push(' '); + line.push_str(&finding.message); + } + line +} + +/// Render the report as pretty-printed JSON: every finding (displayed or +/// not, flagged as such) plus a summary carrying the exit-code verdict. +pub fn render_json(report: &LintReport, options: &LintOptions) -> String { + let findings: Vec = report + .findings + .iter() + .map(|finding| { + json!({ + "letter": finding.letter.to_string(), + "tag": finding.tag_name, + "severity": finding.severity_name(), + "package": finding.package, + "processable_type": finding.processable_type, + "message": finding.message, + "origin": match finding.origin { + Origin::Native => "native", + Origin::Lintian => "lintian", + }, + "overridden": finding.letter == 'O', + "displayed": is_displayed(finding, options), + "explanation": if finding.explanation.is_empty() { + json!(null) + } else { + json!(finding.explanation) + }, + }) + }) + .collect(); + + let document = json!({ + "source": report.source_name, + "wrapper_unavailable": report.wrapper_unavailable, + "notes": report.notes, + "findings": findings, + "summary": { + "failed": exit_code(report, options) == 1, + }, + }); + serde_json::to_string_pretty(&document).expect("lint report JSON is serializable") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn options(fail_on: &[Level]) -> LintOptions { + LintOptions { + path: std::path::PathBuf::from("."), + native: false, + fail_on: fail_on.to_vec(), + info: false, + display_info: false, + pedantic: false, + experimental: false, + show_overrides: false, + suppress_tags: Vec::new(), + only_checks: Vec::new(), + repack: false, + json: false, + color: ColorMode::Never, + dist: None, + series: None, + } + } + + fn report(findings: &[(&str, char)]) -> LintReport { + LintReport { + findings: findings + .iter() + .map(|(tag, letter)| Finding { + letter: *letter, + tag_name: (*tag).to_string(), + message: String::new(), + package: "hello".to_string(), + processable_type: Some("source".to_string()), + explanation: Vec::new(), + origin: Origin::Lintian, + }) + .collect(), + source_name: "hello".to_string(), + wrapper_unavailable: false, + notes: Vec::new(), + } + } + + #[test] + fn fail_on_parses_and_rejects_unknown_levels() { + assert_eq!( + parse_fail_on("error, warning").unwrap(), + vec![Level::Error, Level::Warning] + ); + assert!(parse_fail_on("bogus").is_err()); + assert!(parse_fail_on(" ").is_err()); + } + + #[test] + fn exit_code_only_counts_displayed_severity_letters() { + let default_opts = options(&[Level::Error]); + // Errors fail, warnings alone do not (the default threshold). + assert_eq!(exit_code(&report(&[("t", 'E')]), &default_opts), 1); + assert_eq!( + exit_code(&report(&[("t", 'W'), ("u", 'I')]), &default_opts), + 0 + ); + // Overridden findings never fail unless explicitly requested. + assert_eq!(exit_code(&report(&[("t", 'O')]), &default_opts), 0); + assert_eq!( + exit_code( + &report(&[("t", 'O')]), + &options(&[Level::Error, Level::Overridden]) + ), + 1 + ); + // Classification tags never fail. + assert_eq!(exit_code(&report(&[("t", 'C')]), &default_opts), 0); + } + + #[test] + fn text_rendering_respects_display_levels() { + let base = report(&[("e", 'E'), ("i", 'I'), ("o", 'O'), ("x", 'X')]); + let default = render_text(&base, &options(&[Level::Error])); + assert!(default.contains("E: hello source: e")); + assert!(!default.contains("I: ")); + assert!(!default.contains("O: ")); + assert!(!default.contains("X: ")); + assert!(default.ends_with("N: 1 tag(s) shown (1 E); 3 hidden\n")); + + let everything = LintOptions { + display_info: true, + experimental: true, + show_overrides: true, + ..options(&[Level::Error]) + }; + let full = render_text(&base, &everything); + for prefix in ["E:", "I:", "O:", "X:"] { + assert!( + full.contains(&format!("{prefix} hello source:")), + "{prefix}" + ); + } + assert!(full.ends_with("N: 4 tag(s) shown (1 E, 1 I, 1 X, 1 O); 0 hidden\n")); + } + + #[test] + fn colorization_is_render_only_and_mode_switchable() { + let run_report = report(&[("e", 'E'), ("w", 'W'), ("o", 'O')]); + + let plain = options(&[Level::Error]); + let mut forced = options(&[Level::Error]); + forced.color = ColorMode::Always; + + let plain = render_text(&run_report, &plain); + let colored = render_text(&run_report, &forced); + assert!( + !plain.contains('\u{1b}'), + "never/auto-on-pipe must stay plain" + ); + assert!(colored.contains('\u{1b}'), "always must colorize"); + // Colors wrap the letters only; the lintian line shape is intact. + assert!(colored.contains("hello source: e")); + // Mode parsing round-trips. + assert_eq!(ColorMode::parse("always"), Some(ColorMode::Always)); + assert_eq!(ColorMode::parse("bogus"), None); + } + + #[test] + fn json_round_trips_with_verdicts() { + let run_report = report(&[("e", 'E'), ("w", 'W')]); + let default_opts = options(&[Level::Error]); + let document: serde_json::Value = + serde_json::from_str(&render_json(&run_report, &default_opts)).unwrap(); + assert_eq!(document["source"], "hello"); + assert_eq!(document["findings"][0]["letter"], "E"); + assert_eq!(document["findings"][0]["severity"], "error"); + assert_eq!(document["findings"][0]["origin"], "lintian"); + assert_eq!(document["summary"]["failed"], true); + + // An error finding fails at the warning threshold too, but not at + // pedantic-only, which nothing in this report reaches. + let warn_only = options(&[Level::Warning]); + assert_eq!(exit_code(&run_report, &warn_only), 1); + let document: serde_json::Value = + serde_json::from_str(&render_json(&run_report, &warn_only)).unwrap(); + assert_eq!(document["summary"]["failed"], true); + + let pedantic_only = options(&[Level::Pedantic]); + assert_eq!(exit_code(&run_report, &pedantic_only), 0); + let document: serde_json::Value = + serde_json::from_str(&render_json(&run_report, &pedantic_only)).unwrap(); + assert_eq!(document["summary"]["failed"], false); + } +} diff --git a/src/lint/tag.rs b/src/lint/tag.rs new file mode 100644 index 0000000..b14a700 --- /dev/null +++ b/src/lint/tag.rs @@ -0,0 +1,85 @@ +//! Lint tag model: severities, certainties and static tag metadata. +//! +//! Tags are the atomic diagnostics of a lint run, named after lintian's +//! model: a stable machine-readable name (`missing-debian-copyright-file`), +//! a severity, a certainty and a description. Checks declare the tags they +//! may emit as static [`Tag`] values; the wrapper's parsed findings carry the +//! letter lintian printed instead. + +/// Severity of a finding, matching lintian's severity ladder. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Severity { + /// Policy violation or broken package data (`E:`). + Error, + /// Probable bug or policy deviation (`W:`). + Warning, + /// Informational note about packaging choices (`I:`). + Info, + /// Nitpick most packages may legitimately ignore (`P:`). + Pedantic, +} + +impl Severity { + /// The output letter lintian displays this severity as (`E`, `W`, ...). + pub fn letter(self) -> char { + match self { + Severity::Error => 'E', + Severity::Warning => 'W', + Severity::Info => 'I', + Severity::Pedantic => 'P', + } + } + + /// Lowercase name used in `--fail-on` values and JSON output. + pub fn name(self) -> &'static str { + match self { + Severity::Error => "error", + Severity::Warning => "warning", + Severity::Info => "info", + Severity::Pedantic => "pedantic", + } + } +} + +/// How sure a check is that a finding is real. Not acted upon yet (a future +/// `--fail-on error,certain` would consume it), but captured from day one so +/// severity tuning is data-driven later. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Certainty { + /// The finding is a fact (e.g. a missing file). + Certain, + /// The finding is likely but has legitimate exceptions. + Possible, + /// The finding is a guess from weak signals. + WildGuess, +} + +/// Static metadata of one tag: what checks declare and the renderer resolves. +#[derive(Debug)] +pub struct Tag { + /// Stable machine-readable name, lintian-compatible kebab-case + /// (`pkh-debian-changes-not-committed`). + pub name: &'static str, + /// Severity the tag reports at. + pub severity: Severity, + /// How sure checks are when emitting this tag. + pub certainty: Certainty, + /// Whether the tag is experimental (`X:` output, hidden by default). + pub experimental: bool, + /// One-paragraph explanation, shown by `--info` and `--list-tags`. + pub description: &'static str, + /// References (policy sections, URLs) shown by `--info`. + pub references: &'static [&'static str], +} + +impl Tag { + /// The output letter for this tag: experimental tags render as `X:` + /// regardless of their severity, like lintian. + pub fn letter(&self) -> char { + if self.experimental { + 'X' + } else { + self.severity.letter() + } + } +} diff --git a/src/lint/wrapper.rs b/src/lint/wrapper.rs new file mode 100644 index 0000000..066c311 --- /dev/null +++ b/src/lint/wrapper.rs @@ -0,0 +1,531 @@ +//! The lintian wrapper: pkh's day-one feature-parity layer. +//! +//! Lintian only accepts built package files, never source trees, so the +//! wrapper needs a source artifact. It lints the `pkh build` output next to +//! the tree when it matches the current changelog entry and nothing in the +//! tree is newer than it (the fast path: no packing at all); otherwise it +//! packs the tree fresh with `dpkg-source -b` inside a temporary directory +//! and lints the resulting `.dsc` (`--repack` forces that path). Nothing is +//! written back into the linted tree; the temp directory is removed on drop. +//! +//! Parsing is load-bearing here (findings are merged with the native ones, +//! rendered uniformly and exported as JSON), so the parser is pinned by +//! golden tests captured from real lintian output. Lintian's own exit code +//! is *not* authoritative: lintian uses 2 both for "fail-on met" and for +//! runtime errors, while pkh derives the verdict from the parsed findings +//! and reserves 2 for actual runtime failures. + +use std::io::ErrorKind; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::time::{SystemTime, UNIX_EPOCH}; + +use regex::Regex; + +use crate::lint::LintOptions; +use crate::lint::emit::{Finding, Origin}; + +/// What one wrapper run produced: the parsed lintian findings plus `N:` +/// notes about how the artifact being linted was obtained. +pub struct WrapperOutcome { + /// Lintian's findings. + pub findings: Vec, + /// Human-visible notes (artifact reuse, fresh packing) rendered as `N:`. + pub notes: Vec, +} + +/// Run the wrapper over the source tree at `root`: +/// - `Ok(Some(outcome))` — lintian ran; these are its findings and notes, +/// - `Ok(None)` — lintian is not installed; the caller falls back to +/// native-only with a notice, +/// - `Err(message)` — runtime failure (tree unpacked badly, lintian crashed +/// without reportable output); the caller exits 2. +/// +/// The artifact linted is the `pkh build` output next to the tree when it +/// matches the current changelog entry and no tree content is newer; +/// otherwise the tree is packed fresh (with `--repack` forcing that path). +pub fn run(root: &Path, options: &LintOptions) -> Result, String> { + let tmp = TempDir::new()?; + let root = root + .canonicalize() + .map_err(|e| format!("Cannot lint '{}': {e}", root.display()))?; + + let (dsc, notes) = match usable_build_output(&root, options.repack) { + Some(dsc) => { + let note = format!( + "linting pkh build output {}", + crate::report::display_path(&dsc) + ); + (dsc, vec![note]) + } + None => { + let dsc = pack(&root, &tmp)?; + let reason = if options.repack { + "the tree was packed fresh with dpkg-source (--repack ignored \ + the existing pkh build output)" + .to_string() + } else if expected_build_output(&root).is_some() { + "the tree changed since pkh build, so it was packed fresh \ + with dpkg-source (rerun pkh build to lint the build output)" + .to_string() + } else { + "no pkh build output next to the tree, so it was packed fresh \ + with dpkg-source (pkh build produces one)" + .to_string() + }; + (dsc, vec![reason]) + } + }; + + let mut lintian = Command::new("lintian"); + lintian + .env("LC_ALL", "C") + .args(["--no-cfg", "--color", "never"]) + .arg("--info") + .arg(&dsc); + // Per-distro scoping: lintian auto-detects the *host* vendor, but pkh + // knows the *target* distro (-d); make the two agree, which matters on + // cross-distro hosts (linting an Ubuntu package on Debian or back). + if let Some(dist) = &options.dist + && matches!(dist.as_str(), "ubuntu" | "debian") + { + lintian.arg("--profile").arg(dist); + } + if options.display_info { + lintian.arg("--display-info"); + } + if options.pedantic { + lintian.arg("--pedantic"); + } + if options.experimental { + lintian.arg("--display-experimental"); + } + if options.show_overrides { + lintian.arg("--show-overrides"); + } + if !options.suppress_tags.is_empty() { + lintian + .arg("--suppress-tags") + .arg(options.suppress_tags.join(",")); + } + + let output = match lintian.output() { + Ok(output) => output, + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(format!("Could not run lintian: {e}")), + }; + + let stdout = String::from_utf8_lossy(&output.stdout); + let findings = parse(&stdout); + if !output.status.success() && findings.is_empty() { + return Err(format!( + "lintian exited with {} without reporting findings:\n{}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + )); + } + Ok(Some(WrapperOutcome { findings, notes })) +} + +/// Pack the tree into an ephemeral source package and return its `.dsc`. +/// +/// Compression is deliberately weak (`-Zgzip -z1`): the artifact only ever +/// goes to lintian and is deleted with the temp directory, and xz on a +/// large tree dominates the whole run (measured: 9.8 s xz vs 2.7 s gzip on +/// a 111 MB tree). Locale-independent subprocess output: dpkg messages can +/// be localized, and the error sniffing relies on English wording. +fn pack(root: &Path, tmp: &TempDir) -> Result { + // 3.0 (quilt) trees need the orig tarball(s) reachable from the working + // directory, and dpkg-source searches cwd — link them from the tree's + // parent, where pkh build / git ubuntu export-orig leave them. + link_orig_tarballs(root, tmp.path()); + + let output = Command::new("dpkg-source") + .env("LC_ALL", "C") + .args(["-b", "-Zgzip", "-z1"]) + .arg(root) + .current_dir(tmp.path()) + .output() + .map_err(|e| format!("Could not run dpkg-source: {e}"))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + if stderr.to_lowercase().contains("uncommitted") { + return Err( + "The source tree has uncommitted changes that dpkg-source refuses \ + to pack. Commit them first, or use --native to lint with pkh's \ + native checks only." + .to_string(), + ); + } + return Err(format!( + "dpkg-source -b failed, the tree may not be a valid source package:\n{}", + stderr.trim() + )); + } + + find_dsc(tmp.path()) +} + +/// The `pkh build` output matching the tree's current changelog entry, when +/// it exists and no tree content is newer than it: linting a stale artifact +/// would report the packaging of the past, so staleness forces a fresh pack. +fn usable_build_output(root: &Path, force_repack: bool) -> Option { + if force_repack { + return None; + } + let dsc = expected_build_output(root)?; + let built = std::fs::metadata(&dsc).ok()?.modified().ok()?; + if tree_newer_than(root, built) { + return None; + } + Some(dsc) +} + +/// The `pkh build` output path matching the tree's current changelog entry +/// (`../_.dsc`, pkh build's own naming), if it exists. +fn expected_build_output(root: &Path) -> Option { + let entry = + crate::debian::changelog::parse_changelog_entry(&root.join("debian/changelog")).ok()?; + let dsc = root + .parent()? + .join(format!("{}_{}.dsc", entry.source, entry.version.no_epoch())); + std::fs::metadata(&dsc).ok()?; + Some(dsc) +} + +/// Whether any tree content is newer than `built`. Skips `.git` and `.pc`: +/// commits and quilt bookkeeping churn their mtimes without touching what +/// the source package contains. +fn tree_newer_than(root: &Path, built: SystemTime) -> bool { + const SKIP: &[&str] = &[".git", ".pc"]; + let mut stack = vec![root.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + if SKIP.iter().any(|skip| name.to_string_lossy() == *skip) { + continue; + } + match entry.file_type() { + Ok(ft) if ft.is_dir() => stack.push(entry.path()), + _ => { + let newer = entry + .metadata() + .ok() + .and_then(|m| m.modified().ok()) + .is_some_and(|modified| modified > built); + if newer { + return true; + } + } + } + } + } + false +} + +/// Symlink the orig (and orig component) tarballs for the tree's upstream +/// version from the tree's parent into `dest`, ignoring absence — native +/// trees have none, and missing tarballs surface as a dpkg-source error. +fn link_orig_tarballs(root: &Path, dest: &Path) { + let Some(parent) = root.parent() else { + return; + }; + let Ok(entry) = crate::debian::changelog::parse_changelog_entry(&root.join("debian/changelog")) + else { + return; + }; + let prefixes = [ + format!("{}_{}.orig.tar.", entry.source, entry.version.upstream), + format!("{}_{}.orig-", entry.source, entry.version.upstream), + ]; + let Ok(entries) = std::fs::read_dir(parent) else { + return; + }; + for candidate in entries.flatten() { + let name = candidate.file_name(); + let name = name.to_string_lossy(); + if !candidate.file_type().is_ok_and(|ft| ft.is_file()) + || !prefixes + .iter() + .any(|prefix| name.starts_with(prefix.as_str())) + { + continue; + } + let _ = std::os::unix::fs::symlink(parent.join(name.as_ref()), dest.join(name.as_ref())); + } +} + +/// Parse lintian's output into findings. Tag lines carry the finding; `N:` +/// note lines following a tag line are its explanation (`--info` output) and +/// attach to it. Anything else is ignored. +fn parse(text: &str) -> Vec { + let tag_line = Regex::new(r"^(?P[EWIPXOC]): (?P.*)$").expect("static regex"); + let subject = Regex::new( + r"^(?P\S+?)(?: (?Psource|binary|udeb|changes|buildinfo))?: (?P\S+)(?: (?P
.*))?$", + ) + .expect("static regex"); + let note_line = Regex::new(r"^N:(?: (?P.*))?$").expect("static regex"); + + let mut findings: Vec = Vec::new(); + for line in text.lines() { + if let Some(note) = note_line.captures(line) { + // Attach to the finding above, like lintian lays out --info. + if let (Some(text), Some(last)) = (note.name("text"), findings.last_mut()) + && !text.as_str().trim().is_empty() + { + last.explanation.push(text.as_str().trim().to_string()); + } + continue; + } + let Some(head) = tag_line.captures(line) else { + continue; + }; + let Some(subject) = subject.captures(&head["rest"]) else { + continue; + }; + findings.push(Finding { + letter: head["letter"].chars().next().unwrap_or('E'), + tag_name: subject["tag"].to_string(), + message: subject + .name("details") + .map_or(String::new(), |d| d.as_str().to_string()), + package: subject["pkg"].to_string(), + processable_type: subject.name("ptype").map(|p| p.as_str().to_string()), + explanation: Vec::new(), + origin: Origin::Lintian, + }); + } + findings +} + +/// The single `.dsc` the ephemeral source package produced. +fn find_dsc(dir: &Path) -> Result { + let mut entries: Vec = std::fs::read_dir(dir) + .map_err(|e| format!("Cannot read the temporary build directory: {e}"))? + .flatten() + .map(|entry| entry.path()) + .filter(|path| path.extension().is_some_and(|ext| ext == "dsc")) + .collect(); + match entries.pop() { + Some(dsc) => Ok(dsc), + None => Err( + "dpkg-source produced no .dsc; the tree may not be a valid source package".to_string(), + ), + } +} + +/// Temporary directory removed on drop; the hand-rolled stand-in for +/// `tempfile`, which is dev-only in this crate. +struct TempDir(PathBuf); + +impl TempDir { + fn new() -> Result { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let path = std::env::temp_dir().join(format!("pkh-lint-{}-{unique}", std::process::id())); + std::fs::create_dir(&path) + .map_err(|e| format!("Could not create a temporary directory: {e}"))?; + Ok(TempDir(path)) + } + + fn path(&self) -> &Path { + &self.0 + } +} + +impl Drop for TempDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Captured from lintian 2.129.0ubuntu2.1 on a broken native source + /// package (default display level). + const SOURCE_OUTPUT: &str = "\ +E: hello source: malformed-debian-changelog-version 0.1-1 (for native) [debian/changelog:1] +E: hello source: package-uses-debhelper-but-lacks-build-depends [debian/rules] +W: hello source: debhelper-but-no-misc-depends hello +W: hello source: debhelper-compat-file-is-missing +W: hello source: no-debian-copyright-in-source +"; + + /// Captured from lintian 2.129.0ubuntu2.1 on a binary package: binary + /// findings carry no processable type after the package name. + const BINARY_OUTPUT: &str = "\ +E: badpkg: description-too-short test +E: badpkg: extended-description-is-empty +W: badpkg: empty-binary-package +W: badpkg: recommended-field badpkg_1.0-1_all.deb Priority +"; + + /// Captured with --show-overrides: overridden findings print as `O:`. + const OVERRIDDEN_OUTPUT: &str = "\ +E: hello source: malformed-debian-changelog-version 0.1-1 (for native) [debian/changelog:1] +O: hello source: debhelper-compat-file-is-missing +"; + + /// Captured with --info: each tag line is followed by `N:` explanation + /// lines that belong to it. + const INFO_OUTPUT: &str = "\ +N: +E: hello source: malformed-debian-changelog-version 0.1-1 (for native) [debian/changelog:1] +N: +N: The version string in the latest changelog entry was not parsed correctly. +N: Usually, that means it does not conform to policy. +N: +N: +E: hello source: package-uses-debhelper-but-lacks-build-depends [debian/rules] +N: +N: If a package uses debhelper, it must declare a Build-Depends on debhelper +N: or on the debhelper-compat virtual package. For example: +N: +"; + + #[test] + fn parses_source_output_with_type() { + let findings = parse(SOURCE_OUTPUT); + assert_eq!(findings.len(), 5); + let first = &findings[0]; + assert_eq!(first.letter, 'E'); + assert_eq!(first.package, "hello"); + assert_eq!(first.processable_type.as_deref(), Some("source")); + assert_eq!(first.tag_name, "malformed-debian-changelog-version"); + assert_eq!(first.message, "0.1-1 (for native) [debian/changelog:1]"); + assert_eq!(findings[2].letter, 'W'); + } + + #[test] + fn parses_binary_output_without_type() { + let findings = parse(BINARY_OUTPUT); + assert_eq!(findings.len(), 4); + let first = &findings[0]; + assert_eq!(first.package, "badpkg"); + assert_eq!(first.processable_type, None); + assert_eq!(first.tag_name, "description-too-short"); + // Details containing a file name with dots survive intact. + assert_eq!(findings[3].message, "badpkg_1.0-1_all.deb Priority"); + } + + #[test] + fn parses_overridden_lines() { + let findings = parse(OVERRIDDEN_OUTPUT); + assert_eq!(findings.len(), 2); + assert_eq!(findings[1].letter, 'O'); + assert_eq!(findings[1].tag_name, "debhelper-compat-file-is-missing"); + assert_eq!(findings[1].message, ""); + } + + #[test] + fn attaches_info_notes_to_the_preceding_finding() { + let findings = parse(INFO_OUTPUT); + assert_eq!(findings.len(), 2); + assert_eq!( + findings[0].explanation, + vec![ + "The version string in the latest changelog entry was not parsed correctly.", + "Usually, that means it does not conform to policy.", + ] + ); + assert_eq!(findings[1].explanation.len(), 2); + assert!(findings[1].explanation[0].starts_with("If a package uses debhelper")); + } + + #[test] + fn ignores_stray_lines() { + let findings = + parse("N: lintian ran\ngarbage line\nC: hello source: some-classification\n"); + // The C: classification line parses (kept for JSON), garbage drops. + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].letter, 'C'); + } + + /// A lintable tree at `/` with a changelog entry + /// `pkg (1.0-1) resolute`; returns the outer dir (the tree's parent, + /// where pkh build would place artifacts). + fn tree() -> (tempfile::TempDir, std::path::PathBuf) { + let outer = tempfile::tempdir().unwrap(); + let root = outer.path().join("pkg-1.0"); + std::fs::create_dir_all(root.join("debian")).unwrap(); + std::fs::write( + root.join("debian/changelog"), + "pkg (1.0-1) resolute; urgency=medium\n\n * x\n\n -- J Sat, 19 Sep 2026 12:00:00 +0000\n", + ) + .unwrap(); + (outer, root) + } + + fn dsc_of(outer: &tempfile::TempDir) -> std::path::PathBuf { + outer.path().join("pkg_1.0-1.dsc") + } + + #[test] + fn current_build_output_is_reused() { + let (outer, root) = tree(); + std::fs::write(dsc_of(&outer), "dummy dsc").unwrap(); + // The dsc was written after every tree file: current. + assert_eq!(usable_build_output(&root, false), Some(dsc_of(&outer))); + // Forcing repack skips it. + assert_eq!(usable_build_output(&root, true), None); + } + + #[test] + fn stale_or_mismatched_build_output_is_rejected() { + let (outer, root) = tree(); + // A tree file written after the dsc makes the artifact stale. The + // sleep crosses the coarse clock tick mtimes are stamped with, so + // the control file is strictly newer than the dsc. + std::fs::write(dsc_of(&outer), "dummy dsc").unwrap(); + std::thread::sleep(std::time::Duration::from_millis(10)); + std::fs::write(root.join("debian/control"), "Source: pkg\n").unwrap(); + assert_eq!(usable_build_output(&root, false), None); + + // A dsc of a different version does not represent this tree. + let old_dsc = outer.path().join("pkg_0.9-1.dsc"); + std::fs::write(&old_dsc, "dummy dsc").unwrap(); + assert_eq!(usable_build_output(&root, false), None); + } + + #[test] + fn missing_build_output_is_rejected() { + let (outer, root) = tree(); + assert_eq!(usable_build_output(&root, false), None); + assert!(!dsc_of(&outer).exists()); + } + + #[test] + fn orig_tarballs_are_linked_for_packing() { + let (outer, root) = tree(); + std::fs::write(outer.path().join("pkg_1.0.orig.tar.xz"), "orig").unwrap(); + std::fs::write(outer.path().join("pkg_1.0.orig-data.tar.gz"), "comp").unwrap(); + std::fs::write(outer.path().join("unrelated_1.0.orig.tar.xz"), "no").unwrap(); + std::fs::write(outer.path().join("pkg_1.0-1.dsc"), "no").unwrap(); + + let dest = tempfile::tempdir().unwrap(); + link_orig_tarballs(&root, dest.path()); + + assert!( + dest.path() + .join("pkg_1.0.orig.tar.xz") + .symlink_metadata() + .is_ok() + ); + assert!( + dest.path() + .join("pkg_1.0.orig-data.tar.gz") + .symlink_metadata() + .is_ok() + ); + assert!(!dest.path().join("unrelated_1.0.orig.tar.xz").exists()); + // The dsc is not an orig tarball and must not be linked. + assert!(!dest.path().join("pkg_1.0-1.dsc").exists()); + } +} diff --git a/src/main.rs b/src/main.rs index 3f1aeaa..df28395 100644 --- a/src/main.rs +++ b/src/main.rs @@ -217,6 +217,63 @@ fn main() { .arg(arg!(--verbose "Show raw tool output instead of the live build view").required(false) .long_help("Show raw tool output instead of the live build view.\nAlso implied by RUST_LOG=debug for pkh's own logs.")), ) + .subcommand( + Command::new("lint") + .about("Lint the package (lintian wrapper + pkh-native checks)") + .arg(arg!([path] "Source tree to lint (default: the current directory)").required(false)) + .arg(arg!(-d --dist "Target distribution (debian, ubuntu)").required(false)) + .arg(arg!(-s --series "Target distribution series").required(false)) + .arg(arg!(--native "Run pkh-native checks only, without the lintian wrapper").required(false)) + .arg(arg!(--info "Show tag explanations under each finding").required(false)) + .arg( + clap::Arg::new("display_info") + .long("display-info") + .action(clap::ArgAction::SetTrue) + .help("Also display info-level tags (I:)"), + ) + .arg(arg!(--pedantic "Also display pedantic tags (P:)").required(false)) + .arg(arg!(--experimental "Also display experimental tags (X:)").required(false)) + .arg( + clap::Arg::new("show_overrides") + .long("show-overrides") + .action(clap::ArgAction::SetTrue) + .help("Also display overridden tags (O:)"), + ) + .arg( + clap::Arg::new("fail_on") + .long("fail-on") + .value_name("LEVELS") + .help("Comma-separated severities failing the run: error, warning, info, pedantic, experimental, override (default: error)"), + ) + .arg( + clap::Arg::new("suppress_tags") + .long("suppress-tags") + .value_name("LIST") + .help("Comma-separated tag names to ignore for this run"), + ) + .arg( + clap::Arg::new("check") + .long("check") + .value_name("NAME") + .action(clap::ArgAction::Append) + .help("Run only this pkh-native check (can be specified multiple times)"), + ) + .arg(arg!(--repack "Ignore existing pkh build output and pack the tree fresh for linting").required(false)) + .arg(arg!(--json "Emit the report as JSON").required(false)) + .arg( + clap::Arg::new("color") + .long("color") + .value_name("WHEN") + .value_parser(["auto", "always", "never"]) + .help("Colorize the report: auto, always or never (default: auto)"), + ) + .arg( + clap::Arg::new("list_tags") + .long("list-tags") + .action(clap::ArgAction::SetTrue) + .help("Print the pkh-native tag catalog and exit"), + ), + ) .subcommand( Command::new("context") .about("Manage contexts") @@ -785,6 +842,77 @@ fn main() { } } } + Some(("lint", sub_matches)) => { + if sub_matches.get_flag("list_tags") { + print!("{}", pkh::lint::list_tags()); + std::process::exit(0); + } + + let path = sub_matches + .get_one::("path") + .map(std::path::PathBuf::from) + .unwrap_or_else(current_dir_or_exit); + let fail_on = match pkh::lint::output::parse_fail_on( + sub_matches + .get_one::("fail_on") + .map(String::as_str) + .unwrap_or("error"), + ) { + Ok(levels) => levels, + Err(e) => { + error!("{}", e); + std::process::exit(2); + } + }; + let options = pkh::lint::LintOptions { + path, + native: sub_matches.get_flag("native"), + fail_on, + info: sub_matches.get_flag("info"), + display_info: sub_matches.get_flag("display_info"), + pedantic: sub_matches.get_flag("pedantic"), + experimental: sub_matches.get_flag("experimental"), + show_overrides: sub_matches.get_flag("show_overrides"), + suppress_tags: sub_matches + .get_one::("suppress_tags") + .map(|list| { + list.split(',') + .map(str::trim) + .filter(|tag| !tag.is_empty()) + .map(str::to_string) + .collect() + }) + .unwrap_or_default(), + only_checks: sub_matches + .get_many::("check") + .map(|values| values.cloned().collect()) + .unwrap_or_default(), + repack: sub_matches.get_flag("repack"), + json: sub_matches.get_flag("json"), + color: match sub_matches.get_one::("color").map(String::as_str) { + Some("always") => pkh::lint::output::ColorMode::Always, + Some("never") => pkh::lint::output::ColorMode::Never, + _ => pkh::lint::output::ColorMode::Auto, + }, + dist: sub_matches.get_one::("dist").cloned(), + series: sub_matches.get_one::("series").cloned(), + }; + + match pkh::lint::run(&options) { + Ok(report) => { + if options.json { + println!("{}", pkh::lint::output::render_json(&report, &options)); + } else { + print!("{}", pkh::lint::output::render_text(&report, &options)); + } + std::process::exit(pkh::lint::output::exit_code(&report, &options)); + } + Err(e) => { + error!("{}", e); + std::process::exit(2); + } + } + } _ => unreachable!("Exhausted list of subcommands and subcommand_required prevents `None`"), } }