Skip to content

fix: build with cython 3.3.0 - #1820

Merged
bdraco merged 2 commits into
masterfrom
fix-cython-330
Aug 28, 2026
Merged

fix: build with cython 3.3.0#1820
bdraco merged 2 commits into
masterfrom
fix-cython-330

Conversation

@bdraco

@bdraco bdraco commented Aug 26, 2026

Copy link
Copy Markdown
Member

Summary

Cython 3.3.0 now resolves subscripted annotations like list[str] or set[DNSPointer] when it knows the inner type, then rejects them as incompatible with the plain list/set/dict declarations in the .pxd; seven modules fail to build. It also treats an annotated local that is listed in a .pxd @cython.locals as a redeclaration.

The fix routes each failing annotation through an alias Cython cannot resolve, the same _str = str idiom the codebase already uses, so the annotation degrades to the plain container and the .pxd stays the single source of truth; mypy still sees the full types through the aliases. The three conflicting locals (query_buckets, next_scheduled, known_answers_set) keep their Python annotations and drop their @cython.locals entries, which declared the same types.

Details

  • On Cython 3.2.9 (the locked version) the generated C for every function body is identical to master after normalizing line numbers; the only additions are the module level alias bindings executed once at import.
  • Argument checks are unchanged, for example unique_types still compiles to __Pyx_ArgTypeTest(..., 0, "unique_types", 2), and the Py_None guard counts per module are identical, so no new None checks landed in the hot paths.
  • The annotated HTML from cython -a shows every pre existing source line keeps its exact score on both 3.2.9 and 3.3.0; only the new alias lines add score, at import time.

Test plan

  • all 18 TO_CYTHONIZE modules compile with Cython 3.3.0 and 3.2.9
  • full pytest suite passes with extensions built by 3.2.9 and by 3.3.0
  • pre-commit (ruff, mypy, flake8, cython-lint) passes

Fixes #1818

@bdraco

bdraco commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

perf and html look the same so should be good to go if ci passes

@bdraco
bdraco marked this pull request as ready for review August 26, 2026 15:13
@bdraco bdraco mentioned this pull request Aug 26, 2026
Closed
@bluetoothbot

bluetoothbot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

@bdraco: perf and html look the same so should be good to go if ci passes

Confirmed on my read — no runtime behavior change, only annotations and .pxd declarations.

Two return annotations look unnecessary. browser.py:268 and registry.py:83. Siblings browser.py:227, _cache.py:185, registry.py:79 keep plain list[DNSOutgoing] / list[DNSRecord] / list[ServiceInfo] against the same list pxd declarations, with inner types cimported, and compile. DNSOutgoing_ serves only line 268.

One gap on "if CI passes": CI won't build with 3.3.0. poetry.lock:291 pins 3.2.9, and the use_cython matrix installs from the lock. Only build_wheels resolves Cython>=3.0.8 unpinned, and it runs after release against master. Run poetry update cython here so the matrix compiles what this PR fixes.

@bluetoothbot

bluetoothbot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

PR Review — fix: build with cython 3.3.0

Solid, well-scoped compatibility fix. Merge-ready; all findings are non-blocking.

The diff is disciplined in the ways that matter for a Cython change. Every altered parameter annotation maps to a real .pxd declaration, and the ones that would have been redundant were correctly left alone — _history.py:45 keeps known_answers: set[DNSRecord] because _history.pxd never cimports DNSRecord, and _updates.py:54 keeps list[RecordUpdate] for the same reason. Each module reuses its own local alias convention (_str/_int in _cache/query_handler/registry, trailing-underscore float_/RecordUpdate_ in info/browser) rather than imposing one. record_manager.py:195 aliases only questions, leaving listener: RecordUpdateListener matching the pxd's RecordUpdateListener listener. The seven touched modules and three dropped @cython.locals entries match the description exactly, and the generated-C / cython -a comparison is the right evidence to bring for a change like this.

