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

Add support for yanked releases and files (PEP-592) #5841

Merged
merged 6 commits into from
Aug 21, 2022
Merged
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
6 changes: 6 additions & 0 deletions src/poetry/inspection/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,15 @@ def __init__(self, path: Path | str, *reasons: BaseException | str) -> None:
class PackageInfo:
def __init__(
self,
*,
name: str | None = None,
version: str | None = None,
summary: str | None = None,
platform: str | None = None,
requires_dist: list[str] | None = None,
requires_python: str | None = None,
files: list[dict[str, str]] | None = None,
yanked: str | bool = False,
cache_version: str | None = None,
) -> None:
self.name = name
Expand All @@ -85,6 +87,7 @@ def __init__(
self.requires_dist = requires_dist
self.requires_python = requires_python
self.files = files or []
self.yanked = yanked
self._cache_version = cache_version
self._source_type: str | None = None
self._source_url: str | None = None
Expand Down Expand Up @@ -117,6 +120,7 @@ def asdict(self) -> dict[str, Any]:
"requires_dist": self.requires_dist,
"requires_python": self.requires_python,
"files": self.files,
"yanked": self.yanked,
"_cache_version": self._cache_version,
}

Expand Down Expand Up @@ -163,6 +167,7 @@ def to_package(
source_type=self._source_type,
source_url=self._source_url,
source_reference=self._source_reference,
yanked=self.yanked,
)
if self.summary is not None:
package.description = self.summary
Expand Down Expand Up @@ -450,6 +455,7 @@ def from_package(cls, package: Package) -> PackageInfo:
requires_dist=list(requires),
requires_python=package.python_versions,
files=package.files,
yanked=package.yanked_reason if package.yanked else False,
)

