design: write down what belongs in the `all` extra, and why

[JarryShaw]

Is your feature request related to a problem? Please describe.

The all extra has no written inclusion criterion. It began, in the maintainer's words on #899, as "originally for all deps (when we only had 3 3p engines and crawler deps)", and the library has since grown past that. Today it is a curated list of eight requirements whose three exclusions each rest on a different ad-hoc justification, so a contributor adding a new extra has nothing to check their decision against.

Measured on origin/main:

ALL       DPKT         ['dpkt']
ALL       PyPCAPFile   ['pypcapfile']
ALL       PyShark      ['pyshark']
ALL       Scapy        ['scapy']
ALL       cli          ['emoji']
ALL       crypto       ['cryptography>=3.4']
ALL       vendor       ['requests', 'beautifulsoup4']
EXCLUDED  NGAP         ['pycrate']                        -- size (~238 MB) + LGPL-2.1+ vs BSD-3-Clause
EXCLUDED  PCAP_CT      ['pcap-ct', 'libpcap']             -- published only as pre-releases
EXCLUDED  PyPCAP       ['pypcap']                         -- sdist-only C extension, needs a compiler
EXCLUDED  docs         [Sphinx, furo, ...]                -- not a runtime feature
partial   test         [pytest, pytest-xdist, isort, ...] -- only requests/beautifulsoup4 reach `all`

Three different reasons for three exclusions, and no rule that predicts them. Size, licence, release maturity and build-toolchain requirements are all cited somewhere in pyproject.toml's comments, each argued locally and at length. Nothing says which of those disqualify an extra in general.

Describe the solution you'd like

Decide and write down the criterion — not merely a revised list. The list follows from the rule; a list without one drifts again the next time an extra is added. Candidate axes the current comments already appeal to:

• Licence — should all ever pull something less permissive than this package's BSD-3-Clause? pycrate is excluded partly for being LGPL-2.1+.
• Size — is there a threshold? pycrate lands ~238 MB for one 4.9 MB module, a 50x multiplier over the useful part.
• Release maturity — pcap-ct and libpcap are excluded for being pre-release only.
• Build requirements — pypcap needs a compiler and libpcap headers, and fails where only the header is present.
• Audience — is all "every runtime feature an end user might want", or "everything"? vendor (crawler deps) is in, test tooling is mostly out, docs is out. That boundary is real but unstated.

Then reconcile two specific things the audit will surface:

1. all duplicates other extras' contents inline rather than referencing them. requests[socks]/beautifulsoup4[html5lib] appear in both vendor and all as literals, so the two drift independently — exactly what happened on fix(ngap): source ProcedureCode/ProtocolIE from pycrate, stop the shared -1 mint (#880) #899, where adding pycrate to vendor did not put it in all and the workflow had to ask for both.
2. test is partial, which is almost certainly right but is currently an accident of which names happen to overlap rather than a decision.

Describe alternatives you've considered

Leaving it and deciding case by case. That is the status quo and it is what produced three incompatible justifications; it also means each new extra costs a fresh argument.

Additional context

Raised by the maintainer on #899: "I think that extra might need some revisit to determine what should fit in and what not … Now that the library grew, a new audit on it should be done."

Answering the narrower question he asked there: cryptography>=3.4 is already fully in all via the crypto extra, so crypto needs no change. pycrate is the live question, and its documented exclusion reasons still hold — adding it would make pip install pypcapkit[all] pull an LGPL-2.1+ dependency silently, which is the specific hazard pyproject.toml's own comment names.

#899 itself is unaffected and needs no change for this: it only touched vendor, and all still excludes pycrate as intended.

[JarryShaw]

My preference is that all means everything that an end user may want or need, like crypto and pycrate as they are addon-alike musts for the library to function at full functionality.

[JarryShaw]

Ruling recorded, verbatim:

My preference is that all means everything that an end user may want or need, like crypto and pycrate as they are addon-alike musts for the library to function at full functionality.

That reverses my recommendation and I accept it — I argued for leaving pycrate out. "Full functionality" is a coherent criterion and it is the one the extra's name implies, which is more than the current list can say for itself.

One consequence stated once, because your own pyproject.toml argued it at length and it should be a decision rather than a side effect: pip install pypcapkit[all] will now pull an LGPL-2.1+ dependency into a BSD-3-Clause package's "everything" extra, and ~238 MB of pycrate_asn1dir with it. The comment on the NGAP extra puts the objection as "something pip install pypcapkit[all] pulls in silently: LGPL carries obligations BSD does not". Under this ruling that trade is accepted deliberately — which is fine, and it is worth the comment saying so explicitly rather than being left contradicting the code. I will have that rewritten as part of the work rather than left to be discovered.

