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

Switch cargo-clippy over to use rustup's clippy-driver #2901

Closed
Manishearth opened this issue Jul 8, 2018 · 6 comments
Closed

Switch cargo-clippy over to use rustup's clippy-driver #2901

Manishearth opened this issue Jul 8, 2018 · 6 comments

Comments

@Manishearth
Copy link
Member

Manishearth commented Jul 8, 2018

Currently cargo-clippy calls the associated cargo installd clippy-driver. Now that we ship clippy-preview, we can instead make it call the one from rustup!

Currently I have to hack this together to run clippy with clippy-preview: https://twitter.com/ManishEarth/status/1015818090548944896

$ CARGO_MANIFEST_DIR=. ~/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/clippy-driver src/main.rs --emit=dep-info,metadata 

We can use ${RUSTUP_HOME}/toolchains/${RUSTUP_TOOLCHAIN} to find the driver. Alternatively, we perhaps can do something like rustfmt so that it's in the path (not sure how this works)

Perhaps we should be shipping cargo-clippy via rustup too? This is what rustfmt does, and it's one fewer step for installing clippy.

cc @kennytm @oli-obk

@Manishearth Manishearth changed the title Switch cargo-clippy over to use clippy-driver Switch cargo-clippy over to use rustup's clippy-driver Jul 8, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jul 8, 2018

Yea, seems like the most sensible thing to just ship cargo-clippy

@Manishearth
Copy link
Member Author

cc @kennytm what should we do here for this?

@Manishearth
Copy link
Member Author

Also, how do we get rustup to put these things in the PATH?

@kennytm
Copy link
Member

kennytm commented Jul 11, 2018

I agreed with @oli-obk that we should just ship cargo-clippy, like cargo-fmt.

@Manishearth The rustfmt support in rustup was added in rust-lang/rustup#1294. You'll need to add cargo-clippy (and maybe clippy-driver too) to the DUP_TOOLS array in src/rustup-cli/self_update.rs and TOOLS array in rc/rustup-win-installer/src/lib.rs, and have rustup release a new version.

@Manishearth
Copy link
Member Author

FWIW it should be okay if preexisting cargo-clippy is overwritten since clippy is going to break anyway

@camsteffen
Copy link
Contributor

Looks like this is completed by rust-lang/rustup#1461?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants