Skip to content

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

Open
paldepind wants to merge 6 commits into
mainfrom
rust/inconsistent-feature
Open

paldepind wants to merge 6 commits into
mainfrom
rust/inconsistent-feature

Conversation

@paldepind

@paldepind paldepind commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Oct 20, 2026
@paldepind
paldepind force-pushed the rust/inconsistent-feature branch from 3b50eaf to a3e8174 Compare October 20, 2026 11:25
@paldepind
paldepind marked this pull request as ready for review October 22, 2026 06:33
@paldepind
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 AI 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.

🟡 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

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
Defer dependency extraction until all workspaces have been considered,
prefer configurations enabling more Cargo features, and use
deterministic tie-breaking.

Co-authored-by: Copilot <[email protected]>
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 <[email protected]>
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 <[email protected]>
Copilot-Session: 3def7ce6-eea3-4d78-963d-f3675492bb16
@paldepind
paldepind force-pushed the rust/inconsistent-feature branch from a3e8174 to c3fdbd8 Compare October 22, 2026 09:30

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants