Skip to content

Change versioning scheme to be more robust #57

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

Merged
merged 9 commits into from
Sep 14, 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
3 changes: 3 additions & 0 deletions stub_uploader/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import sys

assert sys.version_info >= (3, 9)
140 changes: 88 additions & 52 deletions stub_uploader/get_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@

from __future__ import annotations

import argparse
import os.path
from collections.abc import Iterable
from typing import Any, Optional
from typing import Any, Union

import requests
import tomli
from packaging.requirements import Requirement
from packaging.specifiers import SpecifierSet
from packaging.version import Version
from requests.adapters import HTTPAdapter
from urllib3.util.retry import Retry

from stub_uploader.const import *
from stub_uploader.metadata import Metadata

PREFIX = "types-"
URL_TEMPLATE = "https://pypi.org/pypi/{}/json"
Expand All @@ -33,7 +31,7 @@
TIMEOUT = 3


def fetch_pypi_versions(distribution: str) -> Iterable[str]:
def fetch_pypi_versions(distribution: str) -> list[Version]:
url = URL_TEMPLATE.format(PREFIX + distribution)
retry_strategy = Retry(total=RETRIES, status_forcelist=RETRY_ON)
with requests.Session() as session:
Expand All @@ -45,28 +43,7 @@ def fetch_pypi_versions(distribution: str) -> Iterable[str]:
return []
raise ValueError("Error while retrieving version")
releases: dict[str, Any] = resp.json()["releases"]
return releases.keys()


def read_base_version(typeshed_dir: str, distribution: str) -> str:
"""Read distribution version from metadata."""
metadata_file = os.path.join(
typeshed_dir, THIRD_PARTY_NAMESPACE, distribution, "METADATA.toml"
)
with open(metadata_file, "rb") as f:
data = tomli.load(f)
version = data["version"]
assert isinstance(version, str)
if version.endswith(".*"):
version = version[:-2]
# Check that the version parses
Version(version)
return version


def strip_dep_version(dependency: str) -> str:
"""Strip a possible version suffix, e.g. types-six>=0.1.4 -> types-six."""
return Requirement(dependency).name
return [Version(release) for release in releases.keys()]


def check_exists(distribution: str) -> bool:
Expand All @@ -83,28 +60,87 @@ def check_exists(distribution: str) -> bool:
raise ValueError("Error while verifying existence")


def main(typeshed_dir: str, distribution: str, version: Optional[str]) -> int:
"""A simple function to get version increment of a third-party stub package.

Supports basic reties and timeouts (as module constants).
"""
pypi_versions = fetch_pypi_versions(distribution)
if not version:
# Use the METADATA.toml version, if not given one.
version = read_base_version(typeshed_dir, distribution)
matching = [v for v in pypi_versions if v.startswith(f"{version}.")]
if not matching:
return -1
increment = max(int(v.split(".")[-1]) for v in matching)
return increment


