diff --git a/docs/source/conf.py b/docs/source/conf.py index 248f69d3be..9d43243b69 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -1247,26 +1247,11 @@ def read_own_enum_new(obj: type, name: str, *defargs: 'Any') -> 'Any': return safe_getattr(obj, name, *defargs) -# 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 done properly elsewhere or make the -# build unusable: -# -# ``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 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. -# -# ``source_read`` printed every document's entire source text, which on this -# project is several thousand pages of build log. -# -# If the ``TYPE_CHECKING`` behaviour is ever wanted again, ``bind_type_checking_names`` -# is where it belongs, not a reload hook. +# NB: the ``TYPE_CHECKING`` names belong to ``bind_type_checking_names`` above, +# not to a hook that flips :data:`typing.TYPE_CHECKING` and reloads each module: +# the binding does the same per module, without mutating a global mid-build. Nor +# should a ``source-read`` handler print each document: on this project that is +# several thousand pages of build log. def setup(app: 'Sphinx') -> None: diff --git a/docs/source/contributing/conventions/extension-header-subclassing.rst b/docs/source/contributing/conventions/extension-header-subclassing.rst index a9476281ea..bb56611fe2 100644 --- a/docs/source/contributing/conventions/extension-header-subclassing.rst +++ b/docs/source/contributing/conventions/extension-header-subclassing.rst @@ -148,8 +148,8 @@ was left behind, and that was deliberate. The owner ruled that the ``IPv6_GenericExt`` name goes for good: it was an intermediate state, and it was never released. -What makes that safe rather than merely decided: the old name existed on ``main`` from -``b3551cb63`` to ``93cf940b3``, under four hours on one day, and no release tag falls +What makes that safe rather than merely decided: ``IPv6_GenericExt`` existed on +``main`` from ``b3551cb63`` to ``93cf940b3``, under four hours on one day, and no release tag falls between the two commits -- ``git tag --contains`` names the same tags for both -- so it appears in **no** release and the break has no callers to inconvenience. Do not reintroduce it as an alias, and do not cite it as a former public name. diff --git a/docs/source/contributing/conventions/mint-criterion.rst b/docs/source/contributing/conventions/mint-criterion.rst index a0ae938b04..7ad3cca6f9 100644 --- a/docs/source/contributing/conventions/mint-criterion.rst +++ b/docs/source/contributing/conventions/mint-criterion.rst @@ -143,8 +143,8 @@ 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. -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 +Further ranges in :mod:`pcapkit.const.reg.ethertype` 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. Neither is among the company names, and neither is suffixed for the reason the next section gives. diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index d5e2d3f292..4483add55c 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -44,8 +44,8 @@ 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: some of them used to be in ``all``. -* ``PyPCAP`` **and** ``PCAP_CT`` **were already excluded for installability**, which the + changed: the 1.4 ``all`` carried some of them. +* ``PyPCAP`` **and** ``PCAP_CT`` **are also excluded for installability**, a reason the ruling leaves untouched. ``pypcap`` is an sdist-only C extension needing a compiler and the libpcap development files; ``pcap-ct`` and ``libpcap`` are published only as pre-releases, and ``all`` should not be how somebody acquires a diff --git a/docs/source/contributing/conventions/protocol-layer-placement.rst b/docs/source/contributing/conventions/protocol-layer-placement.rst index a4463886fe..d05261c44b 100644 --- a/docs/source/contributing/conventions/protocol-layer-placement.rst +++ b/docs/source/contributing/conventions/protocol-layer-placement.rst @@ -153,12 +153,12 @@ Each is deliberate. Do not "fix" any of them. **A parsing base may live in another subpackage.** :mod:`pcapkit.protocols.application.rarp` imports -:class:`~pcapkit.protocols.link.arp.ARP` from :mod:`pcapkit.protocols.link.arp`. This -is the first ``application/ → link/`` dependency in the package, and it is the honest -consequence of letting layer and parser disagree. +:class:`~pcapkit.protocols.link.arp.ARP` from :mod:`pcapkit.protocols.link.arp`. That +``application/ → link/`` dependency is the honest consequence of letting layer and +parser disagree. **The dispatch tier is decoupled from the subpackage.** A protocol's subpackage says -nothing about which registry reaches it, and the move did not touch any dispatch key: +nothing about which registry reaches it, and no dispatch key depends on it: .. list-table:: :header-rows: 1 diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index 30b4a49f03..0197970f88 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -45,7 +45,7 @@ The owner ruled on :issue:`877` that a registry may subclass a bare base enum ou subclassing that base in turn for use by the mutable ones. So a **closed** set inherits :class:`~pcapkit.corekit.enum.EnumLookup` directly and is never handed a ``register`` it would have to refuse; an **open** -registry inherits :class:`~pcapkit.corekit.enum.EnumRegistry` exactly as before. +registry inherits :class:`~pcapkit.corekit.enum.EnumRegistry`. What settled the split is the owner's own second thought about carrying ``register`` on the base: if the base carries ``register``, there is no principled reason for it @@ -54,9 +54,9 @@ Following that through leaves :class:`~pcapkit.corekit.enum.EnumRegistry` holdin 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 -:class:`~pcapkit.corekit.enum.EnumRegistry` leaves the member data type exactly where -it was -- ``LinkType -> EnumRegistry -> EnumLookup -> IntEnum -> int`` -- so +**Both tiers are plain classes, and that is load-bearing.** The parent above +:class:`~pcapkit.corekit.enum.EnumRegistry` leaves the member data type with the enum +base -- ``LinkType -> EnumRegistry -> EnumLookup -> IntEnum -> int`` -- so ``_member_type_`` still comes from the enum base. Had either tier subclassed ``aenum.Enum`` in order to "be an enum", it would have become the member type itself and broken ``int``, ``str`` and flag registries at once. @@ -348,15 +348,15 @@ resolvable. And a folding override carries **only** the fold. :meth:`TransportProtocol.get ` is -the worked example: since :issue:`923` it lowers ``key``, forwards ``default`` verbatim -and delegates to ``super().get()``, and that is all it 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 was +the worked example: it lowers ``key``, forwards ``default`` verbatim and delegates to +``super().get()``, and that is all it does. :issue:`923`'s ruling retired the conversion +of the base's name-miss :exc:`KeyError` into a :exc:`ValueError` that it also carried; +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 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 now be -empty is deleted, not kept as a pass-through** -- a ``get`` that only calls +:meth:`~pcapkit.corekit.enum.EnumLookup.get` unchanged. **An override left with nothing +to add is deleted, not kept as a pass-through** -- a ``get`` that only calls ``super().get()`` reads as though it were doing something, and the next reader has to diff it against the base to find out that it is not. @@ -525,10 +525,9 @@ That leaves the classes with something to decide: :class:`~pcapkit.protocols.internet.mh.LocalizedRoutingStatus` never carried a ``get`` at all, so they had no string lookup to fold. :class:`~pcapkit.protocols.application.ngap.Criticality` had one when this audit - was taken and no longer does: :issue:`877` re-parented it onto + was taken and inherits ``get`` unchanged: :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 inherits ``get`` - unchanged. + conversion that was the override's only remaining job. - **case-sensitive** -- conforms * - :class:`~pcapkit.protocols.misc.pcapng.WireGuardKeyLabel` - ``draft-tuexen-opsawg-pcapng`` @@ -609,7 +608,8 @@ rather than changing it. :class:`~pcapkit.const.ftp.command.Command`, :class:`~pcapkit.const.ftp.command.FEATCode`, :class:`~pcapkit.const.http.method.Method`, - :class:`~pcapkit.const.pcapng.option_type.OptionType` and + :class:`~pcapkit.const.pcapng.option_type.OptionType`, + :class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol` and :class:`~pcapkit.const.reg.apptype.apptype.AppType` -- and probing one of those measures the override rather than the base. The unconditionally clean witness is ``tests/corekit/test_enum_lookup_base_unit.py``'s own ``_Str``, a purpose-built diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index d5b90509a3..f373c86a30 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -62,12 +62,11 @@ leaves the type out of it. The type stays importable by its dotted path there, f annotation or an ``is`` guard; it is only ``import *`` that does not offer it. The ``__all__`` there does name :class:`~pcapkit.corekit.sentinels.Sentinel`, the abstract base every sentinel type shares, since that is no sentinel's own type. A private -sentinel such as ``ABSENT`` is in neither, which is what private means here -- dropping -its leading underscore did not add it to either list, and +sentinel such as ``ABSENT`` is in neither, which is what private means here, and :class:`~pcapkit.corekit.sentinels.AbsentType` and :data:`~pcapkit.corekit.sentinels.ABSENT` are documented on :doc:`the sentinels API page ` as private and not for -public use rather than left off it, since the name alone no longer says so. +public use rather than left off it, since nothing in the name says so. .. note:: diff --git a/docs/source/contributing/conventions/test-coverage.rst b/docs/source/contributing/conventions/test-coverage.rst index d9858108b6..606114172b 100644 --- a/docs/source/contributing/conventions/test-coverage.rst +++ b/docs/source/contributing/conventions/test-coverage.rst @@ -39,7 +39,9 @@ known-failure table keep their cases in a generator, beside the captures it also * - :file:`tests/protocols/test_dispatch_registry_unit.py` - :file:`examples/generators/dispatch.py` - every ``__proto__`` entry dispatches to the class ``PINNED_TARGETS`` names; - known failures in ``KNOWN_DEGRADED`` (:issue:`496`) + known degrades in ``DispatchRegistryTests.KNOWN_DEGRADED``, which the + generator's own ``KNOWN_DEGRADED`` mirrors for its command-line report + (:issue:`496`) The rules those tables carry: @@ -56,7 +58,8 @@ The rules those tables carry: Routing a case around a defect hides it: the generator's ``HIP_COPIES`` once put two 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 + ``KNOWN_DEGRADED`` entries have a different shape -- the fragments of the probe's + ``'error'`` value the entry accepts, any one of which may match, and a reason -- and they record why the probe's placeholder payload cannot reach the target, not a defect to fix, so the name-and-issue rule does not apply to them. * **A regression is fixed; a pre-existing defect is recorded.** A case that a change diff --git a/docs/source/contributing/pep.rst b/docs/source/contributing/pep.rst index 346f2e2f9f..597b430bb1 100644 --- a/docs/source/contributing/pep.rst +++ b/docs/source/contributing/pep.rst @@ -198,7 +198,7 @@ What is left, and why: It wants a registry of its own rather than :meth:`~pcapkit.protocols.protocol.Protocol._decode_next_layer`, which is only ever called at a layer boundary and appends to the frame's protocol chain. - An MLD address record inside a mobility option did not follow MH on the wire, + An MLD address record inside a mobility option does not follow MH on the wire, so putting it in that chain would make ``layer=`` and ``protocol=`` limits behave wrongly. Changing the parsed shape from :obj:`bytes` to :class:`~pcapkit.protocols.misc.raw.Raw` also changes what existing captures dump to, so @@ -276,7 +276,7 @@ complaint and yields nothing useful. :attr:`TCP.__proto__ ` and :attr:`UDP.__proto__ `, out of a registry carrying every IANA service name: TCP 20 to FTP-DATA and 21 to FTP, - ports 80 and 8080 to HTTP on both TCP and UDP, and UDP 1701 to L2TP. + ports 80 and 8080 to HTTP on both TCP and UDP, and UDP 1701 to L2TPv2. * :class:`~pcapkit.const.sctp.payload_protocol_identifier.PayloadProtocolIdentifier` -- the values in :attr:`SCTP.__proto__ `: PPIDs 60 and 66, each to @@ -386,8 +386,8 @@ 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. -The larger wins this section listed, all outside the parse path, have landed in -full or in part: +The larger costs this section listed all lie outside the parse path, and each is +addressed in full or in part: * **The flow dumper reopens its output file once per frame**, in :class:`~pcapkit.dumpkit.pcap.PCAPIO`'s ``__call__``. It does not @@ -407,11 +407,11 @@ full or in part: measured at 86% of the IP-reassembly cost plus a 133 ms garbage-collection bill. Whether unfragmented frames should be emitted at all is the design question that is left, and it is worth settling deliberately. -* **Done.** Every option was parsed twice -- 2274 schema unpacks for 1137 - options, the pre-parse always discarded, about 15% of a PCAP-NG extraction. - :class:`~pcapkit.corekit.fields.collections.OptionField` reads only the base - schema's type field to choose the option schema, rather than unpacking the - whole base schema and throwing it away. +* **Done.** :class:`~pcapkit.corekit.fields.collections.OptionField` reads only + the base schema's type field to choose the option schema, rather than unpacking + the whole base schema and throwing it away. Unpacking it parsed every option + twice -- 2274 schema unpacks for 1137 options, the pre-parse always discarded, + about 15% of a PCAP-NG extraction. Some traps for anyone benchmarking this library. ``reassembly=True`` and ``trace=True`` are **no-ops** without ``ip=``/``tcp=``, so a benchmark that passes @@ -423,10 +423,10 @@ Recursive Dissection, With a Depth Limit **Decided, open to revisit.** Each layer dissects the next from inside its own parse, so dissecting a frame recurses once per layer, at 9 to 12 Python -frames a layer. Deep nesting -- IP in IP, stacked 802.1Q tags, long chains of -IPv6 extension headers -- used to exhaust the recursion limit and leave the rest -of the frame raw, silently, at a depth that varied with the interpreter and the -caller's stack (:issue:`1604`, :issue:`1610`). +frames a layer. Without a bound, deep nesting -- IP in IP, stacked 802.1Q tags, +long chains of IPv6 extension headers -- exhausts the recursion limit and leaves the +rest of the frame raw, silently, at a depth that varies with the interpreter and +the caller's stack (:issue:`1604`, :issue:`1610`). Recursion stays, with a counted bound: :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT` caps a frame at 64 layers by @@ -622,9 +622,9 @@ Anyone wanting to run the tests wants the repository, which is where they are. ``dictdumper``'s ``PLIST`` writer formats every :class:`~datetime.datetime` as ``'%Y-%m-%dT%H:%M:%S.%fZ'``, always with six fractional digits, and - :mod:`plistlib`'s ```` pattern admits none, so :func:`plistlib.load` - failed on every ``format='plist'`` report with ``AttributeError: 'NoneType' - object has no attribute 'groupdict'``. The upstream defect is + :mod:`plistlib`'s ```` pattern admits none, so without the workaround + below :func:`plistlib.load` fails on every ``format='plist'`` report with + ``AttributeError: 'NoneType' object has no attribute 'groupdict'``. The upstream defect is `JarryShaw/DictDumper#122 `__. :func:`pcapkit.dumpkit.common.make_dumper` works around it (:issue:`1448`): it writes each date in UTC to whole seconds, as :func:`plistlib.dump` does. The diff --git a/docs/source/contributing/releasing.rst b/docs/source/contributing/releasing.rst index a38bad9518..c712e45e73 100644 --- a/docs/source/contributing/releasing.rst +++ b/docs/source/contributing/releasing.rst @@ -144,10 +144,11 @@ One Commit per Release Every job that checks out, tags, builds or publishes pins ``github.sha`` (:issue:`1634`), so a run releases the commit it was created for however long it waits on approval. -Before that, run 38008819343 was created for ``b6b95bc8e``, approved three -days later, and tagged ``main``'s tip ``2c0008c3c`` as both ``v1.5.0b9`` and -``conda-1.5.0b9+0``: ``action-gh-release`` defaulted ``target_commitish`` to -the default branch, and ``tag`` pulled ``main`` before tagging. +Without the pins, a run releases whatever ``main``'s tip is once approved: run +38008819343 was created for ``b6b95bc8e``, approved three days later, and +tagged ``main``'s tip ``2c0008c3c`` as both ``v1.5.0b9`` and +``conda-1.5.0b9+0``, since ``action-gh-release`` defaults ``target_commitish`` to +the default branch, and ``tag`` pulls ``main`` before tagging. * On ``workflow_run``, where ``github`` creates the ``v*`` tag, it passes ``target_commitish`` ``github.sha``; on a ``v*`` push the tag @@ -176,7 +177,7 @@ the default branch, and ``tag`` pulled ``main`` before tagging. ``src/github.ts``, on a release that exists). A file the release already has is left alone (:issue:`1640`): the action - deleted and re-uploaded it, and a rebuild against live PyPI is not + deletes and re-uploads it, and a rebuild against live PyPI is not byte-identical to the published copy. Only an asset in state ``uploaded`` counts: `Upload a release asset `__ @@ -185,15 +186,15 @@ 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. -* ``gh release upload`` also fails, deliberately, where the action did not. +* ``gh release upload`` also fails, deliberately, where the action does not. With no release for the tag, the upload fails with ``gh``'s - "release not found", where the action made one, with no notes and + "release not found", where the action makes one, with no notes and ``target_commitish`` unset (``release()`` and ``createRelease()`` there), which Create a release defaults to "the repository's default branch"; the release is ``github``'s to make, at the run's commit. A file pattern matching nothing exits 1 with ``::error title=Nothing to upload``, where the action, - ``fail_on_unmatched_files`` unset, only warned (``run()`` in ``src/run.ts``) and - published the release without that file. + ``fail_on_unmatched_files`` unset, only warns (``run()`` in ``src/run.ts``) and + publishes the release without that file. Pinning the target may have a cost, and GitHub documents the rule two ways. Its REST docs for `Create a release @@ -246,10 +247,10 @@ exists". It also uploads only to a published release, read first with ``gh release view``. With none, the action would create one, and its tag, at ``commit`` (``Action.checkForMissingReleaseError()`` and ``Action.updateDraftOrCreateRelease()``). Conda Update bumped 1.5.0b9's build -three times while that run waited on approval; once ``conda-dist`` runs on -every bump (:issue:`1632`), -that would tag the pending version at ``main``'s tip and fail Create Release's -tag check. The action would also retarget a draft at ``commit`` +three times while that run waited on approval, and ``conda-dist`` runs on +every pushed bump (:issue:`1632`), so a release the action created there would +tag the pending version at ``main``'s tip and fail Create Release's tag check. +The action would also retarget a draft at ``commit`` (``Action.updateDraftOrCreateRelease()`` and ``Action.updateRelease()``), which ``omitDraftDuringUpdate`` keeps a draft, but Create Release's ``workflow_run`` run finds the draft and resets its target to the run's sha (``action-gh-release`` 3.0.2's ``release()`` and @@ -305,7 +306,7 @@ the release test gate, ask for a second approval and race the first run's uploads from another concurrency group, so ``version_check``'s ``if:`` skips a ``push`` whose ``github.actor_id`` is ``vars.RELEASE_APP_BOT_ID``, and ``release_status`` reports it with ``::notice title=Nothing to release``. A -tag pushed by a person runs as before. Whether a tag made +tag pushed by a person is not skipped. Whether a tag made through the Releases API fires ``push``, and with the bot as actor, is unmeasured; if not, the guard never matches and the cost is one approval to reject. @@ -324,23 +325,26 @@ the job; that a re-run reads variables and secrets set since is unmeasured. Per-Job Evidence, Not a Shared Proxy ------------------------------------ -Until :issue:`888`, every job -past ``version_check`` shared one guard: +One guard shared by every job past ``version_check`` would read "the ``v*`` tag +exists" as a proxy for "this version was already published": .. code-block:: yaml if: ${{ startsWith(github.ref_name, 'v') || needs.version_check.outputs.PCAPKIT_TAG_EXISTS == 'false' }} -using "the ``v*`` tag exists" as a proxy for "this version was already -published". Those diverge exactly when a release half-completes -- ``github`` -tags, a later job fails or is rejected, and the tag is left behind with the -publish incomplete -- which is the whole subject of `Precautions`_ below. -``github`` still uses that guard for its own artefact, since the tag it -checks *is* the one it is about to create: +Those diverge exactly when a release half-completes -- ``github`` tags, a later +job fails or is rejected, and the tag is left behind with the publish +incomplete -- which is the whole subject of `Precautions`_ below +(:issue:`888`). Of the publishers, only ``github`` uses that clause, for its own +artefact, since +the tag it checks *is* the one it is about to create; the clause beside it skips +a version whose tag names another commit (`One Commit per Release`_): .. code-block:: yaml - if: ${{ startsWith(github.ref_name, 'v') || needs.version_check.outputs.PCAPKIT_TAG_EXISTS == 'false' }} + if: >- + ${{ (startsWith(github.ref_name, 'v') || needs.version_check.outputs.PCAPKIT_RELEASED_ELSEWHERE != 'true') && + (startsWith(github.ref_name, 'v') || needs.version_check.outputs.PCAPKIT_TAG_EXISTS == 'false') }} ``tag``, ``pypi`` and ``conda`` each read their own evidence instead: @@ -401,7 +405,7 @@ on ``tag``), and GitHub Actions skips a job whose ``needs:`` included a job that itself skipped -- *before* that job's own ``if:`` is even evaluated -- unless the ``if:`` calls a status-check function itself. ``github`` legitimately skips whenever its own tag already exists, which is exactly the -retry case this fix is for. Left alone, that skip would cascade: ``pypi``'s +retry case this guard is for. Left alone, that skip would cascade: ``pypi``'s own evidence might correctly say "not yet on PyPI, please run", but the default status check GitHub Actions prepends to an ``if:`` that does not itself call a status-check function -- effectively ``success()`` over @@ -478,10 +482,9 @@ What still makes one approval mean *the whole release* is the ``needs:`` graph: ``tag`` and ``pypi`` both depend on ``github``, and ``conda`` depends on ``tag`` and ``github`` -- so nothing downstream of ``github`` can start before it is approved, and rejecting it leaves nothing tagged and -nothing published. Before that removal ``tag`` depended on ``version_check`` -alone, so removing its reviewer without moving this dependency would have let -it push a commit to ``main`` and cut a ``conda-*`` tag *ahead of* the -approval it was supposed to wait for. +nothing published. Without its ``needs: github``, ``tag``, which has no +reviewer of its own, could push a commit to ``main`` and cut a ``conda-*`` tag +*ahead of* the approval it is supposed to wait for. Precautions ----------- @@ -521,7 +524,7 @@ Neither is worth the risk of a double upload to an index that cannot take one back, and neither is needed, since a plain re-run self-heals; see `Recovery`_ below. -``environment: pypi`` **stays even though its reviewer is gone.** ``pypi`` +``environment: pypi`` **stays even though it has no reviewer.** ``pypi`` publishes through PyPI's OIDC trusted publishing (the job's ``environment: pypi`` and ``id-token: write``), and PyPI's trusted-publisher configuration can be scoped to a GitHub Actions environment name. If this diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index c2ef1ba483..277574413a 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -357,8 +357,8 @@ starting an independent run after the caller finishes. The calls target ``needs.head.result``. Cron Head (:issue:`1967`) is -one copy of the test gate and the ``verdict`` job, which Conda Update, Vendor -Update and GitHub Pages used to carry each. It holds ``contents: read`` and no +the one copy of the test gate and the ``verdict`` job that Conda Update, Vendor +Update and GitHub Pages share. It holds ``contents: read`` and no more, which is what each caller grants its ``head`` job and what ``unit-tests.yml`` is called with. Every expression in it reads the caller's ``github`` context, so its jobs decide on the event that started the caller and check out the caller's ``github.sha``. A @@ -378,13 +378,13 @@ orders them: ``workflow_run`` path. Create Release reads only the Vendor Update run that triggered it, through its ``vendor_update`` job, so the time of the others does not matter to it. -* At one time they cost more. Vendor Update and Conda Update both ran at 10:00 - until :issue:`1967` spread them, and each scheduled pair read back from 2026-09-26 on - checked out the same commit -- on 2026-10-17, Conda Update 39023770904 and - Vendor Update 39023891165 both gated 44ed96fc3, their ``Gate (full suite, - …)`` jobs taking twelve and ten minutes. Their writing jobs share the ``repository-maintenance`` group, so ``conda-update`` - then waited for ``vendor-update`` to finish, and a third job joining that - group cancels the one pending there. +* Aligned, they cost more. With Vendor Update and Conda Update both at 10:00, + each scheduled pair read back from 2026-09-26 on checked out the same commit + (:issue:`1967`) -- on 2026-10-17, Conda Update 39023770904 and Vendor Update + 39023891165 both gated 44ed96fc3, their ``Gate (full suite, …)`` jobs taking + twelve and ten minutes. Their writing jobs share the ``repository-maintenance`` + group, so ``conda-update`` waited for ``vendor-update`` to finish, and a third + job joining that group cancels the one pending there. Spread, each ``head`` gates alone, and Conda Update runs once Vendor Update's Create Release has had the day to publish, so its ``Check the GitHub release @@ -717,8 +717,9 @@ fire while the pool stays saturated, and the JUnit timings are what show whether not fail the run (:issue:`1732`): on every job of :file:`experimental.yml`, which no required context names, on every :ref:`KTLO ` leg, a job whose name ends in ``(KTLO)`` (:issue:`1633`), and on a job whose name ends in ``(experimental)`` in any workflow. That -last covers the experimental legs that ran in :file:`unit-tests.yml` under -``test``'s 45-minute cap before the split. The finding keeps its ``H`` id and says it is non-blocking. A +last also covers older runs of :file:`unit-tests.yml`, whose experimental legs ran under +``test``'s 45-minute cap until :issue:`1732` moved them to :file:`experimental.yml`. +The finding keeps its ``H`` id and says it is non-blocking. A KTLO leg past its job's ``timeout-minutes`` is still cancelled, and its check run reads red, though since :issue:`1738` nothing gating waits on it, so that warning is worth acting on before the cap. @@ -739,13 +740,13 @@ permission to the reusable workflow. fails the push run that GitHub Pages, Conda Update and Vendor Update (and so Create Release) wait on. The cost is wall time: the push run, and those workflows, finish about one slowest cell later, about 6 min before any queue wait (estimate). -* ``engine-tests`` has no ``DPKT`` cell. Its only path was +* ``engine-tests`` has no ``DPKT`` cell. Its only path would be :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``. + on every leg. What that gives up is the file in a venv holding only ``test`` and + ``DPKT``. -Another cut, running the then-prerelease 3.15 engine cell on ``main`` pushes only, -ended with :issue:`1633`: 3.15 is -in the supported tier, so its cells block on every run. +The 3.15 engine cells are not among the cuts: 3.15 is in the supported tier +(:issue:`1633`), so its cells block on every run. Required Status Checks ---------------------- @@ -783,9 +784,8 @@ grepping every workflow file for its name: - job ``compatibility``, matrix leg ``3.15`` - ``python-compatibility.yml``, key ``"3.15"`` of ``.github/python-versions.toml`` -That is the list after :issue:`1633`, -which added ``Compat Python 3.15`` and removed ``Compat Python 3.10``. Moving a version -between the tiers below means editing the ruleset in the same change. +Moving a version between the tiers below means editing the ruleset in the same +change, as :issue:`1633` did for 3.15 and 3.10. ``Required checks passed`` is defined by exactly one, the ``name:`` of the ``required-checks`` job in ``unit-tests.yml``. ``Compat Python`` is defined by @@ -797,8 +797,8 @@ context matches one. The comment above ``required-checks`` mentions the string without defining it. ``Required checks passed`` is an aggregate, not a single check run. The ruleset -requires it and the ``Compat Python 3.11``-``3.15`` legs, nothing else, as -that comment records. It ``needs:`` the gating jobs of ``unit-tests.yml`` -- +requires it and the ``Compat Python 3.11``-``3.15`` legs, with ``Lint``, and +nothing else. It ``needs:`` the gating jobs of ``unit-tests.yml`` -- ``test``, ``integration``, ``engine-tests`` and ``pypcap-parity``, plus ``project-tests``, the one run of the project pins, and ``changes`` for the docs-only path -- and checks each result explicitly, in its "Check every @@ -902,9 +902,8 @@ setup-python reads them (``3.14t``, ``graalpy-25.4``). Each runs a leg of ``experimental``, the matrix job of :file:`.github/workflows/experimental.yml` ("Experimental Tests"), named ``Python (experimental)``, and nothing else: no ``integration``, ``Compat``, wheel or conda leg, so never a required -context. They were legs of ``test`` until :issue:`1732`, -which moved them to a -workflow of their own, by the owner's ruling that nothing waits on them. **That +context. They run in a workflow of their own, rather than as legs of ``test``, by +the owner's ruling on :issue:`1732` 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 but CI Reports', which posts its finished runs (see `CI Reports`_), so the Unit @@ -1012,8 +1011,8 @@ skipped every step after its tests, ``Post Run actions/checkout@v7`` too, whose `__ "defaults to ``always()``". On a lost runner, then, the ``[monitor]`` lines in the 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. +``continue-on-error`` with no cap of their own, so they change neither the caps above +nor the check names. A lost runner fails the job, and so its check, whatever ``continue-on-error`` says, so ``Run unit tests`` holds the tests to what leaves the runner alive, and a breach @@ -1047,7 +1046,7 @@ warnings: caps hold over the whole tier; on ``timeout``'s SIGINT the run in progress writes its JUnit file and no later one starts. Each shard's run writes a JUnit file of its own beside ``junit.xml``, such as ``junit-tests-protocols.xml``, and ``Upload test - timings`` takes them all. 3.14t's leg runs pytest once, as before. + timings`` takes them all. 3.14t's leg runs pytest once. * ``timeout --signal=INT --kill-after=3m 110m`` ends pytest under the step's 120-minute cap, so it still writes the junit XML, and exits 124; ``prlimit --fsize=2147483648`` caps any file at 2 GiB. The ``timeout`` is GNU's, ``gnutimeout`` where it exists @@ -1272,7 +1271,7 @@ engine cells that ``engine-tests``' ``Write the per-cell script`` writes; ``util/cap_file_size.py`` sets ``RLIMIT_FSIZE``, soft and hard, to 2 GiB, or keeps a lower hard limit, and execs the command, so the step's exit status and -``continue-on-error`` behave as before and every process the command starts +``continue-on-error`` behave as without it and every process the command starts inherits the cap. A write past it raises ``OSError: [Errno 27] File too large`` in Python, since CPython ignores ``SIGXFSZ``, so the test errors; any other program is killed by the signal, whose default action the script restores before it execs. @@ -1753,7 +1752,7 @@ Distribution`` steps rewrite the former, and Dependabot reads only a ``FROM`` li that names its image, while the benchmark's :file:`Dockerfile` takes its base image through ``ARG PYTHON_IMAGE`` from :file:`examples/benchmark/python-images.txt`. -PyUp, which opened the same kind of bump before, opens none: :file:`.pyup.yml` sets +PyUp opens no bump: :file:`.pyup.yml` sets ``update: False`` and lists no ``requirements:``. A file listed there takes its own ``update``, which defaults to ``all``, so the list would override the global setting. Deleting :file:`.pyup.yml` would not stop the bot either, as without it @@ -1763,8 +1762,8 @@ PyUp runs on its defaults. The PyUp App's installation, and the Python Versions --------------- -Since :issue:`1660`, -:file:`.github/python-versions.toml` declares every Python version CI runs: one +:file:`.github/python-versions.toml` (:issue:`1660`) declares every Python version +CI runs: one line per version, with its tier -- ``supported``, ``ktlo``, ``experimental``, or ``pre-release``, which no version carries -- and, where ``ubuntu-latest`` cannot provide it, the ``runner`` it needs. ``latest`` names the newest supported CPython, which every @@ -1785,7 +1784,7 @@ the coverage leg's version. Its tests, and the other tests that 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. +non-blocking mechanics no workflow has, so the script refuses one. **Python Versions** (:file:`cron-python-versions.yml`) proposes those edits. Every Monday, or on dispatch, its ``detect`` job runs