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

Pass Clippy args also trough RUSTFLAGS #6441

Merged
merged 2 commits into from
Dec 13, 2020
Merged

Conversation

ebroto
Copy link
Member

@ebroto ebroto commented Dec 11, 2020

This removes a hack (__CLIPPY_HACKERY__) to add another one :)

It allows this workflow to work:

cargo clippy                             # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop    # no warnings emitted

Before this change the new flag was not taken into consideration in cargo's fingerprint and the warning was emitted again. I guess that ideally we could add a specific env var for compiler wrapper arguments, but in the meantime this should do the job.

changelog: Pass clippy arguments through RUSTFLAGS so that changing them will trigger a rebuild

r? @flip1995
cc @ehuss (I think this may count as another step towards stabilizing RUSTC_WORKSPACE_WRAPPER 😄)

Fixes #5214 and avoids frustration for users unfamiliar with the issue

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 11, 2020
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

It took me some time to understand what this was doing exactly (which is probably because of the hackiness of our driver). I think we can improve this a bit by moving the handling of the rustflags around.

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@ebroto
Copy link
Member Author

ebroto commented Dec 12, 2020

It took me some time to understand what this was doing exactly

I don't blame you, this is an ugly hack :) but I think for the moment it's a big win in usability.

I think you already got it, but the general idea is to pass our args twice, once in RUSTFLAGS. When the driver gets called, RUSTFLAGS has already been transformed in command-line arguments, so we want to remove them if we delegate to normal compilation (hence why we still use CLIPPY_ARGS, to be able to recognise them).

@ebroto ebroto added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 12, 2020
@flip1995
Copy link
Member

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Dec 13, 2020

📌 Commit f93d965 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Dec 13, 2020

⌛ Testing commit f93d965 with merge 684f17e...

@bors
Copy link
Collaborator

bors commented Dec 13, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 684f17e to master...

@bors bors merged commit 684f17e into rust-lang:master Dec 13, 2020
@ebroto ebroto deleted the use_rustflags branch December 13, 2020 23:38
bors added a commit that referenced this pull request Dec 22, 2020
Revert "Pass Clippy args also trough RUSTFLAGS"

Reverts #6441

r? `@ebroto`

changelog: Revert "Pass Clippy args also trough RUSTFLAGS"
@ghost ghost mentioned this pull request Mar 3, 2021
bors added a commit that referenced this pull request Mar 8, 2021
Let Cargo track CLIPPY_ARGS

This PR makes `clippy-driver` emit `CLIPPY_ARGS` in its `dep-info` output.

Just like #6441, this allows this workflow to work:
```shell
cargo clippy # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop # no warnings emitted
```
But without rebuilding all dependencies.

cc https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/CLIPPY_ARGS.20is.20not.20tracked.20by.20Cargo/near/228599088

Changelog: Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed.
bors added a commit that referenced this pull request Mar 8, 2021
Let Cargo track CLIPPY_ARGS

This PR makes `clippy-driver` emit `CLIPPY_ARGS` in its `dep-info` output.

Just like #6441, this allows this workflow to work:
```shell
cargo clippy # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop # no warnings emitted
```
But without rebuilding all dependencies.

cc https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/CLIPPY_ARGS.20is.20not.20tracked.20by.20Cargo/near/228599088

changelog: Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLIPPY_FLAGS to pass options to clippy/ enable disable lints via env vars
4 participants