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

x clippy implementation is deeply coupled with the core bootstrap #122825

Closed
onur-ozkan opened this issue Mar 21, 2024 · 0 comments · Fixed by #122883
Closed

x clippy implementation is deeply coupled with the core bootstrap #122825

onur-ozkan opened this issue Mar 21, 2024 · 0 comments · Fixed by #122883
Assignees
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@onur-ozkan
Copy link
Member

It's not possible to run x clippy only on certain paths. When running x clippy $tool, clippy starts giving warnings on the entire compiler and library tree as well. Therefore we are unable to run clippy with different set of rules indivudually for each tool/path.

The root cause of this problem is that bootstrap overrides all the commands before invocations with clippy as seen here:

/// Like `cargo`, but only passes flags that are valid for all commands.
pub fn bare_cargo(
&self,
compiler: Compiler,
mode: Mode,
target: TargetSelection,
cmd: &str,
) -> Command {
let mut cargo = if cmd == "clippy" {
self.cargo_clippy_cmd(compiler)

And that is because clippy is sharing the same logic with x check:

Kind::Check | Kind::Clippy | Kind::Fix => describe!(
check::Std,
check::Rustc,
check::Rustdoc,
check::CodegenBackend,
check::Clippy,
check::Miri,
check::CargoMiri,
check::MiroptTestTools,
check::Rls,
check::Rustfmt,
check::RustAnalyzer,
check::Bootstrap,
),

To prevent this problem we can create a new build step for clippy and then remove clippy-specific conditions from the core bootstrap flow.

@onur-ozkan onur-ozkan added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Mar 21, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 21, 2024
@onur-ozkan onur-ozkan removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 21, 2024
@onur-ozkan onur-ozkan self-assigned this Mar 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 16, 2024
…bertlarsan68

refactor clippy in bootstrap

Previously, using clippy in bootstrap was not very useful as explained in rust-lang#122825. In short, regardless of the given path clippy would always check the entire compiler and std tree. This makes it impossible to run clippy on different paths with different set of rules. This PR fixes that by allowing developers to run clippy with specific rules on specific paths (e.g., we can run `x clippy compiler -Aclippy::all -Dclippy::correctness` and `x clippy library/std -Dclippy::all` and none of them will affect each other).

Resolves rust-lang#122825
@bors bors closed this as completed in 8229a34 Apr 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 17, 2024
Rollup merge of rust-lang#122883 - onur-ozkan:clippy-build-step, r=albertlarsan68

refactor clippy in bootstrap

Previously, using clippy in bootstrap was not very useful as explained in rust-lang#122825. In short, regardless of the given path clippy would always check the entire compiler and std tree. This makes it impossible to run clippy on different paths with different set of rules. This PR fixes that by allowing developers to run clippy with specific rules on specific paths (e.g., we can run `x clippy compiler -Aclippy::all -Dclippy::correctness` and `x clippy library/std -Dclippy::all` and none of them will affect each other).

Resolves rust-lang#122825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants