diff --git a/docs/source/conf.py b/docs/source/conf.py index f637e454c..468bcb10a 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -93,10 +93,10 @@ # # These are deliberately opt-in per site rather than an automatic ``#NNN`` rule, # because no digit-keyed pattern can tell a citation from a packet-diagram label: -# ``#\d{3}`` misses every two- and four-digit number, while ``#\d+`` catches the 45 -# one-digit RFC diagram labels across ``pcapkit/protocols/internet/hip.py`` (36), -# ``pcapkit/protocols/transport/sctp.py`` (7) and -# ``pcapkit/protocols/schema/internet/hip.py`` (2) -- ``DH GROUP ID #1``, +# ``#\d{3}`` misses every two- and four-digit number, while ``#\d+`` catches the +# one-digit RFC diagram labels in ``pcapkit/protocols/internet/hip.py``, +# ``pcapkit/protocols/transport/sctp.py`` and +# ``pcapkit/protocols/schema/internet/hip.py`` -- ``DH GROUP ID #1``, # ``Gap Ack Block #1`` and the like -- which are not references to anything. A role # nobody writes cannot corrupt them. # @@ -109,7 +109,7 @@ # in the source even though GitHub redirects between ``/issues/NNN`` and ``/pull/NNN`` # either way. # -# ``:discussion:`` is needed because a handful of cited numbers are GitHub +# ``:discussion:`` is needed because some cited numbers are GitHub # Discussions rather than issues -- the issues API returns 404 for them, so # ``:issue:`` would link to a page that does not exist. # @@ -344,8 +344,8 @@ def bind_type_checking_names(app: 'Sphinx') -> None: :exc:`NameError`, which :func:`sphinx.util.typing.get_type_hints` catches. Once it resolves, Python 3.14's :mod:`annotationlib` rejects the leading space with a :exc:`SyntaxError` that :func:`~sphinx.util.typing.get_type_hints` does - *not* catch, so it aborts the whole build. Two such annotations existed and both are - fixed at source; a third would stop the build rather than be worked around + *not* catch, so it aborts the whole build. Such annotations existed and are + fixed at source; another would stop the build rather than be worked around here, which is the right way round for a typo. Args: @@ -530,17 +530,17 @@ def restore_enum_member_docstring(app: 'Sphinx', what: str, name: str, # pylint lines[:] = original -# NB: four handlers used to sit here -- ``remove_module_docstring``, +# NB: handlers used to sit here -- ``remove_module_docstring``, # ``process_docstring``, ``process_fields`` and ``source_read`` -- each with its # ``app.connect`` call commented out in ``setup`` below, so none of them had run # for as long as they had been in the file. They are deleted rather than restored, -# because restoring them would either duplicate work now done properly or make the +# because restoring them would either duplicate work done properly elsewhere or make the # build unusable: # -# ``remove_module_docstring`` and ``process_docstring`` were two attempts at the +# ``remove_module_docstring`` and ``process_docstring`` were both attempts at the # same thing -- flipping :data:`typing.TYPE_CHECKING` and reloading each module # so autodoc could see the ``if TYPE_CHECKING:`` names. ``bind_type_checking_names`` -# above now solves that, per module and without mutating a global mid-build. +# above solves that, per module and without mutating a global mid-build. # # ``process_fields`` only ever printed; the ``lines.append`` that was its point # was itself commented out. diff --git a/docs/source/contributing/conventions/documentation.rst b/docs/source/contributing/conventions/documentation.rst index 34be9176a..a084a80d8 100644 --- a/docs/source/contributing/conventions/documentation.rst +++ b/docs/source/contributing/conventions/documentation.rst @@ -30,9 +30,7 @@ and ``to`` stay lowercase unless they lead, which was the reading put to the own **The casing is the easy half.** A heading can be in Title Case already and still break the rule by being a clause rather than a title: *What a Failed Lookup Raises* was -correctly cased and is now *Failed-Lookup Exceptions*. Of the 62 headings under -:file:`docs/source/contributing/` when this was ruled, 22 needed rewriting and 16 only -re-casing. +correctly cased and was renamed *Failed-Lookup Exceptions*. The line to draw: a **finite verb** makes a heading a sentence and earns a rewrite, while a gerund or infinitive phrase is a noun phrase and needs only re-casing -- so @@ -49,18 +47,17 @@ while a gerund or infinitive phrase is a noun phrase and needs only re-casing -- Renaming a Heading ~~~~~~~~~~~~~~~~~~ -Two mechanical traps, both of which have bitten rather than being hypothetical. +Mechanical traps, each of which has bitten rather than being hypothetical. **Re-measure the underline against the new text.** A case flip usually keeps the length; a rewrite almost never does, and docutils reports an underline shorter than -its heading in a build that does not fail on warnings. Re-check every heading; none -of the 62 was short. +its heading in a build that does not fail on warnings. Re-check every heading. **A heading is a link target, so a rename is a repository-wide sweep** rather than a :file:`docs/source/` one. In scope: ``.rst`` prose, ``:ref:`` and ``:doc:`` link text, toctree entries, implicit ``` `Text`_ ``` references, **and** :mod:`pcapkit` docstrings -and the files under :file:`tests/`. A heading rename once left **five** stale -references, spread across :file:`pcapkit/corekit/sentinels.py`, +and the files under :file:`tests/`. A heading rename once left stale +references spread across :file:`pcapkit/corekit/sentinels.py`, :file:`tests/corekit/test_sentinel_exports_unit.py` and :file:`tests/project/test_conventions_doc_claims.py`. The first of those is a shipped module docstring that renders into the API reference, so a reader following it searches @@ -76,7 +73,7 @@ the page for a string no longer on it. Nothing in CI catches a reference a rename left behind. :file:`docs/source/conf.py` sets no ``nitpicky`` and :file:`docs/Makefile` leaves ``SPHINXOPTS`` empty, so the build runs with neither ``-n`` nor ``-W``: a dead reference renders as the plain text -it used to be, and the build still succeeds. :issue:`934` found sixteen of them at once +it used to be, and the build still succeeds. :issue:`934` found a batch of them at once that way. One thing a rename breaks that no tool checks at all is the prose around it. Turning a @@ -89,7 +86,7 @@ Mermaid for Flows Where the subject is a flow, prefer a Mermaid graph to the paragraph or the ASCII diagram that would otherwise carry it -- a graph is read faster than its own description. The owner ruled this on :issue:`719`, asking for it where it is necessary and -helpful, which bounds it in three directions: +helpful, which bounds it: * **A short sequence does not earn a graph.** Two steps read perfectly well as a sentence, and a diagram of them costs a reader a context switch for nothing. @@ -101,8 +98,8 @@ helpful, which bounds it in three directions: already shown, and a second copy of a flow is exactly that. The style model is the set already in the tree, every one of which builds. The sweep -excludes this page, which writes the directive name three times in its own prose and -would otherwise inflate both counts: +excludes this page, which names the directive in its own prose and would otherwise +count itself: .. code-block:: shell @@ -155,10 +152,10 @@ every toctree.** Measured against a built tree rather than assumed: the caption renders nowhere. That is why most of the per-package toctrees are deliberately bare -- a caption on a -package index would be invisible on every page but one. The three in +package index would be invisible on every page but one. The root ones in :file:`docs/source/index.rst` carry captions, and ``Subpackages`` in -:file:`docs/source/pcapkit/index.rst` is the single nested one. A built tree says so -- -excluding this page, which names both captions in its own prose and would otherwise +:file:`docs/source/pcapkit/index.rst` is the only nested one. A built tree says so -- +excluding this page, which names these captions in its own prose and would otherwise count itself: .. code-block:: shell @@ -166,9 +163,9 @@ count itself: grep -rl 'API Reference' --include='*.html' --exclude='documentation.html' \ docs/build/html | wc -l # every other page grep -rl 'Subpackages' --include='*.html' --exclude='documentation.html' \ - docs/build/html | wc -l # exactly one + docs/build/html | wc -l # the pcapkit index page alone -Those root toctrees are ``:hidden:`` because, without it, each of the three captions +Those root toctrees are ``:hidden:`` because, without it, each of their captions rendered twice on the root page -- once inline in the body, once in the sidebar -- which is the duplication the owner ruled out on :issue:`719`. @@ -182,7 +179,7 @@ is the duplication the owner ruled out on :issue:`719`. One setting there was decided by measurement rather than by preference: ``toc_object_entries`` carries, in a comment beside it, the figures that settled it, so that nobody reopens the question blind. That is worth copying the next time a - setting is chosen that way, but it is a single precedent and not yet a rule the owner + setting is chosen that way, but it is a single precedent and not a rule the owner has ruled on. Paraphrasing a Ruling @@ -190,7 +187,7 @@ Paraphrasing a Ruling Write a ruling down in your own words. **Do not quote the owner verbatim** -- a standing instruction on :issue:`719`, and the one every page in this directory follows. -:issue:`949` went back over the five pages that then existed and replaced their quoted +:issue:`949` went back over the pages that then existed and replaced their quoted rulings with paraphrase. What a quotation costs is not style. A quoted sentence is pinned to the moment it was @@ -228,9 +225,15 @@ prose and code disagree the code wins and the prose is what gets fixed -- :issue own charter -- and a docstring outliving the thing it described is a demonstrated failure mode here rather than a hypothetical one. -**Re-derive a count; do not copy one.** Better still, write down the command that -produces it, as :ref:`process` does, so the figure can be rechecked rather than -trusted. A keyword search is not a sweep: the reliable method takes every cited +**State no count.** A count of things in the codebase or the documentation drifts as +soon as what it counts changes, and keeping it in step by hand does not scale, so +living prose words the claim without one -- *every registered option*, *the remaining +entries* -- and drops a count that was the only content. Protocol and specification +quantities, code constants, versions and issue numbers are not counts, and a +measurement that justifies a design choice stays, stated as a measurement. Ruled on +:issue:`1103`; the released changelogs are a frozen record and keep theirs. + +A keyword search is not a sweep: the reliable method takes every cited repository path in each of the forms it gets written in -- slash path, dotted module, bare filename, and the elided spellings -- intersects them with the diff since the merge base, and re-measures the intersection. A tense-keyword grep misses a claim @@ -244,22 +247,21 @@ shape: One did not: the RARP page declared ``pcapkit.protocols.data.link.rarp``, which has never existed, because RARP and DRARP reuse ARP's data class. Fixed in ``68fbccd90``. * :issue:`911`'s ruling -- export the sentinel objects and leave their types out -- was - read as describing all three modules that then held a sentinel. One ran the other + read as describing every module that then held a sentinel. One ran the other way: :mod:`pcapkit.corekit.fields.field` exported neither, so applying the rule there meant *adding* a name rather than removing one. -**Some numbers on these pages are pinned, and knowing which is part of writing one.** -:file:`tests/project/test_conventions_doc_claims.py` re-derives figures for the three -pages that have a figure test of their own -- ``extension-header-subclassing``, -``process`` and ``registry-protocol``, the last most heavily of all, down to both halves -of an *N of M* -- so a stale count there goes red instead of quiet. Every other figure, -this page's included, is pinned by nothing: the entry it adds to that module's -``ANCHORS`` fixes the page's existence, anchor and toctree position, not its arithmetic. -Re-derive before trusting, and do not read a number as load-bearing just because it is -written down. The same module checks something a reader cannot see and Sphinx cannot +**Some claims on these pages are pinned, and knowing which is part of writing one.** +:file:`tests/project/test_conventions_doc_claims.py` checks what several pages assert +against the tree by name rather than by figure -- the extension-header table against +the classes it classifies, :ref:`process`'s changelog grouping against the entries, +:ref:`registry-protocol`'s helpers against the code -- so a stale claim there goes red +instead of quiet. A page's entry in that module's ``ANCHORS`` fixes only its existence, +anchor and toctree position; what the page goes on to say is pinned by nothing unless a +test reads it. The same module checks something a reader cannot see and Sphinx cannot warn about: in a ``` `#NNN <.../NNN>`__ ``` link the displayed number has to match the -number in its own URL. A review round once corrupted roughly thirty of them across two -pages and nothing caught it, because both halves were well-formed. +number in its own URL. A review round once corrupted a run of them and nothing caught +it, because both halves were well-formed. Resolvable Targets ~~~~~~~~~~~~~~~~~~ @@ -283,19 +285,18 @@ settled this on :issue:`719` found exactly one, and repeating it is cheap: ``.. data:: pcapkit.x._T`` under ``.. module:: pcapkit.x`` registers ``pcapkit.x.pcapkit.x._T``, which no reference resolves to exactly. Where the object lives in a module other than the current one, a ``:module:`` option names it, as the -``LINKTYPE`` entry on :doc:`/pcapkit/protocols/link/index` does. :issue:`1775` found 44 -directives spelled the long way, which left seven TypeVar references ambiguous; +``LINKTYPE`` entry on :doc:`/pcapkit/protocols/link/index` does. :issue:`1775` found +directives spelled the long way, which left TypeVar references ambiguous; :file:`tests/project/test_docs_ambiguous_xrefs_1775_unit.py` fails on the next one. -**Every importable shipped module is documented, with one deliberate exception.** The 33 +**Every importable shipped module is documented, with one deliberate exception.** The modules under :file:`pcapkit/protocols/*/NotImplemented/` are the set with no entry: placeholder sources for dissectors nobody has written yet, carrying no class a reader could use. The exception is a choice rather than a consequence of packaging, which is -worth stating because the opposite is easy to assume -- measured on ``1.5.0b8``, all 33 -are in the source distribution *and* in the wheel, and each imports as a -namespace-package submodule. What they are not is a *package*: none of the four -directories carries an :file:`__init__.py`, so ``find_packages`` returns 73 here and -omits all of them. +worth stating because the opposite is easy to assume -- measured on ``1.5.0b8``, every one +is in the source distribution *and* in the wheel, and each imports as a +namespace-package submodule. What they are not is a *package*: none of those +directories carries an :file:`__init__.py`, so ``find_packages`` omits all of them. When something is removed, its documentation entry goes with it -- carrying a deprecation note where a user could have depended on the thing, and deleted outright @@ -315,9 +316,9 @@ Format and Mechanics ships only if setuptools or an ``include`` line adds it. * **Prose names the real defining module, not a re-export.** One strand of the consistency sweep on :doc:`the roadmap `. The sentinels are the - worked example: all four are defined in :mod:`pcapkit.corekit.sentinels`, and + worked example: every one is defined in :mod:`pcapkit.corekit.sentinels`, and :ref:`sentinel-convention`'s table names it rather than any of the modules that - use them, which no longer re-export them (:issue:`1744`). + use them, which do not re-export them (:issue:`1744`). * **A verbatim upstream port is exempt from these conventions.** :ref:`sentinel-convention` already carries the case that settled it, the ``cached_property`` backport in :file:`pcapkit/utilities/compat.py`: the value of a diff --git a/docs/source/contributing/conventions/extension-header-subclassing.rst b/docs/source/contributing/conventions/extension-header-subclassing.rst index e4186a387..f956911bb 100644 --- a/docs/source/contributing/conventions/extension-header-subclassing.rst +++ b/docs/source/contributing/conventions/extension-header-subclassing.rst @@ -62,10 +62,10 @@ is reachable as an IPv4 payload in this library -- :class:`~pcapkit.protocols.internet.hopopt.HOPOPT` and :class:`~pcapkit.protocols.internet.ipv6_frag.IPv6_Frag` included. Registry membership therefore says nothing at all about standalone-ness, and a classification -derived from it would make all eight headers standalone. +derived from it would make every one of them standalone. -Nor does IANA's *IPv6 Extension Header Types* registry discriminate: it lists all -eight of the implemented headers, plus ``Shim6`` (140) and 253/254. Being *in* that +Nor does IANA's *IPv6 Extension Header Types* registry discriminate: it lists every +one of the implemented headers, plus ``Shim6`` (140) and 253/254. Being *in* that registry is what makes something an extension header; it is not evidence about whether the same header is also a protocol in its own right. @@ -115,7 +115,7 @@ Explicit Base Declarations Name the second base **explicitly**, even though it is already in the MRO. :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` derives -:class:`~pcapkit.protocols.internet.internet.Internet`, so every one of the eight +:class:`~pcapkit.protocols.internet.internet.Internet`, so every one of them reaches ``Internet`` transitively and an ``__mro__`` check cannot tell the two groups apart. The declaration is the only place the classification exists, which is why ``tests/protocols/internet/test_ipv6_ext_unit.py`` pins it against ``__bases__``: @@ -126,7 +126,7 @@ apart. The declaration is the only place the classification exists, which is why Add a header to that set in the same change that adds its second base, and keep the RFC ground in the ``#:`` comment beside it. The test walks -``IPv6_Ext.__subclasses__()`` rather than a hard-coded list, so a ninth header is +``IPv6_Ext.__subclasses__()`` rather than a hard-coded list, so any further header is held to the convention whether or not anyone remembers this page. The Retired ``IPv6_GenericExt`` Name diff --git a/docs/source/contributing/conventions/index.rst b/docs/source/contributing/conventions/index.rst index a6fd0cbe2..5c75d45d0 100644 --- a/docs/source/contributing/conventions/index.rst +++ b/docs/source/contributing/conventions/index.rst @@ -8,17 +8,20 @@ House Conventions automated contributor would otherwise have to rediscover by reading a closed issue thread. Each ruling names where it was settled. - :ref:`mint-criterion` governs :mod:`pcapkit.const`, which is where the - settled questions have mostly arisen, :ref:`sentinel-convention` - :mod:`pcapkit.corekit`, and :ref:`registry-protocol` :mod:`pcapkit.const` - again. :ref:`extension-header-subclassing` governs a protocol class - hierarchy, :ref:`protocol-layer-placement` which subpackage a dissector - belongs in, :ref:`process` the repository rather than any of its code, - :ref:`documentation` the prose itself -- on these pages and in the API - reference -- rather than any code at all, :ref:`test-coverage` what the - test suite has to prove, and :ref:`utilities-or-corekit` which of - :mod:`pcapkit.utilities` and :mod:`pcapkit.corekit` a support module belongs - in. + What each page governs: + + * :ref:`mint-criterion` -- :mod:`pcapkit.const`, which is where the settled + questions have mostly arisen; + * :ref:`sentinel-convention` -- :mod:`pcapkit.corekit`; + * :ref:`registry-protocol` -- :mod:`pcapkit.const` again; + * :ref:`extension-header-subclassing` -- a protocol class hierarchy; + * :ref:`protocol-layer-placement` -- which subpackage a dissector belongs in; + * :ref:`process` -- the repository rather than any of its code; + * :ref:`documentation` -- the prose itself, on these pages and in the API + reference, rather than any code at all; + * :ref:`test-coverage` -- what the test suite has to prove; + * :ref:`utilities-or-corekit` -- which of :mod:`pcapkit.utilities` and + :mod:`pcapkit.corekit` a support module belongs in. **A ruling that stays in its thread is a ruling that gets rediscovered.** So when a question is answered in a way the code cannot express on its own -- a diff --git a/docs/source/contributing/conventions/mint-criterion.rst b/docs/source/contributing/conventions/mint-criterion.rst index 5f7df9760..d7eeb6c49 100644 --- a/docs/source/contributing/conventions/mint-criterion.rst +++ b/docs/source/contributing/conventions/mint-criterion.rst @@ -3,15 +3,15 @@ Minting an Unrecognised Value ----------------------------- -125 of the 127 registries under :mod:`pcapkit.const` resolve an unrecognised value through +Nearly every registry under :mod:`pcapkit.const` resolves an unrecognised value through a ``_missing_`` override, which decides what happens when a value has no member. Only :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` and :class:`~pcapkit.const.pcapng.tls_key_label.TLSKeyLabel` have none anywhere in their -MRO, so they carry no declared-but-unassigned range to resolve. 121 define one -themselves; the other four are the transport subclasses of +MRO, so they carry no declared-but-unassigned range to resolve. The rest define one +themselves, except the transport subclasses of :class:`~pcapkit.const.reg.apptype.apptype.AppType`, which inherit -``AppType._missing_`` (:file:`pcapkit/const/reg/apptype/apptype.py:2994`). Three -behaviours follow from it, and which one a given range gets is a **design decision, not +``AppType._missing_`` (:file:`pcapkit/const/reg/apptype/apptype.py:2994`). The +behaviours below follow from it, and which one a given range gets is a **design decision, not a style preference**: ``extend_enum(cls, name, value)`` -- *mint* @@ -33,29 +33,29 @@ a style preference**: Measured against the tree, by class: -* **1 mints**: :class:`~pcapkit.const.mh.cga_type.CGAType`, whose values are 128-bit +* **Mints**: :class:`~pcapkit.const.mh.cga_type.CGAType` alone, whose values are 128-bit CGA extension tags rather than an IANA-style range of codes. -* **117 unmint.** The four transport subclasses (``TCP``, ``UDP``, ``SCTP``, ``DCCP``) - inherit the method; of the remaining 114 that call ``_unregistered_member`` - themselves, 113 can reach it, the 114th being ``AppType``, which holds no members and - raises for every value through its ``cls.__registry__ is None`` guard. +* **Unmint**: the bulk of the registries. The transport subclasses (``TCP``, ``UDP``, + ``SCTP``, ``DCCP``) inherit the method; every other registry here calls + ``_unregistered_member`` itself, though ``AppType`` never gets there: it holds no + members and raises for every value through its ``cls.__registry__ is None`` guard. - Of those 113, **at least 12 can never reach it**, because every range they declare is + **Some of the rest can never reach the call either**, because every range they declare is already covered by their own members -- ``ToSDelay`` declares ``0..1`` and has members - ``[0, 1]``, so its ``_unregistered_member`` call is dead code. The twelve are + ``[0, 1]``, so its ``_unregistered_member`` call is dead code. Those found are ``ipv4.option_class``, ``ipv4.tos_del``, ``ipv4.tos_ecn``, ``ipv4.tos_pre``, ``ipv4.tos_rel``, ``ipv4.tos_thr``, ``ipv6.option_action``, ``ipv6.seed_id``, ``ipv6.smf_dpd_mode``, ``l2tp.type``, ``mh.dhcp_support_mode`` and - ``vlan.priority_level``. A further three are ``str``-valued and were not probed - (``ftp.command.Command``, ``ftp.command.FEATCode``, ``http.method.Method``), so 12 is - a floor and the true figure is at most 15. -* **5 cache in the value table only**: :class:`~pcapkit.const.tcp.flags.Flags` and the - four Mobility Header flag registries, such as + ``vlan.priority_level``. The ``str``-valued ``ftp.command.Command``, + ``ftp.command.FEATCode`` and ``http.method.Method`` were not probed, so that list is a + floor. +* **Cache in the value table only**: :class:`~pcapkit.const.tcp.flags.Flags` and the + Mobility Header flag registries, such as :class:`~pcapkit.const.mh.binding_ack_flag.BindingACKFlag`. They range-check and hand back to ``super()._missing_``, which for an ``aenum.IntFlag`` reaches ``_create_pseudo_member_`` and ends in ``cls._value2member_map_.setdefault(value, pseudo_member)``. -* **4 raise for any unknown value**: ``AppType``, ``ExtensionHeader``, ``TLSKeyLabel`` +* **Raise for any unknown value**: ``AppType``, ``ExtensionHeader``, ``TLSKeyLabel`` and :class:`~pcapkit.const.hip.transport.Transport`. ``Transport`` raises from its own range check: its declared range ``0..3`` is exactly its member set, so no unassigned value is left for ``super()._missing_`` (the line after the check is @@ -65,12 +65,12 @@ Note that these are ``aenum`` classes: ``issubclass(Flags, enum.IntFlag)`` is `` against the standard library and ``True`` against ``aenum.IntFlag``, so a probe against ``enum`` answers about the wrong hierarchy. -The mint criterion below therefore applies only to the 117 that unmint, where it +The mint criterion below therefore applies only to the registries that unmint, where it decides the *name* an unregistered member carries -- the registrar's own label suffixed with the individual code, or the bare block label on its own -- not whether it -registers. In the other 10 it never runs: ``CGAType`` names its tag from the value, -the five flag registries compose a name from their members' bits, and the four that -raise name nothing. The two crawlers that generate the ranges record that narrowing in +registers. In the others it never runs: ``CGAType`` names its tag from the value, +the flag registries compose a name from their members' bits, and those that +raise name nothing. The crawlers that generate the ranges record that narrowing in their own comments: :file:`pcapkit/vendor/reg/ethertype.py`'s ``UNASSIGNED_ROW_NAMES`` and :file:`pcapkit/vendor/ipx/socket.py`'s ``UNASSIGNED_RANGE_NAMES``. @@ -123,15 +123,15 @@ Worked Examples occupies it; the real name arrives with the allocation. *Suffixed.* :mod:`pcapkit.const.reg.ethertype`'s company names -- ``Xyplex``, -``Datability``, ``Qualcomm``, ``Motorola`` and forty-two others, 46 names across 50 -range blocks, since four of them hold two blocks each -- and +``Datability``, ``Qualcomm``, ``Motorola`` and many others, some of them holding more +than one range block -- and :mod:`pcapkit.const.ipx.socket`'s ``Registered by Xerox``. A company name is the assignment, for the reason in the next section. -Two further ranges in that file carry a suffixed name without being company names at +Further ranges in that file carry a suffixed name without being company names at all: ``IEEE802.3 Length Field``, which names a field in a standard, and -``Berkeley Trailer encap/IP``, which names an encapsulation. Both are outside the 46, -and neither is suffixed for the reason the next section gives. +``Berkeley Trailer encap/IP``, which names an encapsulation. Neither is among the company names, and +neither is suffixed for the reason the next section gives. .. note:: @@ -188,9 +188,9 @@ only ``ast.Name`` nodes finds every mint and no unmint at all: .. note:: The walk reads each file's own ``_missing_``, so it reports ``AppType`` as an - unmint although it raises, prints nothing for the five flag registries and + unmint although it raises, prints nothing for the flag registries and ``Transport`` (they call neither function) nor for ``ExtensionHeader`` and - ``TLSKeyLabel`` (they define no ``_missing_``), and does not see the four + ``TLSKeyLabel`` (they define no ``_missing_``), and does not see the transport subclasses, which inherit theirs. Reconcile it against a runtime probe for the split in the list above. @@ -198,9 +198,9 @@ only ``ast.Name`` nodes finds every mint and no unmint at all: Calling ``Cls(value)`` on a registry that mints or hands back to a flag base **mutates the class**. A probe is not a read: it installs something that every later - lookup then finds. Six registries do this. :class:`~pcapkit.const.mh.cga_type.CGAType` + lookup then finds. :class:`~pcapkit.const.mh.cga_type.CGAType` grows ``__members__``, ``_member_map_`` and ``_value2member_map_``; - :class:`~pcapkit.const.tcp.flags.Flags` and the four Mobility Header flag registries + :class:`~pcapkit.const.tcp.flags.Flags` and the Mobility Header flag registries grow ``_value2member_map_`` only. Snapshot ``{member.value for member in Cls}`` before any lookup regardless -- that snapshot cannot see the flag registries' cache, so compare ``len(Cls._value2member_map_)`` too -- and use a throwaway process per diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index d3c744743..4a8f65521 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -3,7 +3,7 @@ Running the Repository ---------------------- -The five pages before this one are about writing library code. The rulings here are +The pages before this one are about writing library code. The rulings here are about running the repository -- what an install carries, what a changelog entry is, and what the issue and pull request labels mean. None of them is derivable from a module, and none fits a code-convention page, so the owner ruled on @@ -18,7 +18,7 @@ The ``all`` Extra ``all`` means **core addons only** -- the things that let the library itself run at full functionality -- rather than everything a user might conceivably want. The owner settled that on :issue:`910` and named -the three that qualify: the CLI addon, the crypto addon, and ``pycrate``. +the ones that qualify: the CLI addon, the crypto addon, and ``pycrate``. On the tree, in :file:`pyproject.toml`: @@ -30,7 +30,7 @@ On the tree, in :file:`pyproject.toml`: all = [ "emoji", "cryptography>=3.4", "pycrate" ] -So ``all`` is exactly the union of those three extras, written out as literals rather +So ``all`` is exactly the union of those extras, written out as literals rather than referenced. Two groups stay out, and the reasons are **different** rather than two versions of @@ -41,7 +41,7 @@ once: ``PyShark``, ``PyPCAPFile``, ``PyPCAP`` and ``PCAP_CT`` are not core addons, so they are on demand -- one extra at a time, ``pip install pypcapkit[Scapy]`` and so on. This holds however easy they are to install, and it is what the ruling - changed: four of them used to be in ``all``. + changed: some of them used to be in ``all``. * ``PyPCAP`` **and** ``PCAP_CT`` **were already excluded for installability**, which the ruling leaves untouched. ``pypcap`` is an sdist-only C extension needing a compiler and the libpcap development files; ``pcap-ct`` and ``libpcap`` are @@ -57,7 +57,7 @@ back to the default engine rather than raising when a requested engine is absent The ``dev`` extra exists **because** of this narrowing, and is not a second catch-all. pylint, mypy and autodoc all resolve imports against what is installed, - so the four engines leaving ``all`` would have made them newly unresolvable to the + so the engines leaving ``all`` would have made them newly unresolvable to the toolchain. ``dev`` is defined by what the toolchain must be able to *see*, and the workflows that need full resolution install ``.[all,dev]``. Do not add ``pypcap``/``pcap-ct`` to it to silence a lint finding; :file:`.github/workflows/lint.yml` @@ -142,12 +142,12 @@ statement someone made rather than a value derived from the change. That matters for the ``review:`` family below: such a label is not evidence of the state it names, it is a record that the owner asserted it. -Two paths are automated, and both are worth knowing about because a label they set has +Some paths are automated, and each is worth knowing about because a label they set has had no human judgement behind it: * **dependabot** puts ``dependencies`` and ``python`` on its own pull requests, and - only those two: :file:`.github/dependabot.yml` configures a single ecosystem, - ``pip``, so it never opens a workflow bump here. ``github_actions`` exists as a + only those: :file:`.github/dependabot.yml` configures only the ``pip`` + ecosystem, so it never opens a workflow bump here. ``github_actions`` exists as a label but is **hand-applied like the rest** -- naming it as dependabot's would tell a reader the opposite of this section's point. * **The issue templates** apply an issue-kind label from their front matter, before @@ -160,9 +160,9 @@ Nothing else automates a label. :file:`.github/release.yml` only *reads* existin labels to bucket release notes, and the ``PCAPKIT_CONDA_LABEL`` in the release workflows is a conda channel label, unrelated to these. -The ones that carry meaning here fall into five groups, which stack rather than +The ones that carry meaning here fall into the groups below, which stack rather than compete: a pull request normally carries one from the first group and as many of the -rest as apply. **The five groups are not the whole label set** -- the repository also +rest as apply. **These groups are not the whole label set** -- the repository also has GitHub's own defaults, of which ``wontfix``, ``invalid``, ``help wanted`` and ``duplicate`` are all in live use and ``good first issue`` is archived. GitHub cannot archive a label, so an archived one keeps its name and description and is recoloured @@ -170,8 +170,7 @@ grey (``cccccc``); it is kept rather than deleted so the name stays reserved. Those are documented by GitHub rather than here, and are counted rather than listed so this page does not go stale every time one is added:: - $ gh label list -R JarryShaw/PyPCAPKit --limit 100 --json name -q '.[].name' | wc -l - 31 + gh label list -R JarryShaw/PyPCAPKit --limit 100 --json name -q '.[].name' | wc -l One of those defaults carries a local ruling worth knowing: an issue closed as unnecessary takes ``invalid`` (or the nearest applicable) rather than ``bug``, since @@ -228,14 +227,14 @@ these, so that its status is readable without opening it: remembered -- a command someone else can run and get an answer from. * ``needs: decision`` -- waiting on the owner, and on nothing else. -The four are **mutually exclusive**, so moving between them is a swap rather than an +They are **mutually exclusive**, so moving between them is a swap rather than an addition: dispatching work on a ``pending`` issue removes that label as it adds ``wip``. ``pending`` exists because the alternative was an unlabelled issue, and nothing then distinguished *accepted but untouched* from *nobody has looked at the labels* -- the two want different responses, and the second is a gap to close rather than a state to leave alone. -Two things the board shows rather than the rule: ``wip`` and ``needs: decision`` +Things the board shows rather than the rule: ``wip`` and ``needs: decision`` legitimately **co-occur**, when the bulk of an issue is being worked and one sub-question is held for the owner -- :issue:`918` itself was labelled that way. And an open issue with no state label at all is a gap rather than a category, which is @@ -250,8 +249,8 @@ worth checking for rather than assuming away: has a status of its own. ``review: pending`` means no verdict **and nobody producing one** -- never reviewed, or the head moved since the last verdict; ``review: running`` means a cross-review is in flight against this head, so a verdict is coming; -``review: good-to-go`` and ``review: needs-changes`` are the two verdicts. Because all -four are keyed on the head rather than on the pull request, a new push invalidates the +``review: good-to-go`` and ``review: needs-changes`` are the verdicts. Because all of +them are keyed on the head rather than on the pull request, a new push invalidates the label -- a verdict that outlives the commit it was given on is worse than none, and so is a ``running`` label pointing at a review of a commit that is no longer the head. @@ -312,10 +311,10 @@ carrying it are that kind: **The label is not applied uniformly across the repository's history, and a census that assumes it is will be wrong.** It is dense on pull requests from the examples-directory change above onward and effectively absent below it: the only - earlier carriers are seven pre-``0.15`` pull requests, clustered at the very start - of the numbering, with nothing labelled at all between them and that change. Three - of those seven are distribution rollups, each also carrying ``release``; the other - four are early ``refactor``/``feat`` work from before the project stabilised. On + earlier carriers are a few pre-``0.15`` pull requests, clustered at the very start + of the numbering, with nothing labelled at all between them and that change. Some + of those are distribution rollups, each also carrying ``release``; the others + are early ``refactor``/``feat`` work from before the project stabilised. On issues it is sparser still, appearing only from :issue:`719` up. So ``breaking``'s absence on an old pull request is weak evidence at best. The current figures, rather than these: @@ -377,7 +376,7 @@ scopes, because ``GITHUB_TOKEN`` cannot write a user-owned project; **without th secret every run skips with a notice** rather than failing, and Status falls back to being set by hand. -The board has seven views: *All items*; *Board*, the open items in one column per +The board has these views: *All items*; *Board*, the open items in one column per Status; *Release 1.5*, the open milestone; *Needs decision* and *Blocked*, each the open items in that state; *Open pull requests*; and *History*, everything *Done*. Rename *Release 1.5* when the next milestone opens. Sorting and grouping are set in diff --git a/docs/source/contributing/conventions/protocol-layer-placement.rst b/docs/source/contributing/conventions/protocol-layer-placement.rst index 167c0dea0..999bcc470 100644 --- a/docs/source/contributing/conventions/protocol-layer-placement.rst +++ b/docs/source/contributing/conventions/protocol-layer-placement.rst @@ -64,7 +64,7 @@ defines every layer by its purpose rather than by what encapsulates it. IANA Assignments Carry No Layer Claim ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -All three registries this package dispatches on -- *Protocol Numbers*, *Ether Types*, +Every registry this package dispatches on -- *Protocol Numbers*, *Ether Types*, and *Service Name and Transport Protocol Port Number* -- have **no layer field**. *Protocol Numbers* frames itself as identifying *"the next level protocol"*, i.e. the **encapsulated** one. A registry assignment is evidence of encapsulation and of @@ -72,11 +72,11 @@ nothing else. .. caution:: - This **retires the registration grid at** :file:`docs/source/ext.rst` (the + This **retires the registration grid on** :doc:`/ext` (the Protocol Type → Registry Function table) **as a placement rule.** It is a guide to which registrar to call when extending the library, and it was never a layer taxonomy. Read literally as one it makes ARP, RARP and both VLAN tags - internet-layer and all eight IPv6 extension headers transport-layer. A + internet-layer and every IPv6 extension header transport-layer. A contributor looking for a placement rule will find that grid first, which is why this page says so explicitly. @@ -146,10 +146,10 @@ either base order. :class:`~pcapkit.protocols.link.arp.ARP`'s chain reaches Ordering layer-first is correct in both cases, which is the point: a contributor does not have to work out which case they are in. -Two Things That Will Look Like Mistakes -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Things That Will Look Like Mistakes +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Both are deliberate. Do not "fix" either. +Each is deliberate. Do not "fix" any of them. **A parsing base may live in another subpackage.** :mod:`pcapkit.protocols.application.rarp` imports @@ -187,14 +187,14 @@ protocol number is refused. Counting the names in each class's own namespace, whether new or overriding :class:`~pcapkit.protocols.protocol.ProtocolBase`'s, and including those ``ProtocolBase.__init_subclass__`` sets there, ``Link`` has -ten -- ``__data__``, ``__layer__``, ``__post_init__``, ``__proto__``, +``__data__``, ``__layer__``, ``__post_init__``, ``__proto__``, ``__schema__``, ``_read_protos``, ``_shipped_proto``, ``from_data``, ``register`` and -``layer`` -- and ``Application`` nine: the six they share (``__data__``, ``__layer__``, +``layer``, and ``Application`` has what they share (``__data__``, ``__layer__``, ``__post_init__``, ``__schema__``, ``from_data``, ``layer``) plus ``__index__``, ``_decode_next_layer`` and ``_import_next_layer``. -``Link`` overrides four of those that ``Application`` does not -- ``__proto__``, +``Link`` overrides names that ``Application`` does not -- ``__proto__``, ``register``, ``_read_protos`` and ``_shipped_proto``, the last set because ``Link`` -defines its own ``__proto__`` -- but all four also exist on ``ProtocolBase``, so +defines its own ``__proto__`` -- but each also exists on ``ProtocolBase``, so the strict difference ``Link`` minus ``Application`` minus ``ProtocolBase`` is **empty** and every one of them still resolves on ``OSPF``. What changes is which registry it resolves *to*: ``OSPF.__proto__`` is ``ProtocolBase.__proto__`` rather than diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index f266ee4dd..acb90f54a 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -17,7 +17,7 @@ The Registry Protocol class LinkType(EnumRegistry, IntEnum): ... -Six registries define their own ``__new__`` to carry extra attributes and so do not +These registries define their own ``__new__`` to carry extra attributes and so do not share the generated template: :class:`~pcapkit.const.ftp.command.Command`, :class:`~pcapkit.const.ftp.return_code.ReturnCode`, :class:`~pcapkit.const.http.method.Method`, @@ -51,7 +51,7 @@ What settled the split is the owner's own second thought about carrying ``regist on the base: if the base carries ``register``, there is no principled reason for it not to carry ``register_alias`` as well, and the ruling would be the worse for it. Following that through leaves :class:`~pcapkit.corekit.enum.EnumRegistry` holding only -three methods, too thin to justify a second class -- so the two tiers collapse into one, +a few methods, too thin to justify a second class -- so the two tiers collapse into one, which is the opposite of what was ruled. **Both tiers are plain classes, and that is load-bearing.** Inserting a parent above @@ -91,7 +91,7 @@ The check is the *range*, not everything ``_missing_`` decides. A value inside t that matches neither a member nor an unassigned range -- ``CauseCode(0)``, say -- still fails to resolve through ``get``, and ``register`` may mint it. -Three things about it are easy to get wrong: +What is easy to get wrong about it: * **It guards, it does not convert.** The return type is :obj:`None` deliberately, so that an override cannot normalise a value on its way through and silently change @@ -106,7 +106,7 @@ Three things about it are easy to get wrong: ``get`` re-raises a :exc:`ValueError` that is already a :exc:`~pcapkit.utilities.exceptions.BaseError` exactly as the override raised it, and converts only ``aenum``'s and :mod:`enum`'s own "no member carries this - value". Two things follow, and both are the point of the discrimination rather + value". The consequences are the point of the discrimination rather than side effects: the override's **own message** survives to the caller instead of being replaced by the base's, and the error is **logged once** rather than twice, since :class:`~pcapkit.utilities.exceptions.BaseError` logs on @@ -165,7 +165,7 @@ Which branch a key takes, and what each miss ends in: VALUE -->|"ValueError, no usable default"| VALERR["EnumValueError
loud, derives ValueError"] Do not "improve" on the shape by making both misses report identically. Converting -one into the other is exactly what :issue:`923` retired, and it was retired in three +one into the other is exactly what :issue:`923` retired, and it was retired in several places at once: ``TransportProtocol.get`` and ``Criticality.get`` had each turned the base's :exc:`KeyError` into a :exc:`ValueError`, and ``FastBindingAcknowledgmentStatus.get`` raised @@ -189,16 +189,16 @@ logged error, and needs a reason. ``get`` Override Obligations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Three rulings settled what an override owes -:meth:`~pcapkit.corekit.enum.EnumLookup.get`, and the last of them deleted two +Rulings settled what an override owes +:meth:`~pcapkit.corekit.enum.EnumLookup.get`, and the last of them deleted overrides outright. They are collected here because each was reached by the same argument: the base's contract is house-wide, so an override that diverges from it is a defect rather than a local judgement about its own callers. The first item below is -not a ruling but a language constraint, recorded with them because the three rulings +not a ruling but a language constraint, recorded with them because the rulings all presuppose it. **An override that delegates is a** ``@classmethod``, and this one is forced by -Python rather than decided. Zero-argument :func:`super` binds the enclosing +Python rather than decided. Zero-argument :class:`super` binds the enclosing function's **first positional parameter** as its instance, whatever that parameter is named -- so inside ``@staticmethod def get(key, default=NO_DEFAULT)`` it binds ``key``, the lookup key, and ``return super().get(key)`` fails on the key rather @@ -213,16 +213,16 @@ instance nor the type:: TypeError: super(type, obj): obj must be an instance or subtype of type Both forms share ``instance or subtype of type``, which is the only part of the -message anything here relies on -- pinning either full sentence would fail on two or -three of the five supported versions. +message anything here relies on -- pinning either full sentence would fail on some +of the supported versions. -Two corollaries worth stating, because the obvious summary of this is wrong in both +Corollaries worth stating, because the obvious summary of this is wrong in both directions. ``RuntimeError: super(): no arguments`` is a *different* failure, raised only when the enclosing function takes no parameters at all -- no real ``get`` override qualifies, since every one takes ``key``. And the ``TypeError`` is not guaranteed either: pass a first argument that *is* an instance of the class and the delegation **silently succeeds**, so a ``@staticmethod`` override cannot even be -relied on to fail loudly. Measured, all three cases, rather than reasoned about. +relied on to fail loudly. Measured, every case, rather than reasoned about. :meth:`~pcapkit.corekit.enum.EnumLookup.get` is itself a ``@classmethod``. The precedent is :issue:`903`, which made ``FEATCode.get`` a ``@classmethod def get(cls, key, default=NO_DEFAULT)`` ending in @@ -338,7 +338,7 @@ does. It used to convert the base's name-miss :exc:`KeyError` into a :exc:`ValueError` as well, and :issue:`923`'s ruling retired that; the :issue:`808` refusal to extend the class at all is untouched by the retirement, since only the exception class moved. -``Criticality.get`` went further and no longer exists: conversion was the *only* +``Criticality.get`` went further and was deleted: conversion was the *only* thing it added over the base, so once that went there was nothing left for an override to hold, and the class inherits :meth:`~pcapkit.corekit.enum.EnumLookup.get` unchanged. **An override that would @@ -431,7 +431,7 @@ That leaves the classes with something to decide: RFC 5797 §2 above is. Note also that the §2 sentence is wrapped across a line break in the RFC's text file, at ``case-`` / ``insensitive``, so a line-oriented grep for the phrase finds nothing. - - **case-insensitive** -- was a defect; ``get`` now folds + - **case-insensitive** -- was a defect; ``get`` folds * - :class:`~pcapkit.const.http.method.Method` - :rfc:`9110#section-9.1` - *"The method token is case-sensitive."* Explicitly the opposite of limb 1. @@ -457,7 +457,7 @@ That leaves the classes with something to decide: all 14,536 rows (``tcp`` 6608, ``udp`` 6357, blank 1467, ``sctp`` 93, ``dccp`` 11, zero upper-case). - **case-insensitive** -- ``get`` folds, and this is the owner's own example. - Folding is now the *only* thing that override adds (:issue:`923`) + Folding is the *only* thing that override adds (:issue:`923`) * - :class:`~pcapkit.const.reg.apptype.apptype.AppType` - -- - Moot: its ``get`` takes a port number and refuses a non-:class:`int` outright, @@ -489,37 +489,37 @@ That leaves the classes with something to decide: exactly as for the :class:`int` tier above; ``PDUKind``'s are :class:`str` ASN.1 identifiers from 3GPP TS 38.413, and ASN.1 identifiers are case-significant by construction, so it lands the same way for a different reason. - **None** of them defines a ``get`` of its own. Two did when this + **None** of them defines a ``get`` of its own. Some did when this audit was taken -- ``FastBindingAcknowledgmentStatus`` and ``IPv6AddressPrefixCode``, for signature reasons (no ``default``, and an :class:`int`/:class:`str` dispatch) rather than for case -- and both were deleted as redundant rather than widened, on the ruling recorded in the section above and given in review of the widening - itself; each now inherits ``get`` from + itself; each inherits ``get`` from :class:`~pcapkit.corekit.enum.EnumLookup` unchanged. ``LMAAddressCode`` and ``LocalizedRoutingStatus`` never carried a ``get`` at all, so they had no string lookup to fold. ``Criticality`` had one when this audit was taken and no longer does: :issue:`877` re-parented it onto :class:`~pcapkit.corekit.enum.EnumLookup` and :issue:`923` retired the exception - conversion that was the override's only remaining job, so it now inherits + conversion that was the override's only remaining job, so it inherits ``get`` unchanged. - **case-sensitive** -- conforms * - ``WireGuardKeyLabel`` - ``draft-tuexen-opsawg-pcapng`` - - The draft names the four labels outright (*"The key type is one of + - The draft names the labels outright (*"The key type is one of LOCAL_STATIC_PRIVATE_KEY, ..."*) and, as for ``OptionType`` above, never discusses their case. Both authorities write them upper case. - **case-sensitive** -- no override, conforms * - Every other non-registry enumeration - -- - - All 14 are pcapkit's own discriminators and bit labels, with no registrar behind + - Each is one of pcapkit's own discriminators and bit labels, with no registrar behind them at all -- ``Completion``, ``ftp.Type``, ``httpv1.Type``, ``FinalisedState``, - ``ESPStatus``, ``PacketDirection``, ``PacketReception``, and the 7 httpv2 + ``ESPStatus``, ``PacketDirection``, ``PacketReception``, and the httpv2 ``Flags``. - **case-sensitive** -- nothing to cite, nothing to change -Two rows the audit deliberately left open rather than acting on, because each is +Rows the audit deliberately left open rather than acting on, because each is wider than a case fix: * **A service-name lookup on the** ``AppType`` **transport registries.** This is the @@ -527,7 +527,7 @@ wider than a case fix: case-insensitive, and ``TCP``/``UDP``/``SCTP``/``DCCP`` take their member names from that column -- but ``AppType.get`` refuses a non-:class:`int` key, so no service-name lookup exists for the rule to apply to. Implementing one is new public API on a - 6,000-member registry where one name maps to many ports, which is a ``get_all`` + registry of thousands of members where one name maps to many ports, which is a ``get_all`` design question rather than a case fold. * ``CommandType`` **and** ``ConformanceRequirement``. By parity with ``TransportProtocol`` -- an :class:`int`-valued enumeration whose *names* are the @@ -571,7 +571,7 @@ rather than changing it. :issue:`903` that class overrides ``get`` too, so the output is only the base's because its override delegates an exact name-or-value hit straight through. Measure the base on a registry that does - **not** override ``get`` at all. **Five** do -- + **not** override ``get`` at all. **These** do -- :class:`~pcapkit.const.ftp.command.Command`, :class:`~pcapkit.const.ftp.command.FEATCode`, :class:`~pcapkit.const.http.method.Method`, diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index d6fc2c96e..0a50fe129 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -9,7 +9,7 @@ caller might legitimately pass. The house rule, from the maintainer, covers the the sentinel object's type class is named ``Type``. That is, the class takes the instance's name in CamelCase with ``Type`` appended. It -says nothing about the **object**'s own name, which is what let three casings diverge +says nothing about the **object**'s own name, which is what let the casings diverge with no rule naming any of them wrong. GitHub issue :issue:`937` closed that gap: the owner ruled for SCREAMING_SNAKE and accepted the resulting breaking change outright, with no backport. So the object is named in SCREAMING_SNAKE and the type-naming rule above @@ -36,7 +36,7 @@ derives from it mechanically -- title-case each underscore-separated word and ap - ``AbsentType`` - :mod:`pcapkit.corekit.sentinels` -GitHub issue :issue:`911`'s housing ruling -- one module for all four -- is why the table +GitHub issue :issue:`911`'s housing ruling -- one module for all of them -- is why the table names a single defining module, and it is the only import path. The modules that *use* a sentinel import it from there but do not re-export it: :issue:`1744` dropped the per-module re-exports, under the ruling on :issue:`1715` that a moved name keeps no @@ -69,7 +69,7 @@ public use rather than left off it, since the name alone no longer says so. .. note:: - Of the four, only :class:`~pcapkit.corekit.sentinels.NullType` is a full worked + Of the sentinels, only :class:`~pcapkit.corekit.sentinels.NullType` is a full worked example. :class:`~pcapkit.corekit.sentinels.NoValueType` follows the naming rule but is **not** a singleton (``NoValueType() is NO_VALUE`` is :obj:`False`) and has no ``__repr__`` of its own, so it demonstrates the name and nothing else; ``AbsentType`` @@ -110,7 +110,7 @@ ever sees. The rule above is for sentinels this package writes itself. Per-Sentinel Dunder Methods ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The four sentinels deliberately differ, and the differences are **needs, not +The sentinels deliberately differ, and the differences are **needs, not inconsistencies**: ``__new__`` returning a cached instance @@ -134,7 +134,7 @@ inconsistencies**: prevent. ``__copy__`` / ``__deepcopy__`` / ``__reduce__`` - :class:`~pcapkit.corekit.sentinels.NullType` is the only one of the four that has + :class:`~pcapkit.corekit.sentinels.NullType` is the only sentinel that has them, because ``NULL`` is stored in a :class:`~pcapkit.corekit.module.ModuleDescriptor` field, so a caller's :func:`copy.deepcopy` or :mod:`pickle` can walk into it and would otherwise @@ -148,6 +148,6 @@ inconsistencies**: :attr:`FieldBase.default ` through a field copy once per field per packet, keeps its identity there because :meth:`FieldBase.__copy__ ` is - shallow and shares the reference. Add all three when, and only when, the sentinel + shallow and shares the reference. Add them all when, and only when, the sentinel becomes reachable from a copy that is not. diff --git a/docs/source/contributing/conventions/test-coverage.rst b/docs/source/contributing/conventions/test-coverage.rst index ee4d2e55d..405365c0d 100644 --- a/docs/source/contributing/conventions/test-coverage.rst +++ b/docs/source/contributing/conventions/test-coverage.rst @@ -20,7 +20,7 @@ no tolerance in between: a cycle that comes back with different octets is a bug, when both halves are internally consistent. Coverage is **enumerated from the registries rather than hand-picked**, so a code -registered tomorrow gets a case tomorrow. The two registry-driven suites that carry a +registered tomorrow gets a case tomorrow. The registry-driven suites that carry a known-failure table keep their cases in a generator, beside the captures it also writes: .. list-table:: @@ -44,7 +44,7 @@ The rules those tables carry: * **Every registered code has a case.** ``test_every_registered_code_has_a_case`` fails otherwise. The option suite's ``SKIP`` table is the only exit, and each of its - three entries states why a case would assert nothing: ``Multipath_TCP`` is the + entries states why a case would assert nothing: ``Multipath_TCP`` is the envelope of the ``tcp-mptcp`` family and would duplicate one of its subtypes, and ``PadN`` under HOPOPT and IPv6-Opts emits the same option-less header as ``Pad1``. A code that fails is never skipped. @@ -55,7 +55,7 @@ The rules those tables carry: parameters in every HIP packet, so the unrepresentable length of a lone one (:issue:`651`) was never constructed. ``KNOWN_DEGRADED`` entries have a different shape -- a fragment and a reason -- and - its two record why the probe's placeholder payload cannot reach the target, not a + they record why the probe's placeholder payload cannot reach the target, not a defect to fix, so the ``file:line``-and-issue rule does not apply to them. * **A regression is fixed; a pre-existing defect is recorded.** A case that a change breaks -- including a code it registers that does not close -- is fixed in that diff --git a/docs/source/contributing/conventions/utilities-or-corekit.rst b/docs/source/contributing/conventions/utilities-or-corekit.rst index c74385c1e..871dd7b5a 100644 --- a/docs/source/contributing/conventions/utilities-or-corekit.rst +++ b/docs/source/contributing/conventions/utilities-or-corekit.rst @@ -4,7 +4,7 @@ Choosing Between ``utilities`` and ``corekit`` ---------------------------------------------- Which of the two support packages a module belongs in. Settled by the owner on -:issue:`1712`, over two alternatives that sorted modules by packet knowledge or by +:issue:`1712`, over alternatives that sorted modules by packet knowledge or by who uses them. This rule can be checked from the import graph alone, and it needed the fewest moves: one. diff --git a/docs/source/contributing/pep.rst b/docs/source/contributing/pep.rst index a3c0f5af2..c43809874 100644 --- a/docs/source/contributing/pep.rst +++ b/docs/source/contributing/pep.rst @@ -71,9 +71,9 @@ SCTP ~~~~ **Done.** :class:`~pcapkit.protocols.transport.sctp.SCTP` is a first-class -transport layer protocol implemented per :rfc:`9260`: the common header, all -thirteen chunk types the RFC defines, its eight chunk parameters, its thirteen -error causes, and CRC32c checksum verification. +transport layer protocol implemented per :rfc:`9260`: the common header, every +chunk type, chunk parameter and error cause the RFC defines, and CRC32c checksum +verification. One note on how it differs from its siblings. The next layer is dispatched on the DATA chunk's *payload protocol identifier* through @@ -85,9 +85,8 @@ What is still wanted is the registered-but-unimplemented type codes. IANA registers considerably more than :rfc:`9260` defines, and the surplus falls through to the generic handlers rather than failing the extraction -- so a capture using one parses, but yields an opaque chunk instead of its fields. -As things stand that is 17 of the 30 registered chunk types, 24 of the 32 -chunk parameters and 10 of the 23 error causes; :doc:`/pcapkit/const/sctp` -lists them all. +That is true of many of the registered chunk types, chunk parameters and error +causes; :doc:`/pcapkit/const/sctp` lists them all. The other thing wanted for SCTP is reassembly, which no protocol beyond IP and TCP has -- see `Reassembly Beyond IP and TCP`_ below, since it needs work in @@ -100,19 +99,18 @@ ESP decrypts when a Security Association is supplied through the protocol keyed :mod:`pcapkit.corekit.context` channel. -What is still wanted there is wider algorithm coverage. The two enumerations -under :doc:`/pcapkit/const/esp` carry every transform IANA has registered -- 36 -:class:`~pcapkit.const.esp.cipher.Cipher` members and 15 -:class:`~pcapkit.const.esp.integrity.Integrity` members -- but -:data:`~pcapkit.protocols.internet.esp.CIPHER_SUITES` and -:data:`~pcapkit.protocols.internet.esp.INTEGRITY_SUITES` apply only five of -each, and two of those ten are the no-ops ``ENCR_NULL`` and ``NONE``. So the -real coverage is AES-CBC and AES-GCM at all three tag lengths for encryption, -and HMAC-SHA1-96 plus the three :rfc:`4868` HMAC-SHA2 truncations for -integrity. +What is still wanted there is wider algorithm coverage. The enumerations +under :doc:`/pcapkit/const/esp` -- +:class:`~pcapkit.const.esp.cipher.Cipher` and +:class:`~pcapkit.const.esp.integrity.Integrity` -- carry every transform IANA has +registered, but :data:`~pcapkit.protocols.internet.esp.CIPHER_SUITES` and +:data:`~pcapkit.protocols.internet.esp.INTEGRITY_SUITES` apply only a few of +them, the no-ops ``ENCR_NULL`` and ``NONE`` among them. So the real coverage is +AES-CBC and AES-GCM at every tag length for encryption, and HMAC-SHA1-96 plus +the :rfc:`4868` HMAC-SHA2 truncations for integrity. Not implemented, roughly in the order a capture is likely to want them: -AES-CTR [:rfc:`3686`], AES-CCM at all three tag lengths [:rfc:`4309`], +AES-CTR [:rfc:`3686`], AES-CCM at every tag length [:rfc:`4309`], ChaCha20-Poly1305 [:rfc:`7634`], AES-XCBC-MAC-96 [:rfc:`3566`], AES-CMAC-96 [:rfc:`4494`], the AES-GMAC family and its ``ENCR_NULL_AUTH_AES_GMAC`` counterpart [:rfc:`4543`], and then the Camellia, implicit-IV [:rfc:`8750`] and @@ -121,12 +119,12 @@ either, and are the one item on this list that is not simply a table entry: the high-order 32 bits are never transmitted, so recovering them is stateful, and they widen both the ICV coverage and the AEAD associated data. -Two structural notes for anyone starting. Adding an integrity algorithm that is +Some structural notes for anyone starting. Adding an integrity algorithm that is not an HMAC needs more than a row -- :class:`~pcapkit.protocols.internet.esp.IntegritySuite` records the digest as the name of a :mod:`hashlib` constructor, which AES-XCBC and AES-CMAC are not. And :func:`~pcapkit.protocols.internet.esp.load_cryptography` imports only the -``ciphers`` submodules of :mod:`cryptography`, so the AEAD and CMAC primitives +``ciphers`` submodules of ``cryptography``, so the AEAD and CMAC primitives have to be added there before a cipher suite can reach them. Unsupported algorithms are refused loudly, when the Security Association is constructed rather than when a packet is read, so nothing decodes wrongly in the meantime. @@ -136,8 +134,8 @@ Mobility Header **Done.** :class:`~pcapkit.protocols.internet.mh.MH` decodes and constructs the whole -registry: **all 24 registered message data types**, **all 4 CGA extensions**, and -**all 71 registered options**. Every one of them is registered in +registry: **every registered message data type**, **every CGA extension**, and +**every registered option**. Every one of them is registered in :attr:`~pcapkit.protocols.internet.mh.MH.__message__`, :attr:`~pcapkit.protocols.internet.mh.MH.__option__` or :attr:`~pcapkit.protocols.internet.mh.MH.__extension__` with both a ``_read_`` @@ -147,7 +145,7 @@ Every message type and every one of those options round-trips byte-for-byte through the public API -- ``make`` then ``read`` then ``make`` again reproduces the same octets, which :file:`tests/protocols/test_option_roundtrip_unit.py` checks for the whole -registry. The **four CGA extensions round-trip end to end as well**: the CGA +registry. The **CGA extensions round-trip end to end as well**: the CGA Parameters option (type 12) is the only thing that can carry a CGA extension on the wire, so registering it reached the whole :attr:`~pcapkit.protocols.internet.mh.MH.__extension__` registry with it, and no @@ -157,17 +155,17 @@ The sub-registries are the easy half: binding revocation types and triggers, handoff indicators, access network identifier sub-options, flow identification and flow binding sub-options, LMA-controlled MAG parameters, DNS update status, traffic selector formats and QoS attributes are all generated in full under -:doc:`/pcapkit/const/mh`. Two value sets live in ``mh.py`` itself rather than in -:mod:`pcapkit.const.mh`, because IANA registers neither: the localized routing -acknowledgment status codes of :rfc:`6705#section-10.2` +:doc:`/pcapkit/const/mh`. Value sets that IANA does not register live in ``mh.py`` itself rather than +in :mod:`pcapkit.const.mh`: the localized routing acknowledgment status codes of +:rfc:`6705#section-10.2` (:class:`~pcapkit.protocols.internet.mh.LocalizedRoutingStatus`) and the local mobility anchor address option codes of :rfc:`5949#section-6.2.2` -(:class:`~pcapkit.protocols.internet.mh.LMAAddressCode`), alongside two -:rfc:`5568` sets. +(:class:`~pcapkit.protocols.internet.mh.LMAAddressCode`). Sets from :rfc:`5568` +live there alongside them. What is left, and why: -* **Payloads that belong to another protocol** are carried opaquely for now. The +* **Payloads that belong to another protocol** are carried opaquely. The multicast options (54, 56, 57, 60 and 61) embed :rfc:`3810` MLD or :rfc:`3376` IGMP address records, and the traffic selectors of :rfc:`6089` and :rfc:`7222` embed the flag-driven range lists of :rfc:`6088`. Each is a separate registry @@ -188,14 +186,14 @@ What is left, and why: with the rest rather than inventing a convention; and once a dissector for one of these formats exists, registering it needs no change at the option site. - None of those four keys is checked against the values its RFC allows, on + None of those keys is checked against the values its RFC allows, on either side. :rfc:`7028` says ``protocol`` MUST be 143 or 131 for MLD, or ``0x12``, ``0x16`` or ``0x22`` for IGMP, yet :meth:`~pcapkit.protocols.internet.mh.MH._make_opt_mcast_sel` builds and :meth:`~pcapkit.protocols.internet.mh.MH._read_opt_mcast_sel` parses any octet - without a warning, and the other three keys behave the same. The registry is + without a warning, and the other keys behave the same. The registry is where an unknown key would first be noticed, so the check belongs there rather - than in four separate makers. + than in each maker separately. It wants a registry of its own rather than :meth:`~pcapkit.protocols.protocol.Protocol._decode_next_layer`, which is @@ -205,10 +203,10 @@ What is left, and why: behave wrongly. Changing the parsed shape from :obj:`bytes` to ``Raw`` also changes what existing captures dump to, so it is its own change. -Two wire-format traps are worth knowing before touching this code, since both -look like ordinary fields and are not: +Wire-format traps are worth knowing before touching this code, since each +looks like an ordinary field and is not: -* :rfc:`7411`'s two multicast options measure their length field in **32-bit +* :rfc:`7411`'s multicast options measure their length field in **32-bit words**, and exclude the option-code and status octets as well as the type and length ones -- so the option occupies ``4 + length * 4`` octets, not ``length + 2``. @@ -232,9 +230,9 @@ that does not exist: DTLS Key Management chunk parameter, ``0x8006`` of :class:`~pcapkit.const.sctp.parameter.Parameter` -- both fall through to the generic handlers, so they parse as opaque; -* four of its error causes, 100 to 103 of +* its error causes, 100 to 103 of :class:`~pcapkit.const.sctp.cause_code.CauseCode`; -* seven payload protocol identifiers -- 47 (Diameter over DTLS/SCTP), 66 to 69 +* payload protocol identifiers -- 47 (Diameter over DTLS/SCTP), 66 to 69 (NGAP, XnAP, F1AP and E1AP, each over DTLS over SCTP) and 4242 (DTLS chunk key management). @@ -262,32 +260,29 @@ small part of each is bound to a dissector; everything else resolves to :class:`~pcapkit.protocols.misc.raw.Raw`, so the capture parses without complaint and yields nothing useful. -* **5 of the 219** :class:`~pcapkit.const.reg.linktype.LinkType` values -- +* :class:`~pcapkit.const.reg.linktype.LinkType` -- ``NULL``, ``ETHERNET``, ``LOOP``, ``IPV4`` and ``IPV6``, declared identically in :class:`~pcapkit.protocols.misc.pcap.frame.Frame` and :class:`~pcapkit.protocols.misc.pcapng.PCAPNG`, which also read ``RAW`` as ``IPV4`` or ``IPV6`` by its version nibble. -* **17 of the 151** :class:`~pcapkit.const.reg.transtype.TransType` values, in +* :class:`~pcapkit.const.reg.transtype.TransType` -- the values in :attr:`Internet.__proto__ `. -* **7 of the 160** :class:`~pcapkit.const.reg.ethertype.EtherType` values, in +* :class:`~pcapkit.const.reg.ethertype.EtherType` -- the values in :attr:`Link.__proto__ `: ARP, RARP, IPv4, IPv6, IPX and both VLAN tags. -* **7 bindings over 5 port numbers**, against a - :class:`~pcapkit.const.reg.apptype.AppType` registry carrying every IANA - service name -- TCP 20 to FTP-DATA and 21 to FTP, port 80 and 8080 to HTTP on - both TCP and UDP, and UDP 1701 to L2TP. The two counts differ because 80 and - 8080 are each bound twice, once per transport. -* **2 of the 75** - :class:`~pcapkit.const.sctp.payload_protocol_identifier.PayloadProtocolIdentifier` - values, in :attr:`SCTP.__proto__ - `: PPIDs 60 and 66, both to +* :class:`~pcapkit.const.reg.apptype.apptype.AppType`, a registry carrying every + IANA service name -- TCP 20 to FTP-DATA and 21 to FTP, port 80 and 8080 to HTTP + on both TCP and UDP, and UDP 1701 to L2TP. +* :class:`~pcapkit.const.sctp.payload_protocol_identifier.PayloadProtocolIdentifier` + -- the values in :attr:`SCTP.__proto__ + `: PPIDs 60 and 66, each to :class:`~pcapkit.protocols.application.ngap.NGAP`. Every other DATA chunk payload is ``Raw`` until something calls :func:`~pcapkit.foundation.registry.protocols.register_sctp`. -Each pair above is measurable, which is why the command is written down rather -than trusted to stay current: +How much of each registry is bound is measurable, so measure it rather than +trust a figure written down here: .. code-block:: shell @@ -296,7 +291,7 @@ than trusted to stay current: print(len(Internet.__proto__), len(list(TransType)))" Most of those want a dissector written and are covered by the stub list above. -A handful wanted only a table entry, because the dissector was already there: +Some wanted only a table entry, because the dissector was already there: * **Done.** :class:`~pcapkit.protocols.application.ospf.OSPF` is bound at ``TransType`` 89 (``OSPFIGP``) and @@ -325,7 +320,7 @@ A handful wanted only a table entry, because the dissector was already there: (:issue:`1574`). ``RAW`` has no binding of its own: its bare IPv4 or IPv6 is dispatched by the version nibble of the first octet (:issue:`1502`). -Three follow-ups the above deliberately left alone: +Follow-ups to the bindings above: * ``TransType`` 115 (``L2TP``) stays unbound, and the reason is structural: it references :rfc:`3931`, i.e. L2TPv3 over IP, and @@ -353,7 +348,7 @@ Three follow-ups the above deliberately left alone: instead of guessing, since a guess path is wrong on non-HTTP input. Beyond those, the gaps most likely to be met in a real capture are ICMP (1), -ICMPv6 (58) and IGMP (2) on the internet layer, all three of which have stubs; +ICMPv6 (58) and IGMP (2) on the internet layer, each of which has a stub; and ``LINUX_SLL`` and ``LINUX_SLL2`` at the link layer, which every ``tcpdump -i any`` capture uses and which have no stub. @@ -369,8 +364,9 @@ Maybe Even Faster? ------------------ **Partly done.** A measured benchmark exists, and acting on it cut extraction -time on a 1117-frame HTTP capture by about 46% with byte-identical output. Four -things were wrong on the hot path, none of them the ones the thread predicted: +time on a 1117-frame HTTP capture by about 46% with byte-identical output. The +profiling found these wrong on the hot path, none of them the ones the thread +predicted: * character-set detection was uncached, and accounted for 30% of an HTTP extraction -- 3011 :func:`chardet.detect` calls over 163 distinct @@ -379,20 +375,20 @@ things were wrong on the hot path, none of them the ones the thread predicted: :func:`copy.copy` machinery, 63207 times per extraction; * :attr:`Field.length ` recomputes :func:`struct.calcsize` on each read and was read two to four times per field; -* :meth:`Schema.__setattr__ ` - re-entered itself once per field assigned. +* :class:`~pcapkit.protocols.schema.schema.Schema`'s ``__setattr__`` re-entered + itself once per field assigned. -Two predictions the profiling **contradicted**, recorded so nobody spends time on +Predictions the profiling **contradicted**, recorded so nobody spends time on them again: :class:`~pcapkit.corekit.infoclass.Info` construction -- the usual suspect -- is 0.0% of its own cost and 4.4% of a parse, and the logging integration is 0.03% even in the heaviest shape, since every call site is a lazy ``%s``. The IO-batching idea the thread proposed was never the bottleneck. -Of the three larger wins this section listed, all outside the parse path, one has -landed in full and the other two in part: +The larger wins this section listed, all outside the parse path, have landed in +full or in part: * **The flow dumper reopens its output file once per frame**, in - :meth:`PCAPIO.__call__ `. It does not + :class:`~pcapkit.dumpkit.pcap.PCAPIO`'s ``__call__``. It does not *also* rebuild a whole :class:`~pcapkit.protocols.misc.pcap.frame.Frame` to obtain bytes it already holds; dropping that re-dissection was about 82% of the cost of a flow-traced @@ -415,7 +411,7 @@ landed in full and the other two in part: schema's type field to choose the option schema, rather than unpacking the whole base schema and throwing it away. -Two traps for anyone benchmarking this library. ``reassembly=True`` and +Some traps for anyone benchmarking this library. ``reassembly=True`` and ``trace=True`` are **no-ops** without ``ip=``/``tcp=``, so a benchmark that passes only the switch measures nothing; and timing several capture shapes in one process inflates them by up to 73%, so each shape wants its own interpreter. @@ -505,7 +501,7 @@ both of the following are worth knowing before reaching for them: * **pypcapfile** -- the released 0.12.0 imports the ``imp`` module, removed in Python 3.12. Precisely, ``pcapfile/linklayer.py`` imports it at module scope and ``pcapfile.savefile`` imports ``linklayer``, so while a bare - ``import pcapfile`` still succeeds on 3.12+, the two modules the engine + ``import pcapfile`` still succeeds on 3.12+, the modules the engine actually needs raise :exc:`ModuleNotFoundError`. Upstream ``master`` (0.12.1, unreleased) has fixed this, and that is what the engine was verified against. The extra therefore installs a version that only works on Python 3.10 and @@ -558,7 +554,7 @@ both of the following are worth knowing before reaching for them: says so, rather than one ``engine=`` string silently meaning two different implementations. - Two caveats remain. Both distributions are still beta releases, and ``pcap-ct`` + Caveats remain. Both distributions are still beta releases, and ``pcap-ct`` targets the ``pypcap`` 1.2.3 API -- within what this engine uses, but worth knowing. And despite the `libpcap `__ wheel bundling prebuilt binaries, a runtime ``libpcap`` shared library is still @@ -578,8 +574,8 @@ them. Adding a further engine does not mean adding handler methods to :class:`~pcapkit.foundation.extraction.Extractor`, as the thread describes. A new engine subclasses -:class:`pcapkit.foundation.engines.engine.Engine` and implements just two -methods, :meth:`~pcapkit.foundation.engines.engine.EngineBase.run` +:class:`pcapkit.foundation.engines.engine.Engine` and implements just +:meth:`~pcapkit.foundation.engines.engine.EngineBase.run` and :meth:`~pcapkit.foundation.engines.engine.EngineBase.read_frame`; passing the ``engine`` class keyword registers it. See :doc:`/ext` for a worked example. What does still apply is the unified auxiliary tools in :mod:`pcapkit.toolkit`, where @@ -643,7 +639,7 @@ Reassembly Beyond IP and TCP ---------------------------- **Still open.** -:doc:`/pcapkit/foundation/reassembly/index` covers three protocols and no more. +:doc:`/pcapkit/foundation/reassembly/index` covers IPv4, IPv6 and TCP, and no more. IPv4 and IPv6 share the :rfc:`791` procedure, and TCP uses the :rfc:`815` hole-descriptor algorithm, which does handle out-of-order and overlapping segments. SCTP has nothing: a user message split across DATA chunks is never put @@ -657,7 +653,7 @@ reassembler needs, on ``flags.E`` for the beginning and ending fragment bits -- and nothing reads those two bits outside the construction path. So each fragment is dispatched on its own PPID as though it were a whole PDU, and a registered upper layer is -handed half a message. Two related holes compound it: only the *first* DATA +handed half a message. Related holes compound it: only the *first* DATA chunk of a bundle is dispatched at all, and I-DATA [:rfc:`8260`], chunk type 64, is not dissected -- so the MID and FSN fields that exist precisely to make interleaved reassembly tractable are never parsed, and an I-DATA-only packet @@ -666,25 +662,24 @@ likewise unhandled, so partial-reliability stream advancement is invisible. Writing the reassembler is the smaller half of the job. It would go at ``pcapkit/foundation/reassembly/sctp.py``, subclass -:class:`~pcapkit.foundation.reassembly.reassembly.Reassembly` and implement two -methods, +:class:`~pcapkit.foundation.reassembly.reassembly.Reassembly` and implement :meth:`~pcapkit.foundation.reassembly.reassembly.Reassembly.reassembly` and :meth:`~pcapkit.foundation.reassembly.reassembly.Reassembly.submit`. Registering it, though, is inert: :meth:`~pcapkit.foundation.extraction.Extractor.register_reassembly` accepts any protocol name, while :class:`~pcapkit.foundation.extraction.Extractor` only ever instantiates the -three it names literally, and +ones it names literally, and :class:`~pcapkit.foundation.reassembly.ReassemblyManager` is a fixed-field -container with no room for a fourth. Making SCTP reachable therefore also means +container with no room for another. Making SCTP reachable therefore also means touching that manager and its read-side twin, the construction branch in :class:`~pcapkit.foundation.extraction.Extractor`, one adapter per engine in :mod:`pcapkit.toolkit`, the call site in each of those engines, and :func:`~pcapkit.interface.core.reassemble`. Generalising that wiring so a registered reassembler is actually used is worth doing on its own account, and -would make the fourth protocol cheaper than the third rather than dearer. +would make each further protocol cheaper than the last rather than dearer. -Two smaller items in the same subsystem: +Smaller items in the same subsystem: * **Flow tracing is TCP only**, and blocked on the same generalisation -- :class:`~pcapkit.foundation.traceflow.TraceFlowManager` holds a single field, @@ -715,7 +710,7 @@ Two smaller items in the same subsystem: flow is finalised only by proof that nothing more can arrive: a new connection's SYN on the same endpoints, or the end of the capture, via :meth:`TraceFlow.finish - `. Distinguishing + `. Distinguishing that SYN from the peer's SYN-ACK is what the recorded teardown is for. One case remains undecided rather than solved: a capture that *starts* in the @@ -724,14 +719,14 @@ Two smaller items in the same subsystem: :class:`~pcapkit.foundation.traceflow.data.tcp.Packet`, which would make ``syn and not ack`` a definitive new-connection test on its own. - **The application layer is wired into flow tracing** as well. Of the two ways + **The application layer is wired into flow tracing** as well. Of the ways of buffering a traced payload, the tracer **delegates to** :class:`~pcapkit.foundation.reassembly.tcp.TCP` rather than growing a per-direction payload buffer of its own. A buffer that concatenated payloads in capture order would be silently wrong on the first retransmission or reordered segment, where the :rfc:`815` hole-descriptor algorithm already in the reassembler is not -- so - :class:`~pcapkit.foundation.traceflow.data.tcp.Packet` carries the four segment + :class:`~pcapkit.foundation.traceflow.data.tcp.Packet` carries the segment fields (``seq``, ``ack``, ``header``, ``payload``) that reassembler needs, and the tracer hands each traced segment straight to it. @@ -753,9 +748,9 @@ Two smaller items in the same subsystem: adapter at all; asking for it there warns and falls back. * **Timing a partial datagram out** is implemented, for IP. :meth:`Reassembly.expire - ` abandons a + ` abandons a buffer whose first-arriving fragment is older than - :attr:`~pcapkit.foundation.reassembly.reassembly.ReassemblyBase.timeout` + :attr:`~pcapkit.foundation.reassembly.reassembly.Reassembly.timeout` seconds, and the buffer models carry the timestamp that makes it possible. The clock is the **capture's own timestamps**, not the host's: an offline @@ -784,7 +779,7 @@ Two smaller items in the same subsystem: reassembly a deadline and an idle connection is ordinary rather than pathological; ``timeout`` enables one on request. - Two related things are *not* done. :rfc:`8200#section-4.5` and + Related things are *not* done. :rfc:`8200#section-4.5` and :rfc:`1122#section-3.3.2` both want an ICMP Time Exceeded sent on expiry when the offset-zero fragment has been received; a parser sends nothing, so the condition is only reported in the log record. And the memory motive is @@ -801,7 +796,7 @@ is worth knowing before benchmarking against them: ``pyshark``, ``pypcap`` and Checksum and Integrity Verification ----------------------------------- -Ten protocols parse a checksum or CRC field — +These protocols parse a checksum or CRC field — :class:`~pcapkit.protocols.internet.hip.HIP`, :class:`~pcapkit.protocols.internet.hopopt.HOPOPT`, :class:`~pcapkit.protocols.internet.ipv4.IPv4`, @@ -811,13 +806,13 @@ Ten protocols parse a checksum or CRC field — :class:`~pcapkit.protocols.application.ospf.OSPF`, :class:`~pcapkit.protocols.transport.sctp.SCTP`, :class:`~pcapkit.protocols.transport.tcp.TCP` and -:class:`~pcapkit.protocols.transport.udp.UDP` — and exactly one of them checks +:class:`~pcapkit.protocols.transport.udp.UDP` — and only one of them checks whether the value is *right*: :attr:`SCTP.checksum_valid `. -For the other nine the field is recorded and never questioned, so a corrupted +For the others the field is recorded and never questioned, so a corrupted capture parses as cleanly as an intact one. -**These are two different problems and they want separating**, because only one +**These are different problems and they want separating**, because only one of them is cryptographic and the distinction decides what is actually hard. **The one's-complement Internet checksum**, which covers IPv4's header, TCP, UDP, @@ -835,7 +830,7 @@ the SCTP packet alone. Note also that SCTP's CRC32c is a hand-rolled lookup tabl in :mod:`pcapkit.protocols.transport.sctp`, not a library call, so it is not precedent for a dependency either. -**The cryptographic integrity checks** are where the :mod:`cryptography` +**The cryptographic integrity checks** are where the ``cryptography`` dependency — introduced for ESP — genuinely buys something new, and half of that is already done. :class:`~pcapkit.protocols.internet.esp.ESP` **already verifies its ICV** when a Security Association is supplied, and deliberately reports a @@ -847,7 +842,7 @@ packet. Verifying them needs a keying context in the shape ESP already established through :mod:`pcapkit.corekit.context`, which is the reusable part of the design rather than something to invent. -All three of the policy questions this raised have been settled by the repo +The policy questions this raised have all been settled by the repo owner, and each lands on a precedent the library already has: * **Verification is opt-in.** Checksumming every packet costs real time on a @@ -913,8 +908,8 @@ from the same test. Worth spelling out how that workflow is *reached*, because reading its ``on:`` block alone suggests it is not reachable from an ordinary commit at all — it -lists only ``push`` on ``v*`` tags and a ``workflow_run``. The chain is three -hops, not two: a push to ``main`` triggers **Unit Tests** +lists only ``push`` on ``v*`` tags and a ``workflow_run``. The chain has an +extra hop: a push to ``main`` triggers **Unit Tests** (``unit-tests.yml``), whose completion triggers **Vendor Update** (``cron-vendor.yml``) via ``workflow_run`` -- ``cron-vendor.yml`` carries no ``push`` trigger of its own, deliberately, per its own comment at @@ -945,10 +940,10 @@ tag that output answers for is the one ``github`` is about to create. ``tag``, ``pypi`` and ``conda`` each gate on their own target instead -- whether the matching Conda tag exists, whether PyPI's file count for the version has reached the expected total, whether Anaconda's upload is complete. Each of -those three additionally requires ``version_check`` and the release test gate +those additionally requires ``version_check`` and the release test gate (``unit-tests``) to have succeeded, and ``github`` -- and, for ``conda``, ``tag`` -- to have succeeded *or skipped*. ``github`` itself needs the gate -directly, and the gate runs only when one of the four would. +directly, and the gate runs only when one of the publishing jobs would. Those conditions open with ``!cancelled()``, which suppresses the ``success()`` Actions would otherwise prepend; accepting ``skipped`` is then what stops an upstream job's legitimate skip cascading into skipping a job whose own evidence @@ -958,8 +953,8 @@ cleanly, instead of letting it run against an artefact that was never produced. That is why an ordinary commit does not publish: the version string has not -moved, so each job's own check finds its target already there and all four -skip, as does the full-suite release test gate ahead of them. Changing the version string is what flips every one of those checks, and +moved, so each job's own check finds its target already there and every one of +them skips, as does the full-suite release test gate ahead of them. Changing the version string is what flips every one of those checks, and the next push then tags and publishes. So: .. list-table:: @@ -1031,7 +1026,7 @@ instead of defaulting to ``LinkType.NULL``. **Wave 3 — the remaining protocols** from the same list, taken three or four at a time. QUIC, DSL, FDDI and ISDN come last: QUIC because it is really HTTP/3 over a -new transport and wants that settled first, the other three because a capture in +new transport and wants that settled first, the rest because a capture in the wild is rare enough that the work buys little until the commoner protocols are in. @@ -1049,7 +1044,7 @@ with nothing but ``Raw`` to register. The counter-argument is real though, and worth weighing rather than dismissing: changing the parsed shape from :obj:`bytes` to ``Raw`` is the disruptive half, since it changes what existing captures dump to, so there is a case for taking that churn early and registering -dissectors later. The third format, the :rfc:`6088` traffic selectors of +dissectors later. The remaining format, the :rfc:`6088` traffic selectors of :rfc:`6089` and :rfc:`7222`, has no wave at all — it is a registry of its own and nobody has claimed it. @@ -1060,10 +1055,10 @@ working per-entry read/make pair behind it. The goal of this item is to close that gap to zero for every code whose format is actually specified. Both halves of the gap are measurable — the registered codes against the -eighteen families :file:`examples/generators/options.py` walks, and the shortfall +families :file:`examples/generators/options.py` walks, and the shortfall against ``EXPECTED_FAILURES`` in :file:`tests/protocols/test_option_roundtrip_unit.py` — so the commands are -written down instead of a pair of figures that will be stale by the next merge: +written down instead of figures that will be stale by the next merge: .. code-block:: shell @@ -1079,13 +1074,13 @@ written down instead of a pair of figures that will be stale by the next merge: spec.loader.exec_module(m); \ print(collections.Counter(k.split('/')[0] for k in m.EXPECTED_FAILURES))" -Two things to be precise about before anyone starts. First, the round-trip half +Things to be precise about before anyone starts. First, the round-trip half is closed: ``EXPECTED_FAILURES`` is empty, so every case the harness enumerates round-trips, and a new entry there would record a defect rather than a deferral. Second, **round-tripping is not the same as being implemented** — the harness walks the codes each protocol registers a handler for, not every code its enumeration names, so what remains is the codes with no per-entry handler, -such as the 17 SCTP chunk types under `SCTP`_ above, which fall through to the +such as the SCTP chunk types under `SCTP`_ above, which fall through to the generic handlers and parse as opaque. This is deliberately **not** the same request as the ``__proto__`` dispatch @@ -1103,7 +1098,7 @@ protocol's dispatch reaches it. **Between wave 1 and wave 2 — a library-wide consistency sweep.** Clearing defects one at a time, each found because something else was being worked on nearby, is a poor way to find the rest of them. So before wave 2 starts the -library gets swept deliberately, in five strands: +library gets swept deliberately, in strands: * **Prose against code.** Docstrings, the README, and inline comments checked against what the code does. A docstring outliving the thing it described is a @@ -1122,30 +1117,30 @@ library gets swept deliberately, in five strands: * **Packet formats against the specifications.** The most valuable strand, and the one with the clearest evidence behind it: reading the RFC field-by-field against the schema is what produced - `#472 `__ — two HIP + `#472 `__ — HIP parameters sizing their list entries at one octet where :rfc:`5770` §5.4 and :rfc:`7402` §5.1.2 specify sixteen bits — and the same method then *cleared* - fourteen further + the further ``EnumField(length=1)`` sites in the same file against :rfc:`7401`, :rfc:`8002`, :rfc:`8003`, :rfc:`5770` and :rfc:`6078`. It both finds real defects and retires suspicion, which is why it is worth doing exhaustively rather than opportunistically. * **Duplicated logic that wants a shared module.** The same method or the same arithmetic written out in several protocols, where one definition would do. - This strand has the sharpest evidence of the five, because duplication here has + This strand has the sharpest evidence of them all, because duplication here has already *caused* defects rather than merely offended tidiness: - `#487 `__ existed because - ``IPv6_Route.make()`` computed ``Hdr Ext Len`` in **two** branches with two - different wrong units, and the read side computed the inverse in a third - place with a fourth. The fix was not new arithmetic but one helper per + ``IPv6_Route.make()`` computed ``Hdr Ext Len`` in **separate** branches with + different wrong units, and the read side computed the inverse elsewhere with + yet another. The fix was not new arithmetic but one helper per direction, each documenting the :rfc:`8200#section-4.4` unit and saying what its counterpart is. - `#483 `__ was the scapy toolkit adapter passing a fragment offset unscaled where every sibling - adapter scaled it. Five adapters each restate the same field mapping, so a - convention that holds in four and breaks in the fifth is invisible until a - capture is wrong. + adapter scaled it. Every adapter restates the same field mapping, so a + convention that holds in all but one is invisible until a capture is + wrong. So the strand is not a style pass. Look for arithmetic that encodes a wire unit, field-mapping tables restated per engine, and guards of the same shape @@ -1166,7 +1161,7 @@ both wrong when re-derived. **Queued for wave 2 or 3 — checksum and integrity verification.** See `Checksum and Integrity Verification`_ above. It splits in two, and the halves are not equally blocked: the cryptographic half (HIP's ``HIP_MAC``/``RVS_HMAC``/ -``RELAY_HMAC`` and its two signature parameters) could start whenever, since +``RELAY_HMAC`` and its signature parameters) could start whenever, since :class:`~pcapkit.protocols.internet.esp.ESP` has already established both the keying-context channel and the report-rather-than-raise policy. The one's-complement half (IPv4, TCP, UDP, ICMP/ICMPv6, OSPF) needs the IP diff --git a/docs/source/contributing/releasing.rst b/docs/source/contributing/releasing.rst index eb85a0c38..da9db408d 100644 --- a/docs/source/contributing/releasing.rst +++ b/docs/source/contributing/releasing.rst @@ -60,21 +60,21 @@ citation left behind fails the gate well before any approval is requested. Either run the script, or move both fields by hand in the same commit. This is the **only** edit a person makes to get a release started -- the tag, -the Release, and every upload are the workflow's job from here. Two other -checks run unconditionally on the same path, though neither is a step in *this* +the Release, and every upload are the workflow's job from here. Other +checks run unconditionally on the same path, though none is a step in *this* process: the ``changelog`` job (``unit-tests.yml:1624-1643``) fails outright if ``CHANGELOG.md`` has drifted from its source entry under :file:`docs/source/changelog/`, and the release-body step warns, without failing, if that entry's heading still reads "unreleased" -(``create-release.yml:677-678``). Both enforce what the changelog convention +(``create-release.yml:677-678``). Each enforces what the changelog convention already asks for. -Two Ways to Start a Release ---------------------------- +Ways to Start a Release +----------------------- -``create-release.yml`` has two triggers -(``.github/workflows/create-release.yml:2-9``), and both are first-class -- -neither is a workaround for the other: +``create-release.yml``'s triggers +(``.github/workflows/create-release.yml:2-9``) are all first-class -- none is +a workaround for another: ``push: tags: ['v*']`` Pushing a tag matching ``v*`` yourself. This chooses **which commit** to @@ -108,10 +108,14 @@ neither is a workaround for the other: while ``github.event.workflow_run.head_sha`` -- the commit Vendor Update ran *on* -- was the bump's parent, ``945c3c3d8``. -Both paths converge on the same job graph from ``version_check`` onward; only +The paths converge on the same job graph from ``version_check`` onward; only the trigger and ``github.ref_name`` differ, which is what the guards below -read. A ``v*`` ref always runs the release test gate; on ``main`` it runs only -when some target's evidence is not yet ``'true'`` +read. The release test gate runs only once ``version_check`` has passed, and +``version_check`` itself skips the release App's own tag push and a Vendor +Update that did not conclude ``success`` or skipped ``vendor-update`` (below). +Past it, a ``v*`` ref always runs the gate; on ``main`` it +runs only when the version is not tagged at another commit and some target's +evidence is not yet ``'true'`` (`#1052 `__), so the common Vendor Update run that bumped nothing costs a minute, not a full-suite run. @@ -155,7 +159,7 @@ the default branch, and ``tag`` pulled ``main`` before tagging. tag. A run created for a *later* commit of the same version therefore never publishes a second tree under that version. ``version_check`` resolves the ``v*`` tag the same way first (``:536-554``), and off a ``v*`` ref a tag - naming another commit skips the gate and all four publishers + naming another commit skips the gate and every publisher (`#1748 `__): ``Release status`` posts a ``::notice`` if that release is complete, or a ``::warning`` naming both commits and what is missing (``:1624-1634``), and @@ -176,7 +180,7 @@ the default branch, and ``tag`` pulled ``main`` before tagging. again (``--clobber``). ``conda``'s upload is not gated on ``check_leg``, so a leg whose Anaconda upload landed but whose GitHub upload failed still gets its asset on a re-run. -* Two failures differ from the action, deliberately. With no release for the tag, the +* Where it fails, it differs from the action deliberately. With no release for the tag, the upload fails with ``gh``'s "release not found", where the action made one, with no notes and ``target_commitish`` unset (``src/github.ts:617-628,1026``), which Create a release defaults to "the repository's default branch"; the @@ -210,7 +214,7 @@ The two readings disagree on run 38008819343. Against the default branch, its ``github`` would have failed at Create Release. As the commit's own change, ``b6b95bc8e`` touches no workflow file -- its diff against its parent is :file:`CITATION.cff`, :file:`conda/build`, :file:`pcapkit/__init__.py` and -two ``apptype`` modules -- so it would not. Which reading GitHub enforces is +``apptype`` modules -- so it would not. Which reading GitHub enforces is not measured here. `softprops/action-gh-release#411 `__ was closed with "The latest repro still hits ``403 Resource not accessible by integration`` @@ -226,7 +230,7 @@ re-checks the target (unmeasured). ``cron-conda.yml``'s ``conda-dist`` does: it attaches packages with ``ncipollo/release-action@v1.21.0`` and ``allowUpdates: true``, which updates the release before uploading (``src/Action.ts:53-66,107-120``). It therefore -names the default branch -- ``github.ref_name`` on both its triggers -- as +names the default branch -- ``github.ref_name`` whichever trigger raised it -- as ``commit``, which that update sends as ``target_commitish`` (``src/Releases.ts:150-174``), so the rule checks ``main`` itself; the tag stays put, as Update a release calls the target "Unused if the Git tag already @@ -245,7 +249,7 @@ against retargeting a release Create Release owns. Either way the build goes to anaconda.org only, with a ``::notice``. The update also sends the release's name, notes and ``prerelease`` and ``draft`` flags, which unset inputs make the tag, ``""`` and ``false`` -(``src/Inputs.ts:70-92,128-135,210-240``), so the step sets the four +(``src/Inputs.ts:70-92,128-135,210-240``), so the step sets the ``omit*DuringUpdate`` inputs that leave them out (`#1657 `__). @@ -334,7 +338,7 @@ checks *is* the one it is about to create: * ``pypi`` compares PyPI's file count for the version against the expected one -- a wheel per supported leg plus the sdist kept only on the 3.15 leg; the KTLO legs' wheels (`#1633 `__) are not counted. This job is - already idempotent (``skip-existing: true`` on both upload steps), so the + already idempotent (``skip-existing: true`` on every upload step), so the check exists only to save the cost of the build matrix, not for correctness. * ``conda`` compares Anaconda's aggregate count against the expected one (the ``linux-64`` and ``osx-64`` files for each Python version of its matrix; the @@ -347,7 +351,7 @@ checks *is* the one it is about to create: ``force:`` (never set), so re-uploading a file it already has fails outright rather than being absorbed. -All three evidence checks run inside ``version_check``, curling PyPI's and +These evidence checks all run inside ``version_check``, curling PyPI's and Anaconda's public, unauthenticated JSON APIs; the per-leg check runs inside ``conda`` itself, once per matrix leg, against the same Anaconda endpoint. @@ -387,7 +391,7 @@ this run, it *skipped*. ``cron-vendor.yml``, ``deploy-pages.yml`` and ``cron-conda.yml`` each open with ``!cancelled() && needs.unit-tests.result != 'failure'`` so that a skipped gate does not cascade-skip them, while an outright failure or a -genuine run cancellation still does. The three jobs here are stricter about +genuine run cancellation still does. The publishing jobs here are stricter about their *publishing* predecessors -- ``github``, and for ``conda`` also ``tag``: ``!cancelled() && (needs..result == 'success' || needs..result == 'skipped')``, not ``!= 'failure'``. The two spellings differ only when a @@ -398,7 +402,7 @@ check that ``conda-+0`` names this run's commit, a tag ``tag`` never pushed, trading a clean skip for a failed step; the equality pair skips ``conda`` outright instead. -``version_check`` is the exception, and deliberately so: all three require +``version_check`` is the exception, and deliberately so: each of them requires ``needs.version_check.result == 'success'`` with no ``|| 'skipped'``. It produces the evidence the gates read, so a skipped or cancelled ``version_check`` leaves them nothing to decide on. ``unit-tests`` is held to @@ -406,7 +410,7 @@ the same bar for a different reason: it is the release test gate, and it skips when nothing is publishable (`#1052 `__), so accepting ``skipped`` there would publish untested. ``github`` names it in ``needs:`` instead and relies on the implicit -``success()``. The gate's own ``if:`` is the union of the four publishers' +``success()``. The gate's own ``if:`` is the union of the publishers' conditions, with ``!= 'true'`` so unknown evidence runs it; should the two ever drift apart, the failure is a release that did not happen, which ``release_status`` reports loudly, never one that went out untested. @@ -440,7 +444,7 @@ The Release Pipeline A transitive reduction: ``version_check`` and ``unit-tests`` are also direct ``needs:`` of ``tag``, ``pypi`` and ``conda`` in the file, but ``github`` already implies them (``github`` itself depends on both), so drawing them would -add six lines without changing which job can start before which. ``release_status``'s +add edges without changing which job can start before which. ``release_status``'s edges are drawn in full, since it depends directly on *every* job precisely so that it can run regardless of which of them skipped. @@ -484,7 +488,7 @@ Precautions **Do not hand-make a tag to route around a stuck release.** This is narrower than "never tag by hand": pushing a *fresh* ``v`` tag to start a -release, as in `Two Ways to Start a Release`_, is fine. What is not sanctioned +release, as in `Ways to Start a Release`_, is fine. What is not sanctioned is deleting and re-pushing a tag that a stuck run already created, to force a retry. A ``git push`` of a tag ref that already points at the same commit produces no new event (documented GitHub Actions behaviour, not measured @@ -544,7 +548,7 @@ already has (`#1640 `__). listings, because** ``release_status`` **does it.** That job runs on every ``Create Release`` attempt regardless of what else skipped, and reconciles each of ``github``/``tag``/``pypi``/``conda`` *against its own evidence* -rather than demanding the same outcome from all four -- a target counts as +rather than demanding the same outcome from all of them -- a target counts as reconciled if it succeeded, or if it skipped *because* its own evidence already said it was done. That is what makes a self-heal reportable: a retry after a partial release legitimately produces a **mixed** result @@ -552,18 +556,24 @@ after a partial release legitimately produces a **mixed** result ``pypi``/``conda`` finish the rest), and a check demanding uniformity would read that mix as the failure it exists to detect. -With that reconciliation, ``release_status`` reports exactly one of: nothing -to release because the trigger had nothing to do (quiet ``::notice``), -nothing to release because every target was already reconciled by skipping, -i.e. the version was already fully out before this run started (quiet -``::notice``), the release is now fully out because every target reconciled --- whether by a fresh full run, or by the mixed self-heal above (quiet -``::notice`` either way), a release job failed or was cancelled outright -(``::warning``, already visible as a red job elsewhere), or -- the shape -`#888 `__ was actually -about -- at least one target skipped despite its own evidence saying it is -still incomplete, and nothing else failed to explain that (``::error``, and -the job itself fails). That last case should be unreachable given the +With that reconciliation, ``release_status`` reports exactly one of: + +* nothing to release, because the trigger had nothing to do (quiet + ``::notice``); +* nothing to release, because every target was already reconciled by skipping, + i.e. the version was already fully out before this run started (quiet + ``::notice``); +* the release is fully out, because every target reconciled, whether by a fresh + full run or by the mixed self-heal above (quiet ``::notice`` either way); +* a release job failed or was cancelled outright (``::warning``, already + visible as a red job elsewhere); +* at least one target skipped despite its own evidence saying it is still + incomplete, and nothing else failed to explain that (``::error``, and the job + itself fails) -- the shape + `#888 `__ was actually + about. + +That last case should be unreachable given the evidence-based guards above, since a target's own incomplete evidence is what makes its job run rather than skip; reaching it anyway is a signal that those checks themselves need attention, not that the release needs a diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index c127802f3..c5afd0249 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -118,7 +118,7 @@ At a Glance .. note:: Only Coverage Comment fires *exclusively* on ``workflow_run``: it has - nothing to do without a completed Unit Tests run. Each of the other four + nothing to do without a completed Unit Tests run. Each of the others that carry one pairs it with a direct trigger of its own -- a Saturday ``schedule`` for Vendor Update, Conda Update and GitHub Pages, a ``v*`` tag push for Create Release -- for the path where no upstream run exists yet to @@ -236,15 +236,16 @@ verdict Unit Tests already reached instead of re-running the gate a second time, and acts on it only once its ``verdict`` job, described under the edge list below, says it is a verdict on the commit it builds. Create Release has no ``schedule``, and its gate job's ``if:`` reads ``version_check``'s evidence -rather than the event, so its gate runs on both of its triggers: on a ``v*`` -tag push, and on a ``workflow_run`` from Vendor Update when some target is not -yet published (:doc:`releasing` gives the conditions). +rather than the event, so its gate can run whichever way the run was raised, once +``version_check`` itself has passed: on a ``v*`` tag push, and on a +``workflow_run`` from Vendor Update when some target is not yet published +(:doc:`releasing` gives the conditions). ``workflow_run`` Edges ---------------------- -Five in total, found by grepping every ``on:`` block rather than trusting a -hand-maintained list: +Found by grepping every ``on:`` block rather than trusting a hand-maintained +list: * ``.github/workflows/cron-vendor.yml:14-16`` -- **Vendor Update** fires on completion of **Unit Tests**. @@ -259,8 +260,8 @@ hand-maintained list: ``workflow_run`` fires for *every* completion of the named workflow -- success, failure, or skipped -- and regardless of what triggered it, -including a pull request's own run of Unit Tests. Every one of the five -downstream workflows therefore re-checks ``github.event.workflow_run.event`` +including a pull request's own run of Unit Tests. Every +downstream workflow therefore re-checks ``github.event.workflow_run.event`` and ``.head_branch`` (or, for Create Release and Coverage Comment, ``.conclusion``) itself before doing anything with an outward effect; none of the filtering happens in the ``on:`` block. @@ -300,8 +301,8 @@ Reusable-Workflow Calls (``uses:``) A ``uses:`` call is a different relationship from triggering: it runs inside the caller's own workflow run, as one of the caller's own jobs, rather than -starting an independent run after the caller finishes. All four calls in this -repository target the same reusable workflow, with the same input: +starting an independent run after the caller finishes. Every call in this +repository targets the same reusable workflow, with the same input: * ``.github/workflows/create-release.yml:595`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. @@ -332,7 +333,7 @@ comment (``unit-tests.yml:1612-1618``): ``create-release.yml`` feeds where a drifted file must not go unchecked. Confirmed on run `36210743295 `__ (a schedule-triggered GitHub Pages run): both ``Changelog drift`` and ``Gate -(full suite, Python 3.14)`` succeeded, with the four matrix jobs that +(full suite, Python 3.14)`` succeeded, with the matrix jobs that existed then -- ``Python ${{ matrix.python-version }}``, ``Integration Python …``, ``Engines Python …`` and ``PyPCAP/PyPCAPFile parity Python …`` -- all reporting ``skipped``. @@ -340,7 +341,7 @@ Python …``, ``Engines Python …`` and ``PyPCAP/PyPCAPFile parity Python The Skip Cascade (`#888 `__) ------------------------------------------------------------------------------- -The two relationships above compose into a failure mode worth seeing on its +The relationships above compose into a failure mode worth seeing on its own graph. Create Release's ``version_check`` -- ahead of the ``uses:`` call above -- carries: @@ -363,8 +364,8 @@ anything other than ``success`` -- including `#1748 `__ it is also false for a Vendor Update that concluded ``success`` with ``vendor-update`` skipped -- its ``verdict`` job found the Unit Tests verdict was for another commit -- which -the ``vendor_update`` job reads from that run's jobs first, as run 38650191283 -was. ``!cancelled()`` keeps a skipped ``vendor_update``, on a push, from +the ``vendor_update`` job reads from that run's jobs first (observed on run +38650191283). ``!cancelled()`` keeps a skipped ``vendor_update``, on a push, from skipping it. ``unit-tests`` then depends on that job with ``needs: [ version_check ]`` (``create-release.yml:583``) and an ``if:`` that calls no status-check function, so a skipped dependency skips it too -- and @@ -409,7 +410,7 @@ notice. Observed in production on runs `36511610205 `36510774776 `__, both ``workflow_run``-triggered **Create Release** runs on ``main`` that concluded -``skipped`` with every one of their six jobs -- ``Release test gate``, +``skipped`` with every one of their jobs -- ``Release test gate``, ``Check Version``, ``GitHub Release``, ``Conda Tag (release)``, ``PyPI distribution for Python …`` and ``Conda deployment (release) …`` -- reporting ``skipped`` in turn. @@ -469,7 +470,7 @@ report as the ``coverage-html`` artifact (30 days), with the bare numbers as the The PR comment comes from **Coverage Comment** (:file:`coverage-comment.yml`), not from ``unit-tests.yml``. That file is also a reusable workflow, a called -workflow can only narrow the token its caller passes, and all four callers grant +workflow can only narrow the token its caller passes, and every caller grants ``contents: read``, so a ``pull-requests: write`` job there would break them. Coverage Comment runs no tests. On ``workflow_run`` it uses the default branch's file and its own token, so fork pull requests get a comment too. It @@ -594,13 +595,13 @@ acting on before the cap. Work Kept Off the Pull-Request Queue (`#1538 `__) ----------------------------------------------------------------------------------------------------- -Two cuts in :file:`unit-tests.yml` keep work that cannot gate a merge from competing +Cuts in :file:`unit-tests.yml` keep work that cannot gate a merge from competing with pull-request jobs for the 20-runner pool. None touches a required check or adds a permission to the reusable workflow. * ``unittest-ordering`` runs on ``main`` pushes only, per `#1052 `__ item H, and has - ``needs: [test]``, so its eleven cells join the queue once ``test`` finishes rather + ``needs: [test]``, so its cells join the queue once ``test`` finishes rather than when the push run is created, ahead of that run's own ``Python`` legs and of pull-request jobs created later in a merge wave. Its ``if:`` opens with ``!cancelled()``, so the cells still run when ``test`` fails, and a red cell still @@ -611,7 +612,7 @@ permission to the reusable workflow. :file:`tests/toolkit/test_dpkt_unit.py`, which ``test`` runs with ``DPKT`` installed on every leg. Dropped with it: that file in a venv holding only ``test`` and ``DPKT``. -A third cut, running the then-prerelease 3.15 engine cell on ``main`` pushes only, +Another cut, running the then-prerelease 3.15 engine cell on ``main`` pushes only, ended with `#1633 `__: 3.15 is in the supported tier, so its cells block on every run. @@ -670,7 +671,7 @@ for the docs-only path -- and checks each result explicitly (``unit-tests.yml:1936-1990``). Rulesets match check names literally, with no wildcard, so requiring the matrix cells one by one would mean editing the ruleset whenever a matrix changes; the aggregate keeps the required list fixed. -It accepts the four legs as ``skipped`` only on a docs-only pull request, where +It accepts those legs as ``skipped`` only on a docs-only pull request, where ``project-tests`` -- every test that reads ``docs/`` or Markdown -- must pass instead. The ``Compat`` legs stay required separately because they come from ``python-compatibility.yml``, and a job cannot ``needs:`` a job in another @@ -709,14 +710,14 @@ with supported legs. Every job holding KTLO legs works the same way: report the same way. * Never job-level ``continue-on-error`` alone: it passes the *run*, but the leg's check run still concludes ``failure``, which showed every commit red while - ``Compat Python 3.7 (KTLO)`` carried it. ``unit-tests.yml``'s three KTLO jobs + ``Compat Python 3.7 (KTLO)`` carried it. ``unit-tests.yml``'s KTLO jobs carry it on top of the step-level guards, as a backstop for the failures no step contains, as ``experimental.yml``'s job does; it "prevents a workflow run from failing when a job fails" (`Workflow syntax `__), so the Unit Tests run its ``workflow_run`` consumers read stays green. -Two errors remain on a failed KTLO step, neither changing a conclusion: the +Errors remain on a failed KTLO step, though none changes a conclusion: the runner's ``Process completed with exit code N.`` for a ``run:`` step, and an action's own. And `#1634 `__'s tag check still blocks the ``pypi`` KTLO legs: it fails every leg alike, and masked, the leg would go on to publish. @@ -746,7 +747,7 @@ workflow of their own, by the owner's ruling that nothing waits on them. **That workflow reports separately and holds up nothing**: a job cannot ``needs:`` one in another workflow, nothing calls it, and no ``workflow_run`` names it, so the Unit Tests run completes without it, and with it ``Required checks passed``, -``unittest-ordering`` and the four workflows that wait on Unit Tests completing +``unittest-ordering`` and the workflows that wait on Unit Tests completing (``coverage-comment.yml``, ``deploy-pages.yml``, ``cron-vendor.yml`` and ``cron-conda.yml``). It runs on Unit Tests' ``push`` and ``pull_request`` events, skips a docs-only pull request by the same ``changes`` script, and cancels a @@ -771,14 +772,14 @@ job, and none on a step. Promoting one whose leg stays green is a hand edit, whi :file:`util/python_versions.py` refuses until the build has a wheel, a conda build and a ``Compat`` leg. -The tests' 120 minutes are sized for GraalPy, the slower of the two legs that share +The tests' 120 minutes are sized for GraalPy, the slowest of the legs that share them: its leg reached 26% of the unit tier in 25 minutes (job 115915976024), where 3.14t's ran the whole tier in 5.8 (run 38581834697). By the owner's ruling the round-trip sweeps run on ``latest`` alone: every leg but ``latest`` adds ``--ignore-glob='*roundtrip*'``, which leaves out each unit-tier module whose path holds ``roundtrip`` (``ROUNDTRIP_SWEEP_GLOB`` in :file:`tests/_tiers.py`), and ``latest``, the coverage leg, runs them as before. The subset covers -``integration`` too: off ``latest`` its legs leave out the three +``integration`` too: off ``latest`` its legs leave out the ``*roundtrip*_runtime.py`` modules (``ROUNDTRIP_RUNTIME_GLOB``), dropping them from ``fixture_tier_paths()``'s selection and adding the glob as an ``--ignore-glob``, since pytest applies no ignore to a path it was handed. On 2026-10-14 they were 41 modules @@ -804,7 +805,7 @@ skipped every step after its tests, ``Post Run actions/checkout@v7`` too, whose `post-if `__ "defaults to ``always()``". On a lost runner, then, the ``[monitor]`` lines in the -tests' log are the record. The monitor's three steps, start, stop and upload, are each +tests' log are the record. The monitor's steps -- start, stop and upload -- are each ``continue-on-error`` with no cap of their own, so the caps above and the check names are as they were. @@ -873,7 +874,7 @@ jobs' ``!= '3.15'``. ``tests/project/test_python_versions_1660.py`` fails on a pin the file does not list, on an exemption with no comment, and on a classifier, doc or comment that no longer matches the file -- a tier range, the coverage leg's version. Its tests, and the other tests that -read the file, skip on the KTLO interpreters, which have no ``tomllib``. Two +read the file, skip on the KTLO interpreters, which have no ``tomllib``. Some edits it names rather than makes: a new ``pypi`` KTLO leg needs its own ``ktlo_*`` output for ``release_status``, and a pre-release leg needs non-blocking mechanics no workflow has yet, so the script refuses one. @@ -884,7 +885,7 @@ Every Monday, or on dispatch, its ``detect`` job runs release cycle, PyPy's releases, setup-python's ``versions-manifest.json`` and the runner-images README, recomputes the tiers by the rules in that script's docstring, rewrites the file and the prose these tests pin to it, and runs the -three audit modules on the result. Only when that changes something does ``pr`` +audit modules on the result. Only when that changes something does ``pr`` enter ``github-release``, so the owner approves only those runs. It mints a release App token (:doc:`releasing`, "The Release App") with contents and pull requests write, force-pushes ``bot/python-versions`` and opens or updates its @@ -913,8 +914,8 @@ proposal. Deployment ``environment:`` ---------------------------- -Five jobs declare a deployment ``environment:`` and can therefore pause for an -approval, four in ``create-release.yml`` and one in ``cron-python-versions.yml``: +These jobs declare a deployment ``environment:`` and can therefore pause for an +approval, in ``create-release.yml`` and ``cron-python-versions.yml``: * ``github`` -- ``environment: github-release`` (``create-release.yml:618``) * ``tag`` -- ``environment: conda-tag`` (``create-release.yml:806``) @@ -926,7 +927,7 @@ approval, four in ``create-release.yml`` and one in ``cron-python-versions.yml`` No other workflow in scope here declares one -- including ``deploy-pages.yml``, whose ``deploy-pages`` job pushes straight to the ``gh-pages`` branch via ``JamesIves/github-pages-deploy-action`` rather than -through an ``environment:``-gated deployment. Of the four environments, only +through an ``environment:``-gated deployment. Of these environments, only ``github-release`` carries a required reviewer; :doc:`releasing` covers why one approval on that single environment is enough to gate the whole release graph, and is the page to read for the reasoning rather than diff --git a/docs/source/demo.rst b/docs/source/demo.rst index 234b8f7ee..c9519e471 100644 --- a/docs/source/demo.rst +++ b/docs/source/demo.rst @@ -4,7 +4,9 @@ How to ... Basic Samples ------------- -:mod:`pcapkit` is built around three verbs. A few common scenarios: +:mod:`pcapkit` is built around the :func:`~pcapkit.interface.core.extract`, +:func:`~pcapkit.interface.core.reassemble` and :func:`~pcapkit.interface.core.trace` +verbs. Some common scenarios: 1. extract a PCAP file and dump the result to a specific file (with no reassembly) @@ -28,10 +30,9 @@ Basic Samples extraction = extract(fin='in.pcap', nofile=True) frame0 = extraction.frame[0] # check if IP (IPv4 or IPv6) in this frame - flag = IP in frame0 if IP in frame0: - # fetch the IP packet from this frame - ip = frame0[IP] + # fetch the IP packet from this frame + ip = frame0[IP] 3. extract a PCAP file and reassemble TCP payload (with no output file nor frame storage) @@ -50,7 +51,7 @@ Basic Samples CLI Samples ----------- -:mod:`pcapkit`'s CLI (command line interface) can be reached two ways. +:mod:`pcapkit`'s CLI (command line interface) can be reached in these ways. * through console scripts diff --git a/docs/source/ext.rst b/docs/source/ext.rst index 5af2bd1ba..919fcaee8 100644 --- a/docs/source/ext.rst +++ b/docs/source/ext.rst @@ -172,7 +172,7 @@ The following code snippet shows how to create a new protocol class: .. note:: Following is a PoC implementation of a new internet layer protocol. For - demonstration purpose, we use the IPv4 protocol as an example, where the + demonstration purposes, we use the IPv4 protocol as an example, where the comprehensive implementation can be found in the :mod:`pcapkit.protocols.internet.ipv4` module. @@ -244,7 +244,7 @@ The following code snippet shows how to create a new protocol class: ``**kwargs`` to the construction and to the parse, so a keyword only ``read`` declares still travels through ``make``. - Two shapes a signature cannot express are declared on the class instead, via + Shapes a signature cannot express are declared on the class instead, via :attr:`ProtocolBase.__keywords__ `: @@ -262,15 +262,17 @@ The following code snippet shows how to create a new protocol class: Setting it to :obj:`None` skips the check entirely, and is meant only for a *dispatcher* whose real signature belongs to a class chosen at call time -- - :meth:`HTTP.make ` is the one - such class in the library. Unlike a set, :obj:`None` is not inherited, so a + :class:`~pcapkit.protocols.application.http.HTTP`, whose + :meth:`~pcapkit.protocols.application.http.HTTP.make` builds the HTTP version it + is given, is the + only such class in the library. Unlike a set, :obj:`None` is not inherited, so a subclass of a dispatcher is checked normally. Prefer a declared parameter to either: it is also what documents the keyword to your callers. Parsing is unaffected. :meth:`ProtocolBase.from_data ` checks the keywords its caller passes just as strictly, raising - :exc:`~pcapkit.utilities.exceptions.UnsupportedCall`, but only warns + :exc:`~pcapkit.utilities.exceptions.UnsupportedCall`, but only warns with :exc:`~pcapkit.utilities.warnings.UnknownFieldWarning` for a key that comes from your ``_make_data`` -- a mismatch there is a disagreement between two of your own mappings rather than a caller's typo. @@ -437,7 +439,7 @@ The available engines and the file formats they read: .. note:: `PyPCAP`_ and `pcap-ct`_ are two distributions of one :manpage:`libpcap(3)` - interface, and both install a top-level :mod:`pcap` module, so they are two + interface, and both install a top-level ``pcap`` module, so they are two engines rather than one: ``engine='pypcap'`` and ``engine='pcap_ct'``. Upstream `PyPCAP`_ cannot be installed on Python 3.12 or newer, and `pcap-ct`_ can -- which is why both exist. See @@ -895,7 +897,7 @@ produced. A reassembly callback runs on every submission and takes one argument, the list of datagrams that submission produced. A flow tracing callback runs as each flow is finalised and takes that flow's -:class:`~pcapkit.foundation.traceflow.data.tcp.Index`. Either returns +:class:`~pcapkit.foundation.traceflow.data.tcp.Index`. Each should return :obj:`None`; any return value is ignored. A reassembly callback can modify its list in place, which discards datagrams, diff --git a/docs/source/index.rst b/docs/source/index.rst index f0ca1a934..0d9414523 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -12,7 +12,7 @@ construction and analysis. The whole project declares **Python 3.6** or later. CI supports 3.11 to 3.15, and still tests the end-of-life 3.7 to 3.10 without letting a failure block anything; - 3.7 cannot import the package yet. + 3.7 cannot import the package. .. .. contents:: .. :depth: 2 @@ -65,7 +65,7 @@ About :mod:`PyPCAPKit ` project as its CLI support. Unlike popular PCAP extractors such as :mod:`Scapy `, -:mod:`dpkt ` and `PyShark`_, :mod:`pcapkit` aims to be more +`DPKT`_ and `PyShark`_, :mod:`pcapkit` aims to be more comprehensive: more detailed packet information, and a more *Pythonic* interface. @@ -73,7 +73,7 @@ interface. Module Structure ---------------- -:mod:`pcapkit` is made of nine subpackages. +:mod:`pcapkit` is made of these subpackages. - Interface (:mod:`pcapkit.interface`) @@ -83,12 +83,12 @@ Module Structure - Foundation (:mod:`pcapkit.foundation`) Synthesises file I/O and protocol analysis, coordinates - information exchange in all network layers, as well as + information exchange in all network layers, and provides the fundamental functions for :mod:`pcapkit`. - Protocols (:mod:`pcapkit.protocols`) - Collection of all protocol family, with detailed + Collection of all protocol families, with detailed implementation and methods. - Utilities (:mod:`pcapkit.utilities`) @@ -135,7 +135,7 @@ spelled ``'pcapkit'``) is :mod:`pcapkit`'s own parser and the only one with no third-party requirement. `PyPCAP`_ and `pcap-ct`_ are two independent distributions of the same -:manpage:`libpcap(3)` interface and both install a top-level :mod:`pcap` module, +:manpage:`libpcap(3)` interface and both install a top-level ``pcap`` module, so they are two engines rather than one: upstream `PyPCAP`_ stops at Python 3.11, `pcap-ct`_ covers 3.10 and newer. **Install exactly one of them** -- with both present, ``pcap-ct`` wins the import and the other becomes unselectable. @@ -193,7 +193,7 @@ Engine 3.10 3.11 3.12 3.13 3.14 3.15 [*]_ reasons under `Engine prerequisites`_. **Python 3.11 is the last version on which every engine can run** -- and even there ``pypcap`` and ``pcap_ct`` remain mutually exclusive for the reason given above, so no single environment ever has -all seven at once. +every engine at once. Test Environment ---------------- @@ -217,28 +217,27 @@ first extraction is discarded as a warm-up. Values are milliseconds per packet. =================== ======= ======= ======= ======= ======= Engine 3.10 3.11 3.12 3.13 3.14 =================== ======= ======= ======= ======= ======= -``pypcapfile`` [2]_ 0.0133 0.0093 -- -- -- +``pypcapfile`` [1]_ 0.0133 0.0093 -- -- -- ``dpkt`` 0.0163 0.0113 0.0118 0.0117 0.0125 -``pypcap`` [3]_ 0.0289 0.0212 -- -- -- -``pcap_ct`` [4]_ 0.0364 0.0320 0.0366 0.0322 0.0396 +``pypcap`` [2]_ 0.0289 0.0212 -- -- -- +``pcap_ct`` [3]_ 0.0364 0.0320 0.0366 0.0322 0.0396 ``scapy`` 0.1082 0.0822 0.0877 0.0832 0.0967 ``pcapkit`` 0.2342 0.1906 0.1985 0.1920 0.2392 -``pyshark`` [1]_ 18.8158 18.7852 19.2694 20.3751 -- +``pyshark`` [4]_ 18.8158 18.7852 19.2694 20.3751 -- =================== ======= ======= ======= ======= ======= The unavailable cells were attempted. They are not zeroes and must not be compared with a measured row. -.. [1] `PyShark`_ 0.6 cannot create the implicit event loop it expects on Python - 3.14; it ran on 3.10--3.13. +.. [1] `PyPCAPFile`_ 0.12.0 imports :mod:`imp`, which Python removed in 3.12. -.. [2] `PyPCAPFile`_ 0.12.0 imports :mod:`imp`, which Python removed in 3.12. +.. [2] `PyPCAP`_ 1.3.0 is unsupported on Python 3.12+, and its build does not + search Homebrew's libpcap library prefix. -.. [3] `PyPCAP`_ 1.3.0 could not be built here: its build does not search - Homebrew's libpcap library prefix. It is also unsupported on Python 3.12+. +.. [3] `pcap-ct`_ and `libpcap`_ are published only as pre-releases. -.. [4] The prerelease `pcap-ct`_ / `libpcap`_ wheels load Linux ``libc.so.6`` on - this macOS host, so their engine could not be preflighted. +.. [4] `PyShark`_ 0.6 cannot create the implicit event loop it expects on Python + 3.14; it ran on 3.10--3.13. Installation ============ @@ -257,7 +256,7 @@ Installation is intent rather than something that works. ``bpc-poseur`` 0.4.3.post1 crashes on positional-only parameters declared on a *method* rather than a plain function, and exits 0 so the build does not notice; such methods -- including - the 14 on :class:`~pcapkit.corekit.io.SeekableReader`, plus a dozen more across + those on :class:`~pcapkit.corekit.io.SeekableReader` and others across the protocols, foundation, dumpkit and corekit modules -- then survive into the installed package and ``import pcapkit`` fails with :exc:`SyntaxError`. @@ -296,7 +295,7 @@ the package caches inside the project directory: make setup -This resolves two common local setup issues on macOS/Homebrew installations: +This resolves common local setup issues on macOS/Homebrew installations: ``pipenv`` cache permission errors under :file:`~/Library/Caches`, and ``lxml`` builds failing to locate Homebrew's ``libxml2``/``libxslt`` headers. @@ -360,7 +359,7 @@ plug-in functions, you may want to install the optional ones: .. warning:: **Install only one of** ``pypcap`` **and** ``pcap-ct``. Both own the top-level - :mod:`pcap` module, and ``pip`` will install both without complaint. With both + ``pcap`` module, and ``pip`` will install both without complaint. With both present the ``pcap-ct`` package wins the import and ``pypcap``'s extension module is shadowed and unreachable, so ``engine='pypcap'`` stops working -- measured on Python 3.10. :mod:`pcapkit` detects that state and warns with an @@ -371,13 +370,13 @@ plug-in functions, you may want to install the optional ones: Engine Prerequisites -------------------- -Four of the engines need something beyond a ``pip install``. Each constraint +Some of the engines need something beyond a ``pip install``. Each constraint below is also enforced in code, by the :meth:`~pcapkit.foundation.engines.engine.EngineBase.unsupported_reason` preflight described under `Engine Comparison`_. :class:`~pcapkit.foundation.engines.pyshark.PyShark` - Two requirements, and neither is visible to an import: the package imports + Requirements, none of them visible to an import: the package imports cleanly and then fails when used. - Drives Wireshark's :program:`tshark` binary. It need not be on ``PATH``: @@ -408,7 +407,7 @@ preflight described under `Engine Comparison`_. # macOS brew install libpcap - Two caveats, both upstream packaging problems rather than :mod:`pcapkit` ones: + Caveats, each an upstream packaging problem rather than a :mod:`pcapkit` one: - ``pypcap`` 1.3.0 ships a **pre-generated** :file:`pcap.c` produced by Cython 0.29.x, which does not compile against the Python **3.12+** C API. Having @@ -431,7 +430,7 @@ preflight described under `Engine Comparison`_. ``libpcap`` dependency ship ``py3-none-any`` wheels. Verified reading a capture on Python 3.10 and 3.14. - Two caveats: + Caveats: - **A system** :manpage:`libpcap(3)` **is still required at run time.** The ``libpcap`` distribution ships a vendored :file:`libpcap.so` and, as @@ -449,12 +448,12 @@ preflight described under `Engine Comparison`_. `PyPCAPFile`_ 0.12.0 imports the ``imp`` module, which was **removed in Python 3.12**, so ``pcapfile.savefile`` -- the module needed to read a capture -- cannot be imported at all on 3.12 or newer. Upstream ``master`` - has fixed this but no release carries the fix yet, so this engine also + has fixed this but no release carries the fix, so this engine also requires Python **3.11 or older** until 0.12.1 is published. .. note:: - Only the four engines above carry extra constraints. :mod:`pcapkit` itself and + Only the engines above carry extra constraints. :mod:`pcapkit` itself and its ``default``, ``dpkt`` and ``scapy`` engines need nothing beyond their ``pip install`` -- ``dpkt`` 1.9.8 and ``scapy`` 2.7.0 were both measured reading a capture on Python 3.14. The ``scapy`` engine needs **scapy 2.6.0 or @@ -471,7 +470,6 @@ For CLI usage, you will need to install the optional packages: # or explicitly... pip install pypcapkit emoji -================== Indices and Tables ================== diff --git a/docs/source/pcapkit/const/index.rst b/docs/source/pcapkit/const/index.rst index 262411126..4bb3a2def 100644 --- a/docs/source/pcapkit/const/index.rst +++ b/docs/source/pcapkit/const/index.rst @@ -20,11 +20,11 @@ wrong type, raises :exc:`ValueError`. In almost every registry that object is **not** installed: the registry does not grow, and a second lookup of the same value returns an equal but distinct object. -A handful behave otherwise, and :ref:`mint-criterion` gives the full split: +Some behave otherwise, and :ref:`mint-criterion` gives the full split: * :class:`~pcapkit.const.mh.cga_type.CGAType` installs a permanent member, so ``__members__`` and iteration grow. -* :class:`~pcapkit.const.tcp.flags.Flags` and the four Mobility Header flag +* :class:`~pcapkit.const.tcp.flags.Flags` and the Mobility Header flag registries, such as :class:`~pcapkit.const.mh.binding_ack_flag.BindingACKFlag`, hand back to ``aenum``'s flag base, which caches the composed object in the value lookup diff --git a/docs/source/pcapkit/foundation/engines/index.rst b/docs/source/pcapkit/foundation/engines/index.rst index 9e69d054b..304c9b95a 100644 --- a/docs/source/pcapkit/foundation/engines/index.rst +++ b/docs/source/pcapkit/foundation/engines/index.rst @@ -129,7 +129,7 @@ their row says otherwise: Availability ------------ -A third-party engine is usable only where its backing package is, and three are +A third-party engine is usable only where its backing package is, and some are harder to obtain than a plain :program:`pip install` suggests. A missing package is not fatal -- :mod:`pcapkit` emits an :class:`~pcapkit.utilities.warnings.EngineWarning` and falls back to its own @@ -144,7 +144,7 @@ parser -- but the extraction then does not use the engine requested. | :class:`~pcapkit.foundation.engines.dpkt.DPKT`, | nothing -- both are pure Python, and neither | | :class:`~pcapkit.foundation.engines.scapy.Scapy` | overrides ``unsupported_reason()`` | +-----------------------------------------------------------------+---------------------------------------------------------------+ -| :class:`~pcapkit.foundation.engines.pyshark.PyShark` | Wireshark's :program:`tshark` binary -- on :envvar:`PATH`, or | +| :class:`~pcapkit.foundation.engines.pyshark.PyShark` | Wireshark's :program:`tshark` binary -- on ``PATH``, or | | | wherever ``pyshark``'s :file:`config.ini` points -- **and** | | | Python **3.13 or older**: ``pyshark`` 0.6 builds its event | | | loop with :func:`asyncio.get_event_loop`, which raises from | @@ -188,7 +188,7 @@ Choosing Between PyPCAP and PCAP_CT :class:`~pcapkit.foundation.engines.pypcap.PyPCAP` and :class:`~pcapkit.foundation.engines.pcap_ct.PCAP_CT` read the same :manpage:`libpcap(3)` interface from two independent distributions, both of which -install a top-level :mod:`pcap` module. They are separate engines because they +install a top-level ``pcap`` module. They are separate engines because they are separate projects, with different authors, different install requirements and different interpreter coverage: @@ -222,7 +222,7 @@ installed.** Pick on availability, not on features. hard error it would otherwise be. **The two distributions collide, so install exactly one.** Both own the top-level -:mod:`pcap` module, and pip will happily install both -- on Python 3.10, +``pcap`` module, and pip will happily install both -- on Python 3.10, the ``pcap-ct`` package then wins the import and upstream's extension module is shadowed and unreachable. Each engine therefore detects which distribution it actually got, via diff --git a/docs/source/pcapkit/foundation/reassembly/ip/index.rst b/docs/source/pcapkit/foundation/reassembly/ip/index.rst index 02978b540..5231712cc 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/index.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/index.rst @@ -94,7 +94,7 @@ Because completion frees the buffer (:rfc:`791#section-3.2`), a fragment that arrives after its datagram has completed opens a new buffer, which is later reported as a separate incomplete datagram; this is intended (:issue:`1507`). -Three departures from the procedure above: +Departures from the procedure above: * Only the final fragment's partial last ``RCVBT`` block is set. A non-final fragment carries a multiple of 8 octets, so one ending mid-block was cut or diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst index a377d0d50..792a9e890 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst @@ -1,5 +1,5 @@ -Base Class -========== +IP Reassembly Base Class +======================== .. module:: pcapkit.foundation.reassembly.ip diff --git a/docs/source/pcapkit/foundation/reassembly/reassembly.rst b/docs/source/pcapkit/foundation/reassembly/reassembly.rst index 7867ad4af..fe2da535a 100644 --- a/docs/source/pcapkit/foundation/reassembly/reassembly.rst +++ b/docs/source/pcapkit/foundation/reassembly/reassembly.rst @@ -1,5 +1,5 @@ -Base Class -========== +Reassembly Base Class +===================== .. module:: pcapkit.foundation.reassembly.reassembly diff --git a/docs/source/pcapkit/foundation/traceflow/traceflow.rst b/docs/source/pcapkit/foundation/traceflow/traceflow.rst index 5a46929a7..81dc938a9 100644 --- a/docs/source/pcapkit/foundation/traceflow/traceflow.rst +++ b/docs/source/pcapkit/foundation/traceflow/traceflow.rst @@ -1,5 +1,5 @@ -Base Class -========== +Flow Tracing Base Class +======================= .. module:: pcapkit.foundation.traceflow.traceflow diff --git a/docs/source/pcapkit/index.rst b/docs/source/pcapkit/index.rst index ddec18adc..712f12ed1 100644 --- a/docs/source/pcapkit/index.rst +++ b/docs/source/pcapkit/index.rst @@ -9,8 +9,8 @@ Module Documentation .. _DictDumper: https://dictdumper.jarryshaw.me -Unlike popular PCAP file extractors, such as `Scapy`_, `DPKT`_, -`PyShark`_, and etc, :mod:`pcapkit` tends to provide comprehensive +Unlike popular PCAP file extractors, such as `Scapy`_, `DPKT`_ and +`PyShark`_, :mod:`pcapkit` tends to provide comprehensive support to all protocols, including but not limited to their parameters and/or options, etc. Therefore, :mod:`pcapkit` contains enumeration registries used by protocols, independent protocol @@ -169,7 +169,9 @@ Environment Variables :data:`pcapkit.utilities.logging.SPHINX_TYPE_CHECKING` -The tunable limits are read by :mod:`pcapkit.utilities.config`, which lists each +The ``PCAPKIT_*`` tunable limits, such as +:data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`, are read by +:mod:`pcapkit.utilities.config`, which lists each with its default, its accepted range and its risks. The :mod:`pcapkit.vendor` crawlers' :envvar:`PCAPKIT_VENDOR_RETRY`, :envvar:`PCAPKIT_CI_MODE`, :envvar:`PCAPKIT_HTTP_PROXY` and :envvar:`PCAPKIT_HTTPS_PROXY` are read by diff --git a/docs/source/pcapkit/protocols/application/application.rst b/docs/source/pcapkit/protocols/application/application.rst index 952e3e2a2..3627683c2 100644 --- a/docs/source/pcapkit/protocols/application/application.rst +++ b/docs/source/pcapkit/protocols/application/application.rst @@ -1,5 +1,5 @@ -Base Protocol -============= +Application Layer Base Protocol +=============================== .. module:: pcapkit.protocols.application.application diff --git a/docs/source/pcapkit/protocols/internet/internet.rst b/docs/source/pcapkit/protocols/internet/internet.rst index 8c3b9d59c..21ebdfdd7 100644 --- a/docs/source/pcapkit/protocols/internet/internet.rst +++ b/docs/source/pcapkit/protocols/internet/internet.rst @@ -1,5 +1,5 @@ -Base Protocol -============= +Internet Layer Base Protocol +============================ .. module:: pcapkit.protocols.internet.internet diff --git a/docs/source/pcapkit/protocols/link/index.rst b/docs/source/pcapkit/protocols/link/index.rst index 57cb1d7df..9e70c8581 100644 --- a/docs/source/pcapkit/protocols/link/index.rst +++ b/docs/source/pcapkit/protocols/link/index.rst @@ -8,6 +8,16 @@ Link Layer :mod:`pcapkit.protocols.link` is collection of all protocols in link layer, with detailed implementation and methods. +.. versionremoved:: 1.5.0 + + ``pcapkit.protocols.link.OSPF``, ``pcapkit.protocols.link.RARP`` and + ``pcapkit.protocols.link.DRARP``, and ``OSPF`` and + ``OSPF_CrytographicAuthentication`` in ``pcapkit.protocols.data.link`` and + ``pcapkit.protocols.schema.link``, with no alias. The protocols moved to + :mod:`pcapkit.protocols.application` (:issue:`719`); see + :class:`~pcapkit.protocols.application.ospf.OSPF` and + :class:`~pcapkit.protocols.application.rarp.RARP`. + .. toctree:: :maxdepth: 1 diff --git a/docs/source/pcapkit/protocols/link/link.rst b/docs/source/pcapkit/protocols/link/link.rst index 791dfb6ac..8f716dadc 100644 --- a/docs/source/pcapkit/protocols/link/link.rst +++ b/docs/source/pcapkit/protocols/link/link.rst @@ -1,5 +1,5 @@ -Base Protocol -============= +Link Layer Base Protocol +======================== .. module:: pcapkit.protocols.link.link diff --git a/docs/source/pcapkit/protocols/transport/transport.rst b/docs/source/pcapkit/protocols/transport/transport.rst index 47cdbaaf5..3bf94d2d1 100644 --- a/docs/source/pcapkit/protocols/transport/transport.rst +++ b/docs/source/pcapkit/protocols/transport/transport.rst @@ -1,5 +1,5 @@ -Base Protocol -============= +Transport Layer Base Protocol +============================= .. module:: pcapkit.protocols.transport.transport