compliance: Reduce noisiness when checks were successful (#53515)

Finn Evers created

This will ensure we do not post a second Slack message in case the first
check was successful. We still _run_ the second check, but do not notify

Release Notes:

- N/A

Change summary

.github/workflows/compliance_check.yml                |   5 
.github/workflows/release.yml                         |  12 
tooling/xtask/src/tasks/workflows/compliance_check.rs |  43 +--
tooling/xtask/src/tasks/workflows/release.rs          | 139 +++++++++---
tooling/xtask/src/tasks/workflows/vars.rs             |   2 
5 files changed, 125 insertions(+), 76 deletions(-)

Detailed changes

.github/workflows/compliance_check.yml 🔗

@@ -34,13 +34,14 @@ jobs:
         echo "Checking compliance for $TAG"
         echo "tag=$TAG" >> "$GITHUB_OUTPUT"
     - id: run-compliance-check
-      name: compliance_check::scheduled_compliance_check::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"
       env:
-        LATEST_TAG: ${{ steps.determine-version.outputs.tag }}
         GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }}
         GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}
+        LATEST_TAG: ${{ steps.determine-version.outputs.tag }}
+      continue-on-error: true
     - name: '@actions/upload-artifact compliance-report-${GITHUB_REF_NAME}.md'
       if: always()
       uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4

.github/workflows/release.yml 🔗

@@ -307,7 +307,7 @@ jobs:
         cache: rust
         path: ~/.rustup
     - id: run-compliance-check
-      name: release::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"
       env:
@@ -328,7 +328,7 @@ jobs:
             STATUS="✅ Compliance check passed for $COMPLIANCE_TAG"
             MESSAGE=$(printf "%s\n\nReport: %s" "$STATUS" "$ARTIFACT_URL")
         else
-            STATUS="❌ Compliance check failed for $COMPLIANCE_TAG"
+            STATUS="❌ Preliminary compliance check failed (but this can still be fixed while the builds are running!) for $COMPLIANCE_TAG"
             MESSAGE=$(printf "%s\n\nReport: %s\nPRs needing review: %s" "$STATUS" "$ARTIFACT_URL" "https://github.com/zed-industries/zed/pulls?q=is%3Apr+is%3Aclosed+label%3A%22PR+state%3Aneeds+review%22")
         fi
 
@@ -340,6 +340,8 @@ jobs:
         COMPLIANCE_OUTCOME: ${{ steps.run-compliance-check.outcome }}
         COMPLIANCE_TAG: ${{ github.ref_name }}
         ARTIFACT_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}#artifacts
+    outputs:
+      outcome: ${{ steps.run-compliance-check.outputs.outcome }}
     timeout-minutes: 60
   bundle_linux_aarch64:
     needs:
@@ -641,6 +643,7 @@ jobs:
   validate_release_assets:
     needs:
     - upload_release_assets
+    - compliance_check
     runs-on: namespace-profile-2x4-ubuntu-2404
     steps:
     - name: release::validate_release_assets
@@ -673,13 +676,12 @@ jobs:
         cache: rust
         path: ~/.rustup
     - id: run-compliance-check
-      name: release::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"
       env:
         GITHUB_APP_ID: ${{ secrets.ZED_ZIPPY_APP_ID }}
         GITHUB_APP_KEY: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }}
-      continue-on-error: true
     - name: '@actions/upload-artifact compliance-report-${GITHUB_REF_NAME}.md'
       if: always()
       uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4
@@ -689,7 +691,7 @@ jobs:
         if-no-files-found: error
         overwrite: true
     - name: send_compliance_slack_notification
-      if: always()
+      if: failure() || needs.compliance_check.outputs.outcome != 'success'
       run: |
         if [ "$COMPLIANCE_OUTCOME" == "success" ]; then
             STATUS="✅ Compliance check passed for $COMPLIANCE_TAG"

tooling/xtask/src/tasks/workflows/compliance_check.rs 🔗

