From 3338611c1bafa90622ca64b38cb74e2aa485f28e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 25 Oct 2024 10:28:45 +0200 Subject: [PATCH 1/3] Switch CI from bors to merge queue --- .github/workflows/clippy_dev.yml | 44 +++++++--------- .../{clippy_bors.yml => clippy_mq.yml} | 51 +++++++------------ .../workflows/{clippy.yml => clippy_pr.yml} | 13 +---- .github/workflows/remark.yml | 44 +++++++--------- CONTRIBUTING.md | 16 +----- book/src/development/README.md | 1 - 6 files changed, 54 insertions(+), 115 deletions(-) rename .github/workflows/{clippy_bors.yml => clippy_mq.yml} (82%) rename .github/workflows/{clippy.yml => clippy_pr.yml} (83%) diff --git a/.github/workflows/clippy_dev.yml b/.github/workflows/clippy_dev.yml index cf0a8bde202..cefeb4fc5f9 100644 --- a/.github/workflows/clippy_dev.yml +++ b/.github/workflows/clippy_dev.yml @@ -1,10 +1,7 @@ name: Clippy Dev Test on: - push: - branches: - - auto - - try + merge_group: pull_request: # Only run on paths, that get checked by the clippy_dev tool paths: @@ -47,28 +44,21 @@ jobs: cargo check git reset --hard HEAD - # These jobs doesn't actually test anything, but they're only used to tell - # bors the build completed, as there is no practical way to detect when a - # workflow is successful listening to webhooks only. - # - # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! - - end-success: - name: bors dev test finished - if: github.event.pusher.name == 'bors' && success() + conclusion_dev: + needs: [ clippy_dev ] + # We need to ensure this job does *not* get skipped if its dependencies fail, + # because a skipped job is considered a success by GitHub. So we have to + # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run + # when the workflow is canceled manually. + # + # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! + if: ${{ !cancelled() }} runs-on: ubuntu-latest - needs: [clippy_dev] - steps: - - name: Mark the job as successful - run: exit 0 - - end-failure: - name: bors dev test finished - if: github.event.pusher.name == 'bors' && (failure() || cancelled()) - runs-on: ubuntu-latest - needs: [clippy_dev] - - steps: - - name: Mark the job as a failure - run: exit 1 + # Manually check the status of all dependencies. `if: failure()` does not work. + - name: Conclusion + run: | + # Print the dependent jobs to see them in the CI log + jq -C <<< '${{ toJson(needs) }}' + # Check if all jobs that we depend on (in the needs array) were successful. + jq --exit-status 'all(.result == "success")' <<< '${{ toJson(needs) }}' diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_mq.yml similarity index 82% rename from .github/workflows/clippy_bors.yml rename to .github/workflows/clippy_mq.yml index 026771e6fcf..49622048050 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_mq.yml @@ -1,10 +1,7 @@ -name: Clippy Test (bors) +name: Clippy Test (merge queue) on: - push: - branches: - - auto - - try + merge_group: env: RUST_BACKTRACE: 1 @@ -13,11 +10,6 @@ env: CARGO_INCREMENTAL: 0 RUSTFLAGS: -D warnings -concurrency: - # For a given workflow, if we push to the same branch, cancel all previous builds on that branch. - group: "${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}" - cancel-in-progress: true - defaults: run: shell: bash @@ -218,28 +210,21 @@ jobs: env: INTEGRATION: ${{ matrix.integration }} - # These jobs doesn't actually test anything, but they're only used to tell - # bors the build completed, as there is no practical way to detect when a - # workflow is successful listening to webhooks only. - # - # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! - - end-success: - name: bors test finished - if: github.event.pusher.name == 'bors' && success() + conclusion: + needs: [ changelog, base, metadata_collection, integration_build, integration ] + # We need to ensure this job does *not* get skipped if its dependencies fail, + # because a skipped job is considered a success by GitHub. So we have to + # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run + # when the workflow is canceled manually. + # + # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! + if: ${{ !cancelled() }} runs-on: ubuntu-latest - needs: [changelog, base, metadata_collection, integration_build, integration] - steps: - - name: Mark the job as successful - run: exit 0 - - end-failure: - name: bors test finished - if: github.event.pusher.name == 'bors' && (failure() || cancelled()) - runs-on: ubuntu-latest - needs: [changelog, base, metadata_collection, integration_build, integration] - - steps: - - name: Mark the job as a failure - run: exit 1 + # Manually check the status of all dependencies. `if: failure()` does not work. + - name: Conclusion + run: | + # Print the dependent jobs to see them in the CI log + jq -C <<< '${{ toJson(needs) }}' + # Check if all jobs that we depend on (in the needs array) were successful. + jq --exit-status 'all(.result == "success")' <<< '${{ toJson(needs) }}' diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy_pr.yml similarity index 83% rename from .github/workflows/clippy.yml rename to .github/workflows/clippy_pr.yml index 0a0538490cc..15752a86cf9 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy_pr.yml @@ -1,17 +1,6 @@ name: Clippy Test on: - push: - # Ignore bors branches, since they are covered by `clippy_bors.yml` - branches-ignore: - - auto - - try - # Don't run Clippy tests, when only text files were modified - paths-ignore: - - 'COPYRIGHT' - - 'LICENSE-*' - - '**.md' - - '**.txt' pull_request: # Don't run Clippy tests, when only text files were modified paths-ignore: @@ -35,7 +24,7 @@ concurrency: jobs: base: - # NOTE: If you modify this job, make sure you copy the changes to clippy_bors.yml + # NOTE: If you modify this job, make sure you copy the changes to clippy_mq.yml runs-on: ubuntu-latest steps: diff --git a/.github/workflows/remark.yml b/.github/workflows/remark.yml index a1b011dc32d..1c1a9e82454 100644 --- a/.github/workflows/remark.yml +++ b/.github/workflows/remark.yml @@ -1,10 +1,7 @@ name: Remark on: - push: - branches: - - auto - - try + merge_group: pull_request: paths: - '**.md' @@ -45,28 +42,21 @@ jobs: - name: Build mdbook run: mdbook build book - # These jobs doesn't actually test anything, but they're only used to tell - # bors the build completed, as there is no practical way to detect when a - # workflow is successful listening to webhooks only. - # - # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! - - end-success: - name: bors remark test finished - if: github.event.pusher.name == 'bors' && success() + conclusion_remark: + needs: [ remark ] + # We need to ensure this job does *not* get skipped if its dependencies fail, + # because a skipped job is considered a success by GitHub. So we have to + # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run + # when the workflow is canceled manually. + # + # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! + if: ${{ !cancelled() }} runs-on: ubuntu-latest - needs: [remark] - steps: - - name: Mark the job as successful - run: exit 0 - - end-failure: - name: bors remark test finished - if: github.event.pusher.name == 'bors' && (failure() || cancelled()) - runs-on: ubuntu-latest - needs: [remark] - - steps: - - name: Mark the job as a failure - run: exit 1 + # Manually check the status of all dependencies. `if: failure()` does not work. + - name: Conclusion + run: | + # Print the dependent jobs to see them in the CI log + jq -C <<< '${{ toJson(needs) }}' + # Check if all jobs that we depend on (in the needs array) were successful. + jq --exit-status 'all(.result == "success")' <<< '${{ toJson(needs) }}' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1a59238c82..1f6c918fc6c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,6 @@ All contributors are expected to follow the [Rust Code of Conduct]. - [Rust Analyzer](#rust-analyzer) - [How Clippy works](#how-clippy-works) - [Issue and PR triage](#issue-and-pr-triage) - - [Bors and Homu](#bors-and-homu) - [Contributions](#contributions) - [License](#license) @@ -213,16 +212,6 @@ We have prioritization labels and a sync-blocker label, which are described belo Or rather: before the sync this should be addressed, e.g. by removing a lint again, so it doesn't hit beta/stable. -## Bors and Homu - -We use a bot powered by [Homu][homu] to help automate testing and landing of pull -requests in Clippy. The bot's username is @bors. - -You can find the Clippy bors queue [here][homu_queue]. - -If you have @bors permissions, you can find an overview of the available -commands [here][homu_instructions]. - [triage]: https://forge.rust-lang.org/release/triage-procedure.html [l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash [l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug @@ -230,9 +219,6 @@ commands [here][homu_instructions]. [p-medium]: https://github.com/rust-lang/rust-clippy/labels/P-medium [p-high]: https://github.com/rust-lang/rust-clippy/labels/P-high [l-sync-blocker]: https://github.com/rust-lang/rust-clippy/labels/L-sync-blocker -[homu]: https://github.com/rust-lang/homu -[homu_instructions]: https://bors.rust-lang.org/ -[homu_queue]: https://bors.rust-lang.org/queue/clippy ## Contributions @@ -244,7 +230,7 @@ All PRs should include a `changelog` entry with a short comment explaining the c "what do you believe is important from an outsider's perspective?" Often, PRs are only related to a single property of a lint, and then it's good to mention that one. Otherwise, it's better to include too much detail than too little. -Clippy's [changelog] is created from these comments. Every release, someone gets all commits from bors with a +Clippy's [changelog] is created from these comments. Every release, someone gets all merge commits with a `changelog: XYZ` entry and combines them into the changelog. This is a manual process. Examples: diff --git a/book/src/development/README.md b/book/src/development/README.md index 8f09f66f595..b33cdc00ead 100644 --- a/book/src/development/README.md +++ b/book/src/development/README.md @@ -53,7 +53,6 @@ book](../lints.md). > - IDE setup > - High level overview on how Clippy works > - Triage procedure -> - Bors and Homu [ast]: https://rustc-dev-guide.rust-lang.org/syntax-intro.html [hir]: https://rustc-dev-guide.rust-lang.org/hir.html From 9ebe68d8c375eaf41c9901e8921e555f5b389539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 7 Nov 2024 18:13:50 +0100 Subject: [PATCH 2/3] Add conclusion job to PR CI --- .github/workflows/clippy_pr.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/workflows/clippy_pr.yml b/.github/workflows/clippy_pr.yml index 15752a86cf9..8748741ba20 100644 --- a/.github/workflows/clippy_pr.yml +++ b/.github/workflows/clippy_pr.yml @@ -62,3 +62,24 @@ jobs: run: .github/driver.sh env: OS: ${{ runner.os }} + + # We need to have the "conclusion" job also on PR CI, to make it possible + # to add PRs to a merge queue. + conclusion: + needs: [ base ] + # We need to ensure this job does *not* get skipped if its dependencies fail, + # because a skipped job is considered a success by GitHub. So we have to + # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run + # when the workflow is canceled manually. + # + # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! + if: ${{ !cancelled() }} + runs-on: ubuntu-latest + steps: + # Manually check the status of all dependencies. `if: failure()` does not work. + - name: Conclusion + run: | + # Print the dependent jobs to see them in the CI log + jq -C <<< '${{ toJson(needs) }}' + # Check if all jobs that we depend on (in the needs array) were successful. + jq --exit-status 'all(.result == "success")' <<< '${{ toJson(needs) }}' From 843ef1b1f07820d26cbda2fc548f16f6a7a0aa58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 7 Nov 2024 18:26:06 +0100 Subject: [PATCH 3/3] Remove path filter It would cause issues with the required jobs, and it is probably useless anyway, the vast majority of PRs seem to change Rust source files. --- .github/workflows/clippy_dev.yml | 6 ------ .github/workflows/clippy_pr.yml | 6 ------ .github/workflows/remark.yml | 2 -- 3 files changed, 14 deletions(-) diff --git a/.github/workflows/clippy_dev.yml b/.github/workflows/clippy_dev.yml index cefeb4fc5f9..bcb3193ad67 100644 --- a/.github/workflows/clippy_dev.yml +++ b/.github/workflows/clippy_dev.yml @@ -3,12 +3,6 @@ name: Clippy Dev Test on: merge_group: pull_request: - # Only run on paths, that get checked by the clippy_dev tool - paths: - - 'CHANGELOG.md' - - 'README.md' - - '**.stderr' - - '**.rs' env: RUST_BACKTRACE: 1 diff --git a/.github/workflows/clippy_pr.yml b/.github/workflows/clippy_pr.yml index 8748741ba20..2e5b5bd41df 100644 --- a/.github/workflows/clippy_pr.yml +++ b/.github/workflows/clippy_pr.yml @@ -2,12 +2,6 @@ name: Clippy Test on: pull_request: - # Don't run Clippy tests, when only text files were modified - paths-ignore: - - 'COPYRIGHT' - - 'LICENSE-*' - - '**.md' - - '**.txt' env: RUST_BACKTRACE: 1 diff --git a/.github/workflows/remark.yml b/.github/workflows/remark.yml index 1c1a9e82454..0d402fe7064 100644 --- a/.github/workflows/remark.yml +++ b/.github/workflows/remark.yml @@ -3,8 +3,6 @@ name: Remark on: merge_group: pull_request: - paths: - - '**.md' jobs: remark: