Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PEP 771] #13170

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/pip/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import List, Optional

__version__ = "25.0.dev0"
__version__ = "25.0.dev0+pep-771"


def main(args: Optional[List[str]] = None) -> int:
Expand Down
1 change: 1 addition & 0 deletions src/pip/_internal/metadata/_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
("Requires-Python", False),
("Requires-External", True),
("Project-URL", True),
("Default-Extra", True),
("Provides-Extra", True),
("Provides-Dist", True),
("Obsoletes-Dist", True),
Expand Down
15 changes: 15 additions & 0 deletions src/pip/_internal/metadata/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,28 @@ def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requiremen

For modern .dist-info distributions, this is the collection of
"Requires-Dist:" entries in distribution metadata.

In case, no "Extra" is specified, will use "Default-Extra" as specified
per PEP 771.
"""
raise NotImplementedError()

def iter_raw_dependencies(self) -> Iterable[str]:
"""Raw Requires-Dist metadata."""
return self.metadata.get_all("Requires-Dist", [])

def iter_default_extras(self) -> Iterable[NormalizedName]:
"""Extras provided by this distribution.

For modern .dist-info distributions, this is the collection of
"Default-Extra:" entries in distribution metadata.

The return value of this function is expected to be normalised names,
per PEP 771, with the returned value being handled appropriately by
`iter_dependencies`.
"""
raise NotImplementedError()

def iter_provided_extras(self) -> Iterable[NormalizedName]:
"""Extras provided by this distribution.

Expand Down
9 changes: 9 additions & 0 deletions src/pip/_internal/metadata/importlib/_dists.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,22 @@ def _metadata_impl(self) -> email.message.Message:
# until upstream can improve the protocol. (python/cpython#94952)
return cast(email.message.Message, self._dist.metadata)

def iter_default_extras(self) -> Iterable[NormalizedName]:
return [
canonicalize_name(extra)
for extra in self.metadata.get_all("Default-Extra", [])
]

def iter_provided_extras(self) -> Iterable[NormalizedName]:
return [
canonicalize_name(extra)
for extra in self.metadata.get_all("Provides-Extra", [])
]

def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]:
if not extras:
extras = list(self.iter_default_extras())

contexts: Sequence[Dict[str, str]] = [{"extra": e} for e in extras]
for req_string in self.metadata.get_all("Requires-Dist", []):
# strip() because email.message.Message.get_all() may return a leading \n
Expand Down
6 changes: 6 additions & 0 deletions src/pip/_internal/metadata/pkg_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,19 @@ def _metadata_impl(self) -> email.message.Message:
return feed_parser.close()

def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]:
extras = extras or self._dist.default_extras_require

if extras:
relevant_extras = set(self._extra_mapping) & set(
map(canonicalize_name, extras)
)
extras = [self._extra_mapping[extra] for extra in relevant_extras]

return self._dist.requires(extras)

def iter_default_extras(self) -> Iterable[NormalizedName]:
return self._dist.default_extras_require or []

def iter_provided_extras(self) -> Iterable[NormalizedName]:
return self._extra_mapping.keys()

Expand Down
6 changes: 6 additions & 0 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,12 @@ def _prepare_linked_requirement(
self.build_isolation,
self.check_build_deps,
)

# Setting up the default-extra if necessary
default_extras = frozenset(dist.metadata.get_all("Default-Extra", []))
req.extras = req.extras or default_extras
req.req.extras = req.extras or default_extras

return dist

def save_linked_requirement(self, req: InstallRequirement) -> None:
Expand Down
2 changes: 2 additions & 0 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ def __init__(
version=version,
)

template.extras = ireq.extras

