Skip to content

Avoid running cargo+internal lints when not enabled - #5505

Merged
bors merged 2 commits into
rust-lang:masterfrom
flip1995:avoid_running_lints
Apr 25, 2020
Merged

Avoid running cargo+internal lints when not enabled#5505
bors merged 2 commits into
rust-lang:masterfrom
flip1995:avoid_running_lints

Conversation

@flip1995

@flip1995 flip1995 commented Apr 22, 2020

Copy link
Copy Markdown
Member

r? @matthiaskrgr

changelog: none

flip1995
flip1995 commented Apr 22, 2020
View reviewed changes
Comment thread clippy_lints/src/cargo_common_metadata.rs

impl EarlyLintPass for CargoCommonMetadata {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
impl LateLintPass<'_, '_> for CargoCommonMetadata {

@flip1995 flip1995 Apr 22, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Downside: The cargo lints are now LateLintPasses

@tesuji

tesuji commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

What's the motivation of this change?

@matthiaskrgr

matthiaskrgr commented Apr 23, 2020

Copy link
Copy Markdown
Member

@lzutao when running clippy-driver foo.rs in a directory that had a big Cargo.toml somewhere in a parent path, clippy-driver would redundantly search for the nearest cargo manifest run clippy::cargo-lints which invoced cargo-metadata on the entire crate, slowing everything down significantly depending on the project complexity.
https://discordapp.com/channels/442252698964721669/459149871191031810/701877035936645192

https://discordapp.com/channels/442252698964721669/459149871191031810/702159543538417714

When I tested a version of this branch, it managed to speed up my testcase (running clippy-driver on testcases inside the rustc repo) by ~10x :)

@matthiaskrgr

matthiaskrgr commented Apr 23, 2020

Copy link
Copy Markdown
Member

Thanks a lot @flip1995 !
Feel free to r=me once github is working again :)

@flip1995

flip1995 commented Apr 23, 2020

Copy link
Copy Markdown
Member Author

To add to this: I ran flamegraph on clippy-driver, when run inside the clippy repo dir on a single (hello-world) file. Without this change the cargo metadata collection took ~30-40% of the runtime.

@bors r=matthiaskrgr

@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

📌 Commit f31502f has been approved by matthiaskrgr

bors added a commit that referenced this pull request Apr 23, 2020
@bors
Auto merge of #5505 - flip1995:avoid_running_lints, r=matthiaskrgr
a7e6134 
Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

⌛ Testing commit f31502f with merge a7e6134...

@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

💥 Test timed out

@flip1995

flip1995 commented Apr 23, 2020

Copy link
Copy Markdown
Member Author

@bors retry

bors added a commit that referenced this pull request Apr 23, 2020
@bors
Auto merge of #5505 - flip1995:avoid_running_lints, r=matthiaskrgr
dc1adc6 
Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

⌛ Testing commit f31502f with merge dc1adc6...

@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-action_test

@flip1995

flip1995 commented Apr 23, 2020

Copy link
Copy Markdown
Member Author

@bors retry (please?)

bors added a commit that referenced this pull request Apr 23, 2020
@bors
Auto merge of #5505 - flip1995:avoid_running_lints, r=matthiaskrgr
3c3c2cb 
Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

⌛ Testing commit f31502f with merge 3c3c2cb...

@bors

bors commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-action_test

@flip1995 flip1995 added the S-waiting-on-bors Status: The marked PR was approved and is only waiting bors label Apr 23, 2020
@matthiaskrgr

This comment has been minimized.

Sign in to view
@flip1995

flip1995 commented Apr 25, 2020

Copy link
Copy Markdown
Member Author

@bors retry

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Apr 25, 2020
@flip1995
Rollup merge of rust-lang#5505 - flip1995:avoid_running_lints, r=matt…
7c83651 
…hiaskrgr

Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
This was referenced Apr 25, 2020
Closed
Merged
bors added a commit that referenced this pull request Apr 25, 2020
@bors
Auto merge of #5527 - flip1995:rollup-pr2htfd, r=flip1995
77c23b7 
Rollup of 5 pull requests

Successful merges:

 - #5408 (Downgrade match_bool to pedantic)
 - #5505 (Avoid running cargo+internal lints when not enabled)
 - #5516 (Add a note to the beta sections of release.md)
 - #5517 (Deploy time travel)
 - #5523 (Add lifetime test case for `new_ret_no_self`)

Failed merges:

r? @ghost

changelog: rollup
@bors
bors merged commit a33d64a into rust-lang:master Apr 25, 2020
@flip1995
flip1995 deleted the avoid_running_lints branch May 4, 2020 15:29
@flip1995 flip1995 mentioned this pull request May 2, 2022
Merged
sync-agent-neo Bot pushed a commit to aosp-mirror-neo/toolchain_android_rust that referenced this pull request Mar 28, 2026
@jeffvanderstoep
rust-clippy: remove fixes applied to 1.45.0
1a049f1 
Fixes for [1] have been applied to 1.45.0 so we no longer
need to carry these patches.

[1] rust-lang/rust-clippy#5505

Bug: 157238651
Test: Follow test procedure at go/roll-android-rust
Change-Id: I35e46e225bbc80d161639af0206ee77233d488a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

S-waiting-on-bors Status: The marked PR was approved and is only waiting bors

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@flip1995 @tesuji @matthiaskrgr @bors