CodeQL: Have better pack selection (use really the vendored one)#757
Conversation
The MISRA C++ pack from codeql-coding-standards v2.61.0 is incompatible with the CodeQL 2.26.0 standard library (it fails to compile against the newer cpp-all, e.g. the removed TemplateParameter API). Pin the bundle to 2.21.4, which is one of the CodeQL versions the v2.61.0 release officially supports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b7540b7 to
c95c8f7
Compare
| sha256 = "0144d4bc415aee0d5638119dfb626a2d8689e2ff21758ba211a3984862b8522c", | ||
| url = "https://github.com/github/codeql-action/releases/download/codeql-bundle-v2.26.0/codeql-bundle-linux64.tar.gz", | ||
| sha256 = "a94f674bb3c23ea5e9a2ad06b64847dd0277b15014d2517ecd9c41c88e6caa65", | ||
| url = "https://github.com/github/codeql-action/releases/download/codeql-bundle-v2.21.4/codeql-bundle-linux64.tar.gz", |
There was a problem hiding this comment.
How do we know in the future which bundle can we use together with the coding standard version? Should be put at least a comment with a link that explains how they relate to each other and what is supported with what?
| build_file = "//third_party/codeql:codeql_coding_standards.BUILD", | ||
| commit = "06dc6bc32b05152fbe94dbf341a3e854574c9df5", # v2.61.0 | ||
| patch_args = ["-p1"], | ||
| patches = ["//third_party/codeql:codeql_coding_standards_misra.patch"], |
There was a problem hiding this comment.
Can we also create a PR upstream for that? It seems to be that it should be trivial for them to accept
There was a problem hiding this comment.
I already did: github/codeql-coding-standards#1163
| - "cert": "CERT C++ 2016", "autosar": "AUTOSAR C++ R22-11, R21-11, R20-11, R19-11 and R19-03"} | ||
| + "cert": "CERT C++ 2016", | ||
| + "autosar": "AUTOSAR C++ R22-11, R21-11, R20-11, R19-11 and R19-03", | ||
| + "misra": "MISRA C++ 2023", |
There was a problem hiding this comment.
The official name is "MISRA C++:2023"
There was a problem hiding this comment.
I commented in https://github.com/github/codeql-coding-standards/pull/1163/changes#r3643423369
Maybe they intentionally avoid special characters
| ) | ||
|
|
||
| codeql_release_pack( | ||
| name = "codeql_coding_standards_compiled", |
There was a problem hiding this comment.
With this, we have now codeql_coding_standards_compiled, codeql_coding_standards, and codeql_bundle. I guess all 3 have some restriction between them when it comes to the version. The first 2 I understand they should be the same version. Because codeql_coding_standards is a git_repository and we mention the hash, it is not so easy to keep in sync with the same version, we need to remember to update one when we update the other. I would believe there is a way that we can use something different than git_repository and we can also just rever to the version number, in this way we could extract it in a Bazel variable.
For the codeql_bundle it would be good to mention how this version relates to the others.
| recompiling the queries and without downloading anything from the registry. | ||
|
|
||
| This vendored pack is the ONLY supported query source: if it cannot be | ||
| located we raise instead of silently falling back to any other pack (e.g. a |
There was a problem hiding this comment.
| located we raise instead of silently falling back to any other pack (e.g. a | |
| located we raise an error instead of silently falling back to any other pack (e.g. a |
|
|
||
| runfiles = Runfiles.Create() | ||
| anchor = runfiles.Rlocation( | ||
| "codeql_coding_standards_compiled/pack/qlpack.yml") |
There was a problem hiding this comment.
Nitpick: Not sure if the path would change in the future, but it would be better to extract "codeql_coding_standards_compiled/pack/qlpack.yml" into a variable and use it also in the RuntimeError
c95c8f7 to
3c9846a
Compare
generate_guideline_compliance_summary() looks up each applied standard's pretty name in a hard-coded dict that only contained "cert" and "autosar", so a MISRA C++ run aborted with KeyError: 'misra' while rendering the compliance summary. Patch the vendored codeql-coding-standards checkout to add the "misra" -> "MISRA C++ 2023" mapping, and wire the patch into the codeql_coding_standards git_repository. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Analyze against the pre-compiled MISRA C++ query pack that ships with the codeql-coding-standards v2.61.0 release instead of resolving/compiling the pack at analysis time. The github/codeql-coding-standards release publishes a coding-standards-codeql-packs.zip asset containing each standard's pack already compiled (.qlx files plus every library dependency vendored under .codeql/libraries/). A new repository rule (codeql_release_pack) downloads that zip (pinned by sha256), extracts misra-cpp-coding-standards.tgz and exposes it as @codeql_coding_standards_compiled//:pack. This keeps the analysis hermetic and pinned, needs no network at analysis time, requires no query recompilation, and avoids 'codeql pack create', which crashes inside Bazel's linux sandbox. codeql_lint.py now resolves this vendored pack from runfiles and analyzes the bundled misra-cpp-default.qls suite with --additional-packs. The vendored pack is the only supported query source: if it (or its default suite) cannot be located, analysis fails loudly rather than falling back to a registry download or a runtime-compiled pack. 'codeql pack install' is no longer needed and has been dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Switch the MISRA analysis to the codeql-coding-standards user manual's recommended (non-legacy) invocation for released pack artifacts: reference the pack by its declared '<name>@<version>:<suite>' specifier and make it discoverable via --search-path, instead of passing a bare filesystem path to the suite .qls together with --additional-packs (the legacy-style form). The pack name and version are read from the vendored pack's qlpack.yml, so CodeQL now validates the pack's declared identity and an accidental pack or version drift fails loudly rather than silently analyzing whatever suite sits at a path. Verified end to end: 218/218 queries reuse the pre-compiled results (no recompilation), SARIF reports CodeQL 2.21.4 and codeql/misra-cpp-coding-standards 2.61.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The MISRA C++ pack version was pinned in two places: the vendored pre-compiled pack (MODULE.bazel -> @codeql_coding_standards_compiled -> qlpack.yml, which codeql_lint.py reads and passes to 'database analyze') and, redundantly, the 'packs:' list in the code-scanning config.yaml passed to 'database init'. These could drift out of sync. Since the analysis selects the pack explicitly from the vendored pack's declared name@version, the config.yaml 'packs:' entry was dead weight (overridden by the explicit query spec at analyze time). Remove it so the pack has a single source of truth; config.yaml now only carries the analysis-wide query-filters. Verified end to end: unchanged 218/218 queries reuse the pre-compiled results, no registry download, SARIF reports CodeQL 2.21.4 and codeql/misra-cpp-coding-standards 2.61.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review: the query sources/report scripts (@codeql_coding_standards) and the pre-compiled query pack (@codeql_coding_standards_compiled) must come from the same codeql-coding-standards release, but @codeql_coding_standards was a git_repository pinned by commit hash while the pack was pinned by version string, so the two could silently drift and had to be updated in lock-step by hand. Convert @codeql_coding_standards from git_repository (commit) to an http_archive of the release's version tag source tarball, and introduce a single CODING_STANDARDS_VERSION variable that both downloads derive their URL from. A version bump is now a one-line change (plus refreshing the two content sha256s), and the two repositories can no longer reference different versions. Verified end to end: the MISRA patch still applies to the tarball source, and analysis produces the same 218/218 result with reports generated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review nitpick: the vendored pack's runfile path 'codeql_coding_standards_compiled/pack/qlpack.yml' was duplicated in the Rlocation lookup and in the RuntimeError message, so the two could drift if the path ever changes. Hoist it into a single COMPILED_PACK_RUNFILE constant and reference it from both places. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No description provided.