if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument("typeshed_dir", help="Path to typeshed checkout directory")
parser.add_argument("distribution", help="Third-party distribution to build")
parser.add_argument(
"version", nargs="?", help="Base version for which to get increment"
def ensure_specificity(ver: list[int], specificity: int) -> None:
ver.extend([0] * (specificity - len(ver)))


def compute_incremented_version(
version_spec: str, published_versions: list[Version]
) -> Version:
# The most important postcondition is that the incremented version is greater than
# all published versions. This ensures that users who don't pin get the most
# up to date stub. If we ever maintain multiple versions for a stub, this will
# need revisiting.
max_published = max(published_versions, default=Version("0"))

# The second thing we try to do (but don't guarantee), is that the incremented
# version will satisfy the version_spec (defined precisely by the `compatible`
# specifier below). This allows users to have expectations of what a stub package
# will contain based on the upstream version they're targeting.
if version_spec.endswith(".*"):
compatible = SpecifierSet(f"=={version_spec}")
else:
compatible = SpecifierSet(f"=={version_spec}.*")

# Look up the base version and specificity in METADATA.toml.
version_base = Version(version_spec.removesuffix(".*"))
specificity = len(version_base.release)

if max_published.epoch > 0 or version_base.epoch > 0:
raise NotImplementedError("Epochs in versions are not supported")

increment_specificity = specificity + 1
# TODO: uncomment this in follow-up PR
# We'll try to bump the fourth part of the release. So e.g. if our version_spec is
# 1.1, we'll release 1.1.0.0, 1.1.0.1, 1.1.0.2, ...
# But if our version_spec is 5.6.7.8, we'll release 5.6.7.8.0, 5.6.7.8.1, ...
Comment on lines +94 to +96
Copy link
Member

Choose a reason for hiding this comment

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

Why change this behavior? For example, types-tzlocal currently has version 4.2.2, while tzlocal has version 4.2.

Copy link
Contributor Author

@hauntsaninja hauntsaninja Aug 26, 2022

Choose a reason for hiding this comment

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

I think it helps disambiguate "upstream version" from "types version". I've seen someone be confused by this and pin unnecessarily strictly.

E.g. if tzlocal released 4.2.1 that would still be compatible with 4.2 specified in METADATA.toml. But the correct corresponding version of types-tzlocal is still 4.2.2.

The issue comes about because the target version in METADATA.toml is hidden information to the average user. Very few packages use the fourth spot, so defaulting to the fourth spot in practice makes it clearer that the version isn't 1:1 with upstream versions since they are unlikely to overlap.

Is this a change you're open to?

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 fine, but I'm concerned this could break types-tzlocal (and potentially others). I would like to see this changed in a separate PR to keep possible breakage per PR minimal.

Copy link
Contributor Author

@hauntsaninja hauntsaninja Sep 9, 2022

Choose a reason for hiding this comment

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

I'm curious if your concern can be turned into suggestions for tests. I just added a test_version_increment that runs determine_incremented_version on all distributions we publish. This checks that all invariants we assert are met for all currently distributed packages. Let me know if there's anything else that would increase confidence in these changes.

That aside, yes, I can comment this change out for now and raise exception in the change above

# increment_specificity = max(specificity + 1, 4)

if version_base.release < max_published.release[:specificity]:
raise AssertionError("TODO: remove this exception in follow-up PR")

# Our published versions have gone too far ahead the upstream version
# So we can't guarantee our second property.
# In practice, this will only happen if the specificity of version_spec is
# changed or we change our versioning scheme.
# For example, version_base=1.2, max_published=1.3.0.4, return 1.3.0.5
increment_specificity = max(increment_specificity, len(max_published.release))
incremented = list(max_published.release)
ensure_specificity(incremented, increment_specificity)
incremented[-1] += 1

incremented_version = Version(".".join(map(str, incremented)))
assert incremented_version > max_published
# But can't keep versioning compatible with upstream...
assert incremented_version not in compatible
return incremented_version

if version_base.release > max_published.release[:specificity]:
# For example, version_base=1.2, max_published=1.1.0.4, return 1.2.0.0
incremented = list(version_base.release)
ensure_specificity(incremented, increment_specificity)

else:
assert version_base.release == max_published.release[:specificity]
# For example, version_base=1.1, max_published=1.1.0.4, return 1.1.0.5
incremented = list(max_published.release)
ensure_specificity(incremented, increment_specificity)
incremented[-1] += 1

incremented_version = Version(".".join(map(str, incremented)))
assert incremented_version > max_published
assert incremented_version in compatible
return incremented_version


def strip_dep_version(dependency: str) -> str:
"""Strip a possible version suffix, e.g. types-six>=0.1.4 -> types-six."""
return Requirement(dependency).name


def determine_incremented_version(metadata: Metadata) -> str:
published_stub_versions = fetch_pypi_versions(metadata.distribution)
version = compute_incremented_version(
metadata.version_spec, published_stub_versions
)
args = parser.parse_args()
print(main(args.typeshed_dir, args.distribution, args.version))
return str(version)
29 changes: 12 additions & 17 deletions stub_uploader/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,21 @@

import tomli

from stub_uploader import get_version

from .const import META, THIRD_PARTY_NAMESPACE


class Metadata:
def __init__(self, data: Dict[str, Any]):
def __init__(self, distribution: str, data: Dict[str, Any]):
self.distribution = distribution
self.data = data

@classmethod
def from_file(cls, path: str) -> "Metadata":
with open(path, "rb") as f:
return cls(tomli.load(f))
@property
def version_spec(self) -> str:
# The "version" field in METADATA.toml isn't actually a version, it's more
# like a specifier, e.g. we allow it to contain wildcards.
version = self.data["version"]
assert isinstance(version, str)
return version

@property
def requires(self) -> List[str]:
Expand All @@ -37,13 +39,6 @@ def no_longer_updated(self) -> bool:
def read_metadata(typeshed_dir: str, distribution: str) -> Metadata:
"""Parse metadata from file."""
path = os.path.join(typeshed_dir, THIRD_PARTY_NAMESPACE, distribution, META)
return Metadata.from_file(path)


def determine_version(typeshed_dir: str, distribution: str) -> str:
version = get_version.read_base_version(typeshed_dir, distribution)
increment = get_version.main(typeshed_dir, distribution, version)
if increment >= 0:
print(f"Existing version found for {distribution}")
increment += 1
return f"{version}.{increment}"
with open(path, "rb") as f:
data = tomli.load(f)
return Metadata(distribution=distribution, data=data)
8 changes: 5 additions & 3 deletions stub_uploader/upload_changed.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
import subprocess

from stub_uploader import build_wheel, get_changed, update_changelog
from stub_uploader.metadata import determine_version, read_metadata
from stub_uploader.metadata import read_metadata
from stub_uploader.get_version import determine_incremented_version


def main(typeshed_dir: str, commit: str, uploaded: str, dry_run: bool = False) -> None:
Expand All @@ -29,15 +30,16 @@ def main(typeshed_dir: str, commit: str, uploaded: str, dry_run: bool = False) -
)
print("Building and uploading stubs for:", ", ".join(to_upload))
for distribution in to_upload:
version = determine_version(typeshed_dir, distribution)
metadata = read_metadata(typeshed_dir, distribution)
version = determine_incremented_version(metadata)
update_changelog.update_changelog(
typeshed_dir, commit, distribution, version, dry_run=dry_run
)
temp_dir = build_wheel.main(typeshed_dir, distribution, version)
if dry_run:
print(f"Would upload: {distribution}, version {version}")
continue
for dependency in read_metadata(typeshed_dir, distribution).requires:
for dependency in metadata.requires:
build_wheel.verify_dependency(typeshed_dir, dependency, uploaded)
subprocess.run(["twine", "upload", os.path.join(temp_dir, "*")], check=True)
build_wheel.update_uploaded(uploaded, distribution)
Expand Down
8 changes: 5 additions & 3 deletions stub_uploader/upload_some.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
import subprocess

