Skip to content
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

Make x.py clippy download and use beta clippy #106394

Closed
wants to merge 5 commits into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 3, 2023

Rebased + fixed version of #97443.

Builds on #106387. Fixes #95988. Helps with #76495.

This contains two changes to clippy. Both of them are because clippy bypasses the fake rustc shim we set with RUSTC; if cargo-clippy didn't do that, we could avoid those changes.

  1. It fixes the regression from rust-lang/rust-clippy@5907e91#r1060215684, allowing bootstrap to set SYSROOT even if --sysroot is also passed. That allows us to pass the sysroot for build scripts and proc macros, where cargo ignores RUSTFLAGS. (We can't use SYSROOT exclusively because clippy only runs on crates in the workspace, not dependencies.)
  2. It adds a new RUSTC_CLIPPY_IGNORE_BUILD_SCRIPTS_AND_PROC_MACROS environment variable. This is set for stage 1 and above. It's necessary because we really need the shim for those targets at stage 1:
    • for build scripts we need to reuse the stage 0 toolchain so the dependencies are compiled with the same version as the build scripts
    • for proc macros we need to set -Z force-unstable-if-unmarked to allow using rustc_private crates in the sysroot. We can't hack this with -Zcrate-attr=feature(rustc_private) because it won't get passed to proc-macros if we set it through RUSTFLAGS.

Only x clippy --stage 1 has been tested; --stage 0 needs the first change (allowing SYSROOT to be set) to land on beta before it will work.

Note that this also adds x clippy --stage 1 -Awarnings to x86_64-gnu-llvm-13 to make sure it stays working; that won't gate on any clippy warnings, just enforce that clippy doesn't give a hard error.

r? @jyn514

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Jan 3, 2023
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 4, 2023
…bini

Allow passing a specific date to `bump-stage0`

This allows regenerating `src/stage0.json` on changes to the tool, without needing to hard-code the date in the source. It was useful for rust-lang#106394, which added clippy to the list of required components.

r? `@pietroalbini`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 4, 2023
Fix a few clippy lints in libtest

- Remove unnecessary references and dereferences
- Use `.contains` instead of `a <= x && x <= b`
- Use `mem::take` instead of `mem::replace` where possible

cc rust-lang#106394 :)
bors added a commit to rust-lang/rust-clippy that referenced this pull request Jan 12, 2023
Don't pass `--sysroot` twice if SYSROOT is set

This is useful for rust-lang/rust to allow setting a sysroot that's *only* for build scripts, different from the regular sysroot passed in RUSTFLAGS (since cargo doesn't apply RUSTFLAGS to build scripts or proc-macros).

That said, the exact motivation is not particularly important: this fixes a regression from
5907e91#r1060215684.

Note that only RUSTFLAGS is tested in the new integration test; passing --sysroot through `clippy-driver` never worked as far as I can tell, and no one is using it, so I didn't fix it here.

Helps with rust-lang/rust#106394.

---