@@ -1,14 +1,10 @@
-use gh_workflow::{Event, Job, Run, Schedule, Step, Workflow, WorkflowDispatch};
-use indoc::formatdoc;
+use gh_workflow::{Event, Job, Schedule, Workflow, WorkflowDispatch};
 
 use crate::tasks::workflows::{
-    release::{
-        COMPLIANCE_REPORT_PATH, COMPLIANCE_STEP_ID, ComplianceContext,
-        add_compliance_notification_steps,
-    },
+    release::{ComplianceContext, add_compliance_steps},
     runners,
     steps::{self, CommonJobConditions, named},
-    vars::{self, StepOutput},
+    vars::StepOutput,
 };
 
 pub fn compliance_check() -> Workflow {
@@ -37,31 +33,20 @@ fn scheduled_compliance_check() -> steps::NamedJob {
 
     let tag_output = StepOutput::new(&determine_version_step, "tag");
 
-    fn run_compliance_check(tag: &StepOutput) -> Step<Run> {
-        named::bash(
-            formatdoc! {r#"
-                cargo xtask compliance "$LATEST_TAG" --branch main --report-path "{COMPLIANCE_REPORT_PATH}"
-                "#,
-            }
-        )
-        .id(COMPLIANCE_STEP_ID)
-        .add_env(("LATEST_TAG", tag.to_string()))
-        .add_env(("GITHUB_APP_ID", vars::ZED_ZIPPY_APP_ID))
-        .add_env(("GITHUB_APP_KEY", vars::ZED_ZIPPY_APP_PRIVATE_KEY))
-    }
-
     let job = Job::default()
         .with_repository_owner_guard()
         .runs_on(runners::LINUX_SMALL)
         .add_step(steps::checkout_repo().with_full_history())
         .add_step(steps::cache_rust_dependencies_namespace())
-        .add_step(determine_version_step)
-        .add_step(run_compliance_check(&tag_output));
-
-    named::job(add_compliance_notification_steps(
-        job,
-        ComplianceContext::Scheduled {
-            tag_source: tag_output,
-        },
-    ))
+        .add_step(determine_version_step);
+
+    named::job(
+        add_compliance_steps(
+            job,
+            ComplianceContext::Scheduled {
+                tag_source: tag_output,
+            },
+        )
+        .0,
+    )
 }

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

@@ -6,7 +6,7 @@ use crate::tasks::workflows::{
     run_tests,
     runners::{self, Arch, Platform},
     steps::{self, FluentBuilder, NamedJob, dependant_job, named, release_job},
-    vars::{self, StepOutput, assets},
+    vars::{self, JobOutput, StepOutput, assets},
 };
 
 const CURRENT_ACTION_RUN_URL: &str =
@@ -22,7 +22,7 @@ pub(crate) fn release() -> Workflow {
     let check_scripts = run_tests::check_scripts();
 
     let create_draft_release = create_draft_release();
-    let compliance = compliance_check();
+    let (non_blocking_compliance_run, job_output) = compliance_check();
 
     let bundle = ReleaseBundleJobs {
         linux_aarch64: bundle_linux(
@@ -58,7 +58,10 @@ pub(crate) fn release() -> Workflow {
     };
 
     let upload_release_assets = upload_release_assets(&[&create_draft_release], &bundle);
-    let validate_release_assets = validate_release_assets(&[&upload_release_assets]);
+    let validate_release_assets = validate_release_assets(
+        &[&upload_release_assets, &non_blocking_compliance_run],
+        job_output,
+    );
 
     let auto_release_preview = auto_release_preview(&[&validate_release_assets]);
 
@@ -93,7 +96,10 @@ pub(crate) fn release() -> Workflow {
         .add_job(windows_clippy.name, windows_clippy.job)
         .add_job(check_scripts.name, check_scripts.job)
         .add_job(create_draft_release.name, create_draft_release.job)
-        .add_job(compliance.name, compliance.job)
+        .add_job(
+            non_blocking_compliance_run.name,
+            non_blocking_compliance_run.job,
+        )
         .map(|mut workflow| {
             for job in bundle.into_jobs() {
                 workflow = workflow.add_job(job.name, job.job);
@@ -156,25 +162,65 @@ pub(crate) const COMPLIANCE_STEP_ID: &str = "run-compliance-check";
 const NEEDS_REVIEW_PULLS_URL: &str = "https://github.com/zed-industries/zed/pulls?q=is%3Apr+is%3Aclosed+label%3A%22PR+state%3Aneeds+review%22";
 
 pub(crate) enum ComplianceContext {
-    Release,
+    Release { non_blocking_outcome: JobOutput },
     ReleaseNonBlocking,
     Scheduled { tag_source: StepOutput },
 }
 
-pub(crate) fn add_compliance_notification_steps(
+impl ComplianceContext {
+    fn tag_source(&self) -> Option<&StepOutput> {
+        match self {
+            ComplianceContext::Scheduled { tag_source } => Some(tag_source),
+            _ => None,
+        }
+    }
+}
+
+pub(crate) fn add_compliance_steps(
     job: gh_workflow::Job,
     context: ComplianceContext,
-) -> gh_workflow::Job {
+) -> (gh_workflow::Job, StepOutput) {
+    fn run_compliance_check(context: &ComplianceContext) -> (Step<Run>, StepOutput) {
+        let job = named::bash(
+            formatdoc! {r#"
+                cargo xtask compliance {target} --report-path "{COMPLIANCE_REPORT_PATH}"
+                "#,
+                target = if context.tag_source().is_some() { r#""$LATEST_TAG" --branch main"# } else { r#""$GITHUB_REF_NAME""# },
+            }
+        )
+        .id(COMPLIANCE_STEP_ID)
+        .add_env(("GITHUB_APP_ID", vars::ZED_ZIPPY_APP_ID))
+        .add_env(("GITHUB_APP_KEY", vars::ZED_ZIPPY_APP_PRIVATE_KEY))
+        .when_some(context.tag_source(), |step, tag_source| {
+            step.add_env(("LATEST_TAG", tag_source.to_string()))
+        })
+        .when(
+            matches!(
+                context,
+                ComplianceContext::Scheduled { .. } | ComplianceContext::ReleaseNonBlocking
+            ),
+            |step| step.continue_on_error(true),
+        );
+
+        let result = StepOutput::new_unchecked(&job, "outcome");
+        (job, result)
+    }
+
     let upload_step = upload_artifact(COMPLIANCE_REPORT_PATH)
         .if_condition(Expression::new("always()"))
-        .when(matches!(context, ComplianceContext::Release), |step| {
-            step.add_with(("overwrite", true))
-        });
+        .when(
+            matches!(context, ComplianceContext::Release { .. }),
+            |step| step.add_with(("overwrite", true)),
+        );
 
     let (success_prefix, failure_prefix) = match context {
-        ComplianceContext::Release | ComplianceContext::ReleaseNonBlocking => {
+        ComplianceContext::Release { .. } => {
             ("✅ Compliance check passed", "❌ Compliance check failed")
         }
+        ComplianceContext::ReleaseNonBlocking => (
+            "✅ Compliance check passed",
+            "❌ Preliminary compliance check failed (but this can still be fixed while the builds are running!)",
+        ),
         ComplianceContext::Scheduled { .. } => (
             "✅ Scheduled compliance check passed",
             "⚠️ Scheduled compliance check failed",
@@ -198,7 +244,17 @@ pub(crate) fn add_compliance_notification_steps(
 
     let notification_step = Step::new("send_compliance_slack_notification")
         .run(&script)
-        .if_condition(Expression::new("always()"))
+        .if_condition(match &context {
+            ComplianceContext::Release {
+                non_blocking_outcome,
+            } => Expression::new(format!(
+                "failure() || {prior_outcome} != 'success'",
+                prior_outcome = non_blocking_outcome.expr()
+            )),
+            ComplianceContext::Scheduled { .. } | ComplianceContext::ReleaseNonBlocking => {
+                Expression::new("always()")
+            }
+        })
         .add_env(("SLACK_WEBHOOK", vars::SLACK_WEBHOOK_WORKFLOW_FAILURES))
         .add_env((
             "COMPLIANCE_OUTCOME",
@@ -206,8 +262,8 @@ pub(crate) fn add_compliance_notification_steps(
         ))
         .add_env((
             "COMPLIANCE_TAG",
-            match context {
-                ComplianceContext::Release | ComplianceContext::ReleaseNonBlocking => {
+            match &context {
+                ComplianceContext::Release { .. } | ComplianceContext::ReleaseNonBlocking => {
                     Context::github().ref_name().to_string()
                 }
                 ComplianceContext::Scheduled { tag_source } => tag_source.to_string(),
@@ -218,21 +274,21 @@ pub(crate) fn add_compliance_notification_steps(
             format!("{CURRENT_ACTION_RUN_URL}#artifacts"),
         ));
 
-    job.add_step(upload_step).add_step(notification_step)
-}
+    let (compliance_step, check_result) = run_compliance_check(&context);
 
-fn run_compliance_check() -> Step<Run> {
-    named::bash(formatdoc! {r#"
-        cargo xtask compliance "$GITHUB_REF_NAME" --report-path "{COMPLIANCE_REPORT_PATH}"
-        "#,
-    })
-    .id(COMPLIANCE_STEP_ID)
-    .add_env(("GITHUB_APP_ID", vars::ZED_ZIPPY_APP_ID))
-    .add_env(("GITHUB_APP_KEY", vars::ZED_ZIPPY_APP_PRIVATE_KEY))
-    .continue_on_error(true)
+    (
+        job.add_step(compliance_step)
+            .add_step(upload_step)
+            .add_step(notification_step)
+            .when(
+                matches!(context, ComplianceContext::ReleaseNonBlocking),
+                |step| step.outputs([("outcome".to_string(), check_result.to_string())]),
+            ),
+        check_result,
+    )
 }
 
-fn compliance_check() -> NamedJob {
+fn compliance_check() -> (NamedJob, JobOutput) {
     let job = release_job(&[])
         .runs_on(runners::LINUX_SMALL)
         .add_step(
@@ -240,16 +296,17 @@ fn compliance_check() -> NamedJob {
                 .with_full_history()
                 .with_ref(Context::github().ref_()),
         )
-        .add_step(steps::cache_rust_dependencies_namespace())
-        .add_step(run_compliance_check());
+        .add_step(steps::cache_rust_dependencies_namespace());
+
+    let (compliance_job, check_result) =
+        add_compliance_steps(job, ComplianceContext::ReleaseNonBlocking);
+    let compliance_job = named::job(compliance_job);
+    let check_result = check_result.as_job_output(&compliance_job);
 
-    named::job(add_compliance_notification_steps(
-        job,
-        ComplianceContext::ReleaseNonBlocking,
-    ))
+    (compliance_job, check_result)
 }
 
-fn validate_release_assets(deps: &[&NamedJob]) -> NamedJob {
+fn validate_release_assets(deps: &[&NamedJob], context_check_result: JobOutput) -> NamedJob {
     let expected_assets: Vec<String> = assets::all().iter().map(|a| format!("\"{a}\"")).collect();
     let expected_assets_json = format!("[{}]", expected_assets.join(", "));
 
@@ -279,13 +336,17 @@ fn validate_release_assets(deps: &[&NamedJob]) -> NamedJob {
                 .with_full_history()
                 .with_ref(Context::github().ref_()),
         )
-        .add_step(steps::cache_rust_dependencies_namespace())
-        .add_step(run_compliance_check());
+        .add_step(steps::cache_rust_dependencies_namespace());
 
-    named::job(add_compliance_notification_steps(
-        job,
-        ComplianceContext::Release,
-    ))
+    named::job(
+        add_compliance_steps(
+            job,
+            ComplianceContext::Release {
+                non_blocking_outcome: context_check_result,
+            },
+        )
+        .0,
+    )
 }
 
 fn auto_release_preview(deps: &[&NamedJob]) -> NamedJob {

tooling/xtask/src/tasks/workflows/vars.rs 🔗

@@ -167,7 +167,7 @@ impl StepOutput {
                 .run
                 .as_ref()
                 .is_none_or(|run_command| run_command.contains(name)),
-            "Step Output name {name} must occur at least once in run command with ID {step_id}!"
+            "Step output with name '{name}' must occur at least once in run command with ID {step_id}!"
         );
 
         Self { name, step_id }