ci: improve linting

[benoit-pierre]

Cf. #579 (comment).

---

This change is 

[benoit-pierre]

Rewrote the whole thing, main commit:

• fix cppcheck & clang-tidy configurations to avoid parsing, processing, and/or spurious errors.
• parallelize all checks to speed up linting (which is much slower now that cppcheck & clang-tidy can actually do their jobs).
• always lint all files (we care about), instead of trying to determine what changed: even if a source file, say crengine/src/lvtinydom.cpp, was not changed in a PR, some change to included headers may broke something that could be detected by one of the checks.
• plus colors, groups and error annotations on Github Actions!

[benoit-pierre]

How slow you ask? ~4 minutes without lvtinydom.cpp, 9 minutes for everything.

[Frenzie]

That approach looks very nice. :-) (Talking about the code.)

Wrt to the output I'm not the biggest fan of hiding the stuff you actually want to see, though I suppose it's workable.

[benoit-pierre]

Wrt to the output I'm not the biggest fan of hiding the stuff you actually want to see, though I suppose it's workable.

It does make it easier to zero-in on the problematic part, without being flooded with all the traces. And avoid the need GA horrible slow search mechanism. I agree that it would be better if there was a way to create already opened groups (for those containing errors).

Let's see if annotation created by problem matchers can help (but there's a stupidly low limit of 10 annotations max in the summary…).

[benoit-pierre]

But the summary is showing more than 10… Maybe 10 errors and 10 warnings?

But if you look at the PR diff, there are some annotations there, now!

[benoit-pierre]

There are definitively more than 10 warnings in the log, so yeah, 10 max annotations for each type in the summary.

[benoit-pierre]

I was going to say adding problem matchers to the lint check on koreader & koreader-base would be nice too, but I remembered we're using CircleCI. :P

[Frenzie]

I suppose they could still be helpful in time, and in any case the collapsing (and slow search) aren't an issue when running the lint locally. What do you think @poire-z?

[benoit-pierre]

No annotation for cr3gui/data/hyph/languages.json however, although technically possible with a multi-line matcher, I think.

[Frenzie]

PS typo in the first commit message: "may brake" (break)

[Frenzie]

It's not really clear to me why that dependabot thing automatically shows up in some repos and has to be explicitly set up in others.

[benoit-pierre]

To receive Dependabot alerts, you must first enable Dependabot alerts in this repository’s settings.

[Frenzie]

I feel like back in 2022 it just showed up out of the blue without being (explicitly) enabled on for example repos with a requirements.txt (i.e., Python).

But anyway, my implicit point was provided that thing is set to open its trap at most once a week it could keep an eye on trivialities like the version of the checkout action.

[benoit-pierre]

PS typo in the first commit message: "may brake" (break)

D'oh! /o\

[benoit-pierre]

It does not look like clicking on the annotation link to the workflow log get you to the line that created the annotation (in the PR diff). And there's another 10 max limit there too. ;(

[Frenzie]

I have to say I'm somewhat surprised they'd even add that. But presumably it's different for paying customers.

[poire-z]

I suppose they could still be helpful in time, and in any case the collapsing (and slow search) aren't an issue when running the lint locally. What do you think @poire-z?

I probably won't install and run any lint stuff, so I'll be using Github/CircleCI CIs to check how I did :)
The output I see on #582 looks ok to me (a bit long height to scroll to look for the red stuff, but I'll be ok).

Does this mean that ALL our warnings are resolved ?! And any new one shouted by CI will be to solve ? Do all/most warnings get some -- crenginecheck: ignore kind of solution when we don't have a reaol one?

[benoit-pierre]

Previously, the exit codes of clang-tidy / cppcheck were ignored, so the workflow would fail only on linting the hyphenation patterns or CSS files.

Annotations don't fail the workflow, a non-zero exit code by one of the linters does. So this warning in #582 for example would not fail the job.

With cppcheck, some warnings still result in a non-zero exit code, which is the case with the remaining failures. Those will have to be addressed.

It's similar zlib related code in all 3 instances (in crengine/src/pdbfmt.cpp and crengine/src/lvtinydom.cpp).

[poire-z]

Good to go ? Or still working on it?

[benoit-pierre]

This one is ready.

[poire-z]

Or should we wait until the PR that fixes the warnings is ready ? Seeing the CI failure just below?

[benoit-pierre]

I think it's better to merge this now, since that's the whole point: improve linting. And then fix the CI failures in subsequent PR(s).

[poire-z]

You'll be ready soon with your other PR ? Otherwise, we'll keep getting failures, and it will be hard to notice that any other new PR introduce some new ones or bugs because of the noise.

[benoit-pierre]

Let's wait a little then, I want to address your comments about asserts.

[benoit-pierre]

I've added -DUSE_ZSTD=1 to the compilation flags, and have all the necessary fixes ready to pass the lint. Most of #579 is unrelated anyway, since we don't compile. I'll pull the lint related fixes to separate PR(s) (and the uncontroversial stuff too, to move it out of the way).

This can be merged.

[poire-z]

A little git rebase with my date update & sleep trick to update the commits from Jul 23 to 28 svp ?
(Less bothering for ci stuff than for sources, but good to get the habit :))

[benoit-pierre]

Done.