Fix inclusion of extra specified in dependency group when listed in a conflict#19109
Draft
zanieb wants to merge 7 commits intoastral-sh:mainfrom
Draft
Fix inclusion of extra specified in dependency group when listed in a conflict#19109zanieb wants to merge 7 commits intoastral-sh:mainfrom
zanieb wants to merge 7 commits intoastral-sh:mainfrom
Conversation
zanieb
commented
Apr 22, 2026
zanieb
commented
Apr 22, 2026
9c1a1b6 to
614ef75
Compare
When a dependency group references a `pkg[extra]` self-extra and that extra participates in `[tool.uv.conflicts]`, activating the group should activate the extra so that conflict markers on transitive dependencies evaluate correctly. Previously, the extras gained from a group's `pkg[extra]` entries were only used for a one-shot marker check on the group dep itself and never added to the activated set, so `uv sync` could skip gated transitive dependencies that `uv sync --extra <name>` would install. Fixes astral-sh#19106.
The fix in the previous commit addresses the package-level dependency-groups loop, but an analogous block handles dependency groups on the workspace *manifest* (used when the root `pyproject.toml` has `[tool.uv.workspace]` but no `[project]` table). That loop has the same defect: self-extras activated by `pkg[extra]` group entries are never propagated into `activated_extras`, so conflict-gated transitive deps are silently dropped at sync time. Add a test that pins the buggy behavior and a `TODO(astral-sh#19106)` comment at the matching spot in `installable.rs` pointing at the test.
The previous commits removed `TODO(charlie)` because the fix resolved the specific case in astral-sh#19106, but the underlying concern is only partially addressed: within-group ordering and transitive self-extra chains are still mishandled. Restore a TODO (attributed to zanieb) describing the remaining cases, and align the attribution of the TODO on the manifest-level loop and the accompanying test.
Apply the same fix as for the package-level loop: when a group entry on the workspace manifest (used with non-project workspace roots) activates a self-extra via `pkg[extra]`, add `(pkg, extra)` to `activated_extras` so conflict markers on transitive dependencies evaluate correctly. Drop the TODO on the manifest-level loop and flip the `group_activates_self_extra_non_project_workspace` test to assert the correct behavior (bare `uv sync` now installs the gated transitive dep, matching `uv sync --extra=dev`).
Add two integration tests, `group_activates_self_extra_ordering` and `group_activates_self_extra_transitive`, that pin the buggy behavior described in the `TODO(zanieb)` on the package-level dependency-groups loop in `installable.rs`: - Ordering: a group-dep entry whose marker needs an extra activated by a *later* entry is dropped because the earlier entry is evaluated with an incomplete `activated_extras` set. - Transitive: `project[a]` activates `(project, a)` directly, but `(project, b)` reached via `project.optional-dependencies.a = ["project[b]"]` is only populated during the first-pass traversal, after the group-dep loop. Both tests use hand-crafted lockfiles because the resolver tends to emit markers with vacuously-true clauses that mask the bug. They assert the current (buggy) output alongside the `--extra=<name>` output that produces the correct environment, so that once the underlying TODO is fixed the two snapshots can be collapsed.
The two `group_activates_self_extra_ordering` and `group_activates_self_extra_transitive` tests relied on hand-crafted lockfiles because the resolver does not produce lockfile shapes that would trigger either code path. Tests against impossible inputs don't pin real behavior, so drop them. Reword the `TODO(zanieb)` on the package-level group-dep loop to make clear that these two structural soundness gaps are not reachable via lockfiles the resolver currently emits — it either produces self-extra markers (handled by `newly_activated_extras`) or markers with vacuously-true disjuncts — but should still be fixed for correctness if a future resolver change tightens marker simplification.
614ef75 to
c20b6b8
Compare
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.
Closes #19106