Skip to content

Fix extra marker evaluation for requested extras - #14141

Open
KRRT7 wants to merge 15 commits into
pypa:mainfrom
KRRT7:issue-14139-extra-ne-markers
Open

Fix extra marker evaluation for requested extras#14141
KRRT7 wants to merge 15 commits into
pypa:mainfrom
KRRT7:issue-14139-extra-ne-markers

Conversation

@KRRT7

@KRRT7 KRRT7 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #14139.

This updates pip's handling of marker clauses involving extra so they are evaluated against the selected extras set for the requirement/package, instead of evaluating each selected extra independently in a way that made negated markers too broad.

The intended set-wide behavior is:

  • no requested extras still evaluates extra as the empty string
  • extra == "name" and extra in ... match if any selected extra matches
  • extra != "name" and extra not in ... match only if all selected extras match
  • other extra comparisons continue to use packaging's marker comparison semantics, aggregated across the selected extras set

Implementation notes:

  • adds shared marker handling in pip._internal.utils.markers
  • centralizes metadata dependency filtering in BaseDistribution.iter_dependencies() instead of duplicating it across metadata backends
  • models extras in resolvelib's normal criteria flow by identifying requirements/candidates by project name and carrying requested extras on requirements/candidates
  • invalidates a pinned candidate when the active extras set for its project changes, so resolvelib re-pins that project normally and stale extra-conditioned edges are removed by existing criteria cleanup
  • filters unsupported extras before post-install conflict simulation evaluates newly installed package dependencies
  • builds constrained base candidates from extras-free templates before wrapping them with active extras
  • preserves transitive metadata inconsistency diagnostics against the candidate being checked

This deliberately avoids a post-resolution graph repair pass or rerunning the resolver. The resolver continues to converge through resolvelib's existing criterion invalidation/backtracking behavior.

I also opened pypa/packaging#1312 to ask whether packaging wants a public helper for this kind of selected-extras marker evaluation. This PR does not depend on packaging changes.

Tests run:

uv run --group test pytest tests/functional/test_install_check.py tests/functional/test_new_resolver.py::test_new_resolver_inconsistent_metadata_keeps_extras tests/functional/test_new_resolver.py::test_new_resolver_reports_transitive_metadata_mismatch_against_candidate tests/functional/test_new_resolver.py::test_new_resolver_constraint_on_link_with_extra tests/functional/test_new_resolver.py::test_new_resolver_constraint_on_link_with_extra_indirect tests/functional/test_new_resolver.py::test_new_resolver_constrained_link_drops_abandoned_extras -vv
uv run --group test pytest -m 'not network' tests/functional/test_install_extras.py -k 'extra_not_equal_marker' tests/functional/test_new_resolver.py -k 'extra or extras' tests/unit/test_operations_check.py -vv
uv run --group test pytest tests/functional/test_install_report.py -vv
uv run --group test pytest tests/unit/test_req.py tests/unit/metadata/test_metadata.py tests/unit/metadata/test_metadata_pkg_resources.py tests/unit/test_operations_check.py tests/functional/test_invalid_versions_and_specifiers.py::test_show_require_invalid_version -vv
uv run ruff check src/pip/_internal/utils/markers.py src/pip/_internal/metadata/base.py src/pip/_internal/metadata/importlib/_dists.py src/pip/_internal/metadata/pkg_resources.py src/pip/_internal/operations/check.py src/pip/_internal/req/req_install.py src/pip/_internal/resolution/resolvelib/base.py src/pip/_internal/resolution/resolvelib/candidates.py src/pip/_internal/resolution/resolvelib/factory.py src/pip/_internal/resolution/resolvelib/provider.py src/pip/_internal/resolution/resolvelib/requirements.py src/pip/_internal/resolution/resolvelib/resolver.py tests/functional/test_install_check.py tests/functional/test_install_extras.py tests/functional/test_invalid_versions_and_specifiers.py tests/functional/test_new_resolver.py tests/unit/metadata/test_metadata.py tests/unit/metadata/test_metadata_pkg_resources.py tests/unit/test_operations_check.py tests/unit/test_req.py
uv run --python 3.15 --with black black --check src/pip/_internal/utils/markers.py src/pip/_internal/metadata/base.py src/pip/_internal/metadata/importlib/_dists.py src/pip/_internal/metadata/pkg_resources.py src/pip/_internal/operations/check.py src/pip/_internal/req/req_install.py src/pip/_internal/resolution/resolvelib/base.py src/pip/_internal/resolution/resolvelib/candidates.py src/pip/_internal/resolution/resolvelib/factory.py src/pip/_internal/resolution/resolvelib/provider.py src/pip/_internal/resolution/resolvelib/requirements.py src/pip/_internal/resolution/resolvelib/resolver.py tests/functional/test_install_check.py tests/functional/test_install_extras.py tests/functional/test_invalid_versions_and_specifiers.py tests/functional/test_new_resolver.py tests/unit/metadata/test_metadata.py tests/unit/metadata/test_metadata_pkg_resources.py tests/unit/test_operations_check.py tests/unit/test_req.py
git diff --check