@staticmethod
Expand Down
3 changes: 1 addition & 2 deletions src/poetry/installation/chooser.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ def _sort_key(

has_allowed_hash = int(self._is_link_hash_allowed_for_package(link, package))

# TODO: Proper yank value
yank_value = 0
yank_value = int(not link.yanked)

return (
has_allowed_hash,
Expand Down
17 changes: 17 additions & 0 deletions src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def __init__(
self._executed = {"install": 0, "update": 0, "uninstall": 0}
self._skipped = {"install": 0, "update": 0, "uninstall": 0}
self._sections: dict[int, SectionOutput] = {}
self._yanked_warnings: list[str] = []
self._lock = threading.Lock()
self._shutdown = False
self._hashes: dict[str, str] = {}
Expand Down Expand Up @@ -140,6 +141,7 @@ def execute(self, operations: list[Operation]) -> int:
# We group operations by priority
groups = itertools.groupby(operations, key=lambda o: -o.priority)
self._sections = {}
self._yanked_warnings = []
for _, group in groups:
tasks = []
serial_operations = []
Expand Down Expand Up @@ -179,6 +181,9 @@ def execute(self, operations: list[Operation]) -> int:

break

for warning in self._yanked_warnings:
self._io.write_error_line(f"<warning>Warning: {warning}</warning>")

return 1 if self._shutdown else 0

@staticmethod
Expand Down Expand Up @@ -611,6 +616,18 @@ def _install_git(self, operation: Install | Update) -> int:
def _download(self, operation: Install | Update) -> Path:
link = self._chooser.choose_for(operation.package)

if link.yanked:
# Store yanked warnings in a list and print after installing, so they can't
# be overlooked. Further, printing them in the concerning section would have
# the risk of overwriting the warning, so it is only briefly visible.
message = (
f"The file chosen for install of {operation.package.pretty_name} "
f"{operation.package.pretty_version} ({link.show_url}) is yanked."
)
if link.yanked_reason:
message += f" Reason for being yanked: {link.yanked_reason}"
self._yanked_warnings.append(message)

return self._download_link(operation, link)

def _download_link(self, operation: Install | Update, link: Link) -> Path:
Expand Down
1 change: 1 addition & 0 deletions src/poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ def search_for(self, dependency: Dependency) -> list[DependencyPackage]:

packages.sort(
key=lambda p: (
not p.yanked,
not p.is_prerelease() and not dependency.allows_prereleases(),
p.version,
),
Expand Down
10 changes: 10 additions & 0 deletions src/poetry/puzzle/solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ def solve(self, use_latest: list[str] | None = None) -> Transaction:
f" {', '.join(f'({b})' for b in self._overrides)}"
)

for p in packages:
if p.yanked:
message = (
f"The locked version {p.pretty_version} for {p.pretty_name} is a"
" yanked version."
)
if p.yanked_reason:
message += f" Reason for being yanked: {p.yanked_reason}"
self._io.write_error_line(f"<warning>Warning: {message}</warning>")

return Transaction(
self._locked_packages,
list(zip(packages, depths)),
Expand Down
2 changes: 1 addition & 1 deletion src/poetry/repositories/cached.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@


class CachedRepository(Repository, ABC):
CACHE_VERSION = parse_constraint("1.0.0")
CACHE_VERSION = parse_constraint("1.1.0")

def __init__(
self, name: str, disable_cache: bool = False, config: Config | None = None
Expand Down
3 changes: 3 additions & 0 deletions src/poetry/repositories/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ def _links_to_data(self, links: list[Link], data: PackageInfo) -> dict[str, Any]
urls = defaultdict(list)
files: list[dict[str, Any]] = []
for link in links:
if link.yanked and not data.yanked:
# drop yanked files unless the entire release is yanked
continue
if link.is_wheel:
urls["bdist_wheel"].append(link.url)
elif link.filename.endswith(
Expand Down
11 changes: 8 additions & 3 deletions src/poetry/repositories/legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def _find_packages(
"""
Find packages on the remote server.
"""
versions: list[Version]
versions: list[tuple[Version, str | bool]]

key: str = name
if not constraint.is_any():
Expand All @@ -90,7 +90,9 @@ def _find_packages(
return []

versions = [
version for version in page.versions(name) if constraint.allows(version)
(version, page.yanked(name, version))
for version in page.versions(name)
if constraint.allows(version)
]
self._cache.store("matches").put(key, versions, 5)

Expand All @@ -101,8 +103,9 @@ def _find_packages(
source_type="legacy",
source_reference=self.name,
source_url=self._url,
yanked=yanked,
)
for version in versions
for version, yanked in versions
]

def _get_release_info(
Expand All @@ -113,6 +116,7 @@ def _get_release_info(
raise PackageNotFound(f'No package named "{name}"')

links = list(page.links_for_version(name, version))
yanked = page.yanked(name, version)

return self._links_to_data(
links,
Expand All @@ -124,6 +128,7 @@ def _get_release_info(
requires_dist=[],
requires_python=None,
files=[],
yanked=yanked,
cache_version=str(self.CACHE_VERSION),
),
)
Expand Down
14 changes: 14 additions & 0 deletions src/poetry/repositories/link_sources/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,17 @@ def clean_link(self, url: str) -> str:
the link, it will be rewritten to %20 (while not over-quoting
% or other characters)."""
return self.CLEAN_REGEX.sub(lambda match: f"%{ord(match.group(0)):02x}", url)

def yanked(self, name: NormalizedName, version: Version) -> str | bool:
reasons = set()
for link in self.links_for_version(name, version):
if link.yanked:
if link.yanked_reason:
reasons.add(link.yanked_reason)
else:
# release is not yanked if at least one file is not yanked
return False
# if all files are yanked (or there are no files) the release is yanked
if reasons:
return "\n".join(sorted(reasons))
return True
8 changes: 7 additions & 1 deletion src/poetry/repositories/link_sources/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ def links(self) -> Iterator[Link]:
url = self.clean_link(urllib.parse.urljoin(self._url, href))
pyrequire = anchor.get("data-requires-python")
pyrequire = unescape(pyrequire) if pyrequire else None
link = Link(url, requires_python=pyrequire)
yanked_value = anchor.get("data-yanked")
yanked: str | bool
if yanked_value:
yanked = unescape(yanked_value)
else:
yanked = "data-yanked" in anchor.attrib
link = Link(url, requires_python=pyrequire, yanked=yanked)

if link.ext not in self.SUPPORTED_FORMATS:
continue
Expand Down
14 changes: 12 additions & 2 deletions src/poetry/repositories/pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ def _find_packages(
continue

if constraint.allows(version):
packages.append(Package(info["info"]["name"], version))
# PEP 592: PyPI always yanks entire releases, not individual files,
# so we just have to look for the first file
yanked = self._get_yanked(release[0])
packages.append(Package(info["info"]["name"], version, yanked=yanked))
Comment on lines +147 to +150
Copy link
Member

Choose a reason for hiding this comment

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

Clearly outside the scope of this PR, but I wonder if PyPI lets you upload a new file on a yanked release, and if so, if this sets yanked / yanked_reason attributes accordingly.

It's a bit sad that we have to assume that a random file that is yanked means that the entire release is yanked, just because of a limitation from the structure of the API, especially since the releases don't seem to be ordered by upload_time:

$ curl -s https://pypi.org/pypi/cryptography/json | jq '.releases["37.0.3"][].upload_time'
"2022-06-21T19:07:43"
"2022-06-21T19:07:48"
"2022-06-21T19:08:26"
"2022-06-21T19:09:32"
"2022-06-21T19:09:38"
"2022-06-21T19:08:32"
"2022-06-21T19:08:03"
"2022-06-21T19:09:44"
"2022-06-21T19:08:55"
"2022-06-21T19:08:59"
"2022-06-21T19:09:04"
"2022-06-21T19:08:40"
"2022-06-21T19:08:10"
"2022-06-21T19:07:52"
"2022-06-21T19:08:45"
"2022-06-21T19:08:15"
"2022-06-21T19:09:08"
"2022-06-21T19:07:57"
"2022-06-21T19:08:49"
"2022-06-21T19:08:20"
"2022-06-21T19:09:11"
"2022-06-21T19:07:26"

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. For now I'd just rely on the PEP which says:

In Warehouse, the user experience will be implemented in terms of yanking or unyanking an entire release, rather than as an operation on individual files, which will then be exposed via the API as individual files being yanked.


return packages

Expand All @@ -163,7 +166,7 @@ def find_links_for_package(self, package: Package) -> list[Link]:
links = []
for url in json_data["urls"]:
h = f"sha256={url['digests']['sha256']}"
links.append(Link(url["url"] + "#" + h))
links.append(Link(url["url"] + "#" + h, yanked=self._get_yanked(url)))

return links

Expand All @@ -188,6 +191,7 @@ def _get_release_info(
requires_dist=info["requires_dist"],
requires_python=info["requires_python"],
files=info.get("files", []),
yanked=self._get_yanked(info),
cache_version=str(self.CACHE_VERSION),
)

Expand Down Expand Up @@ -254,3 +258,9 @@ def _get(self, endpoint: str) -> dict[str, Any] | None:

json: dict[str, Any] = json_response.json()
return json

@staticmethod
def _get_yanked(json_data: dict[str, Any]) -> str | bool:
if json_data.get("yanked", False):
return json_data.get("yanked_reason") or True # noqa: SIM222
return False
7 changes: 6 additions & 1 deletion src/poetry/repositories/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing import TYPE_CHECKING

from poetry.core.semver.helpers import parse_constraint
from poetry.core.semver.version import Version
from poetry.core.semver.version_constraint import VersionConstraint
from poetry.core.semver.version_range import VersionRange

Expand All @@ -16,7 +17,6 @@
from poetry.core.packages.dependency import Dependency
from poetry.core.packages.package import Package
from poetry.core.packages.utils.link import Link
from poetry.core.semver.version import Version


class Repository:
Expand All @@ -43,6 +43,11 @@ def find_packages(self, dependency: Dependency) -> list[Package]:
ignored_pre_release_packages = []

for package in self._find_packages(dependency.name, constraint):
if package.yanked and not isinstance(constraint, Version):
# PEP 592: yanked files are always ignored, unless they are the only
# file that matches a version specifier that "pins" to an exact
# version
continue
if (
package.is_prerelease()
and not allow_prereleases
Expand Down
8 changes: 6 additions & 2 deletions tests/console/commands/test_add.py
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,9 @@ def test_add_constraint_with_source(
):
repo = LegacyRepository(name="my-index", url="https://my-index.fake")
repo.add_package(get_package("cachy", "0.2.0"))
repo._cache.store("matches").put("cachy:0.2.0", [Version.parse("0.2.0")], 5)
repo._cache.store("matches").put(
"cachy:0.2.0", [(Version.parse("0.2.0"), False)], 5
)

poetry.pool.add_repository(repo)

Expand Down Expand Up @@ -1810,7 +1812,9 @@ def test_add_constraint_with_source_old_installer(
):
repo = LegacyRepository(name="my-index", url="https://my-index.fake")
repo.add_package(get_package("cachy", "0.2.0"))
repo._cache.store("matches").put("cachy:0.2.0", [Version.parse("0.2.0")], 5)
repo._cache.store("matches").put(
"cachy:0.2.0", [(Version.parse("0.2.0"), False)], 5
)

poetry.pool.add_repository(repo)

Expand Down
6 changes: 4 additions & 2 deletions tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@
MOCK_DEFAULT_GIT_REVISION = "9cf87a285a2d3fbb0b9fa621997b3acc3631ed24"


def get_package(name: str, version: str | Version) -> Package:
return Package(name, version)
def get_package(
name: str, version: str | Version, yanked: str | bool = False
) -> Package:
return Package(name, version, yanked=yanked)


def get_dependency(
Expand Down
Loading