Rust: Reuse Cargo target dir when running QL tests - #22446
Open
paldepind wants to merge 1 commit into
Open
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
paldepind
force-pushed
the
rust/reuse-cargo-target-dir
branch
from
0e6875f to
dc29b10
Compare
August 28, 2026 08:01
paldepind
marked this pull request as ready for review
August 28, 2026 08:19
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1 · 2
| Severity | Finding |
|---|---|
| rust/extractor/src/config.rs — cargo_extra_env is applied afterward and can override or remove RUSTFLAGS, while cargo_check… | |
| rust/extractor/src/config.rs — The documented default for cargo_target_dir is now inaccurate: rust/codeql-extractor.yml:38-39… | |
| rust/extractor/src/config.rs — Correct the spelling of “necessary.” |
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.
| File | Description |
|---|---|
| rust/extractor/src/qltest.rs | Configures preliminary Cargo checks to use the shared target directory. |
| rust/extractor/src/config.rs | Selects 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.
Sorry, something went wrong.
Comment thread
rust/extractor/src/config.rs
Comment thread
rust/extractor/src/config.rs
Comment thread
rust/extractor/src/config.rs
Outdated
paldepind
force-pushed
the
rust/reuse-cargo-target-dir
branch
from
dc29b10 to
c83a41e
Compare
August 28, 2026 09:07
paldepind
force-pushed
the
rust/reuse-cargo-target-dir
branch
from
c83a41e to
516815c
Compare
August 28, 2026 09:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.