diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml new file mode 100644 index 00000000000..ae143ac95a1 --- /dev/null +++ b/.github/workflows/changelog.yml @@ -0,0 +1,49 @@ +name: changelog + +on: + pull_request: + paths: + - "CHANGELOG.md" + - "xtask/**/*" + types: + - opened + - synchronize + - reopened + - labeled + - unlabeled + +env: + # + # Dependency versioning + # + + # This is the MSRV used by `wgpu` itself and all surrounding infrastructure. + REPO_MSRV: "1.88" + + # + # Environment variables + # + + CARGO_INCREMENTAL: false + CARGO_TERM_COLOR: always + RUST_LOG: info + RUST_BACKTRACE: "1" + CACHE_SUFFIX: c # cache busting + +jobs: + changelog: + timeout-minutes: 2 + + name: Check changelog for errors + runs-on: ubuntu-latest + + steps: + - name: Checkout repo + uses: actions/checkout@v5 + with: + fetch-depth: 0 + + # NOTE: Keep label name(s) in sync. with `xtask`'s implementation. + - name: Run `cargo xtask changelog …` + run: | + cargo xtask changelog "origin/${{ github.event.pull_request.base.ref }}" ${{ contains(github.event.pull_request.labels.*.name, 'changelog: released entry changed') && '--allow-released-changes' || '' }} diff --git a/xtask/src/changelog.rs b/xtask/src/changelog.rs new file mode 100644 index 00000000000..6b30dc4dc1f --- /dev/null +++ b/xtask/src/changelog.rs @@ -0,0 +1,567 @@ +use pico_args::Arguments; +use xshell::Shell; + +pub(crate) fn check_changelog(shell: Shell, mut args: Arguments) -> anyhow::Result<()> { + const CHANGELOG_PATH_RELATIVE: &str = "./CHANGELOG.md"; + let allow_released_changes = args.contains("--allow-released-changes"); + + let from_branch = args + .free_from_str() + .ok() + .unwrap_or_else(|| "trunk".to_owned()); + let to_commit: Option = args.free_from_str().ok(); + + let from_commit = shell + .cmd("git") + .args(["merge-base", "--fork-point", &from_branch]) + .args(to_commit.as_ref()) + .read() + .unwrap(); + + let diff = shell + .cmd("git") + .args(["diff", &from_commit]) + // NOTE: If `to_commit` is not specified, we compare against the working tree, instead of + // between commits. + .args(to_commit.as_ref()) + .args(["--", CHANGELOG_PATH_RELATIVE]) + .read() + .unwrap(); + + // NOTE: If `to_commit` is not specified, we fetch from the file system, instead of `git show`. + let changelog_contents = if let Some(to_commit) = to_commit.as_ref() { + shell + .cmd("git") + .arg("show") + .arg(format!("{to_commit}:{CHANGELOG_PATH_RELATIVE}")) + .arg("--") + .read() + .unwrap() + } else { + shell.read_file(CHANGELOG_PATH_RELATIVE).unwrap() + }; + + let mut failed = false; + + let hunks_in_a_released_section = hunks_in_a_released_section(&changelog_contents, &diff); + log::info!( + "# of hunks in a released section of `{CHANGELOG_PATH_RELATIVE}`: {}", + hunks_in_a_released_section.len() + ); + if !hunks_in_a_released_section.is_empty() { + failed = true; + + #[expect(clippy::uninlined_format_args)] + { + eprintln!( + "Found hunk(s) in released sections of `{}`, which we don't want:\n", + CHANGELOG_PATH_RELATIVE, + ); + } + + for hunk in &hunks_in_a_released_section { + eprintln!("{hunk}"); + } + } + + if failed { + #[expect(clippy::uninlined_format_args)] + let msg = format!( + "one or more checks against `{}` failed; see above for details", + CHANGELOG_PATH_RELATIVE, + ); + if allow_released_changes { + log::warn!("{msg}"); + Ok(()) + } else { + Err(anyhow::Error::msg(msg)) + } + } else { + Ok(()) + } +} + +/// Given some `changelog_contents` (in Markdown) containing the full end state of the provided +/// `diff` (in [unified diff format]), return all hunks that are (1) below a `## Unreleased` section +/// _and_ (2) above all other second-level (i.e., `## …`) headings. +/// +/// [unified diff format]: https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html +/// +/// This function makes a few assumptions that are necessary to uphold for correctness, in the +/// interest of a simple implementation: +/// +/// - The provided `diff`'s end state _must_ correspond to `changelog_contents`. +/// - The provided `diff` must _only_ contain a single entry for the file containing +/// `changelog_contents`. using hunk information to compare against `changelog_contents`. +/// +/// Failing to uphold these assumptons is not unsafe, but will yield incorrect results. +fn hunks_in_a_released_section<'a>(changelog_contents: &str, diff: &'a str) -> Vec<&'a str> { + let mut changelog_lines = changelog_contents.lines(); + + let changelog_unreleased_line_num = + changelog_lines.position(|l| l == "## Unreleased").unwrap() as u64; + + let changelog_first_release_section_line_num = changelog_unreleased_line_num + + 1 + + changelog_lines.position(|l| l.starts_with("## ")).unwrap() as u64; + + let hunks = { + let first_hunk_match = diff.match_indices("\n@@").next(); + let Some((first_hunk_idx, _)) = first_hunk_match else { + log::info!("no diff found"); + return vec![]; + }; + SplitPrefixInclusive::new("\n@@", &diff[first_hunk_idx..]).map(|s| &s['\n'.len_utf8()..]) + }; + let hunks_in_a_released_section = hunks + .filter(|hunk| { + let (hunk_header, hunk_contents) = hunk.split_once('\n').unwrap(); + + // Reference: This is of the format `@@ -86,6 +88,10 @@ …`. + let hunk_header_re = regex_lite::Regex::new(r"@@ -\d+,\d+ \+(\d+),\d+ @@.*").unwrap(); + let captures = hunk_header_re.captures_at(hunk_header, 0).unwrap(); + let post_change_hunk_start_offset = + captures.get(1).unwrap().as_str().parse::().unwrap(); + + let lines_until_first_change = hunk_contents + .lines() + .take_while(|l| l.starts_with(' ')) + .count() as u64; + + let first_hunk_change_start_offset = + post_change_hunk_start_offset + lines_until_first_change; + + first_hunk_change_start_offset >= changelog_first_release_section_line_num + }) + .collect::>(); + + hunks_in_a_released_section +} + +struct SplitPrefixInclusive<'haystack, 'prefix> { + haystack: Option<&'haystack str>, + prefix: &'prefix str, + current_pos: usize, +} + +impl<'haystack, 'prefix> SplitPrefixInclusive<'haystack, 'prefix> { + pub fn new(prefix: &'prefix str, haystack: &'haystack str) -> Self { + assert!(haystack.starts_with(prefix)); + Self { + haystack: Some(haystack), + prefix, + current_pos: 0, + } + } +} + +impl<'haystack> Iterator for SplitPrefixInclusive<'haystack, '_> { + type Item = &'haystack str; + + fn next(&mut self) -> Option { + let remaining = &self.haystack?[self.current_pos..]; + + let prefix_len = self.prefix.len(); + + // NOTE: We've guaranteed that the prefix is always at the start of what remains. So, skip + // the first match manually, and adjust match indices by `prefix_len` later. + let to_search = &remaining[prefix_len..]; + + match to_search.match_indices(self.prefix).next() { + None => { + self.haystack = None; + Some(remaining) + } + Some((idx, _match)) => { + let length = idx + prefix_len; + self.current_pos += length; + Some(&remaining[..length]) + } + } + } +} + +#[cfg(test)] +mod test_split_prefix_inclusive { + #[collapse_debuginfo(yes)] + macro_rules! assert_chunks { + ($prefix: expr, $haystack: expr, $expected: expr $(,)?) => { + assert_eq!( + super::SplitPrefixInclusive::new($prefix, $haystack).collect::>(), + $expected.into_iter().collect::>(), + ); + }; + } + + #[test] + fn it_works() { + assert_chunks! { + "\n@@", + " +@@ -1,4 +1,5 @@ + + +## Unreleased + +## Recently released + +- This change actually went into the release. +- This change was added after release, reject me! + +## An older release + +- Yada yada. +", + "\ +--- a/CHANGELOG.md ++++ b/CHANGELOG.md +@@ -5,6 +5,7 @@ + ## Recently released +\u{0020} + - This change actually went into the release. ++- This change was added after release, reject me! +\u{0020} + ## An older release +", + [ + "\ +@@ -5,6 +5,7 @@ + ## Recently released +\u{0020} + - This change actually went into the release. ++- This change was added after release, reject me! +\u{0020} + ## An older release +", + ], + } + } + + #[test] + fn change_in_unreleased_not_rejected() { + assert_released_section_changes! { + "\ + + +## Unreleased + +- This change was due to the valiant efforts of a contributor. + +## Recently released + +- This change actually went into the release. + +## An older release + +- Yada yada. +", + "\ +--- a/CHANGELOG.md ++++ b/CHANGELOG.md +@@ -2,6 +2,8 @@ +\u{0020} + ## Unreleased +\u{0020} ++- This change was due to the valiant efforts of a contributor. ++ + ## Recently released +\u{0020} + - This change actually went into the release. +", + [], + } + } + + #[test] + fn change_above_unreleased_not_rejected() { + assert_released_section_changes! { + "\ + + + +## Unreleased + +- This change was due to the valiant efforts of a contributor. + +## Recently released + +- This change actually went into the release. + +## An older release + +- Yada yada. +", + "\ +--- a/CHANGELOG.md ++++ b/CHANGELOG.md +@@ -1,4 +1,5 @@ + ++ +\u{0020} + ## Unreleased +\u{0020} +", + [], + } + } + + #[test] + fn all_reject_and_not_reject_cases_at_once() { + assert_released_section_changes! { + "\ + + +## Unreleased + +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- This change should be accepted. + +## Recently released + +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- Pad out some changes here so we force multiple hunks in a diff. +- This change was added after release, reject me! + +## An older release + +- Yada yada. +", + "\ +--- ../CHANGELOG.md ++++ ../CHANGELOG.md +@@ -1,4 +1,5 @@ + + +## Unreleased + +- This change was due to the valiant efforts of a contributor. + +## Recently released + +- This change actually went into the release. +- terribly + long + entry + please + save + me + +## An older release + +- Yada yada. +", + "\ +diff --git a/../CHANGELOG-old.md b/../CHANGELOG-new.md +index a6bf3614a..c766d7225 100644 +--- a/../CHANGELOG-old.md ++++ b/../CHANGELOG-new.md +@@ -7,6 +7,12 @@ + ## Recently released +\u{0020} + - This change actually went into the release. ++- terribly ++ long ++ entry ++ please ++ save ++ me +\u{0020} + ## An older release +\u{0020} +", + [ + "\ +@@ -7,6 +7,12 @@ + ## Recently released +\u{0020} + - This change actually went into the release. ++- terribly ++ long ++ entry ++ please ++ save ++ me +\u{0020} + ## An older release +\u{0020} +", + ], + } + assert_released_section_changes! { + "\ + + +## Unreleased + +- This change was due to the valiant efforts of a contributor. + +## Recently released + +- This change actually went into the release. + +## An older release + +- Accidentally added before. +- Yada yada. +- Accidentally added after. +", + "\ +diff --git a/../CHANGELOG-old.md b/../CHANGELOG-new.md +index a6bf3614a..5c2dcdc4e 100644 +--- a/../CHANGELOG-old.md ++++ b/../CHANGELOG-new.md +@@ -10,4 +10,6 @@ +\u{0020} + ## An older release +\u{0020} ++- Accidentally added before. + - Yada yada. ++- Accidentally added after. +", + [ + "\ +@@ -10,4 +10,6 @@ +\u{0020} + ## An older release +\u{0020} ++- Accidentally added before. + - Yada yada. ++- Accidentally added after. +", + ] + } + } +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 81e0b6db521..fdf7b257b12 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -6,6 +6,7 @@ use std::process::ExitCode; use anyhow::Context; use pico_args::Arguments; +mod changelog; mod cts; mod miri; mod run_wasm; @@ -42,6 +43,17 @@ Commands: All extra arguments will be forwarded to cargo-nextest (NOT wgpu-info) + changelog [from_branch] [to_commit] + Audit changes in the `CHANGELOG.md` at the root of the repo. Ensure that: + + 1. All changes are in an `Unreleased` section. + + `` is used to determine the base of the diff to be performed. The base is set to fork point between `` and this branch. + + `` is the tip of the `git diff` that will be used for checking (1). + + --allow-released-changes Only reports issues as warnings, rather than reporting errors and forcing a non-zero exit code. + miri Run all miri-compatible tests under miri. Requires a nightly toolchain with the x86_64-unknown-linux-gnu target and miri component installed. @@ -101,6 +113,7 @@ fn main() -> anyhow::Result { shell.change_dir(String::from(env!("CARGO_MANIFEST_DIR")) + "/.."); match subcommand.as_deref() { + Some("changelog") => changelog::check_changelog(shell, args)?, Some("cts") => cts::run_cts(shell, args, passthrough_args)?, Some("run-wasm") => run_wasm::run_wasm(shell, args, passthrough_args)?, Some("miri") => miri::run_miri(shell, args)?,