compliance: Add support for checking singular commit (#54369)

This helps with two things: Testing changes locally against real commits
as well as laying the groundwork for making things less convoluted for
the GitHub worker.

Also removes some useless wrapping left from an earlier direction.

Release Notes:

- N/A

.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 }}

.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 }}

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<E: Into<anyhow::Error>> From<E> for ReviewFailure {
     }
 }
 
-pub struct Reporter<'a> {
+pub struct Reporter {
     commits: CommitList,
-    github_client: &'a GithubClient,
+    github_client: Rc<dyn GithubApiClient>,
 }
 
-impl<'a> Reporter<'a> {
-    pub fn new(commits: CommitList, github_client: &'a GithubClient) -> Self {
+impl Reporter {
+    pub fn new(commits: CommitList, github_client: Rc<dyn GithubApiClient>) -> Self {
         Self {
             commits,
             github_client,
         }
     }
 
+    pub async fn result_for_commit(
+        commit: CommitDetails,
+        github_client: Rc<dyn GithubApiClient>,
+    ) -> 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
         }
     }

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<Self, Self::Err> {
+        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<Item = String> {
+        [
+            "log".to_string(),
+            format!("--pretty=format:{}", CommitDetails::FORMAT_STRING),
+            format!("{sha}~1..{sha}", sha = self.sha),
+        ]
+    }
+}
+
 pub struct CommitsFromVersionToVersion {
     version_tag: VersionTag,
     branch: String,

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<dyn GithubApiClient>,
-}
-
-impl GithubClient {
-    pub fn new(api: Rc<dyn GithubApiClient>) -> Self {
-        Self { api }
-    }
-
-    #[cfg(feature = "octo-client")]
-    pub async fn for_app_in_repo(app_id: u64, app_private_key: &str, org: &str) -> Result<Self> {
-        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 _;

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<String>,
 }
 
-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 {}",

tooling/xtask/src/tasks/workflows/release.rs

@@ -185,7 +185,7 @@ pub(crate) fn add_compliance_steps(
     fn run_compliance_check(context: &ComplianceContext) -> (Step<Run>, 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""# },
             }