No correctness or security concerns: the diff is annotations and .pxd declarations only, with zero runtime behavior change. All new aliases (_ServiceInfo, _DNSIncoming, _RecordUpdate, _DNSQuestion, RecordUpdate_, DNSPointer_) bind names that are imported at runtime, not under TYPE_CHECKING, so no import-time NameError.

  • Two return annotations look over-aliased: browser.py:268 and registry.py:83. Siblings with identical pxd shapes and cimported inner types — browser.py:227, _cache.py:185, registry.py:75/79 — keep plain list[DNSOutgoing] / list[DNSRecord] / list[ServiceInfo] and compile. DNSOutgoing_ exists solely for line 268.
  • CI never builds with 3.3.0: poetry.lock pins 3.2.9 and the use_cython matrix installs from the lock; the only isolated PEP 517 build (build_wheels) runs after release against master. poetry update cython here would make the pipeline prove the fix.
  • The three dropped @cython.locals entries hand hot-path local typing to X | None annotation resolution, where a downgrade to object would be silent (browser.py:531, query_handler.py:358).
  • Minor pre-existing cleanup available in the same block being edited: msgs=list at query_handler.pxd:94 duplicates the cython.list msgs parameter.

🟢 Suggestions

1. Nothing in CI actually builds with Cython 3.3.0
pyproject.toml:90

The PR fixes the 3.3.0 build, but no CI job on this PR will exercise it, so the fix ships unverified by the pipeline and can silently regress.

What I found:

  • poetry.lock:291 pins cython 3.2.9. The use_cython test matrix installs via poetry install --only=main,dev (ci.yml:106-110), so it compiles with 3.2.9.
  • The only PEP 517 isolated build that would resolve build-system.requires = ['Cython>=3.0.8'] to 3.3.0 is the build_wheels cibuildwheel job (ci.yml:355), and that job checks out master / needs.release.outputs.newest_release_tag (ci.yml:318, ci.yml:350) and runs after release.

The practical consequence: because build-system.requires has no upper bound, a 3.3.0 incompatibility breaks the wheel build after the tag is cut, not on the PR that introduced it. That is exactly the failure mode of #1818, and the next one (someone adding a list[DNSRecord] parameter annotation to a pxd-declared function) would land the same way.

Suggested fix: run poetry update cython in this PR so the lock moves to 3.3.0 and the use_cython matrix compiles the fix it is meant to validate. Dependabot will get there eventually, but bumping here is what turns your manual verification into a standing CI guarantee.

cython = "^3.2.9"

Checklist

  • No runtime behavior change (annotations and .pxd declarations only)
  • All new module-level aliases bind runtime (non-TYPE_CHECKING) imports
  • Fix is complete across cythonized modules (unresolvable-inner-type cases correctly skipped)
  • CI exercises the Cython version the PR targets — suggestion ＃1
  • Diff matches PR description scope (7 modules, 3 locals)
  • No security surface touched

Automated review by Kōan (Claude) HEAD=5183d54 7 min 53s

bluetoothbot
bluetoothbot approved these changes Aug 26, 2026
View reviewed changes

@bluetoothbot bluetoothbot left a comment

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.

Tip

No blocking issues found — ready to merge.

@bdraco bdraco mentioned this pull request Aug 28, 2026
Merged
@bdraco
bdraco merged commit df947b2 into master Aug 28, 2026
6 of 35 checks passed
@bdraco
bdraco deleted the fix-cython-330 branch August 28, 2026 14:39
This was referenced Aug 28, 2026
Merged
Open
@bdraco bdraco mentioned this pull request Aug 28, 2026
Merged
1 task
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 28, 2026
@0-wiz-0
py-zeroconf: update to 0.150.2.
9de92ef 
## v0.150.2 (2026-08-28)

### Bug Fixes

- Keep `key=` distinct from a valueless `key` in TXT records
  ([#1813](python-zeroconf/python-zeroconf#1813),
  [`899eaaa`](python-zeroconf/python-zeroconf@899eaaa))


## v0.150.1 (2026-08-28)

### Bug Fixes

- Build with cython 3.3.0 ([#1820](python-zeroconf/python-zeroconf#1820),
  [`df947b2`](python-zeroconf/python-zeroconf@df947b2))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@bluetoothbot bluetoothbot bluetoothbot approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

0.150.0: cython 3.3.0 breaks build

2 participants

@bdraco @bluetoothbot