arcnmx / cargo-clippy

cargo clippy
MIT License
68 stars 11 forks source link

Cargo install in the Readme #7

Closed leoyvens closed 8 years ago

leoyvens commented 8 years ago

Cargo install all the things!

arcnmx commented 8 years ago

Does this actually work? As the readme states:

WARNING: simply copying it is not enough; you must specifically add the directory to your path, or use a symbolic link instead.

Which I'd imagine cargo install isn't smart enough to magically do for us. Changes to either cargo-clippy or cargo install seem necessary...

leoyvens commented 8 years ago

Form the cargo install RFC: "To use installed crates one just needs to add the binary path to their PATH environment variable. This will be recommended when cargo install is run if PATH does not already look like it's configured." So you should have it in your PATH if you're trying to cargo install anything. Works for me anyhow.

arcnmx commented 8 years ago

Sure, what I mean is that cargo-clippy requires an additional file (the clippy rlib) to work properly, which it currently uses its executable path to locate. That file needs to be installed alongside it, or otherwise embedded into the program and extracted to a temporary location upon use, or some other workaround. Just the cargo-clippy exe alone isn't enough for cargo-clippy to work properly.

Basically, https://github.com/rust-lang/rfcs/blob/master/text/1200-cargo-install.md#non-binary-artifacts

leoyvens commented 8 years ago

The way I get it to work is by using cargo install and then adding clippy as a dependency. So I guess I'm using a workaround.

arcnmx commented 8 years ago

Yeah, that kind of defeats the purpose of cargo-clippy being a one-off thing you can run on any project... I'd imagine the embed+tempdir approach is what works best in order to be compatible with cargo install.

leoyvens commented 8 years ago

Agreed. Closing this since it's insufficient.

arcnmx commented 8 years ago

Do you want to create an issue for that or shall I?

leoyvens commented 8 years ago

You could since you understand the issue better than I do.