lintcheck: run clippy on transitive dependencies #8704
Conversation
|
r? @xFrednet (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #8705) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Hm, I'm not really sure if I see the benefit here. I want to avoid as much spurious noise in the logs as possible, so I use a fixed version root crates to run clippy on. If we just care about transitive dependencies and not fixed versions of them, we can probably do something like I'm not fan of losing --fix, the git repo source type was mostly intended for folks at embark who we had a meeting with at some point because of high false positives rates and such iirc, |
|
If you check in the Cargo.lock for https://github.com/matthiaskrgr/clippy-lintcheck it wouldn't show up as spurious, the one in the clippy repo is .gitignore'd, but if you add I went through a few attempts:
My goal is to get lintcheck running more clippy per second, potentially with the aim of running it against PRs (with a smaller set of crates) for say ~1 minute |
|
|
|
☔ The latest upstream changes (presumably #8709) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Aah I see now, you created some kind of "dummy" crate with a few dependencies, One downside of this approach is that this will be less flexible in regards of what kind of crates you can check, for example you won't be able to check two versions of the same crate (cargo 123 and cargo 234), or there can also be weird dependency incompatibilities, for example when two different root crates require different major versions of a c++ crate, I think I have seen something like that with libgit2. Right, running cargo fix on transitive deps like that might not even be possible, I have no idea tbh :/ |
|
Yep! that's it, with the additional step of creating a patch entry for every dep, as being vendored alone isn't enough for cargo to show the warnings from them Two versions of the same crate can work if you give one of them a different name, such as cargo123 = { package = "cargo", version = "1.2.3" }
cargo234 = { package = "cargo", version = "2.3.4" }...but oh god yes native dependencies can be frustrating 💢, there's some that won't let you have multiple versions of themselves in the crate graph. Fortunately that didn't come up with the current list, but it could happen with a different one Today I was looking into how However the That would allow more complicated filtering again also, such as per crate |
I like the general idea, could we maybe have an easy switch for this mode?
lintcheck has a I'm sadly not familiar with the code. @flip1995 or @matthiaskrgr could you maybe take over the review? 🙃 |
|
With the lintcheck tool I always defer to @matthiaskrgr myself 😄 I'm not sure how necessary this change is. Couldn't you just generate a crates list with all dependencies and feed it to the I'm also not really a fan of losing the |
You could yeah, the purpose is to make it run quicker. With the current lintcheck apart from the leaf nodes every crate would be downloaded twice - once by cargo, once by lintcheck. And I think compiled twice as well, once by rustc as a dep, and once by clippy-driver as the primary package
Yeah that's fair, I think I'm going to bring this one back to the drawing board. I'll try another PR with the wrapper method I mentioned earlier, that would allow keeping git sources and per crate options. I believe I can get |
|
I already tried speeding up checking by sharing a target dir between projects |
|
I put together the equivalent list of crates to give it a go with the current lintcheck, runtime was 20mins including download time, 18mins excluding all.toml[crates]
aho-corasick = {name = "aho-corasick", versions = ["0.7.18"]}
anyhow = {name = "anyhow", versions = ["1.0.56"]}
arrayvec = {name = "arrayvec", versions = ["0.5.2"]}
async-trait = {name = "async-trait", versions = ["0.1.53"]}
atty = {name = "atty", versions = ["0.2.14"]}
autocfg = {name = "autocfg", versions = ["0.1.8", "1.1.0"]}
base64 = {name = "base64", versions = ["0.9.3", "0.12.3", "0.13.0"]}
bitflags = {name = "bitflags", versions = ["1.3.2"]}
bitmaps = {name = "bitmaps", versions = ["2.1.0"]}
bstr = {name = "bstr", versions = ["0.2.17"]}
byteorder = {name = "byteorder", versions = ["1.4.3"]}
bytes = {name = "bytes", versions = ["1.1.0"]}
bytesize = {name = "bytesize", versions = ["1.1.0"]}
cargo = {name = "cargo", versions = ["0.61.1"]}
cargo-platform = {name = "cargo-platform", versions = ["0.1.2"]}
cargo-util = {name = "cargo-util", versions = ["0.1.2"]}
cc = {name = "cc", versions = ["1.0.73"]}
cfg-expr = {name = "cfg-expr", versions = ["0.10.2"]}
cfg-if = {name = "cfg-if", versions = ["1.0.0"]}
chrono = {name = "chrono", versions = ["0.4.19"]}
clap = {name = "clap", versions = ["3.1.8"]}
# combine = {name = "combine", versions = ["4.6.3"]}
crates-io = {name = "crates-io", versions = ["0.34.0"]}
# crc32fast = {name = "crc32fast", versions = ["1.3.2"]}
crossbeam-channel = {name = "crossbeam-channel", versions = ["0.5.4"]}
crossbeam-deque = {name = "crossbeam-deque", versions = ["0.8.1"]}
crossbeam-epoch = {name = "crossbeam-epoch", versions = ["0.9.8"]}
crossbeam-utils = {name = "crossbeam-utils", versions = ["0.8.8"]}
crypto-hash = {name = "crypto-hash", versions = ["0.3.4"]}
curl = {name = "curl", versions = ["0.4.43"]}
# curl-sys = {name = "curl-sys", versions = ["0.4.53+curl-7.82.0"]}
cxx = {name = "cxx", versions = ["1.0.66"]}
cxxbridge-flags = {name = "cxxbridge-flags", versions = ["1.0.66"]}
cxxbridge-macro = {name = "cxxbridge-macro", versions = ["1.0.66"]}
either = {name = "either", versions = ["1.6.1"]}
env_logger = {name = "env_logger", versions = ["0.9.0"]}
fastrand = {name = "fastrand", versions = ["1.7.0"]}
filetime = {name = "filetime", versions = ["0.2.15"]}
flate2 = {name = "flate2", versions = ["1.0.23"]}
fnv = {name = "fnv", versions = ["1.0.7"]}
foreign-types = {name = "foreign-types", versions = ["0.3.2"]}
foreign-types-shared = {name = "foreign-types-shared", versions = ["0.1.1"]}
form_urlencoded = {name = "form_urlencoded", versions = ["1.0.1"]}
getrandom = {name = "getrandom", versions = ["0.2.6"]}
git2 = {name = "git2", versions = ["0.14.2"]}
git2-curl = {name = "git2-curl", versions = ["0.15.0"]}
glob = {name = "glob", versions = ["0.3.0"]}
globset = {name = "globset", versions = ["0.4.8"]}
hashbrown = {name = "hashbrown", versions = ["0.11.2"]}
hex = {name = "hex", versions = ["0.3.2", "0.4.3"]}
home = {name = "home", versions = ["0.5.3"]}
http = {name = "http", versions = ["0.2.6"]}
httparse = {name = "httparse", versions = ["1.7.0"]}
humantime = {name = "humantime", versions = ["2.1.0"]}
hyper = {name = "hyper", versions = ["0.10.16"]}
idna = {name = "idna", versions = ["0.1.5", "0.2.3"]}
ignore = {name = "ignore", versions = ["0.4.18"]}
im-rc = {name = "im-rc", versions = ["15.0.0"]}
indexmap = {name = "indexmap", versions = ["1.8.1"]}
iron = {name = "iron", versions = ["0.6.1"]}
itertools = {name = "itertools", versions = ["0.10.3"]}
itoa = {name = "itoa", versions = ["1.0.1"]}
jobserver = {name = "jobserver", versions = ["0.1.24"]}
jsonwebtoken = {name = "jsonwebtoken", versions = ["7.2.0"]}
kstring = {name = "kstring", versions = ["1.0.6"]}
language-tags = {name = "language-tags", versions = ["0.2.2"]}
lazy_static = {name = "lazy_static", versions = ["1.4.0"]}
lazycell = {name = "lazycell", versions = ["1.3.0"]}
libc = {name = "libc", versions = ["0.2.123"]}
# libgit2-sys = {name = "libgit2-sys", versions = ["0.13.2+1.4.2"]}
# libnghttp2-sys = {name = "libnghttp2-sys", versions = ["0.1.7+1.45.0"]}
# libssh2-sys = {name = "libssh2-sys", versions = ["0.2.23"]}
# libz-sys = {name = "libz-sys", versions = ["1.1.5"]}
link-cplusplus = {name = "link-cplusplus", versions = ["1.0.6"]}
linked-hash-map = {name = "linked-hash-map", versions = ["0.5.4"]}
# lintcheck_crates = {name = "lintcheck_crates", versions = ["0.0.1"]}
log = {name = "log", versions = ["0.3.9", "0.4.16"]}
matches = {name = "matches", versions = ["0.1.9"]}
memchr = {name = "memchr", versions = ["2.4.1"]}
memoffset = {name = "memoffset", versions = ["0.6.5"]}
mime = {name = "mime", versions = ["0.2.6"]}
mime_guess = {name = "mime_guess", versions = ["1.8.8"]}
modifier = {name = "modifier", versions = ["0.1.0"]}
num-bigint = {name = "num-bigint", versions = ["0.2.6"]}
num-integer = {name = "num-integer", versions = ["0.1.44"]}
num-traits = {name = "num-traits", versions = ["0.2.14"]}
num_cpus = {name = "num_cpus", versions = ["1.13.1"]}
once_cell = {name = "once_cell", versions = ["1.10.0"]}
opener = {name = "opener", versions = ["0.5.0"]}
openssl = {name = "openssl", versions = ["0.10.38"]}
openssl-probe = {name = "openssl-probe", versions = ["0.1.5"]}
# openssl-sys = {name = "openssl-sys", versions = ["0.9.72"]}
os_info = {name = "os_info", versions = ["3.2.0"]}
os_str_bytes = {name = "os_str_bytes", versions = ["6.0.0"]}
pem = {name = "pem", versions = ["0.8.3"]}
percent-encoding = {name = "percent-encoding", versions = ["1.0.1", "2.1.0"]}
phf = {name = "phf", versions = ["0.7.24"]}
phf_codegen = {name = "phf_codegen", versions = ["0.7.24"]}
phf_generator = {name = "phf_generator", versions = ["0.7.24"]}
phf_shared = {name = "phf_shared", versions = ["0.7.24"]}
pkg-config = {name = "pkg-config", versions = ["0.3.25"]}
plugin = {name = "plugin", versions = ["0.2.6"]}
ppv-lite86 = {name = "ppv-lite86", versions = ["0.2.16"]}
proc-macro2 = {name = "proc-macro2", versions = ["1.0.37"]}
puffin = {name = "puffin", versions = ["0.13.1"]}
quote = {name = "quote", versions = ["1.0.18"]}
rand = {name = "rand", versions = ["0.6.5", "0.8.5"]}
rand_chacha = {name = "rand_chacha", versions = ["0.1.1", "0.3.1"]}
rand_core = {name = "rand_core", versions = ["0.3.1", "0.4.2", "0.5.1", "0.6.3"]}
rand_hc = {name = "rand_hc", versions = ["0.1.0"]}
rand_isaac = {name = "rand_isaac", versions = ["0.1.1"]}
rand_jitter = {name = "rand_jitter", versions = ["0.1.4"]}
rand_os = {name = "rand_os", versions = ["0.1.3"]}
rand_pcg = {name = "rand_pcg", versions = ["0.1.2"]}
rand_xorshift = {name = "rand_xorshift", versions = ["0.1.1"]}
rand_xoshiro = {name = "rand_xoshiro", versions = ["0.4.0"]}
rayon = {name = "rayon", versions = ["1.5.2"]}
rayon-core = {name = "rayon-core", versions = ["1.9.2"]}
regex = {name = "regex", versions = ["1.5.5"]}
regex-automata = {name = "regex-automata", versions = ["0.1.10"]}
regex-syntax = {name = "regex-syntax", versions = ["0.6.25"]}
remove_dir_all = {name = "remove_dir_all", versions = ["0.5.3"]}
# ring = {name = "ring", versions = ["0.16.20"]}
rpmalloc = {name = "rpmalloc", versions = ["0.2.2"]}
# rpmalloc-sys = {name = "rpmalloc-sys", versions = ["0.2.3+b097fd0"]}
rustc-workspace-hack = {name = "rustc-workspace-hack", versions = ["1.0.0"]}
rustfix = {name = "rustfix", versions = ["0.6.0"]}
ryu = {name = "ryu", versions = ["1.0.9"]}
safemem = {name = "safemem", versions = ["0.3.3"]}
same-file = {name = "same-file", versions = ["1.0.6"]}
scopeguard = {name = "scopeguard", versions = ["1.1.0"]}
semver = {name = "semver", versions = ["1.0.7"]}
serde = {name = "serde", versions = ["1.0.136"]}
serde_derive = {name = "serde_derive", versions = ["1.0.136"]}
serde_ignored = {name = "serde_ignored", versions = ["0.1.2"]}
serde_json = {name = "serde_json", versions = ["1.0.79"]}
serde_yaml = {name = "serde_yaml", versions = ["0.8.23"]}
shell-escape = {name = "shell-escape", versions = ["0.1.5"]}
simple_asn1 = {name = "simple_asn1", versions = ["0.4.1"]}
siphasher = {name = "siphasher", versions = ["0.2.3"]}
sized-chunks = {name = "sized-chunks", versions = ["0.6.5"]}
smallvec = {name = "smallvec", versions = ["1.8.0"]}
socket2 = {name = "socket2", versions = ["0.4.4"]}
spin = {name = "spin", versions = ["0.5.2"]}
strip-ansi-escapes = {name = "strip-ansi-escapes", versions = ["0.1.1"]}
strsim = {name = "strsim", versions = ["0.10.0"]}
syn = {name = "syn", versions = ["1.0.91"]}
tame-oidc = {name = "tame-oidc", versions = ["0.4.0"]}
tar = {name = "tar", versions = ["0.4.38"]}
tempfile = {name = "tempfile", versions = ["3.3.0"]}
termcolor = {name = "termcolor", versions = ["1.1.3"]}
textwrap = {name = "textwrap", versions = ["0.15.0"]}
thiserror = {name = "thiserror", versions = ["1.0.30"]}
thiserror-impl = {name = "thiserror-impl", versions = ["1.0.30"]}
thread_local = {name = "thread_local", versions = ["1.1.4"]}
time = {name = "time", versions = ["0.1.43"]}
tinyvec = {name = "tinyvec", versions = ["1.5.1"]}
tinyvec_macros = {name = "tinyvec_macros", versions = ["0.1.0"]}
toml_edit = {name = "toml_edit", versions = ["0.13.4"]}
traitobject = {name = "traitobject", versions = ["0.1.0"]}
typeable = {name = "typeable", versions = ["0.1.2"]}
typemap = {name = "typemap", versions = ["0.3.3"]}
typenum = {name = "typenum", versions = ["1.15.0"]}
unicase = {name = "unicase", versions = ["1.4.2"]}
unicode-bidi = {name = "unicode-bidi", versions = ["0.3.7"]}
# unicode-normalization = {name = "unicode-normalization", versions = ["0.1.19"]}
unicode-width = {name = "unicode-width", versions = ["0.1.9"]}
unicode-xid = {name = "unicode-xid", versions = ["0.2.2"]}
unsafe-any = {name = "unsafe-any", versions = ["0.4.2"]}
untrusted = {name = "untrusted", versions = ["0.7.1"]}
url = {name = "url", versions = ["1.7.2", "2.2.2"]}
utf8parse = {name = "utf8parse", versions = ["0.2.0"]}
version_check = {name = "version_check", versions = ["0.1.5", "0.9.4"]}
vte = {name = "vte", versions = ["0.10.1"]}
vte_generate_state_changes = {name = "vte_generate_state_changes", versions = ["0.1.1"]}
walkdir = {name = "walkdir", versions = ["2.3.2"]}
yaml-rust = {name = "yaml-rust", versions = ["0.4.5"]}
|
|
Somewhat related, I recently encountered a troll on the Rust subreddit asking me how to lint all dependencies. |
That's sadly a big difference. Have you looked into my suggestion, to have a switch for the two modes? 🙃 When we could have the best of both worlds 🙃
Maybe you just found the alt account from one.. ^^ |
By this do you mean a switch for between the current behaviour and running on transitive deps, or between the current crates and fewer crates mode? |
|
Yes, the question is how complicated that would be and if it's worth it. Also one thing I just thought of: The |
|
Any fewer crates mode would definitely be off by default yeah For switching off transitive mode it shouldn't be tooooo bad, I think I would need something like that for |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Ah yeah can close this |
|
Ohh, was this change of the table? I thought we were only waiting on a switch to have the best of both worlds? 🤔 |
|
It turned out to be quite a pain to unify the two ways, bindeps have a few teething issues. I'll still work towards it though it'll just be in a different branch, maybe as an entirely separate mode at first |
|
Okay, thank you for clarifying 🙃 |
cc @matthiaskrgr
changelog: lintcheck: run clippy on transitive dependencies
Switches over to
cargo vendorto download the sources and some creative use ofcargo checkto run clippy on themThe trick is to use the fact that cargo shows warnings for patched dependencies by generating a patch entry for every (unignored) dependency
This brings the number of crates clippy is running on up to 229 (on linux) for the same set of crates (but different versions), the effects being:
Lint count changes
Some more tweaking could be done on the ignore list, but the most substantial outliers are already added
Runtime wise it went from 7:08 to 8:46 on my laptop. Both times are for compiling only, not downloading the sources. My laptop has an old dual core CPU, the difference should be more favourable on higher core count machines due to the improved scheduling.
Feature wise it gained the
--timingsflag to enable cargo timings, which is really handy to track down the slow cratesSome things that are no longer supported are
--fixand git dependencies (I believe path dependencies would still work). You also can no longer set per crate options