Skip to content
Open
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
37 changes: 33 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,47 @@ dependencies = [

[tool.uv]
environments = ["sys_platform == 'darwin' or sys_platform == 'linux'"]
no-build-package = [
"avalara",
"fido2",
"google-crc32c",
"hiredis",
"maxminddb",
"mmh3",
"petname",
"pycountry",
"python-rapidjson",
"python-u2flib-server",
"simplejson",
"zstandard",
]

[[tool.uv.index]]
# We don't allow using public pypi - submit a PR to
# https://github.com/getsentry/pypi to add a dependency.
name = "sentry-pypi"
url = "https://pypi.devinfra.sentry.io/simple"
default = true

explicit = true
# Note: instead of using a build system, we use tools/fast_editable.py
# to install sentry editably as well as console scripts.
# Otherwise, you need to define a build system (uv_build)
# and console scripts here, and call uv sync then uv pip install -e .

# These dependencies don't yet have wheels in upstream PyPI,
# so we get those wheels from internal PyPI.
[tool.uv.sources]
avalara = { index = "sentry-pypi" }
fido2 = { index = "sentry-pypi" }
google-crc32c = { index = "sentry-pypi" }
hiredis = { index = "sentry-pypi" }
maxminddb = { index = "sentry-pypi" }
mmh3 = { index = "sentry-pypi" }
petname = { index = "sentry-pypi" }
pycountry = { index = "sentry-pypi" }
python-rapidjson = { index = "sentry-pypi" }
python-u2flib-server = { index = "sentry-pypi" }
simplejson = { index = "sentry-pypi" }
zstandard = { index = "sentry-pypi" }


[dependency-groups]
dev = [
"codeowners-coverage>=0.3.0",
Expand Down
138 changes: 107 additions & 31 deletions tests/tools/test_lint_requirements.py
Original file line number Diff line number Diff line change
@@ -1,57 +1,139 @@
import json
from unittest.mock import patch

import pytest

from tools import lint_requirements


def test_ok(tmp_path) -> None:
def test_ok_pypi_org(tmp_path) -> None:
f = tmp_path.joinpath("uv.lock")
f.write_text(
"""
[[package]]
name = "amqp"
version = "5.3.1"
source = { registry = "https://pypi.devinfra.sentry.io/simple" }
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "vine", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" },
]
wheels = [
{ url = "https://pypi.devinfra.sentry.io/wheels/amqp-5.3.1-py3-none-any.whl", hash = "sha256:43b3319e1b4e7d1251833a93d672b4af1e40f3d632d479b98661a95f117880a2" },
{ url = "https://files.pythonhosted.org/packages/26/99/amqp-5.3.1-py3-none-any.whl", hash = "sha256:43b3319e1b4e7d1251833a93d672b4af1e40f3d632d479b98661a95f117880a2" },
]
"""
)
assert lint_requirements.main((str(f),)) == 0


def test_not_ok_git_url(tmp_path) -> None:
# note: uv adding
# git+https://github.com/asottile/astpretty@3.0.0#egg=astpretty
# vs astpretty @ git+https://github.com/asottile/astpretty@v3.0.0
# ...results in the same source = { git = ...
def test_ok_internal_pypi_no_upstream_wheels(tmp_path, monkeypatch) -> None:
monkeypatch.setattr(lint_requirements, "_has_upstream_cp313_wheels", lambda name, ver: False)
f = tmp_path.joinpath("uv.lock")
f.write_text(
"""
[[package]]
name = "astpretty"
version = "3.0.0"
source = { git = "https://github.com/asottile/astpretty?rev=v3.0.0#472be812174a2c883ee8e1cab5935e8c32c3a44f" }
name = "hiredis"
version = "2.3.2"
source = { registry = "https://pypi.devinfra.sentry.io/simple" }
wheels = [
{ url = "https://pypi.devinfra.sentry.io/wheels/hiredis-2.3.2-cp313-cp313-manylinux2014_x86_64.whl", hash = "sha256:abcd" },
]
"""
)
assert lint_requirements.main((str(f),)) == 0


def test_ok_internal_pypi_network_unavailable(tmp_path, monkeypatch) -> None:
monkeypatch.setattr(lint_requirements, "_has_upstream_cp313_wheels", lambda name, ver: None)
f = tmp_path.joinpath("uv.lock")
f.write_text(
"""
[[package]]
name = "hiredis"
version = "2.3.2"
source = { registry = "https://pypi.devinfra.sentry.io/simple" }
wheels = [
{ url = "https://pypi.devinfra.sentry.io/wheels/hiredis-2.3.2-cp313-cp313-manylinux2014_x86_64.whl", hash = "sha256:abcd" },
]
"""
)
assert lint_requirements.main((str(f),)) == 0


def test_not_ok_internal_pypi_has_upstream_wheels(tmp_path, monkeypatch) -> None:
monkeypatch.setattr(lint_requirements, "_has_upstream_cp313_wheels", lambda name, ver: True)
f = tmp_path.joinpath("uv.lock")
f.write_text(
"""
[[package]]
name = "xmlsec"
version = "1.3.17"
source = { registry = "https://pypi.devinfra.sentry.io/simple" }
wheels = [
{ url = "https://pypi.devinfra.sentry.io/wheels/xmlsec-1.3.17-cp313-cp313-manylinux2014_x86_64.whl", hash = "sha256:abcd" },
]
"""
)
with pytest.raises(SystemExit) as excinfo:
lint_requirements.main((str(f),))

expected = f"""
The specifier for package astpretty in {f} isn't allowed:

You cannot use dependencies that are not on internal pypi.

You also cannot use non-specifier requirements.
See PEP440: https://www.python.org/dev/peps/pep-0440/#direct-references"""
Package xmlsec==1.3.17 in {f} is sourced from internal
PyPI but upstream PyPI already has cp313 wheels for macOS arm64 and/or Linux
x86_64. Remove it from [tool.uv.sources] and no-build-package in pyproject.toml
so it is fetched from pypi.org directly."""
(msg,) = excinfo.value.args
assert msg == expected.rstrip()


def test_not_ok_public_pypi(tmp_path) -> None:
def _has_wheels(filenames: list[str]) -> bool | None:
import io

payload = json.dumps(
{"urls": [{"packagetype": "bdist_wheel", "filename": fn} for fn in filenames]}
).encode()

class FakeResp(io.BytesIO):
def __enter__(self):
return self

def __exit__(self, *a):
pass

with patch("urllib.request.urlopen", return_value=FakeResp(payload)):
return lint_requirements._has_upstream_cp313_wheels("pkg", "1.0")


def test_has_upstream_wheels_both_platforms() -> None:
assert (
_has_wheels(
[
"pkg-1.0-cp313-cp313-macosx_11_0_arm64.whl",
"pkg-1.0-cp313-cp313-manylinux2014_x86_64.whl",
]
)
is True
)


def test_has_upstream_wheels_pure_python() -> None:
assert _has_wheels(["pkg-1.0-py3-none-any.whl"]) is True


def test_has_upstream_wheels_mac_only() -> None:
# only mac — linux installs would still need internal PyPI
assert _has_wheels(["pkg-1.0-cp313-cp313-macosx_11_0_arm64.whl"]) is False


def test_has_upstream_wheels_linux_only() -> None:
# only linux — mac installs would still need internal PyPI
assert _has_wheels(["pkg-1.0-cp313-cp313-manylinux2014_x86_64.whl"]) is False


def test_has_upstream_wheels_none() -> None:
assert _has_wheels([]) is False


def test_not_ok_git_url(tmp_path) -> None:
# note: uv adding
# git+https://github.com/asottile/astpretty@3.0.0#egg=astpretty
# vs astpretty @ git+https://github.com/asottile/astpretty@v3.0.0
Expand All @@ -60,27 +142,21 @@ def test_not_ok_public_pypi(tmp_path) -> None:
f.write_text(
"""
[[package]]
name = "amqp"
version = "5.3.1"
source = { registry = "https://pypi.org/simple" }
dependencies = [
{ name = "vine", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" },
]
wheels = [
{ url = "https://files.pythonhosted.org/packages/26/99/fc813cd978842c26c82534010ea849eee9ab3a13ea2b74e95cb9c99e747b/amqp-5.3.1-py3-none-any.whl", hash = "sha256:43b3319e1b4e7d1251833a93d672b4af1e40f3d632d479b98661a95f117880a2", size = 50944, upload-time = "2024-11-12T19:55:41.782Z" },
]
name = "astpretty"
version = "3.0.0"
source = { git = "https://github.com/asottile/astpretty?rev=v3.0.0#472be812174a2c883ee8e1cab5935e8c32c3a44f" }
"""
)

with pytest.raises(SystemExit) as excinfo:
lint_requirements.main((str(f),))

expected = f"""
The specifier for package amqp in {f} isn't allowed:

You cannot use dependencies that are not on internal pypi.
The specifier for package astpretty in {f} isn't allowed:

You also cannot use non-specifier requirements.
Packages must come from pypi.org or the internal Sentry PyPI
(https://pypi.devinfra.sentry.io/simple) for packages that lack
suitable upstream wheels. URL/VCS/local dependencies are not allowed.
See PEP440: https://www.python.org/dev/peps/pep-0440/#direct-references"""
(msg,) = excinfo.value.args
assert msg == expected.rstrip()
60 changes: 54 additions & 6 deletions tools/lint_requirements.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,43 @@
from __future__ import annotations

import argparse
import json
import tomllib
import urllib.request
from collections.abc import Sequence

_INTERNAL_PYPI = "https://pypi.devinfra.sentry.io/simple"


def _has_upstream_cp313_wheels(name: str, version: str) -> bool | None:
"""
Returns True if upstream PyPI has satisfactory cp313 wheels for macOS arm64
or Linux x86_64 (or a pure-Python wheel that covers all platforms).
Returns None if the upstream check could not be performed (e.g. no network).
"""
url = f"https://pypi.org/pypi/{name}/{version}/json"
try:
with urllib.request.urlopen(url, timeout=5) as resp:
data = json.load(resp)
except Exception:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The PyPI API call in _has_upstream_cp313_wheels does not canonicalize package names, causing it to fail silently for packages with underscores.
Severity: MEDIUM

Suggested Fix

Before constructing the URL, normalize the package name according to PEP 503 rules. This can be done by replacing underscores, periods, and hyphens with a single hyphen and lowercasing the name. For example: canonical_name = re.sub(r"[-_.]+", "-", name).lower().

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: tools/lint_requirements.py#L22

Potential issue: The function `_has_upstream_cp313_wheels` constructs a PyPI API URL
using the package `name` directly without normalization. According to PEP 503, PyPI
package names in URLs must be canonicalized, with underscores converted to hyphens. If a
package with an underscore in its name is processed, the constructed URL will be
incorrect, resulting in a 404 error. The code catches the resulting exception and
silently treats it as if the check passed, which means the linter will fail to flag
packages that should be flagged, undermining the purpose of the check for any such
packages added in the future.

return None

has_mac = False
has_linux = False

for dist in data.get("urls", []):
if dist.get("packagetype") != "bdist_wheel":
continue
fn = dist["filename"]
if fn.endswith("-none-any.whl"):
return True
if "cp313" in fn and "macosx" in fn and "arm64" in fn:
has_mac = True
if "cp313" in fn and ("manylinux" in fn or "musllinux" in fn) and "x86_64" in fn:
has_linux = True

return has_mac and has_linux


def main(argv: Sequence[str] | None = None) -> int:
"""
Expand All @@ -23,20 +57,34 @@ def main(argv: Sequence[str] | None = None) -> int:
continue

# non-specifier requirements won't have registry as a source
if (
package["source"].get("registry", "")
!= "https://pypi.devinfra.sentry.io/simple"
package_registry = package["source"].get("registry", "")

if package_registry not in (
"https://pypi.org/simple",
_INTERNAL_PYPI,
Comment thread
joshuarli marked this conversation as resolved.
Comment thread
joshuarli marked this conversation as resolved.
):
raise SystemExit(
f"""
The specifier for package {package["name"]} in {filename} isn't allowed:

You cannot use dependencies that are not on internal pypi.

You also cannot use non-specifier requirements.
Packages must come from pypi.org or the internal Sentry PyPI
(https://pypi.devinfra.sentry.io/simple) for packages that lack
suitable upstream wheels. URL/VCS/local dependencies are not allowed.
See PEP440: https://www.python.org/dev/peps/pep-0440/#direct-references"""
)

if package_registry == _INTERNAL_PYPI:
version = package.get("version", "")
has_wheels = _has_upstream_cp313_wheels(package["name"], version)
if has_wheels:
raise SystemExit(
f"""
Package {package["name"]}=={version} in {filename} is sourced from internal
PyPI but upstream PyPI already has cp313 wheels for macOS arm64 and/or Linux
Comment on lines +82 to +83
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The error message uses "and/or" when describing platform requirements, but the code logic requires "and", potentially misleading developers.
Severity: LOW

Suggested Fix

Update the error message string at lines 82-84 to use "and" instead of "and/or". Similarly, correct the docstring for the _has_upstream_cp313_wheels function to accurately reflect that wheels for both macOS and Linux are required.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: tools/lint_requirements.py#L82-L83

Potential issue: The error message displayed by the linter at lines 82-84 is
inconsistent with the underlying code logic. The message states that a package can be
moved if it has wheels for "macOS arm64 and/or Linux x86_64", implying either platform
is sufficient. However, the function `_has_upstream_cp313_wheels` at line 39 requires
wheels for both platforms (`has_mac and has_linux`). This discrepancy can mislead
developers into believing a package is ready to be moved from the internal PyPI when it
does not yet meet the linter's actual requirements. The function's docstring also
contains this same inaccuracy.

Also affects:

  • tools/lint_requirements.py:39~39

x86_64. Remove it from [tool.uv.sources] and no-build-package in pyproject.toml
so it is fetched from pypi.org directly."""
)

return 0


Expand Down
Loading
Loading