The narrower question your ruling does not settle, and it needs your word before the criterion can be written down. Two extras remain excluded, and their reason is installability, not desirability:

• PyPCAP — pypcap is an sdist-only C extension. It needs a compiler and the libpcap development headers, and per the existing comment it "fails where only the header is present -- as on the macOS runners".
• PCAP_CT — pcap-ct and libpcap are published only as pre-releases, so including them means all silently installs a beta.

Both are engines an end user might well want, so under "everything an end user may want or need" they qualify by desire. But including them makes pip install pypcapkit[all] either fail outright on a machine without a toolchain, or install a pre-release nobody asked for.

So: does the criterion read as "everything an end user may want", or "everything an end user may want that installs unattended on a supported platform"? The second keeps those two out for a stated reason rather than an ad-hoc one, and is the only version I can write down as a rule that predicts the current list plus your pycrate decision. Keeping needs: decision for this alone.

Not blocked on it: adding pycrate to all and reconciling the comments can proceed on your ruling as given.

And it makes a comment on the just-merged #899 wrong in a second way. pyproject.toml:202-210 reads "a developer opting into vendor has already accepted a far heavier and more permissively-licensed dependency set than an end user installing all ever would." I had already flagged "more permissively-licensed" as backwards — LGPL-2.1+ is less permissive than BSD-3-Clause. Under this ruling the rest of the sentence is false too, since all will carry pycrate as well. The whole clause goes rather than getting a one-word fix.

[JarryShaw]

I say we only keep core addons in all, meaning only CLI, crypto and pycrate to date.

vendor extra keeps to the vendor crawler and they're not used for end user.

3P engine extras same, only on demand for each one.

Will need to update all scripts that references/uses extras.

[JarryShaw]

Ruling recorded, verbatim:

I say we only keep core addons in all, meaning only CLI, crypto and pycrate to date.

vendor extra keeps to the vendor crawler and they're not used for end user.

3P engine extras same, only on demand for each one.

Will need to update all scripts that references/uses extras.

That is a sharper criterion than your first answer and it changes the shape rather than just adding pycrate. Your earlier wording was "everything that an end user may want or need", which the four third-party engines arguably satisfy; "core addons only" excludes them. Measured — all goes from 8 requirements to 3:

NOW    ['emoji', 'cryptography>=3.4', 'dpkt', 'scapy', 'pyshark',
        "pypcapfile; python_version < '3.12'", 'requests[socks]', 'beautifulsoup4[html5lib]']
RULED  ['emoji', 'cryptography>=3.4', 'pycrate']
REMOVED  dpkt, scapy, pyshark, pypcapfile, requests, beautifulsoup4
ADDED    pycrate

You are right that scripts need updating, and here is the list — seven workflow sites plus three test modules and two docs pages:

lint.yml:186            create-release.yml:437, :444      deploy-pages.yml:111
cron-conda.yml:111, :247, :254                            cron-vendor.yml:122  (already .[all,vendor])
tests/_dependency_gates.py:126, :1407     tests/test_tier_guard.py:1278
docs/source/index.rst:312                 docs/source/pep.rst:539-541

Those three test sites assert all's contents — e.g. _dependency_gates.py:126: ".[all] carries pypcapfile, pyshark and scapy, so a guard…". They will fail, and correctly so; they encode the current shape and move with it.

The finding that needs your word, because it is the one thing the ruling does not resolve: all is currently doing double duty as "the CI install", and those are different sets. lint.yml:178-181 says so in its own words:

.[all] rather than a bare install: pylint and mypy resolve imports, so findings depend on what is importable.

So removing scapy, dpkt and pyshark from all means lint starts reporting unresolved imports for them — its comment already tracks exactly 8 import-error messages from the two engines that are absent by marker, and this would add more. deploy-pages.yml:111 has the same dependency: the docs build already emits "dependency not found" notices for missing optional packages, and autodoc would gain more.

So the ask: should CI get its own extra — a dev or ci set naming everything the toolchain needs to resolve imports — or should each workflow name the extras it needs explicitly (.[all,Scapy,DPKT,PyShark] and so on)? The first is one new extra and seven one-line edits; the second is more verbose but makes each workflow's needs self-documenting, and avoids inventing a second catch-all that drifts the way all just did.

