Skip to content

Rust: Reuse Cargo target dir when running QL tests - #22446

Merged
paldepind merged 1 commit into
mainfrom
rust/reuse-cargo-target-dir
Aug 31, 2026
Merged

Rust: Reuse Cargo target dir when running QL tests#22446
paldepind merged 1 commit into
mainfrom
rust/reuse-cargo-target-dir

Conversation

@paldepind

@paldepindpaldepind commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Currently when we run codeql test for Rust, the Rust extractor runs a preliminary cargo check before it begins extraction. This populates a target directory next to the test. The extractor then starts rust-analyzer with a it's target directory being inside the scratch directory. This means that we're create two target directories.

This PR changes this s.t. by default the target directory next to the tests is used by rust-analyzer as well. This speeds up by uncached (without an existing target directory) as well as cached runs.

I had Copilot run the tests in rust/ql/test/library-tests/dataflow/sources/net 2 times on main and 2 times on this branch. The results are below and show a 21%-22% speedup.

This PR should also make CI for Rust faster. The job "Rust Language Tests Linux" took approximately 10 minutes on this PR and 16m and 20m on some other recent Rust PRs. While there's probably a ton of variance in these numbers, it does at least indicate in the direction that CI is faster.

BranchRunStateTime
main1Clean149.32s
main2Cached130.02s
main3Clean119.26s
main4Cached100.05s
rust/reuse-cargo-target-dir1Clean102.45s
rust/reuse-cargo-target-dir2Cached88.37s
rust/reuse-cargo-target-dir3Clean105.06s
rust/reuse-cargo-target-dir4Cached92.99s
BranchClean averageCached average
main134.29s115.04s
rust/reuse-cargo-target-dir103.76s90.68s
Comparison with mainTime savedReduction
Clean30.54s22.74%
Cached24.36s21.17%

@github-actionsgithub-actionsBot added the Rust Pull requests that update Rust code label Aug 27, 2026
@paldepind
paldepindforce-pushed the rust/reuse-cargo-target-dir branch from 0e6875f to dc29b10CompareAugust 28, 2026 08:01
@paldepind
paldepind marked this pull request as ready for review August 28, 2026 08:19
@paldepind
paldepind requested review from a team as code ownersAugust 28, 2026 08:19
CopilotAI balanced review requested due to automatic review settings August 28, 2026 08:19

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: 1 Medium severity · 2 Low severity

New issues introduced by this change (3)
SeverityFinding
Medium severityrust/​extractor/​src/​config.rscargo_extra_env is applied afterward and can override or remove RUSTFLAGS, while cargo_check
Low severityrust/​extractor/​src/​config.rs — The documented default for cargo_target_dir is now inaccurate: rust/codeql-extractor.yml:38-39
Low severityrust/​extractor/​src/​config.rs — Correct the spelling of “necessary.”
What changed in this PR

Reuses Cargo build artifacts during Rust QL tests to reduce extraction time.

Changes:

  • Shares the test-local Cargo target directory with rust-analyzer.
  • Aligns default RUSTFLAGS between Cargo invocations.
FileDescription
rust/​extractor/​src/​qltest.rsConfigures preliminary Cargo checks to use the shared target directory.
rust/​extractor/​src/​config.rsSelects target directories and configures rust-analyzer’s environment.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadrust/extractor/src/config.rs
Comment threadrust/extractor/src/config.rs
Comment threadrust/extractor/src/config.rs Outdated
@paldepind
paldepindforce-pushed the rust/reuse-cargo-target-dir branch from dc29b10 to c83a41eCompareAugust 28, 2026 09:07
@paldepind
paldepindforce-pushed the rust/reuse-cargo-target-dir branch from c83a41e to 516815cCompareAugust 28, 2026 09:10

@hvitvedhvitved left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks plausible to me; great speedup 🎉

@paldepind
paldepind merged commit 95c30d3 into mainAug 31, 2026
13 checks passed
@paldepind
paldepind deleted the rust/reuse-cargo-target-dir branch August 31, 2026 16:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RustPull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@paldepind@hvitved