Skip to content

bootstrap: allow to set clippy.toml for x.py clippy #137785

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Feb 28, 2025

First two commits adds ability to set clippy.toml for x.py clippy, to be able to tune clippy lints, and set avoid-breaking-exported-api = false for bootstrap and compiler as example.
3rd commit fixes wrong displayed name for rustdoc - now it will be named rustdoc when running python x.py clippy librustdoc, not clippy.

I don't like copypasted paths for lint targets in CI, but can't find better way for it.

@rustbot
Copy link
Collaborator

rustbot commented Feb 28, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 28, 2025

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r? bootstrap

Maybe someone has stronger opinions about clippy...

let config = LintConfig::new(run.builder);
let config = LintConfig::new(
run.builder,
Some("src/etc/clippy_configs/compiler/clippy.toml".into()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a bit of an odd place to put clippy configs.. maybe we can move them to e.g. src/bootstrap, similar to profile files?

I'm also wondering if there's any clippy-native feature for these configs that we could be using instead? For example package-specific configs or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, location not really important, i can move it.

Lints can be set via https://doc.rust-lang.org/nightly/cargo/reference/manifest.html#the-lints-section, but not config for them https://doc.rust-lang.org/clippy/lint_configuration.html, as i understand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I thought we had user-configurable config for e.g. the cfg lints. Maybe those are special cased though.

@rustbot rustbot assigned clubby789 and unassigned Mark-Simulacrum Mar 1, 2025
Comment on lines +122 to +123
// FIXME: this skip merging clippy.toml,
// but maybe there is a better way?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this comment mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function named merge, but it don't merge 1 field, so i left this as note, better rephrase this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we want to merge this field?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
4 participants