I lean a dev extra, because "everything the linters and docs build need to see" is a real, stable set and naming it once is what stops the next drift. But it is your call, and it is the difference between seven edits and seven longer edits.

needs: decision kept for that alone. Everything else in the ruling is unambiguous and the work can start on it.

[JarryShaw]

Sure dev extra it is.

[JarryShaw]

Ruling recorded, verbatim:

Sure dev extra it is.

So all narrows to the core addons and a new dev extra carries what the toolchain needs to resolve imports. That settles every open point on this issue — needs: decision off, worker dispatched.

The shape, as ruled:

all  = [ emoji, cryptography>=3.4, pycrate ]          core addons only
dev  = [ everything lint/mypy/docs need to see ]      new
vendor = crawler deps only, not for end users          unchanged in purpose
3P engine extras                                       on demand, one at a time

Seven workflow sites move to .[dev] (or .[all,dev] where both are genuinely wanted), and the worker has been told to decide per site rather than substituting blindly:

lint.yml:186            create-release.yml:437, :444      deploy-pages.yml:111
cron-conda.yml:111, :247, :254                            cron-vendor.yml:122 (already .[all,vendor])

Three test sites assert all's contents and will fail correctly — they encode the current shape and move with it: tests/_dependency_gates.py:126 and :1407, tests/test_tier_guard.py:1278. Each says something like ".[all] carries pypcapfile, pyshark and scapy", which stops being true.

Two docs pages describe the old all and need updating: docs/source/index.rst:312 and :322, docs/source/pep.rst:539-541. Note pep.rst:539 currently explains why pypcap is excluded from all — that reasoning survives, but its framing ("otherwise pip install pypcapkit[all] would demand a compiler") now applies to dev as well if dev carries the engines.

The judgement I have asked the worker to make explicitly, because it is the one place this could go wrong: lint.yml's comment says its findings "depend on what is importable", and tracks exactly 8 import-error messages from the two engines already absent by Python-version marker. If dev carries the engines, that count should stay at 8; if it does not, lint gains new import-error findings and the comment becomes wrong. So dev has to be defined by what the toolchain must see, and the 8-message count is the check that it was got right.

Worker briefed to report the before/after import-error count rather than assume, and to leave pypcap/pcap-ct out of everything — your ruling keeps engine extras on demand, and those two additionally break or beta-install, which is why they were never in all.

[JarryShaw]

PR up: #914 (b1535e7f3), implementing both rulings. Cross-review dispatched on a different model, briefed to falsify.

What landed, against your words "only keep core addons in all, meaning only CLI, crypto and pycrate to date":

all = [ "emoji", "cryptography>=3.4", "pycrate" ]          # was 8 requirements
dev = [ "dpkt", "scapy", "pyshark",
        "pypcapfile; python_version < '3.12'",
        "requests[socks]", "beautifulsoup4[html5lib]" ]

vendor is unchanged and pypcap/pcap-ct are untouched, so nothing moved that you did not name. The dev extra exists because pylint, mypy and autodoc all resolve imports against what is installed, so narrowing all alone would have made four packages newly unresolvable to them — the three workflows that need full resolution now install .[all,dev], and lint.yml's tracked import-error count is claimed to stay at exactly 8.

Two things I am having the reviewer attack rather than take on trust. First, whether a [all]-only user silently loses four engines — dpkt, scapy, pyshark and pypcapfile were in all and are not now, so the question is whether engine='dpkt' warns and falls back or raises, and whether that is documented anywhere a user reads. That is the one genuine user-visible consequence of the ruling and it needs to be deliberate. Second, two install sites in create-release.yml and cron-conda.yml were left at bare .[all] on the reasoning that nothing downstream consumes them; if that is wrong it only shows on a real release.

The PR also swept stale prose the ruling invalidated — README.md, docs/source/pep.rst, docs/source/index.rst, and the NGAP/vendor extra comments in pyproject.toml, which both asserted pycrate was deliberately kept out of all.

One measurement I could not reconcile and am not relaying as settled: the author reports a Sphinx baseline of 42 warnings where two other measurements of the same commit give 58. I confirmed 58 myself. make html adds no flags and PCAPKIT_DEVMODE is read only by pcapkit/utilities/logging.py, never by conf.py, so neither explains it; missing optional imports account for only 6 of the 16. The reviewer is settling the cause. The delta is 0 either way, which is what the PR actually needs.