Lintcheck: Update crates and expand CI testset to 200 crates

[xFrednet]

This PR adds a new ci_crates.toml to lintcheck for our CI. The 200 crates take about 14 minutes, which is slightly more than the 10 I aimed for but still reasonable. The testset is constructed from:

• 5 crates that compile to binaries
• 4 crates that have been mentioned in ICE issues
• 1 crates "random" crates from lintcheck_crates.toml
• 190 crates from the top 200 crates from crates.io

During testing, I noticed a few panics in lintcheck. I've fixed them where possible, or at least improved the error message.

The new test set generates 500+ MB of json lints, which are compressed to a ~24mb artifact.

---

This PR also updates our lintcheck_crates.toml. I mainly updated the versions, removed some very outdated crates, and added some new ones. I targeted 25 crates as those are pretty fast to lint and a good precursor for our CI.

---

Optional TODO:

• It's likely that some crates are compiled several times. We could potentially safe some time, by using --recursive in our CI.
  This is something I want to investigate, but it shouldn't be a blocker for this PR.

---

r? @Alexendoo

changelog: none

[flip1995]

which is slightly more than the 10 I aimed for but still reasonable.

Could we just cut down the most popular crates to 100-150 to achieve that? 15 minutes is a long time for a CI run. I think all other jobs finish in about 5-7 minutes usually.

[xFrednet]

I think it's fine if lintcheck takes a bit longer, since it will usually pass unless, there are ICEs. I intentionally aimed for the top 200 crates due to this comment from @y21 on Zulip

I almost always run lintcheck on the top 200 crates and it's fairly common that FPs only show up in the lower top 100-200 and nothing with the default crates list

Also, I'm guessing that it's usually the reviewer or triage that looks at the lintcheck output. Then it's better if we have more data. This is also usually async to the PR author.

[Alexendoo]

• It's likely that some crates are compiled several times. We could potentially safe some time, by using --recursive in our CI.
  This is something I want to investigate, but it shouldn't be a blocker for this PR.

Yeah there'll be a whole bunch

Getting --recursive to be deterministic enough to use in CI is something I've been looking at. Currently dependencies of checked crates can change between runs, usually this doesn't cause issues but it definitely would if we're linting those deps

[xFrednet]

Currently dependencies of checked crates can change between runs, usually this doesn't cause issues but it definitely would if we're linting those deps

Is this because cargo doesn't always respect the lock files?

Then let's not block the PR on that, but keep that as a hopeful performance boost in the future

[Alexendoo]

Some things didn't have Cargo.locks IIRC, but that's worth double checking, maybe we don't have to do anything

[xFrednet]

Should I check that in this branch ?

[Alexendoo]

Nah doesn't have to be done here

[Alexendoo]

I had a look and yeah many libraries don't include Cargo.locks in the package because they're gitignored

How long are we talking for 100/150 crates?

[xFrednet]

I had a look and yeah many libraries don't include Cargo.locks in the package because they're gitignored

We could try to hack around that by having a crate in our repo that includes all of them with a Cargo.lock file. But that seems very hacky :/

[xFrednet]

Crates	Time [min]
100	~11
150	~13
200	~14
Without binaries	~8
No deno	10:40
No deno cargo	9:40

[xFrednet]

Removing the binary crates drastically cuts down the time to ~8 min. Even with the other 200 crates 🤔

I would like to at least keep a few binary crates. I can see which ones take the longest:

Deno and cargo. Let's see what happens without deno

[xFrednet]

Without cargo and deno, w#re at slightly below 10 minutes. Does that sound like a better CI time?

[flip1995]

Could we start a parallel job for the binaries?

[xFrednet]

We would need to modify lintcheck to be able to load and compare multiple json files (To keep the uniform output in a single step). I still don't think it's worth it, though. As mentioned, I'm guessing that it will be mostly us as reviewers that look at lintcheck and regular contributors that know about our CI. And since this only runs on PRs, it's less a thing which you check during development, but instead in the PR once you feel like the implementation is mostly done.

So based on this, I think having a longer run time of now 9 minutes (or 10 with cargo) is totally fine.

[flip1995]

You could run lintcheck twice and upload the json files with 2 different names and then also run the comparison twice. No need to modify lintcheck. Just the workflow would get a bit more complicated and we'd need 2 crates.toml files in the repo.

And since this only runs on PRs

Oh, it doesn't run on bors? In that case it would not slow down merging PRs, which was my main concern.

[flip1995]

Yeah, 9 or 10 minutes are fine. Just tried to figure out how we could manage to not drop binaries you planned to include, without increasing CI time.

[Alexendoo]

10 minutes including cargo sounds good to me too, cargo catches a lot since it's a nice large codebase

[xFrednet]

Oh, it doesn't run on bors? In that case it would not slow down merging PRs, which was my main concern.

It only runs on PRs and is meant as input for discussions. The CI is always green, unless there is an ICE. So we assume that an ICE would be caught by the reviewer, but otherwise there is little reason to run it with bors I'd say.

10 minutes including cargo sounds good to me too, cargo catches a lot since it's a nice large codebase

Awesome, then I'll squash the testing commits :D

[xFrednet]

I've only squashed and reordered the commits. The content should be the same :D

[Alexendoo]

🚀

@bors r+

[bors]

📌 Commit 8940bc3 has been approved by Alexendoo

It is now in the queue for this repository.

[bors]

⌛ Testing commit 8940bc3 with merge 057c4ae...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 057c4ae to master...

[xFrednet]

Nice! Lintcheck is getting better every day. Now there are like one or two more small things I want to add, and that should be it for now =^.^=

[flip1995]

Thank you both for all the work on our tooling recently! ❤️