fix(packaging): ship CITATION.cff in the source distribution - #631
Merged
Merged
Conversation
JarryShaw
force-pushed
the
fix/manifest-include-citation-cff
branch
from
September 22, 2026 04:56
e5ef1fd to
c24bb00
Compare
JarryShaw
force-pushed
the
fix/manifest-include-citation-cff
branch
from
September 22, 2026 05:16
c24bb00 to
eea40f8
Compare
Owner
Author
|
@copilot resolve the merge conflicts on this branch. |
Copilot stopped work on behalf of
JarryShaw due to an error
September 22, 2026 05:16
Requested by the owner; no issue is open for it. * `MANIFEST.in` had `include` lines for `README.md`, `LICENSE` and `CHANGELOG.md` but none for `CITATION.cff`, and its two `global-include` patterns are `*.rst` and `*.py`, neither of which matches a `.cff`. The file was therefore in the repository and in no source distribution. * Nor was there a default to fall back on. Removing all three `include` lines and rebuilding shows `README.md` and `LICENSE` shipping regardless -- setuptools adds the latter from `license_files`, logging `adding license file 'LICENSE'` and recording `License-File: LICENSE` in `PKG-INFO` -- while `CHANGELOG.md` disappears. Of the three only `CHANGELOG.md` is load-bearing, and a citation file, which no packaging default covers at all, is in the same position. * The gap is invisible from the web UI, since GitHub renders the "Cite this repository" button from the repository. Citation managers, Zenodo and dependency inventories read the published artifact, which is the surface that was missing it. * It matters now because #625 has just taught `util/bump_version.py` to keep that file's `version` and `date-released` in step with the bump, so a release exercising that path would publish an sdist omitting the artefact under test. #615 flagged the omission when it added the file; #619 did not address it. * It also lets `RepositoryCitationTests` in `tests/project/test_bump_version.py` -- the gate #625 added for the hand-authored bumps that never run the script -- execute against an unpacked sdist, where today it skips itself with "CITATION.cff is not shipped in the source distribution". * Changelog bullet added; `CHANGELOG.md` regenerated with `util/changelog_md.py`, and `--check` exits 0. Rebased onto `cfb81d3f6` after #630 merged; the changelog conflict was resolved keeping both bullets and regenerating `CHANGELOG.md` rather than merging it. Measured both ways with `python -m build --sdist` against that base: `tar tzf | grep -i citation` found nothing before and `pypcapkit-1.5.0b4/CITATION.cff` after; the listings differ by that one entry and nothing else, 860 against 861; the shipped copy is byte-identical to the repository's; and `twine check --strict` reports PASSED on both. No tests were run -- one line of packaging configuration, nothing under `pcapkit/`.
JarryShaw
force-pushed
the
fix/manifest-include-citation-cff
branch
from
September 22, 2026 05:25
eea40f8 to
e143a22
Compare
Owner
Author
Cross-review: GOOD TO GOIndependent cross-review by a subagent on a different model (Claude Haiku), run read-only on its own clones in Verdict on this PR: GOOD TO GO.
What it caught, and what changed as a resultPass 1 — the comment's central analogy was inverted. The first version of the new comment opened "Load-bearing for the same reason as Pass 2 — "nothing outside this file knows a citation file exists" was false. Pass 2 also surfaced a second, independent motivation for this PR, now recorded in the entry: Two PR-body claims were also overstated and were corrected: that the surrounding Independently re-measured on the final head, against the rebased base
|
This was referenced Sep 22, 2026
JarryShaw
added a commit
that referenced
this pull request
Sep 22, 2026
`util/changelog_md.py --check` has failed on `main` since #638 (375e9d4), which inserted three lines into the generated `CHANGELOG.md` directly instead of running the generator. That left the #630 bullet duplicated -- the stale pre-correction text alongside the corrected one -- and the #631 bullet in the wrong position relative to its source entry. Regenerated from `docs/source/changelog/1.5.0.rst`, which was always correct: the phrase appeared once there and twice in the generated file. `Changelog drift` had failed on four consecutive commits (375e9d4, a62aed1, a05f461, da381f2), and because `tests/project/test_changelog_md.py::RepositoryStateTests` asserts the same consistency, all twelve matrix jobs failed with it -- 13 of 14 on run 35748204467. Every open pull request inherited the failure. --check now exits 0.
JarryShaw
added a commit
that referenced
this pull request
Sep 22, 2026
…two false packaging claims (#642) Three names appeared in string annotations that their module never imported, and they were mypy's complete set of ``name-defined`` findings for the package: * ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``. * ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and ``:271`` used ``Optional``; added, ``Protocol`` as ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell it that way, in the same ``payload:`` stub; this makes twenty-two. mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total errors before, 112 after, so nothing else moved. ``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing that puts it in an sdist" and that an sdist without it "cannot be installed at all". Both halves are false, and the file had contradicted itself since #631 wrote the correct mechanism seven lines below without correcting this. Deleting the lines and rebuilding gives a byte-identical sdist listing -- empty diff -- that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md`` is load-bearing. The comment now says that, and the same correction is applied to the #619 entry in ``docs/source/changelog/1.5.0.rst``, which carried the identical claim. Three changelog bullets gained the citation they were missing: ``(#619)`` for the README entry, ``(#570)`` for the ``code=`` keyword and ``(#577)`` for the ``StreamEOFError`` gaps. Two of the five remain uncited because no number could be recovered for them. ``CHANGELOG.md`` regenerated with ``util/changelog_md.py``; ``--check`` exits 0. New ``tests/project/test_annotation_names.py`` resolves every string annotation in the package -- following a nested forward reference such as ``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated`` metadata as the values they are -- against the names its own module binds. It reports the same three findings as mypy on the unfixed tree and none after. 128 -> 132 passed over ``tests/project`` and ``tests/utilities/test_logging.py``, both exit 0, subtests unchanged at 487.
JarryShaw
added a commit
that referenced
this pull request
Sep 22, 2026
…two false packaging claims (#642) Three names appeared in string annotations that their module never imported, and they were mypy's complete set of ``name-defined`` findings for the package: * ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``. * ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and ``:271`` used ``Optional``; added, ``Protocol`` as ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell it that way, in the same ``payload:`` stub; this makes twenty-two. mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total errors before, 112 after, so nothing else moved. ``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing that puts it in an sdist" and that an sdist without it "cannot be installed at all". Both halves are false, and the file had contradicted itself since #631 wrote the correct mechanism seven lines below without correcting this. Deleting the lines and rebuilding gives a byte-identical sdist listing -- empty diff -- that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md`` is load-bearing. The comment now says that, and the same correction is applied to the #619 entry in ``docs/source/changelog/1.5.0.rst``, which carried the identical claim. Three changelog bullets gained the citation they were missing: ``(#619)`` for the README entry, ``(#570)`` for the ``code=`` keyword and ``(#577)`` for the ``StreamEOFError`` gaps. Two of the five remain uncited because no number could be recovered for them. ``CHANGELOG.md`` regenerated with ``util/changelog_md.py``; ``--check`` exits 0. New ``tests/project/test_annotation_names.py`` resolves every string annotation in the package -- following a nested forward reference such as ``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated`` metadata as the values they are -- against the names its own module binds. It reports the same three findings as mypy on the unfixed tree and none after. 127 -> 132 passed over ``tests/project`` and ``tests/utilities/test_logging.py``, both exit 0, subtests unchanged at 487.
JarryShaw
added a commit
that referenced
this pull request
Sep 22, 2026
…a false packaging claim (#642) Three names appeared in string annotations that their module never imported, and they were mypy's complete set of ``name-defined`` findings for the package: * ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``. * ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and ``:271`` used ``Optional``; added, ``Protocol`` as ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell it that way, in the same ``payload:`` stub; this makes twenty-two. mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total errors before, 112 after, so nothing else moved. ``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing that puts it in an sdist" and that an sdist without it "cannot be installed at all". Both halves are false, and the file had contradicted itself since #631 wrote the correct mechanism seven lines below without correcting this. Deleting the lines and rebuilding gives a byte-identical sdist listing -- empty diff -- that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md`` is load-bearing. The comment now says that. New ``tests/project/test_annotation_names.py`` resolves every string annotation in the package -- following a nested forward reference such as ``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated`` metadata as the values they are -- against the names its own module binds. It reports the same three findings as mypy on the unfixed tree and none after. That module named ``ast.TypeAlias`` and ``ast.TypeVar`` directly, and both are PEP 695 nodes added in Python 3.12, so *every* test in it raised ``AttributeError`` on the 3.10 and 3.11 matrix jobs -- ``bound_names`` walks every node of every file, so the attribute is reached whatever a test does. Both are now resolved once at module scope through ``getattr(ast, ..., ())``, leaving the ``isinstance`` branches otherwise untouched: ``isinstance(x, ())`` is always False, so the branches stay live on 3.12+ and are simply unreachable below it. Chosen over a ``sys.version_info`` comparison because it writes no version number down at all -- a comparison states 3.12 next to the attribute it guards, and the two can then drift -- and over a per-node ``getattr`` because a module-level constant lifts the lookup out of a loop that runs on every node of every file. ``ast.TypeVar`` is the branch that earns its keep: it carries its name as a bare ``str`` and emits no ``ast.Name`` node, so forcing ``_TYPE_VAR`` to ``()`` on 3.14.7 turns ``T`` and ``U`` into false findings. ``ast.TypeAlias`` is defensive by comparison -- its name *is* an ``ast.Name`` in ``Store`` context, which the preceding branch already catches -- and is left as it stands rather than removed. A new ``test_a_pep695_type_parameter_is_in_scope`` pins both the guards and the behaviour, skipped below 3.12 because its fixture source cannot parse there. Measured on real interpreters rather than simulated. 3.10.21 and 3.11.15: 5 failed, exit 1 -> 5 passed, 1 skipped, exit 0. 3.14.7: all 6 pass, exit 0. 133 passed over ``tests/project`` and ``tests/utilities/test_logging.py``, exit 0, subtests unchanged at 487. No changelog entry on this branch. Per the rule that no code branch touches ``CHANGELOG.md`` or anything under ``docs/source/changelog/``, this change's entry -- and the wording correction the ``MANIFEST.in`` claim implies for the #619 entry, plus the missing ``(#570)`` and ``(#577)`` citations -- go to the shared changelog pull request #657 instead.
JarryShaw
added a commit
that referenced
this pull request
Sep 22, 2026
…a false packaging claim (#642) (#666) Three names appeared in string annotations that their module never imported, and they were mypy's complete set of ``name-defined`` findings for the package: * ``pcapkit/utilities/logging.py:350`` used ``Any``; added to the ``TYPE_CHECKING`` block beside ``IO``, ``Optional`` and ``Union``. * ``pcapkit/protocols/schema/internet/ipv6_route.py:136`` used ``Protocol`` and ``:271`` used ``Optional``; added, ``Protocol`` as ``ProtocolBase as Protocol``. Twenty-one sibling schema modules already spell it that way, in the same ``payload:`` stub; this makes twenty-two. mypy 2.3.1 over ``pcapkit``: 3 ``name-defined`` errors before, 0 after; 115 total errors before, 112 after, so nothing else moved. ``MANIFEST.in:14-17`` asserted that ``include README.md`` was "the only thing that puts it in an sdist" and that an sdist without it "cannot be installed at all". Both halves are false, and the file had contradicted itself since #631 wrote the correct mechanism seven lines below without correcting this. Deleting the lines and rebuilding gives a byte-identical sdist listing -- empty diff -- that installs with exit 0: ``setuptools/command/sdist.py:59-60`` ships the README unconditionally and ``setuptools/dist.py:460``'s default ``license_files`` glob ships ``LICENSE``. Of the original three ``include`` lines only ``CHANGELOG.md`` is load-bearing. The comment now says that. New ``tests/project/test_annotation_names.py`` resolves every string annotation in the package -- following a nested forward reference such as ``'list["Nested"]'``, while treating ``Literal`` members and ``Annotated`` metadata as the values they are -- against the names its own module binds. It reports the same three findings as mypy on the unfixed tree and none after. That module named ``ast.TypeAlias`` and ``ast.TypeVar`` directly, and both are PEP 695 nodes added in Python 3.12, so *every* test in it raised ``AttributeError`` on the 3.10 and 3.11 matrix jobs -- ``bound_names`` walks every node of every file, so the attribute is reached whatever a test does. Both are now resolved once at module scope through ``getattr(ast, ..., ())``, leaving the ``isinstance`` branches otherwise untouched: ``isinstance(x, ())`` is always False, so the branches stay live on 3.12+ and are simply unreachable below it. Chosen over a ``sys.version_info`` comparison because it writes no version number down at all -- a comparison states 3.12 next to the attribute it guards, and the two can then drift -- and over a per-node ``getattr`` because a module-level constant lifts the lookup out of a loop that runs on every node of every file. ``ast.TypeVar`` is the branch that earns its keep: it carries its name as a bare ``str`` and emits no ``ast.Name`` node, so forcing ``_TYPE_VAR`` to ``()`` on 3.14.7 turns ``T`` and ``U`` into false findings. ``ast.TypeAlias`` is defensive by comparison -- its name *is* an ``ast.Name`` in ``Store`` context, which the preceding branch already catches -- and is left as it stands rather than removed. A new ``test_a_pep695_type_parameter_is_in_scope`` pins both the guards and the behaviour, skipped below 3.12 because its fixture source cannot parse there. Measured on real interpreters rather than simulated. 3.10.21 and 3.11.15: 5 failed, exit 1 -> 5 passed, 1 skipped, exit 0. 3.14.7: all 6 pass, exit 0. 133 passed over ``tests/project`` and ``tests/utilities/test_logging.py``, exit 0, subtests unchanged at 487. No changelog entry on this branch. Per the rule that no code branch touches ``CHANGELOG.md`` or anything under ``docs/source/changelog/``, this change's entry -- and the wording correction the ``MANIFEST.in`` claim implies for the #619 entry, plus the missing ``(#570)`` and ``(#577)`` citations -- go to the shared changelog pull request #657 instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MANIFEST.incarriesinclude README.md(line 18),include LICENSE(19) andinclude CHANGELOG.md(37), but had no line forCITATION.cff. Its twoglobal-includepatterns are*.rstand*.py, and neither can match a.cff, soCITATION.cffwas in the repository and in no source distribution at all. This addsinclude CITATION.cff.Requested by the repository owner; no issue is open for it, so there is no
Fixesline.Why this matters now
#625 has just taught
util/bump_version.pyto keepCITATION.cff'sversionanddate-releasedin step with the version bump, and the owner intends a beta release specifically to exercise that path. Releasing with this gap would publish an sdist that omits the very artefact the release is meant to be testing — the bump would update a file that then does not ship.The omission was flagged when the file landed: #615 added
CITATION.cffand noted it ("CITATION.cffis not in the sdist. … Left alone becauseMANIFEST.inis owned by a concurrent branch this wave"), and #619 touchedMANIFEST.inonly forREADME.md, so it did not address the gap.It also matters independently of the release. GitHub renders the "Cite this repository" button from the repository, so the button works either way and the gap is invisible from the web UI. Citation managers, Zenodo and dependency inventories read the published artifact, which is exactly the surface where the file was missing — so credit stopped travelling with the code at precisely the point where nobody can see the repository.
There was no default to fall back on
Worth establishing rather than assuming, because two of the three neighbouring
includelines turn out to be redundant. Removinginclude README.md,include LICENSEandinclude CHANGELOG.mdall at once from a clean checkout atead73b204and rebuilding gives an sdist whose root is:README.mdandLICENSEstill ship — setuptools adds the licence from itslicense_filesdefault, loggingadding license file 'LICENSE'and recordingLicense-File: LICENSEinPKG-INFO.CHANGELOG.mdis gone. So of the three, onlyinclude CHANGELOG.mdis genuinely load-bearing, and a.cffis in the same position as it: nothing outsideMANIFEST.inknows a citation file exists. That is what the new comment says, and it is why it anchors onCHANGELOG.mdrather than on its immediate neighbour.Proof, both directions
Same command before and after the one-line change:
Before — no output, exit 1. The root of the sdist held
CHANGELOG.md,LICENSE,MANIFEST.in,PKG-INFO,pyproject.toml,README.md,setup.cfg,setup.py.After — exit 0:
Nothing else moved. Diffing the two full archive listings (sorted under
LC_ALL=C) gives exactly one added line and nothing removed — 860 entries before, 861 after:The shipped copy is the repository's copy, not a rewritten one —
sha256of both is9d231a0bb7ab5ceece390497b2f9c42818cd6dbc47b88bb6d52d276cef488a25.twine checkstill passesPASSED, exit 0, on both the before and the after archive. Worth stating explicitly, since #497 records a beta that had to be reshipped as1.5.0b2after PyPI rejected its wheel over atwine checkfailure.The comment
The two commented
includeblocks in this file each explain why the line is load-bearing rather than belt-and-braces, so this one does too, in the same voice: whichglobal-includefails to reach a.cff, that the absence was measured by building one each way rather than assumed, that no setuptools default covers it, and why the published artifact rather than the repository is the surface that matters.Changelog
A
**Fixed**bullet indocs/source/changelog/1.5.0.rst, withCHANGELOG.mdregenerated byutil/changelog_md.pyand--checkconfirmed to exit 0.CHANGELOG.mdwas not hand-edited.No tests were run: this is one line of packaging configuration plus a changelog entry, and touches nothing under
pcapkit/.python -m buildwas the relevant verification and it is quoted above.Notes for whoever merges
docs/source/changelog/1.5.0.rst, so they conflict with each other (and with the other changelog-touching PRs in flight). Whichever lands second needs a rebase that keeps every bullet, then autil/changelog_md.pyregeneration rather than a hand merge ofCHANGELOG.md.MANIFEST.in:14-17(from docs(readme): trim to a landing page and convert to Markdown #619) claimsinclude README.mdis "the only thing that puts it in an sdist" and that "an sdist without it cannot be installed at all". The build above showsREADME.mdshipping with that line removed, so the claim is wrong. Left alone as out of scope — happy to send it as its own PR if wanted.