diff --git a/.github/workflows/compliance_check.yml b/.github/workflows/compliance_check.yml index 144185f95ba95ba902d1239649cdcd8dc8828ef1..f662dfd7073fbe2e34dd047c0f8b9f384559dcad 100644 --- a/.github/workflows/compliance_check.yml +++ b/.github/workflows/compliance_check.yml @@ -36,7 +36,7 @@ jobs: - id: run-compliance-check name: release::add_compliance_steps::run_compliance_check run: | - cargo xtask compliance "$LATEST_TAG" --branch main --report-path "compliance-report-${GITHUB_REF_NAME}.md" + cargo xtask compliance version "$LATEST_TAG" --branch main --report-path "compliance-report-${GITHUB_REF_NAME}.md" env: GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }} GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8081955920823edad55bcaef371a6f2f15f7b386..17e121b9958db65a3f49c7e9e481153e84372673 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -309,7 +309,7 @@ jobs: - id: run-compliance-check name: release::add_compliance_steps::run_compliance_check run: | - cargo xtask compliance "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md" + cargo xtask compliance version "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md" env: GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }} GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }} @@ -678,7 +678,7 @@ jobs: - id: run-compliance-check name: release::add_compliance_steps::run_compliance_check run: | - cargo xtask compliance "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md" + cargo xtask compliance version "$GITHUB_REF_NAME" --report-path "compliance-report-${GITHUB_REF_NAME}.md" env: GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }} GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }} diff --git a/tooling/compliance/src/checks.rs b/tooling/compliance/src/checks.rs index cda37a84d2ae70de5fa5891c2dcd1d15fc2a9080..409df5394f18176139ba49a882c5754f3fa46711 100644 --- a/tooling/compliance/src/checks.rs +++ b/tooling/compliance/src/checks.rs @@ -1,11 +1,11 @@ -use std::{fmt, ops::Not as _}; +use std::{fmt, ops::Not as _, rc::Rc}; use itertools::Itertools as _; use crate::{ git::{CommitDetails, CommitList}, github::{ - CommitAuthor, GithubClient, GithubLogin, PullRequestComment, PullRequestData, + CommitAuthor, GithubApiClient, GithubLogin, PullRequestComment, PullRequestData, PullRequestReview, Repository, ReviewState, }, report::Report, @@ -87,19 +87,28 @@ impl> From for ReviewFailure { } } -pub struct Reporter<'a> { +pub struct Reporter { commits: CommitList, - github_client: &'a GithubClient, + github_client: Rc, } -impl<'a> Reporter<'a> { - pub fn new(commits: CommitList, github_client: &'a GithubClient) -> Self { +impl Reporter { + pub fn new(commits: CommitList, github_client: Rc) -> Self { Self { commits, github_client, } } + pub async fn result_for_commit( + commit: CommitDetails, + github_client: Rc, + ) -> ReviewResult { + Self::new(Default::default(), github_client) + .check_commit(&commit) + .await + } + /// Method that checks every commit for compliance pub async fn check_commit( &self, @@ -293,8 +302,8 @@ mod tests { use crate::git::{CommitDetails, CommitList, CommitSha}; use crate::github::{ - AuthorsForCommits, GithubApiClient, GithubClient, GithubLogin, GithubUser, - PullRequestComment, PullRequestData, PullRequestReview, Repository, ReviewState, + AuthorsForCommits, GithubApiClient, GithubLogin, GithubUser, PullRequestComment, + PullRequestData, PullRequestReview, Repository, ReviewState, }; use super::{Reporter, ReviewFailure, ReviewSuccess}; @@ -466,8 +475,8 @@ mod tests { commit_authors_json: self.commit_authors_json, org_members: self.org_members, }; - let client = GithubClient::new(Rc::new(mock)); - let reporter = Reporter::new(CommitList::default(), &client); + let client = Rc::new(mock); + let reporter = Reporter::new(CommitList::default(), client); reporter.check_commit(&self.commit).await } } diff --git a/tooling/compliance/src/git.rs b/tooling/compliance/src/git.rs index e2581b1c7fa79ae43ad3b01d3f755df2a7bbc508..f5b1a9c32960215aee4f0f1380fc41086667bba3 100644 --- a/tooling/compliance/src/git.rs +++ b/tooling/compliance/src/git.rs @@ -138,6 +138,18 @@ impl CommitDetails { } } +impl FromStr for CommitDetails { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + CommitList::from_str(s).and_then(|list| { + list.into_iter() + .next() + .ok_or_else(|| anyhow!("No commit found")) + }) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Committer { name: String, @@ -318,6 +330,30 @@ impl FromStr for VersionTagList { } } +pub struct InfoForCommit { + sha: String, +} + +impl InfoForCommit { + pub fn new(sha: impl ToString) -> Self { + Self { + sha: sha.to_string(), + } + } +} + +impl Subcommand for InfoForCommit { + type ParsedOutput = CommitDetails; + + fn args(&self) -> impl IntoIterator { + [ + "log".to_string(), + format!("--pretty=format:{}", CommitDetails::FORMAT_STRING), + format!("{sha}~1..{sha}", sha = self.sha), + ] + } +} + pub struct CommitsFromVersionToVersion { version_tag: VersionTag, branch: String, diff --git a/tooling/compliance/src/github.rs b/tooling/compliance/src/github.rs index 29e22c92fc831a7eb1f0e5cda9c3f93ef3a681b2..edd2e4ea2e702e4637299fa476514386d528894d 100644 --- a/tooling/compliance/src/github.rs +++ b/tooling/compliance/src/github.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::HashMap, fmt, ops::Not, rc::Rc}; +use std::{borrow::Cow, collections::HashMap, fmt, ops::Not}; use anyhow::Result; use derive_more::Deref; @@ -210,23 +210,6 @@ pub trait GithubApiClient { ) -> Result<()>; } -#[derive(Deref)] -pub struct GithubClient { - api: Rc, -} - -impl GithubClient { - pub fn new(api: Rc) -> Self { - Self { api } - } - - #[cfg(feature = "octo-client")] - pub async fn for_app_in_repo(app_id: u64, app_private_key: &str, org: &str) -> Result { - let client = OctocrabClient::new(app_id, app_private_key, org).await?; - Ok(Self::new(Rc::new(client))) - } -} - pub mod graph_ql { use anyhow::{Context as _, Result}; use itertools::Itertools as _; diff --git a/tooling/xtask/src/tasks/compliance.rs b/tooling/xtask/src/tasks/compliance.rs index fbe06383cf5115efd41d813a70336c2b3d41605a..46c40fb82c581ce6b5ae943f1db89a35d30d81e2 100644 --- a/tooling/xtask/src/tasks/compliance.rs +++ b/tooling/xtask/src/tasks/compliance.rs @@ -1,20 +1,37 @@ -use std::path::PathBuf; +use std::{path::PathBuf, rc::Rc}; use anyhow::{Context, Result}; -use clap::Parser; +use clap::{Parser, Subcommand}; use compliance::{ checks::Reporter, - git::{CommitsFromVersionToVersion, GetVersionTags, GitCommand, VersionTag}, - github::{GithubClient, Repository}, + git::{CommitsFromVersionToVersion, GetVersionTags, GitCommand, InfoForCommit, VersionTag}, + github::{GithubApiClient as _, OctocrabClient, Repository}, report::ReportReviewSummary, }; #[derive(Parser)] -pub struct ComplianceArgs { +pub(crate) struct ComplianceArgs { + #[clap(subcommand)] + mode: ComplianceMode, +} + +#[derive(Subcommand)] +pub(crate) enum ComplianceMode { + // Check compliance for all commits between two version tags + Version(VersionArgs), + // Check compliance for a single commit + Single { + // The full commit SHA to check + commit_sha: String, + }, +} + +#[derive(Parser)] +pub(crate) struct VersionArgs { #[arg(value_parser = VersionTag::parse)] // The version to be on the lookout for - pub(crate) version_tag: VersionTag, + version_tag: VersionTag, #[arg(long)] // The markdown file to write the compliance report to report_path: PathBuf, @@ -23,7 +40,7 @@ pub struct ComplianceArgs { branch: Option, } -impl ComplianceArgs { +impl VersionArgs { pub(crate) fn version_tag(&self) -> &VersionTag { &self.version_tag } @@ -39,6 +56,35 @@ async fn check_compliance_impl(args: ComplianceArgs) -> Result<()> { let app_id = std::env::var("GITHUB_APP_ID").context("Missing GITHUB_APP_ID")?; let key = std::env::var("GITHUB_APP_KEY").context("Missing GITHUB_APP_KEY")?; + let client = Rc::new( + OctocrabClient::new( + app_id.parse().context("Failed to parse app ID as int")?, + key.as_ref(), + Repository::ZED.owner(), + ) + .await?, + ); + + println!("Initialized GitHub client for app ID {app_id}"); + + let args = match args.mode { + ComplianceMode::Version(version) => version, + ComplianceMode::Single { commit_sha } => { + let commit = GitCommand::run(InfoForCommit::new(&commit_sha))?; + + return match Reporter::result_for_commit(commit, client).await { + Ok(review_success) => { + println!("Check for commit {commit_sha} succeeded. Result: {review_success}",); + Ok(()) + } + + Err(review_failure) => Err(anyhow::anyhow!( + "Check for commit {commit_sha} failed. Result: {review_failure}" + )), + }; + } + }; + let tag = args.version_tag(); let previous_version = GitCommand::run(GetVersionTags)? @@ -69,16 +115,9 @@ async fn check_compliance_impl(args: ComplianceArgs) -> Result<()> { println!("Checking commit range {range}, {} total", commits.len()); - let client = GithubClient::for_app_in_repo( - app_id.parse().context("Failed to parse app ID as int")?, - key.as_ref(), - Repository::ZED.owner(), - ) - .await?; - - println!("Initialized GitHub client for app ID {app_id}"); - - let report = Reporter::new(commits, &client).generate_report().await?; + let report = Reporter::new(commits, client.clone()) + .generate_report() + .await?; println!( "Generated report for version {}", diff --git a/tooling/xtask/src/tasks/workflows/release.rs b/tooling/xtask/src/tasks/workflows/release.rs index e3e0fb78a86208fa09d5b1b2a6697d3b2abc125d..76895a37e3da84798c9dc18db4e0bd8dedadf1a8 100644 --- a/tooling/xtask/src/tasks/workflows/release.rs +++ b/tooling/xtask/src/tasks/workflows/release.rs @@ -185,7 +185,7 @@ pub(crate) fn add_compliance_steps( fn run_compliance_check(context: &ComplianceContext) -> (Step, StepOutput) { let job = named::bash( formatdoc! {r#" - cargo xtask compliance {target} --report-path "{COMPLIANCE_REPORT_PATH}" + cargo xtask compliance version {target} --report-path "{COMPLIANCE_REPORT_PATH}" "#, target = if context.tag_source().is_some() { r#""$LATEST_TAG" --branch main"# } else { r#""$GITHUB_REF_NAME""# }, }