changelog: other: `SYSROOT` and `--sysroot` can now be set at the same time
[#10149](#10149)
<!-- changelog_checked -->
@matthiaskrgr
Copy link
Member

Will this provide a way to still run with master/nightly clippy? This has proven valuable a couple of times to find clippy regressions/crashes.

@jyn514
Copy link
Member Author

jyn514 commented Jan 29, 2023

@matthiaskrgr yes - that's the only thing that currently works right now actually, x clippy --stage 1.

See https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/clippy.201-1 for the current status, clippy has an ICE that only shows up with debug-assertions enabled and I need #107297 to land before I can test --stage 0.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jan 31, 2023

☔ The latest upstream changes (presumably #107297) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:ac593985615ec2ede58e132d2e21d2b1cbd6127c)
Download action repository 'rust-lang/simpleinfra@master' (SHA:055e3b93d15803815fe6f9cbc1b02b11be094e54)
Complete job name: PR (x86_64-gnu-llvm-13, false, ubuntu-20.04-xl)
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: x86_64-gnu-llvm-13
---
Step 7/8 : ENV RUST_CONFIGURE_ARGS       --build=x86_64-unknown-linux-gnu       --llvm-root=/usr/lib/llvm-13       --enable-llvm-link-shared       --set rust.thin-lto-import-instr-limit=10
 ---> Running in 3f0fd176fa78
Removing intermediate container 3f0fd176fa78
 ---> 38598d98e4c2
Step 8/8 : ENV SCRIPT ../x.py --stage 2 test --exclude src/tools/tidy &&            ../x --stage 2 test tests/mir-opt                              --host='' --target=i686-unknown-linux-gnu &&            ../x.ps1 --stage 2 test tests/ui --pass=check                              --host='' --target=i686-unknown-linux-gnu &&            python3 ../x.py --stage 1 clippy -Awarnings &&            python2.7 ../x.py --stage 2 test src/tools/tidy
Removing intermediate container 0b783e6cd9d6
 ---> 65cc3a6cd5e3
Successfully built 65cc3a6cd5e3
Successfully tagged rust-ci:latest
Successfully tagged rust-ci:latest
Built container sha256:65cc3a6cd5e3704efa006d25e25f919c4d37ceb6f8a2c91ec3ccdaa3040a5ae6
Uploading finished image to https://ci-caches.rust-lang.org/docker/bcf14fce040598f1a6b9d3e5198e0d88a2c00cfe6808e9b57b0d90dee3b8e6d0f95daa85f2a2050aeb73f3b9f0cb40f0c2cf491c83dfad8dac94a6b0a8a3f7cb
upload failed: - to s3://rust-lang-ci-sccache2/docker/bcf14fce040598f1a6b9d3e5198e0d88a2c00cfe6808e9b57b0d90dee3b8e6d0f95daa85f2a2050aeb73f3b9f0cb40f0c2cf491c83dfad8dac94a6b0a8a3f7cb Unable to locate credentials
[CI_JOB_NAME=x86_64-gnu-llvm-13]
---
test result: ok. 14183 passed; 0 failed; 200 ignored; 0 measured; 0 filtered out; finished in 83.54s

 finished in 84.285 seconds
Build completed successfully in 0:01:26
+ python3 ../x.py --stage 1 clippy -Awarnings
    Finished dev [unoptimized] target(s) in 0.06s
Building stage0 library artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.27s
Copying stage0 library from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
    Checking rustc_ast_passes v0.0.0 (/checkout/compiler/rustc_ast_passes)
    Checking rustc_expand v0.0.0 (/checkout/compiler/rustc_expand)
    Checking rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `(1, Some(1))`,
 right: `(2, Some(2))`: wrong number of generic parameters for DefId(2:7163 ~ core[cdb4]::iter::traits::iterator::Iterator): [&mut mir::traversal::Preorder<'_, '_>, fn((mir::BasicBlock, &mir::BasicBlockData<'_>)) {std::mem::drop::<(mir::BasicBlock, &mir::BasicBlockData<'_>)>}]', /checkout/compiler/rustc_middle/src/ty/context.rs:1762:13

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.
note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new

note: Clippy version: clippy 0.1.69 (efbb2338 2023-02-01)
query stack during panic:
query stack during panic:
#0 [analysis] running analysis passes on this crate
error: could not compile `rustc_middle`
Build completed successfully in 0:03:15

@jyn514 jyn514 assigned flip1995 and unassigned jyn514 Feb 2, 2023
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-clippy Area: Clippy and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 2, 2023
@jyn514 jyn514 self-assigned this Feb 3, 2023
@albertlarsan68
Copy link
Member

I'll take over this PR, so I moved the commits to #107628.

@jyn514 jyn514 closed this Feb 3, 2023
@jyn514 jyn514 deleted the bootstrap-clippy branch February 3, 2023 07:27
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request May 31, 2023
Fix a few clippy lints in libtest

- Remove unnecessary references and dereferences
- Use `.contains` instead of `a <= x && x <= b`
- Use `mem::take` instead of `mem::replace` where possible

cc rust-lang/rust#106394 :)
@jyn514 jyn514 mentioned this pull request Nov 5, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2023
x clippy

thanks to `@asquared31415` `@albertlarsan68` for all their help, most of this pr is their work

note that this also adds x clippy --stage 0 -Awarnings to x86_64-gnu-llvm-15 to make sure it stays working; that won't gate on any clippy warnings, just enforce that clippy doesn't give a hard error. we can't add --stage 1 until clippy fixes its debug assertions not to panic.

note that `x clippy --stage 1` currently breaks when combined with download-rustc.

unlike the previous prs, this doesn't require changes to clippy (it works by using RUSTC_WRAPPER instead), and supports stage 0

read this commit-by-commit

closes rust-lang#107628; see also rust-lang#106394, rust-lang#97443. fixes rust-lang#95988. helps with rust-lang#76495.

r? bootstrap
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2023
x clippy

thanks to `@asquared31415` `@albertlarsan68` for all their help, most of this pr is their work

note that this also adds x clippy --stage 0 -Awarnings to x86_64-gnu-llvm-15 to make sure it stays working; that won't gate on any clippy warnings, just enforce that clippy doesn't give a hard error. we can't add --stage 1 until clippy fixes its debug assertions not to panic.

note that `x clippy --stage 1` currently breaks when combined with download-rustc.

unlike the previous prs, this doesn't require changes to clippy (it works by using RUSTC_WRAPPER instead), and supports stage 0

read this commit-by-commit

closes rust-lang#107628; see also rust-lang#106394, rust-lang#97443. fixes rust-lang#95988. helps with rust-lang#76495.

r? bootstrap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-clippy Area: Clippy A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix x.py clippy to be less janky
8 participants