From 4c63fb1a101e2dc11617c729663659c66658b49c Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Fri, 10 Apr 2026 18:39:35 +0200 Subject: [PATCH] compliance: Reduce noisiness when checks were successful (#53515) 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 --- .github/workflows/compliance_check.yml | 5 +- .github/workflows/release.yml | 12 +- .../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(-) diff --git a/.github/workflows/compliance_check.yml b/.github/workflows/compliance_check.yml index 7eb53f082dd6aa22e60248acac1fd18529db3b26..e74c38ec5d3701b936448a128ea8076932d83e91 100644 --- a/.github/workflows/compliance_check.yml +++ b/.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 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e40c9f68b5f79c19238fd08da0b73919734f8fa4..17178ab3054a7cddf1dccd2cd9bfa415a56755bd 100644 --- a/.github/workflows/release.yml +++ b/.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" diff --git a/tooling/xtask/src/tasks/workflows/compliance_check.rs b/tooling/xtask/src/tasks/workflows/compliance_check.rs index 941e993403a45c37477f88048376faa8807d2d4f..5918bc476772ae1ffe4c0878bccce1e092a6ac7e 100644 --- a/tooling/xtask/src/tasks/workflows/compliance_check.rs +++ b/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 { - 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, + ) } diff --git a/tooling/xtask/src/tasks/workflows/release.rs b/tooling/xtask/src/tasks/workflows/release.rs index 5a33cc911b5d940b2d52e93568c5f4e5c53a0898..d3ad064a6653c963fdc78d147ffbb3147c009c8d 100644 --- a/tooling/xtask/src/tasks/workflows/release.rs +++ b/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, 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 { - 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 = 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 { diff --git a/tooling/xtask/src/tasks/workflows/vars.rs b/tooling/xtask/src/tasks/workflows/vars.rs index b3f8bdf56e9bb0f93f81992fbc61dab2b9754e63..8afcad7461f936c081111eeb35097709aa0eb13f 100644 --- a/tooling/xtask/src/tasks/workflows/vars.rs +++ b/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 }