engines: add the pcap_ct engine, and make unsupported_reason a real preflight - #405
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Several updated docs/comments overstate CI “verification” of Python 3.15 (it is configured as experimental/allowed-failure), and one user-facing PyShark.unsupported_reason() message references the wrong asyncio API.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds a new pcap_ct extraction engine (backed by the pcap-ct distribution) and introduces shared backend-detection/preflight logic so engines can reliably report “unsupported” reasons (wrong pcap distribution, missing tshark, missing system libpcap, etc.) before import-time failures occur.
Changes:
- Add
pcap_ctengine implementation plus a shared_pcap_backendprobe to distinguishpypcapvspcap-ctwhen both provideimport pcap. - Extend
unsupported_reason()behavior (notably forPyShark,PyPCAP, andPCAP_CT) and update engine registry/type aliases accordingly. - Add extensive unit/runtime tests and update docs/README/examples to document engine constraints and selection.
File summaries
| File | Description |
|---|---|
| tests/toolkit/test_pcap_ct_unit.py | Adds unit tests for the pcapkit.toolkit.pcap_ct adapter surface and its “unsupported” adapters. |
| tests/foundation/engines/test_pyshark_engine.py | Adds unit tests for PyShark.unsupported_reason() covering version ceiling and tshark discovery behavior. |
| tests/foundation/engines/test_pypcap_engine.py | Extends PyPCAP engine tests to cover backend-collision detection and wrong-backend refusal. |
| tests/foundation/engines/test_pcap_ct_engine.py | Adds comprehensive PCAP_CT engine tests (preflight, run/read/close behavior, and real-backend smoke). |
| tests/foundation/engines/test_pcap_backend.py | Adds unit tests for _pcap_backend probing/identification/collision reporting. |
| tests/foundation/engines/test_new_engine_parity_runtime.py | Moves/updates parity test gating so fixture-dependent captures stay out of unit tier and pypcap detection is correct. |
| README.rst | Documents new engine, prerequisites, and adds a Python-version support matrix and updated install guidance. |
| pyproject.toml | Adds PCAP_CT extra (with prerelease pins/markers) and expands rationale/comments around Python support and prerequisites. |
| Pipfile | Adds pcap-ct/libpcap with prerelease-aware rationale for development environments using pipenv. |
| pcapkit/toolkit/pypcap.py | Cross-references the new pcap_ct toolkit adapter in docs. |
| pcapkit/toolkit/pcap_ct.py | Introduces toolkit adapter functions for pcap-ct (packet mapping + explicit UnsupportedCall for reassembly/traceflow). |
| pcapkit/toolkit/init.py | Adds commented import stubs for pcap_ct toolkit functions (paralleling other toolkits). |
| pcapkit/interface/misc.py | Updates Engines type alias and follow_tcp_stream engine fallback list to include pcap_ct. |
| pcapkit/foundation/extraction.py | Adds pcap_ct engine registration and updates engine literals/packet union commentary. |
| pcapkit/foundation/engines/pyshark.py | Implements PyShark.unsupported_reason() with Python ceiling and tshark checks. |
| pcapkit/foundation/engines/pypcap.py | Adds backend probing/collision warning and wrong-backend refusal to avoid mislabeling pcap-ct as pypcap. |
| pcapkit/foundation/engines/pcap_ct.py | Adds the new PCAP_CT engine, including wrong-backend detection and missing-system-libpcap handling. |
| pcapkit/foundation/engines/_pcap_backend.py | New shared module to probe/identify which distribution owns pcap and to generate actionable reasons/messages. |
| pcapkit/foundation/engines/init.py | Exposes PCAP_CT in the engines package. |
| examples/legacy_smoke/_engine_support.py | Updates example harness to include new engines and consult unsupported_reason() for accurate skip messaging. |
| docs/source/pcapkit/toolkit/3rdparty.rst | Documents pcap_ct toolkit functions and their intentional UnsupportedCall behavior. |
| docs/source/pcapkit/foundation/engines/index.rst | Updates engine support overview/table and adds guidance on choosing between PyPCAP and PCAP_CT. |
| docs/source/pcapkit/foundation/engines/3rdparty.rst | Adds detailed docs for PCAP_CT engine and expands PyShark/PyPCAP prereq explanations. |
| docs/source/index.rst | Updates top-level docs to include the new engine and adds engine-by-Python-version matrix + prerequisites narrative. |
| docs/source/ext.rst | Updates extension docs to include PCAP_CT in engine capability tables and notes. |
Review details
Suppressed comments (2)
README.rst:193
- This claims CI “verifies 3.10 through 3.15”, but the workflow runs 3.15 as an experimental/allowed-failure job (continue-on-error + allow-prereleases). That makes the statement stronger than what CI actually enforces.
**Note** -- ``pcapkit`` declares support for **Python 3.6 and later**, and CI
verifies **3.10 through 3.15**.
docs/source/index.rst:227
- This note says CI “verifies 3.10 through 3.15”, but the workflow treats 3.15 as experimental/allowed-failure (continue-on-error + allow-prereleases). Consider wording that distinguishes enforced versions from experimental coverage.
:mod:`pcapkit` declares support for **Python 3.6 and later**, and CI verifies
**3.10 through 3.15**.
- Files reviewed: 25/25 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…reflight Upstream `pypcap` is unusable on a current Python: 1.3.0 compiles a `pcap.c` pre-generated by Cython 0.29.32, which does not build against the 3.12+ C API, and it publishes no wheels. Measured with libpcap present and found: builds on 3.10 and 3.11, fails on 3.12 (`ob_digit`, `curexc_traceback`) and 3.14 (those plus `ma_version_tag` and `_PyLong_AsByteArray` arity). `pcap-ct` is a ctypes reimplementation that provides the same top-level `pcap` module and works across the whole supported range. It is added as its own engine rather than a second backend behind `pypcap`, because the two are **mutually exclusive** -- both distributions own the import name `pcap`, and with both installed pcap-ct wins while upstream's extension module is shadowed and unreachable. One engine silently meaning two different implementations is exactly the ambiguity worth avoiding. New shared, stdlib-only `_pcap_backend.py` is the single place that answers "which backend did I get": `hasattr(pcap, '_pcap')` distinguishes them (pcap-ct is a package, upstream a lone extension module), and `importlib.metadata` reveals the distribution the import did *not* resolve to. Shared deliberately -- two copies would be two chances for the engines to disagree. Each engine exposes `.backend` and warns when both are installed. `unsupported_reason()` now works as a preflight across every engine, which is the case the import test cannot cover: a dependency that installs cleanly and only fails when used. * `pcap`, `pcapng` -- no override; pcapkit's own parsers have no requirement. * `dpkt`, `scapy` -- no override, measured genuinely unconstrained on 3.14. * `pyshark` -- ceiling 3.14 plus a tshark check. It *imports* on 3.14 and fails at use: the call it makes, `asyncio.get_event_loop_policy().get_event_loop()`, returns a loop on 3.10/3.11, warns on 3.12 and raises on 3.14. 3.13 was not available and is inferred, flagged as such in the code. * `pypcap`, `pcap_ct` -- wrong-backend detection; `pcap_ct` also reports a missing system libpcap. * `pypcapfile` -- unchanged from #396. The tshark check delegates to pyshark's own `get_process_path()` rather than `shutil.which`, which is not equivalent: pyshark consults `tshark_path` in its config first, then PATH on POSIX, both Program Files directories on Windows, and `/Applications/Wireshark.app` on macOS, so `which` would refuse working setups. Not cached -- the failing path measures ~287 us against 39 PATH entries and runs once per Extractor, and caching would freeze an answer about the *environment*, so installing Wireshark would not take effect until restart. Three of my own earlier claims that measurement disproved, corrected in the docs rather than left standing: the `libpcap` wheel does **not** remove the need for a system libpcap (its published config sets `LIBPCAP = None`, sending the loader to `find_library("pcap")`); with no libpcap, `import pcap` raises `OSError`, not `ImportError`, so it escapes `import_test` and aborts the extraction rather than degrading; and the PCAP-NG verdict is host-dependent -- libpcap 1.11 reads `dhcp.pcapng` correctly while 1.5.3 silently returns nonsense timestamps, so both engines keep rejecting PCAP-NG, now with the real reason. Fixed while probing: a second probe after a failure reported `NameError: '__about__' is not defined` instead of the real cause, because both `pcap-ct`'s and `libpcap`'s `__init__` open with a non-rerunnable `from .__about__ import * ; del __about__`. `probe()` now purges both module trees on failure. Also moved `test_new_engine_parity.py` to `*_runtime.py`. It read four generated captures from the unit tier -- a tier violation that #393's guard could not catch, because it calls `sample_path(capture)` with a *variable*, which only the runtime layer sees, and that layer is never reached on a host where the engine packages are absent. It was the sole cause of the 5 failures in the pristine baseline below. `examples/legacy_smoke/_engine_support.py` listed only four engines, so the timing harness never exercised the new ones; its skip messages now consult `unsupported_reason()` and name the real cause. No benchmark numbers are published. They were measured, but this host is a shared cloud desktop under load whose figures disagree with the README's existing row in *both* directions, so mixing the two tables row-for-row would be misleading. An engine x Python support matrix is documented instead, which is host-independent: 3.11 is the last version where every engine runs, and even there pypcap and pcap_ct are mutually exclusive, so no single environment has all seven. Verified after rebasing onto 89201df: engine identity asserted rather than inferred (`__engine_name__ == 'PCAP_CT'`, 6 frames, no EngineWarning); `unsupported_reason()` exercised for all eight engines; `files=True` naming checked against #403's bare-`_fext` change, which this engine's per-frame naming depends on. Unit tier 614 passed / 28 skipped / 0 failed, against a pristine-main baseline of 5 failed / 546 passed / 50 skipped.
… clean The benchmark table's footnotes were hand-numbered `[1]_`..`[4]_`, and referenced out of order -- the table cited 3, 1, 4, 2 -- so any insertion meant renumbering every marker and definition by hand. They are now RST auto-numbered footnotes with labels (`[#pyshark-historical]_` and friends), which is the form already used across `docs/source/pcapkit/const/*.rst` as auto-symbol `[*]_`. Labelled rather than bare `[#]_` on purpose: bare auto-numbers are assigned in reference order, so the definitions would have to stay in the table's order to read correctly, and a reordered row would silently repoint a marker at the wrong note. Labels make the pairing explicit and order-independent. Labelled auto-footnotes do take their *numbers* from definition order, though, so the definitions are also reordered to match the table -- otherwise the markers rendered 3, 1, 4, 2, exactly the wart being removed. Verified: markers now render 1, 2, 3, 4 in both the table and the definition list, in `README.rst` and `docs/source/index.rst`. Also removed three Sphinx-only roles this branch had introduced into `README.rst` -- one `:manpage:` and two `:func:` -- which `main` did not have. GitHub renders the README with plain docutils, which knows none of them and emits a visible `Unknown interpreted text role` error for each, the same defect #400 fixed for the admonitions. They are now ``literals``. README again renders with zero diagnostics under plain docutils.
8a32fe4 to
69503cd
Compare
…the README The engine-support matrix hand-wrote its `*` and `†` markers in the cells with loose prose beneath, so nothing linked and the symbols had to be maintained by hand. In `docs/source/index.rst` those are now RST auto-symbol footnotes (`[*]_`), which index themselves. `README.rst` deliberately keeps the literal `*`/`†` characters. GitHub renders it with plain docutils, which does not present indexed footnotes usefully, so the markers there stay plain text -- the same reasoning that keeps admonitions out of that file. One constraint worth recording, because it shaped the layout: **auto-symbol footnotes are strictly one reference per definition.** Two references against one definition is an error (`Too many symbol footnote references: only 1 corresponding footnote available`), and three references mint three *separate* notes rendering `*`, `†`, `‡`. Since `*` appeared in eight cells and `†` in four, a direct conversion would have needed twelve definitions. The markers therefore moved off the individual cells and onto the things they actually qualify -- the `3.13` and `3.15` column headers, and the `pyshark` row label -- which is three references, three definitions, and no loss of precision about which verdicts are inferred. The benchmark table's `[1]_`..`[4]_` footnotes are left numbered as they were. Verified: README renders with zero diagnostics under plain docutils; the Sphinx copy's three auto-symbol footnotes resolve without warnings.
`test_pyshark_engine_currently_breaks_on_modern_asyncio` waited for an `AttributeError` out of pyshark on Python 3.14. `PyShark.unsupported_reason()` (#405) now answers before anything is imported, so the engine is declined with a warning and the extraction falls back to pcapkit's own parser -- the exception cannot happen and the test failed on a clean tree. Renamed and rewritten to assert the current behaviour: an `EngineWarning` naming both the engine and the interpreter, and a fallback that still extracts all six frames. The pre-3.14 branch is untouched and still runs pyshark for real. The test is in `tests/integration/`, which CI excludes for want of untracked fixtures, which is why nothing caught it. Closes #414
Two related things: a third PCAP engine that actually works on a current Python, and
unsupported_reason()extended across every engine as the preflight it was introduced to be.Why a new engine rather than a second backend
Upstream
pypcapis unusable on a modern interpreter. 1.3.0 compiles apcap.cpre-generated by Cython 0.29.32 and never runs Cython at build time, and publishes no wheels. Measured with libpcap present and found, so these are compile failures rather than missing-library failures:pip install pypcapob_digit,curexc_tracebackma_version_tag,_PyLong_AsByteArrayaritypcap-ctis a ctypes reimplementation exposing the same top-levelpcapmodule, and it works across the whole supported range.It is a separate engine because the two are mutually exclusive: both distributions own the import name
pcap. Measured with both installed — pcap-ct wins the import and upstream's extension module is shadowed and unreachable. Oneengine=string silently meaning two different implementations is the ambiguity worth avoiding.New stdlib-only
_pcap_backend.pyanswers "which backend did I get":hasattr(pcap, '_pcap')distinguishes them (pcap-ct is a package, upstream a lone extension module), plusimportlib.metadatato reveal the distribution the import did not resolve to. Shared between both engines deliberately — two copies would be two chances to disagree. Each engine exposes.backend, and warns when both distributions are installed.Note
pcap.ex_nameexists on both, so it looks like a discriminator and is not.unsupported_reason()across every engineThis is the case the import test structurally cannot cover: a dependency that imports fine and fails at use.
pcap,pcapngdpkt,scapypysharkpypcappcap_ctpypcapfilepyshark's ceiling is measured, not guessed. It imports on 3.14 and fails at use. Isolating the exact call it makes —
asyncio.get_event_loop_policy().get_event_loop()— gives: 3.10/3.11 return a loop silently, 3.12 returns one with a DeprecationWarning, 3.14 raisesRuntimeError. So the ceiling is 3.14. 3.13 was unavailable and is inferred, flagged as such in the code rather than presented as measured.The tshark check delegates to pyshark's own
get_process_path(), notshutil.which— they are not equivalent. pyshark consultstshark_pathin its config first, then PATH on POSIX, both Program Files directories on Windows, and/Applications/Wireshark.appon macOS, sowhichwould refuse working installations.Not cached, deliberately. The failing path — exhausting every candidate — measures ~287 µs against 39 PATH entries, and runs once per
Extractor, not per frame. Caching would freeze an answer about the environment, so installing Wireshark wouldn't take effect until restart.End to end on 3.14:
engine='pyshark'now gives one warning naming the asyncio cause and falls back to PCAP with 6 frames, instead of dying.Three of my own earlier claims that measurement disproved
Corrected in the docs rather than left standing:
libpcapwheel ships a vendored.so, but its publishedlibpcap.cfgsetsLIBPCAP = None, which sends the loader tofind_library("pcap"). The same wheel loaded the host's 1.5.3 under one interpreter and linuxbrew's 1.11.0 under another. No compiler is needed; a runtimelibpcap.so.1is.import pcapraisesOSError, notImportError— so it escapesimport_testentirely and aborts the extraction rather than degrading.PCAP_CT.unsupported_reason()now catches it.dhcp.pcapngcorrectly; 1.5.3 silently returns nonsense timestamps (1102274184.0000002). Since the version is a property of the host, both engines keep rejecting PCAP-NG — now with the real reason documented.Fixed while probing
A second probe after a failure reported
NameError: '__about__' is not definedinstead of the real cause, because bothpcap-ct's andlibpcap's__init__open with a non-rerunnablefrom .__about__ import * ; del __about__.probe()now purges both module trees on failure.A tier violation removed
test_new_engine_parity.pyread four generated captures from the unit tier. #393's guard could not catch it: the call issample_path(capture)with a variable, which only the runtime layer sees, and that layer is never reached on a host where the engine packages are absent. It was the sole cause of the 5 failures in the pristine baseline below. Moved totest_new_engine_parity_runtime.pyviagit mv, so the rename is preserved.Relatedly,
examples/legacy_smoke/_engine_support.pylisted only four engines, so the timing harness never exercised the new ones. Its skip messages now consultunsupported_reason()and name the real cause instead of guessing "package is not installed".No benchmark numbers, deliberately
They were measured across four interpreters, and are not published. This host is a shared cloud desktop under load whose figures disagree with the README's existing row in both directions — dpkt and pcapkit ~2× slower, scapy ~1.5× faster — so mixing the two tables row-for-row would mislead. Repeatability was 1.4–3.3%, so the numbers are real; they are simply not comparable with what is already there.
An engine × Python support matrix is documented instead, which is host-independent and more durable. The useful conclusion from it: 3.11 is the last version where every engine runs, and even there
pypcapandpcap_ctare mutually exclusive, so no single environment ever has all seven.Verification, after rebasing onto
89201dff6Engine identity asserted rather than inferred, since a missing engine only warns and falls back:
unsupported_reason()exercised for all eight engines;files=Truenaming checked against #403's bare-_fextchange, which this engine's per-frame naming depends on (it would have producedFrame 1..jsonbefore that PR).origin/main@89201dff6Notes for review
README.rstanddocs/source/index.rst, which overlap with in-flight documentation work; that will be rebased onto this.