from stub_uploader import build_wheel
from stub_uploader.metadata import determine_version, read_metadata
from stub_uploader.metadata import read_metadata
from stub_uploader.get_version import determine_incremented_version


def main(typeshed_dir: str, pattern: str, uploaded: str) -> None:
Expand All @@ -31,8 +32,9 @@ def main(typeshed_dir: str, pattern: str, uploaded: str) -> None:
)
print("Uploading stubs for:", ", ".join(to_upload))
for distribution in to_upload:
version = determine_version(typeshed_dir, distribution)
for dependency in read_metadata(typeshed_dir, distribution).requires:
metadata = read_metadata(typeshed_dir, distribution)
version = determine_incremented_version(metadata)
for dependency in metadata.requires:
build_wheel.verify_dependency(typeshed_dir, dependency, uploaded)
# TODO: Update changelog
temp_dir = build_wheel.main(typeshed_dir, distribution, version)
Expand Down
15 changes: 10 additions & 5 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@
"""
import os
import pytest
from packaging.version import Version
from stub_uploader import get_version, build_wheel
from stub_uploader.metadata import read_metadata

TYPESHED = "../typeshed"
UPLOADED = "data/uploaded_packages.txt"


def test_version() -> None:
def test_fetch_pypi_versions() -> None:
"""Check that we can query PyPI for package increments."""
assert get_version.main(TYPESHED, "six", "0.1") >= 0
assert get_version.main(TYPESHED, "nonexistent-distribution", "0.1") == -1
assert get_version.main(TYPESHED, "typed-ast", "0.1") == -1
assert get_version.main(TYPESHED, "typed-ast", None) >= 0
assert Version("1.16.0") in get_version.fetch_pypi_versions("six")
assert Version("1.5.4") in get_version.fetch_pypi_versions("typed-ast")
assert not get_version.fetch_pypi_versions("nonexistent-distribution")


def test_check_exists() -> None:
Expand All @@ -33,6 +33,11 @@ def test_build_wheel(distribution: str) -> None:
assert list(os.listdir(tmp_dir)) # check it is not empty


@pytest.mark.parametrize("distribution", os.listdir(os.path.join(TYPESHED, "stubs")))
def test_version_increment(distribution: str) -> None:
get_version.determine_incremented_version(read_metadata(TYPESHED, distribution))


def test_verify_dependency() -> None:
# Check some known dependencies that they verify as valid.
build_wheel.verify_dependency(TYPESHED, "types-six", UPLOADED)
Expand Down
Loading