@notatallshaw

Copy link
Copy Markdown
Member

Heads up, I am also taking a look at this and may at some point raise my own PR. It's fine to keep working on, but be aware that this may not be accepted in favor of other work.

It seems like this PR is still in a very experimental state, but some quick feedback, I do not like the idea of have to do a post resolution rewalk or rerunning the resolver more than once. I am even willing to push back at the concept of the spec if it requires some kind of iterative reresolve that we have to hope converges.

@KRRT7
KRRT7 marked this pull request as draft July 4, 2026 04:31
@KRRT7

KRRT7 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Update after rework: agreed with this concern.

The PR no longer uses a post-resolution rewalk or reruns the resolver. Extras are now modeled inside resolvelib's normal project criterion flow; see the updated PR body and the follow-up implementation note below.

@KRRT7

KRRT7 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Current implementation update: this now avoids the post-resolution rewalk/rerun approach and keeps extras in resolvelib's normal criteria flow.

The current shape is:

  • requirements and candidates are identified by project name
  • requirements expose requested extras, with shared helper logic to collect the active extras set for a criterion
  • candidates carry their requested extras, and dependency expansion reads from the candidate rather than a separate provider argument
  • if the active extras set for a project changes, the existing pin no longer satisfies that project criterion, so resolvelib re-pins it normally
  • stale extra-conditioned dependency edges fall out through resolvelib's existing criteria cleanup, rather than through a repair pass after resolution
  • metadata dependency filtering is centralized in BaseDistribution.iter_dependencies()
  • marker comparison semantics are delegated to packaging; pip only changes aggregation across selected extras (any for positive comparisons, all for != / not in)

I also opened pypa/packaging#1312 to ask whether packaging wants a public helper for selected-extras marker evaluation. This PR does not depend on that.

@KRRT7
KRRT7 marked this pull request as ready for review July 4, 2026 06:09
@KRRT7

KRRT7 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@notatallshaw can you retrigger CI? looks like a transient issue.

@notatallshaw

Copy link
Copy Markdown
Member

I generally don't think this is possible, see discussion to ban this: https://discuss.python.org/t/ban-negative-extras-for-extras-marker/108022

A case where this PR silently installs the wrong packages.

project 1.0:
  Provides-Extra: gpu
  Requires-Dist: cpu-only ; extra != "gpu"
  Requires-Dist: gpu-only ; extra == "gpu"
cpu-only 1.0:
  Requires-Dist: project[gpu]
gpu-only 1.0

pip install project (no extras) resolves to:

Processing gpu_only-1.0-py3-none-any.whl (from project[gpu]->cpu-only->project)
Would install gpu-only-1.0 project-1.0

It installs gpu-only, gated behind the gpu extra that was never requested, and drops cpu-only, the default dependency. pip's trace attributes gpu-only to cpu-only, which is not installed. There is no valid install: cpu-only is required only while gpu is inactive, but cpu-only activates gpu.

@ichard26 ichard26 added the skip PR template check Silence the PR template check in CI label Jul 5, 2026
@KRRT7

KRRT7 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Chose this fix because the reported case has no stable valid resolution under the current extra != semantics, so silently installing a graph with a dangling dependency chain is incorrect.

The change is conservative: it does not try to make negative extras globally solvable, but prevents pip from accepting a final result whose provenance depends on a package that resolvelib later dropped. This preserves the existing valid additive/backtracking cases while turning the self-invalidating case into an explicit error.

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

Labels

bot:chronographer:provided skip PR template check Silence the PR template check in CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extra != '...' environment markers always evaluate to true

3 participants