From 4b87dcffad898bfc530071da824e444c130104b7 Mon Sep 17 00:00:00 2001 From: Jussi Elo Date: Thu, 1 Oct 2026 09:01:00 +0000 Subject: [PATCH 1/4] chore(hooks): close hawkeye pin/coverage gaps flagged on #474 Guard CONTRIBUTING.md's hawkeye@7.0.0 pin against drifting from ci.yml's HAWKEYE_VERSION (mirrors the existing cargo-about pin guard in xtask/src/tpn.rs, as a test-only module since hawkeye has no xtask subcommand to host a production constant). Also widen the local license-headers hook's types_or to include markdown, so a markdown-only commit no longer skips the local hook that CI's hawkeye check already enforces against **/*.md. Signed-off-by: Jussi Elo --- .pre-commit-config.yaml | 2 +- xtask/src/hawkeye_pin.rs | 64 ++++++++++++++++++++++++++++++++++++++++ xtask/src/main.rs | 1 + 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 xtask/src/hawkeye_pin.rs diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index de20d2cb5..c7bc00abc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -91,7 +91,7 @@ repos: name: license header check (hawkeye) entry: hawkeye check --config licenserc.toml language: system - types_or: [rust, python, shell] + types_or: [rust, python, shell, markdown] pass_filenames: false groups: [local-tools] # runs in the dedicated `license-headers` CI job (pinned prebuilt binary, no Action) diff --git a/xtask/src/hawkeye_pin.rs b/xtask/src/hawkeye_pin.rs new file mode 100644 index 000000000..5b0e7965a --- /dev/null +++ b/xtask/src/hawkeye_pin.rs @@ -0,0 +1,64 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Guards against the hawkeye version documented in CONTRIBUTING.md drifting +//! from the version `ci.yml`'s `license-headers` job installs. +//! +//! CONTRIBUTING.md tells contributors to `cargo install hawkeye@ +//! --locked` so the local pre-commit hook produces the same verdict as CI's +//! pinned prebuilt binary. Nothing else ties those two strings together, so a +//! version bump in one and not the other would silently let contributors pass +//! locally on a hawkeye build that can still fail in CI (or vice versa). This +//! checks only that the two version strings agree — it does not check the +//! SHA256 pin, the `licenserc.toml` rules, or which file types the local hook +//! scans (see `.pre-commit-config.yaml`'s `license-headers` hook for that). + +#[cfg(test)] +mod tests { + use std::path::{Path, PathBuf}; + + /// hawkeye version CONTRIBUTING.md tells contributors to install. Kept in + /// sync with `ci.yml`'s `HAWKEYE_VERSION` by `pin_matches_ci_workflow` + /// below. + const HAWKEYE_VERSION: &str = "7.0.0"; + + fn repo_root() -> PathBuf { + // CARGO_MANIFEST_DIR is the xtask/ crate dir; its parent is the repo + // root (same idiom as verify_pinned_keys::repo_root). + Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("xtask crate has a parent directory") + .to_path_buf() + } + + /// The documented pin and the version CI installs must never drift: if + /// they do, a contributor's local hook can bless a header hawkeye's CI + /// build would reject, or vice versa. + #[test] + fn pin_matches_ci_workflow() { + let root = repo_root(); + + let contributing = root.join("CONTRIBUTING.md"); + let contributing_text = std::fs::read_to_string(&contributing) + .unwrap_or_else(|e| panic!("reading {}: {e}", contributing.display())); + let documented = format!("cargo install hawkeye@{HAWKEYE_VERSION} --locked"); + assert!( + contributing_text.contains(&documented), + "CONTRIBUTING.md must instruct `{documented}` to match HAWKEYE_VERSION in \ + hawkeye_pin.rs" + ); + + let ci = root.join(".github/workflows/ci.yml"); + let ci_text = std::fs::read_to_string(&ci) + .unwrap_or_else(|e| panic!("reading {}: {e}", ci.display())); + // ci.yml's env var is `v`-prefixed (`HAWKEYE_VERSION: v7.0.0`); CONTRIBUTING.md's + // prose is not (`hawkeye@7.0.0`). Both are formatted from the same bare + // HAWKEYE_VERSION constant above, with the `v` added only on this side. + let installed = format!("HAWKEYE_VERSION: v{HAWKEYE_VERSION}"); + assert!( + ci_text.contains(&installed), + "ci.yml must install `{installed}` to match HAWKEYE_VERSION in hawkeye_pin.rs" + ); + } +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 37b6d89bd..b05a422df 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -18,6 +18,7 @@ mod e2e; mod e2e_prewarm; mod e2e_report; mod env_mutation_contract; +mod hawkeye_pin; mod manifest; mod package; mod paths; From 676485931159aaad2e5088bbee73d8dfe9b7d3bd Mon Sep 17 00:00:00 2001 From: Jussi Elo Date: Fri, 2 Oct 2026 15:00:49 +0000 Subject: [PATCH 2/4] Scope the hawkeye pin guard to ci.yml's env block The guard's whole-file substring match on ci.yml could false-pass on a stray mention elsewhere in the file (a comment, another job), and was unanchored, so a suffixed version (v7.0.0-rc1) would still satisfy a check meant for v7.0.0. Extract the installed version from the license-headers job's env mapping instead, scoped and anchored, and derive the CONTRIBUTING.md assertion from it directly rather than duplicating the version into a third constant. Signed-off-by: Jussi Elo --- xtask/src/hawkeye_pin.rs | 91 +++++++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 20 deletions(-) diff --git a/xtask/src/hawkeye_pin.rs b/xtask/src/hawkeye_pin.rs index 5b0e7965a..1306b286c 100644 --- a/xtask/src/hawkeye_pin.rs +++ b/xtask/src/hawkeye_pin.rs @@ -18,11 +18,6 @@ mod tests { use std::path::{Path, PathBuf}; - /// hawkeye version CONTRIBUTING.md tells contributors to install. Kept in - /// sync with `ci.yml`'s `HAWKEYE_VERSION` by `pin_matches_ci_workflow` - /// below. - const HAWKEYE_VERSION: &str = "7.0.0"; - fn repo_root() -> PathBuf { // CARGO_MANIFEST_DIR is the xtask/ crate dir; its parent is the repo // root (same idiom as verify_pinned_keys::repo_root). @@ -32,33 +27,89 @@ mod tests { .to_path_buf() } + /// Strip a trailing `# …` comment from a YAML line (same idiom as + /// `workflow_contract.rs`'s `strip_comment`). + fn strip_comment(line: &str) -> &str { + line.split_once(" #").map_or(line, |(v, _)| v) + } + + /// Extract one top-level job's complete YAML block by its job id. A local + /// copy of `workflow_contract.rs`'s `job_block`: that extractor is private + /// to its own `#[cfg(test)]` mod, and this guard stays in its own file + /// (see the module doc above) rather than reaching into it. + fn job_block<'a>(text: &'a str, job: &str) -> &'a str { + let marker = format!(" {job}:\n"); + let start = text + .find(&marker) + .unwrap_or_else(|| panic!("workflow defines job `{job}`")); + let rest = &text[start + marker.len()..]; + let end = rest + .match_indices("\n ") + .find_map(|(i, _)| { + rest[i + 1..] + .lines() + .next() + .is_some_and(|line| line.starts_with(" ") && !line.starts_with(" ")) + .then_some(i) + }) + .unwrap_or(rest.len()); + &rest[..end] + } + + /// The anchored value of a scalar `key: value` entry directly under a + /// job's `env:` mapping. + /// + /// Scoped to the `env:` block rather than a whole-file search, so a + /// mention elsewhere in the file (another job, a comment) can't + /// false-pass; and the full line value is extracted for the caller to + /// compare by equality, not matched as a substring, so a version suffix + /// (e.g. `v7.0.0-rc1`) can't satisfy a check meant for `v7.0.0`. + fn job_env_value(block: &str, key: &str) -> String { + let marker = " env:"; + let lines: Vec<&str> = block.lines().collect(); + let env_start = lines + .iter() + .position(|line| *line == marker) + .unwrap_or_else(|| panic!("job has no `env:` mapping")); + let key_marker = format!("{key}:"); + lines[env_start + 1..] + .iter() + .take_while(|line| line.starts_with(" ")) + .find_map(|line| strip_comment(line).trim().strip_prefix(&key_marker)) + .unwrap_or_else(|| panic!("job's `env:` has no `{key}` entry")) + .trim() + .to_owned() + } + /// The documented pin and the version CI installs must never drift: if /// they do, a contributor's local hook can bless a header hawkeye's CI /// build would reject, or vice versa. + /// + /// The version lives in exactly one place — `ci.yml`'s `license-headers` + /// job — and is read from there rather than duplicated into a constant + /// here, so a bump only ever needs the two edits this test actually + /// guards (ci.yml and CONTRIBUTING.md). #[test] fn pin_matches_ci_workflow() { let root = repo_root(); + let ci = root.join(".github/workflows/ci.yml"); + let ci_text = std::fs::read_to_string(&ci) + .unwrap_or_else(|e| panic!("reading {}: {e}", ci.display())); + let job = job_block(&ci_text, "license-headers"); + let installed = job_env_value(job, "HAWKEYE_VERSION"); + // ci.yml's env var is `v`-prefixed (`v7.0.0`); CONTRIBUTING.md's prose + // is not (`hawkeye@7.0.0`). + let version = installed.strip_prefix('v').unwrap_or(&installed); + let contributing = root.join("CONTRIBUTING.md"); let contributing_text = std::fs::read_to_string(&contributing) .unwrap_or_else(|e| panic!("reading {}: {e}", contributing.display())); - let documented = format!("cargo install hawkeye@{HAWKEYE_VERSION} --locked"); + let documented = format!("cargo install hawkeye@{version} --locked"); assert!( contributing_text.contains(&documented), - "CONTRIBUTING.md must instruct `{documented}` to match HAWKEYE_VERSION in \ - hawkeye_pin.rs" - ); - - let ci = root.join(".github/workflows/ci.yml"); - let ci_text = std::fs::read_to_string(&ci) - .unwrap_or_else(|e| panic!("reading {}: {e}", ci.display())); - // ci.yml's env var is `v`-prefixed (`HAWKEYE_VERSION: v7.0.0`); CONTRIBUTING.md's - // prose is not (`hawkeye@7.0.0`). Both are formatted from the same bare - // HAWKEYE_VERSION constant above, with the `v` added only on this side. - let installed = format!("HAWKEYE_VERSION: v{HAWKEYE_VERSION}"); - assert!( - ci_text.contains(&installed), - "ci.yml must install `{installed}` to match HAWKEYE_VERSION in hawkeye_pin.rs" + "CONTRIBUTING.md must instruct `{documented}` to match ci.yml's \ + license-headers job (HAWKEYE_VERSION: {installed})" ); } } From 1ce01384dd556c4809ace159b0559dd29760df7d Mon Sep 17 00:00:00 2001 From: Jussi Elo Date: Fri, 2 Oct 2026 15:14:02 +0000 Subject: [PATCH 3/4] Normalize CRLF when reading ci.yml in the hawkeye pin guard job_block's marker search is \n-literal. A Windows checkout with core.autocrlf can hand back \r\n even though the repo standardises on LF, which made the previous commit's extraction panic with "workflow defines job `license-headers`" on windows-build-and-test. Normalize the same way workflow_contract.rs's read_workflow already does. Signed-off-by: Jussi Elo --- xtask/src/hawkeye_pin.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/xtask/src/hawkeye_pin.rs b/xtask/src/hawkeye_pin.rs index 1306b286c..aa301f84e 100644 --- a/xtask/src/hawkeye_pin.rs +++ b/xtask/src/hawkeye_pin.rs @@ -94,8 +94,13 @@ mod tests { let root = repo_root(); let ci = root.join(".github/workflows/ci.yml"); + // Normalized the same way as workflow_contract.rs's `read_workflow`: + // `job_block`'s marker search is `\n`-literal, and a Windows checkout + // with `core.autocrlf` can hand back `\r\n` even though the repo + // standardises on LF (see .gitattributes). let ci_text = std::fs::read_to_string(&ci) - .unwrap_or_else(|e| panic!("reading {}: {e}", ci.display())); + .unwrap_or_else(|e| panic!("reading {}: {e}", ci.display())) + .replace("\r\n", "\n"); let job = job_block(&ci_text, "license-headers"); let installed = job_env_value(job, "HAWKEYE_VERSION"); // ci.yml's env var is `v`-prefixed (`v7.0.0`); CONTRIBUTING.md's prose From fbf2084c147a5dfea7afe1f755fdb0ed97f43c63 Mon Sep 17 00:00:00 2001 From: Jussi Elo Date: Mon, 5 Oct 2026 07:47:30 +0000 Subject: [PATCH 4/4] Fold hawkeye pin test into workflow_contract.rs Removes the test-only hawkeye_pin module in favor of a test in workflow_contract.rs that reuses its already-fixture-tested job_block and job_mapping extractors. The standalone module's own job_block/ strip_comment were an untested, duplicated copy of those, and its job_env_value extractor claimed scoping and anchoring guarantees that no test actually exercised. Signed-off-by: Jussi Elo --- xtask/src/hawkeye_pin.rs | 120 --------------------------------- xtask/src/main.rs | 1 - xtask/src/workflow_contract.rs | 35 ++++++++++ 3 files changed, 35 insertions(+), 121 deletions(-) delete mode 100644 xtask/src/hawkeye_pin.rs diff --git a/xtask/src/hawkeye_pin.rs b/xtask/src/hawkeye_pin.rs deleted file mode 100644 index aa301f84e..000000000 --- a/xtask/src/hawkeye_pin.rs +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright © Advanced Micro Devices, Inc., or its affiliates. -// -// SPDX-License-Identifier: MIT - -//! Guards against the hawkeye version documented in CONTRIBUTING.md drifting -//! from the version `ci.yml`'s `license-headers` job installs. -//! -//! CONTRIBUTING.md tells contributors to `cargo install hawkeye@ -//! --locked` so the local pre-commit hook produces the same verdict as CI's -//! pinned prebuilt binary. Nothing else ties those two strings together, so a -//! version bump in one and not the other would silently let contributors pass -//! locally on a hawkeye build that can still fail in CI (or vice versa). This -//! checks only that the two version strings agree — it does not check the -//! SHA256 pin, the `licenserc.toml` rules, or which file types the local hook -//! scans (see `.pre-commit-config.yaml`'s `license-headers` hook for that). - -#[cfg(test)] -mod tests { - use std::path::{Path, PathBuf}; - - fn repo_root() -> PathBuf { - // CARGO_MANIFEST_DIR is the xtask/ crate dir; its parent is the repo - // root (same idiom as verify_pinned_keys::repo_root). - Path::new(env!("CARGO_MANIFEST_DIR")) - .parent() - .expect("xtask crate has a parent directory") - .to_path_buf() - } - - /// Strip a trailing `# …` comment from a YAML line (same idiom as - /// `workflow_contract.rs`'s `strip_comment`). - fn strip_comment(line: &str) -> &str { - line.split_once(" #").map_or(line, |(v, _)| v) - } - - /// Extract one top-level job's complete YAML block by its job id. A local - /// copy of `workflow_contract.rs`'s `job_block`: that extractor is private - /// to its own `#[cfg(test)]` mod, and this guard stays in its own file - /// (see the module doc above) rather than reaching into it. - fn job_block<'a>(text: &'a str, job: &str) -> &'a str { - let marker = format!(" {job}:\n"); - let start = text - .find(&marker) - .unwrap_or_else(|| panic!("workflow defines job `{job}`")); - let rest = &text[start + marker.len()..]; - let end = rest - .match_indices("\n ") - .find_map(|(i, _)| { - rest[i + 1..] - .lines() - .next() - .is_some_and(|line| line.starts_with(" ") && !line.starts_with(" ")) - .then_some(i) - }) - .unwrap_or(rest.len()); - &rest[..end] - } - - /// The anchored value of a scalar `key: value` entry directly under a - /// job's `env:` mapping. - /// - /// Scoped to the `env:` block rather than a whole-file search, so a - /// mention elsewhere in the file (another job, a comment) can't - /// false-pass; and the full line value is extracted for the caller to - /// compare by equality, not matched as a substring, so a version suffix - /// (e.g. `v7.0.0-rc1`) can't satisfy a check meant for `v7.0.0`. - fn job_env_value(block: &str, key: &str) -> String { - let marker = " env:"; - let lines: Vec<&str> = block.lines().collect(); - let env_start = lines - .iter() - .position(|line| *line == marker) - .unwrap_or_else(|| panic!("job has no `env:` mapping")); - let key_marker = format!("{key}:"); - lines[env_start + 1..] - .iter() - .take_while(|line| line.starts_with(" ")) - .find_map(|line| strip_comment(line).trim().strip_prefix(&key_marker)) - .unwrap_or_else(|| panic!("job's `env:` has no `{key}` entry")) - .trim() - .to_owned() - } - - /// The documented pin and the version CI installs must never drift: if - /// they do, a contributor's local hook can bless a header hawkeye's CI - /// build would reject, or vice versa. - /// - /// The version lives in exactly one place — `ci.yml`'s `license-headers` - /// job — and is read from there rather than duplicated into a constant - /// here, so a bump only ever needs the two edits this test actually - /// guards (ci.yml and CONTRIBUTING.md). - #[test] - fn pin_matches_ci_workflow() { - let root = repo_root(); - - let ci = root.join(".github/workflows/ci.yml"); - // Normalized the same way as workflow_contract.rs's `read_workflow`: - // `job_block`'s marker search is `\n`-literal, and a Windows checkout - // with `core.autocrlf` can hand back `\r\n` even though the repo - // standardises on LF (see .gitattributes). - let ci_text = std::fs::read_to_string(&ci) - .unwrap_or_else(|e| panic!("reading {}: {e}", ci.display())) - .replace("\r\n", "\n"); - let job = job_block(&ci_text, "license-headers"); - let installed = job_env_value(job, "HAWKEYE_VERSION"); - // ci.yml's env var is `v`-prefixed (`v7.0.0`); CONTRIBUTING.md's prose - // is not (`hawkeye@7.0.0`). - let version = installed.strip_prefix('v').unwrap_or(&installed); - - let contributing = root.join("CONTRIBUTING.md"); - let contributing_text = std::fs::read_to_string(&contributing) - .unwrap_or_else(|e| panic!("reading {}: {e}", contributing.display())); - let documented = format!("cargo install hawkeye@{version} --locked"); - assert!( - contributing_text.contains(&documented), - "CONTRIBUTING.md must instruct `{documented}` to match ci.yml's \ - license-headers job (HAWKEYE_VERSION: {installed})" - ); - } -} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index b05a422df..37b6d89bd 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -18,7 +18,6 @@ mod e2e; mod e2e_prewarm; mod e2e_report; mod env_mutation_contract; -mod hawkeye_pin; mod manifest; mod package; mod paths; diff --git a/xtask/src/workflow_contract.rs b/xtask/src/workflow_contract.rs index a4f1091ac..81eaf0ad9 100644 --- a/xtask/src/workflow_contract.rs +++ b/xtask/src/workflow_contract.rs @@ -2062,6 +2062,41 @@ esac ); } + /// The hawkeye version CONTRIBUTING.md documents and the version `ci.yml`'s + /// `license-headers` job installs must never drift: if they do, a + /// contributor's local pre-commit hook can bless a header hawkeye's CI + /// build would reject, or vice versa. + /// + /// The version lives in exactly one place — `ci.yml`'s `license-headers` + /// job's `env:` mapping — and is read from there with the extractors this + /// file already has fixture tests for (`job_block`, `job_mapping`), rather + /// than duplicated into a constant, so a bump only ever needs the two + /// edits this test actually guards (ci.yml and CONTRIBUTING.md). + #[test] + fn hawkeye_pin_matches_ci_workflow() { + let ci = read_workflow("ci.yml"); + let job = job_block(&ci, "license-headers"); + let env = job_mapping(job, "env"); + let installed = env + .get("HAWKEYE_VERSION") + .unwrap_or_else(|| panic!("license-headers job has no HAWKEYE_VERSION entry")); + // ci.yml's env var is `v`-prefixed (`v7.0.0`); CONTRIBUTING.md's prose + // is not (`hawkeye@7.0.0`). + let version = installed + .strip_prefix('v') + .unwrap_or_else(|| panic!("HAWKEYE_VERSION `{installed}` must be `v`-prefixed")); + + let contributing = repo_root().join("CONTRIBUTING.md"); + let contributing_text = std::fs::read_to_string(&contributing) + .unwrap_or_else(|e| panic!("reading {}: {e}", contributing.display())); + let documented = format!("cargo install hawkeye@{version} --locked"); + assert!( + contributing_text.contains(&documented), + "CONTRIBUTING.md must instruct `{documented}` to match ci.yml's \ + license-headers job (HAWKEYE_VERSION: {installed})" + ); + } + // Extractor guards: prove the helpers actually parse multiline forms, so the // contract tests above can't silently false-pass on a shape they don't handle. #[test]