Rust: Prefer richer feature configurations when extracting dependencies by paldepind · Pull Request #22847 · github/codeql

paldepind · 2026-10-20T08:43:17Z

Problem: When a project under extraction contains multiple Cargo.toml files, the Cargo.toml files might specify overlapping dependencies but enable different features for the dependencies.

The extractor previously extracted each dependency file using whichever Cargo.toml it encountered first in a hash-map. This made extraction non-deterministic.

Fix: This PR defers dependency extraction until all Cargo.toml files have been examined, then selects a configuration per file, preferring the one with more enabled features and using deterministic tie-breaking based on paths. Once that is done, the "winning" manifests are reloaded to extract the dependency files for which they "won".

Note to reviewer:

The first three commits are simple refactors.
The key change is in the main function, specifically how we now have two for loops over manifests and in the first pass defer extracting dependency files and instead call dependencies.add_file.
The added tests might be a bit annoying to figure out. The doc comment for test_dependency_features.py attempt to give an overview.

The DCA report shows some changes. Nothing looks particularly good/bad nor actionable, so I think its fine. The real test is wether future DCA runs have less wobble.

Copilot

🟡 Changes recommended

The new configuration unit test cannot compile, and a failed workspace reload now aborts extraction.

3 open issues

What changed in this PR

This PR makes Rust dependency extraction deterministic across workspaces by selecting a configuration for each dependency file, preferring more enabled features.

Changes:

Defer dependency extraction until workspace configurations have been compared.
Count unique manifests in diagnostics while retaining reload durations.
Add richer-feature and tie-breaking regression cases, and update extraction-step expectations.

File	Description
rust/ql/integration-tests/workspace-with-glob/steps.expected	Records the additional manifest load.
rust/ql/integration-tests/hello-workspace/steps.rust-project.expected	Updates project-manifest load expectations.
rust/ql/integration-tests/hello-workspace/steps.cargo.expected	Updates Cargo manifest load expectations.
rust/ql/integration-tests/hello-project/steps.rust-project.expected	Records the project-manifest reload.
rust/ql/integration-tests/hello-project/steps.cargo.expected	Records the Cargo manifest reload.
rust/ql/integration-tests/dependency-features/test_dependency_features.py	Generates competing feature configurations.
rust/ql/integration-tests/dependency-features/source/second/src/lib.rs	Exercises dependency macros from the second workspace.
rust/ql/integration-tests/dependency-features/source/first/src/lib.rs	Exercises dependency macros from the first workspace.
rust/ql/integration-tests/dependency-features/functions.tie.reversed.expected	Captures reversed tie-case results.
rust/ql/integration-tests/dependency-features/functions.tie.expected	Captures equal-feature selection results.
rust/ql/integration-tests/dependency-features/functions.rich.reversed.expected	Captures reversed richer-feature results.
rust/ql/integration-tests/dependency-features/functions.rich.expected	Captures richer-feature selection results.
rust/ql/integration-tests/dependency-features/functions.ql	Checks extracted calls and resolved targets.
rust/ql/integration-tests/dependency-features/dependencies/beta/src/lib.rs	Supplies feature-gated functions and macros.
rust/ql/integration-tests/dependency-features/dependencies/beta/Cargo.toml	Declares beta’s test features.
rust/ql/integration-tests/dependency-features/dependencies/beta/.cargo-checksum.json	Supports the local dependency source.
rust/ql/integration-tests/dependency-features/dependencies/alpha/src/lib.rs	Supplies feature-gated functions and macros.
rust/ql/integration-tests/dependency-features/dependencies/alpha/Cargo.toml	Declares alpha’s test features.
rust/ql/integration-tests/dependency-features/dependencies/alpha/.cargo-checksum.json	Supports the local dependency source.
rust/ql/integration-tests/dependency-features/.cargo/config.toml	Redirects dependency resolution to local fixtures.
rust/extractor/src/util.rs	Adds optional-value comparison support.
rust/extractor/src/rust_analyzer.rs	Clarifies enclosing-workspace lookup and excludes self-matches.
rust/extractor/src/main.rs	Groups sources deterministically and adds deferred dependency extraction.
rust/extractor/src/diagnostics.rs	Deduplicates manifest counts and adds a regression test.
rust/extractor/src/configuration.rs	Implements dependency configuration ranking and selection.

