Repository navigation
QL4QL: Install CodeQL with setup-codeql rather than init - #22777
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused replacements preserve CLI-path compatibility and provide the required extractors, with no unresolved findings.
Review effort: Balanced
Findings: None
What changed in this PR
Updates QL-for-QL workflows to install the complete CodeQL bundle, ensuring the required YAML extractor is available.
Changes:
- Replaces
initwithsetup-codeqland removes placeholder JavaScript inputs. - Preserves nightly tooling for build and test workflows.
| File | Description |
|---|---|
.github/workflows/ql-for-ql-tests.yml |
Switches both test jobs to CLI-only setup. |
.github/workflows/ql-for-ql-dataset_measure.yml |
Switches dataset measurement to CLI-only setup. |
.github/workflows/ql-for-ql-build.yml |
Switches the build workflow to CLI-only setup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ats workflow Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| env: | ||
| CODEQL_THREADS: 4 # TODO: remove this once it's set by the CLI |
There was a problem hiding this comment.
This is indeed now set by the CLI.
mbg
left a comment
mbg
left a comment
There was a problem hiding this comment.
These changes look good to me from a CodeQL Action / CLI perspective, but it might be good for someone more familiar with these workflows to have a look over this as well.
In particular, I saw that there's some caching going on ("Cache entire extractor" step) and it's not immediately obvious to me whether that is restoring some cached that would otherwise require a full init step to generate. Have you tried a test run with that step removed?
| - name: Find codeql | ||
| id: find-codeql | ||
| uses: github/codeql-action/init@main | ||
| uses: github/codeql-action/setup-codeql@main |
There was a problem hiding this comment.
Nice to see setup-codeql here! Is there a good reason that this was pinned to main? Should we move that to a tag or sha while we are here? (Same question for other instances of this.)
There was a problem hiding this comment.
It's pointing to main to get some internal testing in case we can pick up some bugs before they get released. We can pin it, but perhaps it makes more sense to do that consistently across the other Actions too in a separate PR.
There was a problem hiding this comment.
[..] but perhaps it makes more sense to do that consistently across the other Actions too in a separate PR.
Sure, that's fair.
Good idea, I triggered each of the workflows with the caching steps removed: |
mbg
left a comment
mbg
left a comment
There was a problem hiding this comment.
Thanks for running all the workflows without caching! Pretty happy to approve on that basis 👍🏻
The QL for QL workflows ran the
initAction with a placeholderlanguages: javascriptinput just to get a CodeQL CLI. With per-language bundles, the Action can download a JavaScript-only bundle for that input. This bundle doesn't contain the YAML extractor that the QL extractor uses inpre-finalize.shandqltest.sh, so the build fails with "There's no CodeQL extractor named 'yaml' installed" (example).This uses the
setup-codeqlAction instead, which only installs the CLI and always uses the bundle that contains all languages. Sinceinitno longer runs:CODEQL_THREADSis no longer set, so this passes--threads=0tocodeql database createandcodeql test runinstead. When creating the database, this gives the QL extractor all cores as before and also parallelises TRAP import, which dropped from about 40s to 8s in my test runs. When running tests, it parallelises query evaluation, but doesn't reach the test extractor, which now uses one thread fewer than the number of cores.This also removes the redundant
CODEQL_THREADSsetting from the database stats workflow. I checked locally that its--threads 4argument tocodeql database createalready gives the extractor the same value.I tested this on a branch with
CODEQL_ACTION_PER_LANGUAGE_BUNDLES=true, which makes the build and test workflows fail without this change. With this change, all three workflows passed.