def _prepare_distribution(self) -> BaseDistribution:
preparer = self._factory.preparer
return preparer.prepare_linked_requirement(self._ireq, parallel_builds=True)
Expand Down
11 changes: 9 additions & 2 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def _make_candidate_from_link(
base: Optional[BaseCandidate] = self._make_base_candidate_from_link(
link, template, name, version
)
extras = extras if extras else template.extras
if not extras or base is None:
return base
return self._make_extras_candidate(base, extras, comes_from=template)
Expand Down Expand Up @@ -482,6 +483,9 @@ def _make_requirements_from_install_req(
(or link) and one with the extra. This allows centralized constraint
handling for the base, resulting in fewer candidate rejections.
"""
if ireq.comes_from is not None and hasattr(ireq.comes_from, "extra"):
requested_extras = requested_extras or ireq.comes_from.extras

if not ireq.match_markers(requested_extras):
logger.info(
"Ignoring %s: markers '%s' don't match your environment",
Expand All @@ -504,6 +508,9 @@ def _make_requirements_from_install_req(
name=canonicalize_name(ireq.name) if ireq.name else None,
version=None,
)

extras = ireq.extras or list(cand.dist.iter_default_extras())

if cand is None:
# There's no way we can satisfy a URL requirement if the underlying
# candidate fails to build. An unnamed URL must be user-supplied, so
Expand All @@ -517,10 +524,10 @@ def _make_requirements_from_install_req(
else:
# require the base from the link
yield self.make_requirement_from_candidate(cand)
if ireq.extras:
if extras:
# require the extras on top of the base candidate
yield self.make_requirement_from_candidate(
self._make_extras_candidate(cand, frozenset(ireq.extras))
self._make_extras_candidate(cand, frozenset(extras))
)

def collect_root_requirements(
Expand Down
33 changes: 24 additions & 9 deletions src/pip/_vendor/distlib/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#
"""Implementation of the Metadata for Python packages PEPs.

Supports all metadata formats (1.0, 1.1, 1.2, 1.3/2.1 and 2.2).
Supports all metadata formats (1.0, 1.1, 1.2, 1.3/2.1, 2.2, 2.3, 2.4 and 2.5).
"""
from __future__ import unicode_literals

Expand Down Expand Up @@ -89,13 +89,20 @@ class MetadataInvalidError(DistlibException):

_643_FIELDS = _566_FIELDS + _643_MARKERS

# PEP 771
_771_MARKERS = ('Default-Extra')

_771_FIELDS = _643_FIELDS + _771_MARKERS


_ALL_FIELDS = set()
_ALL_FIELDS.update(_241_FIELDS)
_ALL_FIELDS.update(_314_FIELDS)
_ALL_FIELDS.update(_345_FIELDS)
_ALL_FIELDS.update(_426_FIELDS)
_ALL_FIELDS.update(_566_FIELDS)
_ALL_FIELDS.update(_643_FIELDS)
_ALL_FIELDS.update(_771_FIELDS)

EXTRA_RE = re.compile(r'''extra\s*==\s*("([^"]+)"|'([^']+)')''')

Expand All @@ -115,6 +122,8 @@ def _version2fieldlist(version):
# return _426_FIELDS
elif version == '2.2':
return _643_FIELDS
elif version == '2.5':
return _771_FIELDS
raise MetadataUnrecognizedVersionError(version)


Expand All @@ -125,7 +134,7 @@ def _has_marker(keys, markers):
return any(marker in keys for marker in markers)

keys = [key for key, value in fields.items() if value not in ([], 'UNKNOWN', None)]
possible_versions = ['1.0', '1.1', '1.2', '1.3', '2.1', '2.2'] # 2.0 removed
possible_versions = ['1.0', '1.1', '1.2', '1.3', '2.1', '2.2', '2.5'] # 2.0 removed

# first let's try to see if a field is not part of one of the version
for key in keys:
Expand All @@ -148,6 +157,9 @@ def _has_marker(keys, markers):
if key not in _643_FIELDS and '2.2' in possible_versions:
possible_versions.remove('2.2')
logger.debug('Removed 2.2 due to %s', key)
if key not in _771_FIELDS and '2.5' in possible_versions:
possible_versions.remove('2.5')
logger.debug('Removed 2.5 due to %s', key)
# if key not in _426_FIELDS and '2.0' in possible_versions:
# possible_versions.remove('2.0')
# logger.debug('Removed 2.0 due to %s', key)
Expand All @@ -165,16 +177,19 @@ def _has_marker(keys, markers):
is_2_1 = '2.1' in possible_versions and _has_marker(keys, _566_MARKERS)
# is_2_0 = '2.0' in possible_versions and _has_marker(keys, _426_MARKERS)
is_2_2 = '2.2' in possible_versions and _has_marker(keys, _643_MARKERS)
if int(is_1_1) + int(is_1_2) + int(is_2_1) + int(is_2_2) > 1:
raise MetadataConflictError('You used incompatible 1.1/1.2/2.1/2.2 fields')
is_2_5 = '2.5' in possible_versions and _has_marker(keys, _771_MARKERS)

if int(is_1_1) + int(is_1_2) + int(is_2_1) + int(is_2_2) + int(is_2_5) > 1:
raise MetadataConflictError('You used incompatible 1.1/1.2/2.1/2.2/2.5 fields')

# we have the choice, 1.0, or 1.2, 2.1 or 2.2
# - 1.0 has a broken Summary field but works with all tools
# - 1.1 is to avoid
# - 1.2 fixes Summary but has little adoption
# - 2.1 adds more features
# - 2.2 is the latest
if not is_1_1 and not is_1_2 and not is_2_1 and not is_2_2:
# - 2.2 adds more features
# - 2.5 is the latest
if not is_1_1 and not is_1_2 and not is_2_1 and not is_2_2 and not is_2_5:
# we couldn't find any specific marker
if PKG_INFO_PREFERRED_VERSION in possible_versions:
return PKG_INFO_PREFERRED_VERSION
Expand All @@ -184,10 +199,10 @@ def _has_marker(keys, markers):
return '1.2'
if is_2_1:
return '2.1'
# if is_2_2:
# return '2.2'
if is_2_2:
return '2.2'

return '2.2'
return '2.5'


# This follows the rules about transforming keys as described in
Expand Down
13 changes: 11 additions & 2 deletions src/pip/_vendor/packaging/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ class RawMetadata(TypedDict, total=False):
license_expression: str
license_files: list[str]

# Metadata 2.5 - PEP 771
default_extra: list[str]


_STRING_FIELDS = {
"author",
Expand Down Expand Up @@ -463,8 +466,8 @@ def parse_email(data: bytes | str) -> tuple[RawMetadata, dict[str, list[str]]]:


# Keep the two values in sync.
_VALID_METADATA_VERSIONS = ["1.0", "1.1", "1.2", "2.1", "2.2", "2.3", "2.4"]
_MetadataVersion = Literal["1.0", "1.1", "1.2", "2.1", "2.2", "2.3", "2.4"]
_VALID_METADATA_VERSIONS = ["1.0", "1.1", "1.2", "2.1", "2.2", "2.3", "2.4", "2.5"]
_MetadataVersion = Literal["1.0", "1.1", "1.2", "2.1", "2.2", "2.3", "2.4", "2.5"]

_REQUIRED_ATTRS = frozenset(["metadata_version", "name", "version"])

Expand Down Expand Up @@ -861,3 +864,9 @@ def from_email(cls, data: bytes | str, *, validate: bool = True) -> Metadata:
"""``Provides`` (deprecated)"""
obsoletes: _Validator[list[str] | None] = _Validator(added="1.1")
"""``Obsoletes`` (deprecated)"""
# PEP 771 lets us define a default `extras_require` if none is passed by the
# user.
default_extra: _Validator[list[utils.NormalizedName] | None] = _Validator(
added="2.5",
)
""":external:ref:`core-metadata-default-extra`"""
6 changes: 6 additions & 0 deletions src/pip/_vendor/pkg_resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3070,6 +3070,8 @@ def requires(self, extras: Iterable[str] = ()):
dm = self._dep_map
deps: list[Requirement] = []
deps.extend(dm.get(None, ()))

extras = extras or self.default_extras_require
for ext in extras:
try:
deps.extend(dm[safe_extra(ext)])
Expand Down Expand Up @@ -3322,6 +3324,10 @@ def clone(self, **kw: str | int | IResourceProvider | None):
def extras(self):
return [dep for dep in self._dep_map if dep]

@property
def default_extras_require(self):
return self._parsed_pkg_info.get_all('Default-Extra') or []


class EggInfoDistribution(Distribution):
def _reload_version(self):
Expand Down
7 changes: 6 additions & 1 deletion src/pip/_vendor/resolvelib/resolvers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import collections
import contextlib
import itertools
import operator

Expand Down Expand Up @@ -172,7 +173,11 @@ def _add_to_criteria(self, criteria, requirement, parent):
)
if not criterion.candidates:
raise RequirementsConflicted(criterion)
criteria[identifier] = criterion

with contextlib.suppress(AttributeError):
requirement._extras = requirement._ireq.extras

criteria[requirement.name] = criterion
Comment on lines +176 to +180
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, if PEP 771 is accepted, be aware you will not be able to submit this specific patch to resolvelib. Resolvelib is very generic and has no concept of extras, you will to solve this in someway in src/pip/_internal/resolvelib

Copy link
Author

@DEKHTIARJonathan DEKHTIARJonathan Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notatallshaw who is the best person to have a chat in the future about this PR ? Getting some directions ?
This PEP is a lot more difficult to implement than I anticipated and I'm not entirely sure I took the best approach.

I used the debugger to track up and down how extras are being managed, though it's far from being trivial and my overall confidence in this PR is not very high.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, as far as I'm aware all active pip maintainers volunteer personal time to do their maintenance duties and therefore it's extremely limited.

I see pip maintainers @pfmoore, @pradyunsg, and myself have all contributed or are actively involved in the PEP 771 discussion.

For my part I will review anything that touches the resolution code, but I can't currently help with larger architectural / mentoring tasks sorry.

Copy link
Author

@DEKHTIARJonathan DEKHTIARJonathan Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic thanks for your comment ;)
For now we don't need to worry about. Once a decision is taken on the PEP, I'll see what needs to be done.

And if I have specific questions I can always ask (and hope for the best ahaha)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to help out - I haven't yet taken a look at this PR mainly as it's in draft and I've had some RL issues to deal with.

@ichard26 is correct about vendoring, we'll need to get the patches in the source project(s) and wait for a release before we pick them up. We can carry the patches in the PR until that happens, though, so work here won't be blocked.

As for resolvelib, @notatallshaw is correct. There's code in the resolvelib "examples" directory that shows how to handle extras in resolvelib, if you want to see a simple example before diving into the full glory of pip's implementation.

The way pip handles this is that when you see foo[xtra], you basically treat that as a completely separate "candidate", and inject an additional dependency that says that foo[xtra] depends on the exact same version of foo. The reason you need to do it like this is that candidates must have fixed dependencies in resolvelib, so foo[xtra] and foo must be different because they don't have the same dependencies. You're going to have to change this model a bit, as with default extras, foo could include the default extra or not, depending on whether foo[actual_extra] is requested as well. So when you see "foo", you don't actually know what candidate you need to tell resolvelib about...

For a tricky example to think about, consider the following.

A depends on B. B depends on C. C depends on A[xtra]. Now, if you ask to install A, will the default extra of A get installed? I assume not, as down in the dependency tree, A[xtra] is requested, which "switches off" the default extra. But you can't know this when you process the initial requirement ("A") to pass it to resolvelib - so what dependencies do you say A has? Remember, you can't change your decision later...

Actually, this makes me wonder about the proposal itself. If A has a default extra of X, and X specifies a dependency of B, and B depends on A[Y], then if I install A, will X be installed? Because if it is, A[Y] will be installed, and the fact that you installed extra Y means that X (the default extra) shouldn't be installed. I don't know what the answer is here, but the PEP needs to specify, as you can't leave it to individual tools to pick something.


def _remove_information_from_criteria(self, criteria, parents):
"""Remove information from parents of criteria.
Expand Down