_{Review effort: Max}

Defer dependency extraction until all workspaces have been considered, prefer configurations enabling more Cargo features, and use deterministic tie-breaking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3def7ce6-eea3-4d78-963d-f3675492bb16

Add tests for extraction of dependency files with different features enabled in different manifest files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3def7ce6-eea3-4d78-963d-f3675492bb16

Regenerate existing integration-test expectations to account for manifest reloads in the deferred dependency extraction pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3def7ce6-eea3-4d78-963d-f3675492bb16

geoffw0

Looks plausible, and I'm definitely excited for having better extractor determinism! (light review, one question on the DCA run issue)

Cover macro expansion in a dependency module that is inactive under the preferred configuration, including richer, tied, and reversed manifest cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d675ac38-ec3b-47ee-8b48-268c63fc7777

Rank unambiguous dependency roots using crate graph metadata, preserving semantic ownership checks for roots with differing configurations. Retry inactive modules in lower-priority configurations and bypass selection entirely for single-workspace extractions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d675ac38-ec3b-47ee-8b48-268c63fc7777

paldepind · 2026-10-29T07:27:52Z

Ready for re-review. With the latest changes the increase in database build time is now gone :)

geoffw0

I won't pretend to be clear about all the details here, but the intent and outcome looks good to me (now). 👍

geoffw0 · 2026-10-29T13:18:48Z

+                let sources = entry.get_mut();
+                if let Some(source) = sources
+                    .iter_mut()
+                    .find(|source| source.cfg_options == candidate.cfg_options)


Copilot seems to think this logic is sometimes throwing away entries that we may later need even though their cfg_options are identical. If that's not obviously nonsense it might be worth grilling it about this area (as follow-up).

paldepind added 2 commits October 20, 2026 10:39

Rust: Rename cfg to config

8923c35

Rust: Refactor find_workspace and add doc comment

a5a3b48

github-actions Bot added the Rust Pull requests that update Rust code label Oct 20, 2026

Rust: Extract workspace grouping into a helper

87754be

paldepind force-pushed the rust/inconsistent-feature branch from 3b50eaf to a3e8174 Compare October 20, 2026 11:25

paldepind marked this pull request as ready for review October 22, 2026 06:33

paldepind requested review from a team as code owners October 22, 2026 06:33

Copilot AI max review requested due to automatic review settings October 22, 2026 06:33

Copilot started reviewing on behalf of paldepind October 22, 2026 06:33

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread rust/extractor/src/configuration.rs Outdated

Comment thread rust/extractor/src/main.rs Outdated

Comment thread rust/extractor/src/main.rs Outdated

paldepind and others added 3 commits October 22, 2026 11:26

paldepind force-pushed the rust/inconsistent-feature branch from a3e8174 to c3fdbd8 Compare October 22, 2026 09:30

geoffw0 reviewed Oct 23, 2026

View reviewed changes

paldepind and others added 2 commits October 28, 2026 13:08

paldepind requested a review from geoffw0 October 29, 2026 07:27

geoffw0 approved these changes Oct 29, 2026

View reviewed changes

Navigation Menu

Repository navigation

Rust: Prefer richer feature configurations when extracting dependencies - #22847

Rust: Prefer richer feature configurations when extracting dependencies#22847
paldepind wants to merge 8 commits into
mainfrom
rust/inconsistent-feature

paldepind commented Oct 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

geoffw0 left a comment

Uh oh!

paldepind commented Oct 29, 2026

Uh oh!

geoffw0 left a comment

Uh oh!

geoffw0 Oct 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

paldepind commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

paldepind commented Oct 29, 2026

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

geoffw0 Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paldepind commented Oct 20, 2026 •

edited

Loading