-
Notifications
You must be signed in to change notification settings - Fork 1.6k
📌 Pin Clippy to a nightly 📌 #6401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
613333a
Pin Clippy to a nightly
ebroto 2e8b00a
Apply suggestions from PR review
ebroto 77a32eb
Use new cache key
ebroto 20d84fd
Enable internal lints for every test in CI
flip1995 41cab83
Fix toolchain installation in workflows
flip1995 26dcbf5
Stop caching on CI
flip1995 3f41fe2
Error in integration test, if required toolchain is not installed
flip1995 836325e
Fix integration test runner
flip1995 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,29 +35,11 @@ jobs: | |
with: | ||
github_token: "${{ secrets.github_token }}" | ||
|
||
- name: rust-toolchain | ||
uses: actions-rs/[email protected] | ||
with: | ||
toolchain: nightly | ||
target: x86_64-unknown-linux-gnu | ||
profile: minimal | ||
|
||
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: Run cargo update | ||
run: cargo update | ||
|
||
- name: Cache cargo dir | ||
uses: actions/cache@v2 | ||
with: | ||
path: ~/.cargo | ||
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }} | ||
restore-keys: | | ||
${{ runner.os }}-x86_64-unknown-linux-gnu | ||
|
||
- name: Master Toolchain Setup | ||
run: bash setup-toolchain.sh | ||
- name: Install toolchain | ||
run: rustup show active-toolchain | ||
|
||
# Run | ||
- name: Set LD_LIBRARY_PATH (Linux) | ||
|
@@ -66,13 +48,13 @@ jobs: | |
echo "LD_LIBRARY_PATH=${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" >> $GITHUB_ENV | ||
|
||
- name: Build | ||
run: cargo build --features deny-warnings | ||
run: cargo build --features deny-warnings,internal-lints | ||
|
||
- name: Test | ||
run: cargo test --features deny-warnings | ||
run: cargo test --features deny-warnings,internal-lints | ||
|
||
- name: Test clippy_lints | ||
run: cargo test --features deny-warnings | ||
run: cargo test --features deny-warnings,internal-lints | ||
working-directory: clippy_lints | ||
|
||
- name: Test rustc_tools_util | ||
|
@@ -98,9 +80,3 @@ jobs: | |
cargo dev new_lint --name new_late_pass --pass late | ||
cargo check | ||
git reset --hard HEAD | ||
|
||
# Cleanup | ||
- name: Run cargo-cache --autoclean | ||
run: | | ||
cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache | ||
cargo cache |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ jobs: | |
- uses: rust-lang/simpleinfra/github-actions/cancel-outdated-builds@master | ||
with: | ||
github_token: "${{ secrets.github_token }}" | ||
|
||
- name: Checkout | ||
uses: actions/[email protected] | ||
with: | ||
|
@@ -84,31 +85,11 @@ jobs: | |
sudo apt-get install gcc-multilib libssl-dev:i386 libgit2-dev:i386 | ||
if: matrix.host == 'i686-unknown-linux-gnu' | ||
|
||
- name: rust-toolchain | ||
uses: actions-rs/[email protected] | ||
with: | ||
toolchain: nightly | ||
target: ${{ matrix.host }} | ||
profile: minimal | ||
|
||
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: Run cargo update | ||
run: cargo update | ||
|
||
- name: Cache cargo dir | ||
uses: actions/cache@v2 | ||
with: | ||
path: ~/.cargo | ||
key: ${{ runner.os }}-${{ matrix.host }}-${{ hashFiles('Cargo.lock') }} | ||
restore-keys: | | ||
${{ runner.os }}-${{ matrix.host }} | ||
|
||
- name: Master Toolchain Setup | ||
run: bash setup-toolchain.sh | ||
env: | ||
HOST_TOOLCHAIN: ${{ matrix.host }} | ||
- name: Install toolchain | ||
run: rustup show active-toolchain | ||
|
||
# Run | ||
- name: Set LD_LIBRARY_PATH (Linux) | ||
|
@@ -128,13 +109,13 @@ jobs: | |
SYSROOT=$(rustc --print sysroot) | ||
echo "$SYSROOT/bin" >> $GITHUB_PATH | ||
|
||
- name: Build with internal lints | ||
- name: Build | ||
run: cargo build --features deny-warnings,internal-lints | ||
|
||
- name: Test with internal lints | ||
- name: Test | ||
run: cargo test --features deny-warnings,internal-lints | ||
|
||
- name: Test clippy_lints with internal lints | ||
- name: Test clippy_lints | ||
run: cargo test --features deny-warnings,internal-lints | ||
working-directory: clippy_lints | ||
|
||
|
@@ -155,12 +136,6 @@ jobs: | |
env: | ||
OS: ${{ runner.os }} | ||
|
||
# Cleanup | ||
- name: Run cargo-cache --autoclean | ||
run: | | ||
cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache | ||
cargo cache | ||
|
||
integration_build: | ||
needs: changelog | ||
runs-on: ubuntu-latest | ||
|
@@ -171,29 +146,11 @@ jobs: | |
with: | ||
github_token: "${{ secrets.github_token }}" | ||
|
||
- name: rust-toolchain | ||
uses: actions-rs/[email protected] | ||
with: | ||
toolchain: nightly | ||
target: x86_64-unknown-linux-gnu | ||
profile: minimal | ||
|
||
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: Run cargo update | ||
run: cargo update | ||
|
||
- name: Cache cargo dir | ||
uses: actions/cache@v2 | ||
with: | ||
path: ~/.cargo | ||
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }} | ||
restore-keys: | | ||
${{ runner.os }}-x86_64-unknown-linux-gnu | ||
|
||
- name: Master Toolchain Setup | ||
run: bash setup-toolchain.sh | ||
- name: Install toolchain | ||
run: rustup show active-toolchain | ||
|
||
# Run | ||
- name: Build Integration Test | ||
|
@@ -214,11 +171,6 @@ jobs: | |
name: target | ||
path: target | ||
|
||
# Cleanup | ||
- name: Run cargo-cache --autoclean | ||
run: | | ||
cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache | ||
cargo cache | ||
integration: | ||
needs: integration_build | ||
strategy: | ||
|
@@ -252,29 +204,11 @@ jobs: | |
with: | ||
github_token: "${{ secrets.github_token }}" | ||
|
||
- name: rust-toolchain | ||
uses: actions-rs/[email protected] | ||
with: | ||
toolchain: nightly | ||
target: x86_64-unknown-linux-gnu | ||
profile: minimal | ||
|
||
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: Run cargo update | ||
run: cargo update | ||
|
||
- name: Cache cargo dir | ||
uses: actions/cache@v2 | ||
with: | ||
path: ~/.cargo | ||
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }} | ||
restore-keys: | | ||
${{ runner.os }}-x86_64-unknown-linux-gnu | ||
|
||
- name: Master Toolchain Setup | ||
run: bash setup-toolchain.sh | ||
- name: Install toolchain | ||
run: rustup show active-toolchain | ||
|
||
# Download | ||
- name: Download target dir | ||
|
@@ -288,16 +222,11 @@ jobs: | |
|
||
# Run | ||
- name: Test ${{ matrix.integration }} | ||
run: $CARGO_TARGET_DIR/debug/integration | ||
run: | | ||
RUSTUP_TOOLCHAIN="$(rustup show active-toolchain | grep -o -E "nightly-[0-9]{4}-[0-9]{2}-[0-9]{2}")" \ | ||
$CARGO_TARGET_DIR/debug/integration | ||
env: | ||
INTEGRATION: ${{ matrix.integration }} | ||
RUSTUP_TOOLCHAIN: master | ||
|
||
# Cleanup | ||
- name: Run cargo-cache --autoclean | ||
run: | | ||
cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache | ||
cargo cache | ||
|
||
# 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 | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,16 +22,20 @@ jobs: | |
|
||
steps: | ||
# Setup | ||
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: remove toolchain file | ||
run: rm rust-toolchain | ||
|
||
- name: rust-toolchain | ||
uses: actions-rs/[email protected] | ||
with: | ||
toolchain: nightly | ||
target: x86_64-unknown-linux-gnu | ||
profile: minimal | ||
components: rustfmt | ||
|
||
- name: Checkout | ||
uses: actions/[email protected] | ||
default: true | ||
|
||
# Run | ||
- name: Build | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,8 +70,8 @@ But we can make it nest-less by using [if_chain] macro, [like this][nest-less]. | |
|
||
[`E-medium`] issues are generally pretty easy too, though it's recommended you work on an [`good first issue`] | ||
first. Sometimes they are only somewhat involved code wise, but not difficult per-se. | ||
Note that [`E-medium`] issues may require some knowledge of Clippy internals or some | ||
debugging to find the actual problem behind the issue. | ||
Note that [`E-medium`] issues may require some knowledge of Clippy internals or some | ||
debugging to find the actual problem behind the issue. | ||
|
||
[`T-middle`] issues can be more involved and require verifying types. The [`ty`] module contains a | ||
lot of methods that are useful, though one of the most useful would be `expr_ty` (gives the type of | ||
|
@@ -182,18 +182,26 @@ That's why the `else_if_without_else` example uses the `register_early_pass` fun | |
[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html | ||
[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html | ||
|
||
## Fixing build failures caused by Rust | ||
## Syncing changes between Clippy and [`rust-lang/rust`] | ||
|
||
Clippy currently gets built with `rustc` of the `rust-lang/rust` `master` | ||
branch. Most of the times we have to adapt to the changes and only very rarely | ||
there's an actual bug in Rust. | ||
Clippy currently gets built with a pinned nightly version. | ||
ebroto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
If you decide to make Clippy work again with a Rust commit that breaks it, you | ||
have to sync the `rust-lang/rust-clippy` repository with the `subtree` copy of | ||
Clippy in the `rust-lang/rust` repository. | ||
In the `rust-lang/rust` repository, where rustc resides, there's a copy of Clippy | ||
that compiler hackers modify from time to time to adapt to changes in the unstable | ||
API of the compiler. | ||
|
||
For general information about `subtree`s in the Rust repository see [Rust's | ||
`CONTRIBUTING.md`][subtree]. | ||
We need to sync these changes back to this repository periodically, and the changes | ||
made to this repository in the meantime also need to be synced to the `rust-lang/rust` repository. | ||
|
||
To avoid flooding the `rust-lang/rust` PR queue, this two-way sync process is done | ||
in a bi-weekly basis if there's no urgent changes. This is done starting on the day of | ||
the Rust stable release and then every other week. That way we guarantee that we keep | ||
this repo up to date with the latest compiler API, and every feature in Clippy is available | ||
for 2 weeks in nightly, before it can get to beta. For reference, the first sync | ||
following this cadence was performed the 2020-08-27. | ||
|
||
This process is described in detail in the following sections. For general information | ||
about `subtree`s in the Rust repository see [Rust's `CONTRIBUTING.md`][subtree]. | ||
|
||
### Patching git-subtree to work with big repos | ||
|
||
|
@@ -222,13 +230,14 @@ This shell has a hardcoded recursion limit set to 1000. In order to make this pr | |
you need to force the script to run `bash` instead. You can do this by editing the first | ||
line of the `git-subtree` script and changing `sh` to `bash`. | ||
|
||
### Performing the sync | ||
### Performing the sync from [`rust-lang/rust`] to Clippy | ||
|
||
Here is a TL;DR version of the sync process (all of the following commands have | ||
to be run inside the `rust` directory): | ||
|
||
1. Clone the [`rust-lang/rust`] repository | ||
2. Sync the changes to the rust-copy of Clippy to your Clippy fork: | ||
1. Clone the [`rust-lang/rust`] repository or make sure it is up to date. | ||
2. Checkout the commit from the latest available nightly. You can get it using `rustup check`. | ||
3. Sync the changes to the rust-copy of Clippy to your Clippy fork: | ||
```bash | ||
# Make sure to change `your-github-name` to your github name in the following command | ||
git subtree push -P src/tools/clippy [email protected]:your-github-name/rust-clippy sync-from-rust | ||
|
@@ -246,17 +255,11 @@ to be run inside the `rust` directory): | |
git checkout sync-from-rust | ||
git merge upstream/master | ||
``` | ||
3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to | ||
4. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to | ||
accelerate the process ping the `@rust-lang/clippy` team in your PR and/or | ||
~~annoy~~ ask them in the [Zulip] stream.) | ||
|
||
### Syncing back changes in Clippy to [`rust-lang/rust`] | ||
|
||
To avoid flooding the [`rust-lang/rust`] PR queue, changes in Clippy's repo are synced back | ||
in a bi-weekly basis if there's no urgent changes. This is done starting on the day of | ||
the Rust stable release and then every other week. That way we guarantee that | ||
every feature in Clippy is available for 2 weeks in nightly, before it can get to beta. | ||
For reference, the first sync following this cadence was performed the 2020-08-27. | ||
### Performing the sync from Clippy to [`rust-lang/rust`] | ||
|
||
All of the following commands have to be run inside the `rust` directory. | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
nightly | ||
[toolchain] | ||
channel = "nightly-2020-12-09" | ||
components = ["llvm-tools-preview", "rustc-dev", "rust-src", "rustfmt"] |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we remove the
rust-toolchain
step, from where does therustup
used in this command come from?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GHA ships with stable rust+rustup already installed. See GHA documentation about the pre-installed software
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's convenient :)
BTW thanks for the work on this, you saved me another night fighting the CI 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP, I like CI pain 😄
So can we merge this? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I was hesitant about approving a PR I opened 😄