feat(util): have bump_version.py keep CITATION.cff in step with the bump - #625
Conversation
d4b725b to
586e5b6
Compare
Requested by the owner: "we have version cited in CITATION.cff, might need to have the version_bump.py handle that as well". * `util/bump_version.py` now rewrites `version` and `date-released` in `CITATION.cff` alongside `__version__` in `pcapkit/__init__.py`. Nothing else in the repository maintains that file, so every bump left it naming the previous release -- and GitHub renders it as the "Cite this repository" button. * The rewrite is line-oriented, so the comment header, key ordering and each field's existing quoting survive; `cff-version` and a `references` entry's own `version` are anchored out at column zero. An absent file is reported on stderr and skipped rather than failing the vendor cron before its commit; a file present with no `version` field raises, before anything is written. * The script gains a `main()` guard. It previously ran the bump at import, which is why it had no testable surface at all. * Fixed while here: the `import pcapkit` fallback in the version reader returned `"1.5.0b4'\n"`, quote and newline included, which `packaging` rejects. That path had never worked; CI never reaches it because the package is installed first. * New `tests/project/test_bump_version.py`, 32 tests against a temporary CFF fixture -- the repository's own file is only ever read, by a gate asserting it still names the packaged version. `tests/project` 96 passed, tier guard and docstring contract green. The rewrite of `pcapkit/__init__.py` and `conda/build` is byte-for-byte identical to before, and the produced CFF passes `cffconvert --validate`.
586e5b6 to
3ad3096
Compare
|
Cross-review (independent agent, Sonnet — the change was authored on Opus): GOOD TO GO, no blocking changes. Two non-blocking notes acted on: dedicated tests for the trailing-inline-comment limit and for the One disagreement, resolved against the reviewer: its 10-of-25 hand-authored figure used a different commit set. Full write-up in the PR description. |
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. * 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. * The new line carries a comment, as its neighbours do, saying why it is load-bearing rather than belt-and-braces. * Changelog bullet added; `CHANGELOG.md` regenerated with `util/changelog_md.py`, and `--check` exits 0. Measured both ways with `python -m build --sdist`: `tar tzf | grep -i citation` found nothing before and `pypcapkit-1.5.0b4/CITATION.cff` after; the two archive 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/`.
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 nothing outside `MANIFEST.in` knows about, 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. * Changelog bullet added; `CHANGELOG.md` regenerated with `util/changelog_md.py`, and `--check` exits 0. Measured both ways with `python -m build --sdist`: `tar tzf | grep -i citation` found nothing before and `pypcapkit-1.5.0b4/CITATION.cff` after; the two archive 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/`.
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/`.
Requested by the owner: "we have version cited in CITATION.cff, might need to have the version_bump.py handle that as well". No issue exists for this, so there is no
Fixesline.Two notes on the ask as phrased: the script is
util/bump_version.py, notversion_bump.py, and it rewrote two files rather than one —pcapkit/__init__.pyand, from the working directory,conda/build.The problem
CITATION.cfflanded in #615 carryingversion: 1.5.0b4anddate-released: '2026-09-20'. Nothing in the repository maintains it. A repo-wide grep forCITATIONoutside.githits only changelog prose — no workflow, hook,Makefile,setup.pyorpyproject.tomlmentions it, and the file has exactly two commits in its history. So every version bump left it naming the previous release, which matters more than a stale docs file usually would: GitHub renders it as the Cite this repository button, and citation managers, Zenodo and dependency inventories read it directly, so a wrong number propagates into papers and bills of materials.What the release flow actually does, and what that settles
bump_version.pyis invoked from exactly one place — theBump Versionstep of.github/workflows/cron-vendor.yml:78, between the registry crawl and thegit commit. Completion of that workflow is itself theworkflow_runtrigger ofcreate-release.yml, which readspcapkit.__version__back out of the tree, tagsv<version>, and publishes to PyPI and Anaconda with no manual approval gate (environment: releaseis commented out atcreate-release.yml:234).So in this repository the bump is not a preparatory step that a release later follows — the bump is what causes the release. Measured over the thirty most recent releases:
and the measurement that actually governs a date field: the bump commit's UTC calendar date equals the PyPI upload's UTC calendar date 30 times out of 30, with zero midnight crossings — the weekly cron fires at 10:05 UTC and publishes at 10:07, twelve hours clear of a date boundary.
Recommendation on
date-released: bump it, in UTCBoth fields move. They have the same standing — the file's own header says
versionanddate-releasedboth "describe the newest published release", so at bump time both are equally anticipatory and there is no principled line that moves one and pins the other. Moving onlyversionwould assert something plainly false and leave it there indefinitely: "1.5.0b5, released on the day 1.5.0b4 was". A stale date is wrong in every case where the release does happen, which is eleven times in twelve.UTC, not
date.today(): seven of those thirty bumps were made late evening in US-Eastern, where a naive local date is a day behind the publish it describes.The residual risk, stated rather than hidden. Roughly one bump in twelve historically never reached PyPI at all — eight consecutive weekly bumps between 2024-04-13 and 2024-06-15 (
1.3.1.post10–post17, all tagged), and1.5.0b1last week. For those this writes an optimistic date for a release that never happened. That is an argument for eventually anchoring both fields tocreate-release.yml, which runs only when a publish is actually going out — not for leaving the field stale..github/was outside this change's scope, so it is a recommendation rather than a commit.And the script is no longer the main way versions move. Of the 159 commits that have moved
__version__onmain, 40 are hand-authored and so never ran this script — a quarter over the project's life, and a rising share lately: 11 of the most recent 25. (Two different windows, which is worth stating plainly: 25% lifetime, 44% recent.1.4.0,1.4.1,1.5.0a1and the1.5.0b1–b3run are all in the recent set.) That gap is why the change also adds a gate:RepositoryCitationTestsfails when the committedCITATION.cffhas drifted frompcapkit/__init__.py, so a hand-made bump that forgets the file is caught in CI rather than shipped. Proven non-vacuous — it catches drift in either direction, does not latch ontocff-version, and survives a quoted value.The absent-file case: reported on stderr, then skipped
Deliberately not fatal, and deliberately not silent either.
Silence is out for the reason the whole change exists — a bump that quietly does not update a file it maintains is how this staleness starts. But hard failure is wrong for this script specifically, because of where it runs: the
Bump Versionstep executes underbash -e, before the workflow'sgit commit. A non-zero exit there does not merely skip the citation update; it fails the step, discards the entire vendor crawl the run existed to produce, and the release that would have followed never happens. Trading a published release and a week of registry updates for a missing documentation file is the wrong way round.A file that is present and carries no top-level
versionfield is a different case and does raiseCitationFieldError. There the script is in exactly the situation it was written for, and finding nothing to update means the file's shape changed underneath it — an outcome indistinguishable from success if it were allowed to pass. Unlike an absent file, that cannot happen by accident of packaging. The check runs before anything is written, so the fatal case leaves the tree untouched rather than half-bumped;plan_citationtakes text and returns text, so it has nothing to write with.date-releasedis optional in CFF 1.2.0, so a file without one keeps not having one — the absence is reported and no date is invented.How the rewrite works, and what it refuses to disturb
Line-oriented, not a
yamlload-and-dump. A round trip through a YAML library would reorder keys, normalise quoting and drop the sixteen comment lines explaining why the file omitsdoiandorcid— none of which anyone asked for.cff-version: 1.2.0shares theversion:suffix but must never move, and areferencesentry's ownversionis indented and belongs to that reference. Both are covered by tests.version: 1.5.0b4bare,date-released: '2026-09-20'quoted, the latter deliberately so a validator reads a string rather than casting it to a YAML date. Preserving the style rather than normalising it keeps the bot's diff down to the value that moved. An unquoted version is safe because every stringbump()returns carries a.devN/aN/bN/rcN/.postNsuffix, so none can be read as a YAML number the way a bare2.0would be — asserted bytest_every_bumped_version_carries_a_non_numeric_suffix.#comment after either value would not survive. Neither line carries one, all of the file's commentary being whole-line, and parsing far enough to know the difference is the YAML round trip this avoids. The loss would be visible in the diff.On the real file the rewrite changes exactly two lines:
CITATION.cffitself is not edited by this PR. Maintaining it is the script's job; a hand-edit now would be the bug this change removes.Two other things this touches
The script gains a
main()guard. It previously ran the entire bump at module scope, so importing it rewrote two files as a side effect — which is why it had no testable surface at all and why variant A below cannot even be collected.A latent bug fixed while here. The
import pcapkitfallback in the version reader ranline.split('=')[1].strip(" '"), which stops at the trailing newline and so returned"1.5.0b4'\n"— closing quote and newline included — whichpackagingrejects outright:That path had never worked. It went unnoticed because the only caller installs the package first, so
import pcapkitalways succeeds and the fallback is unreachable in CI. Now.strip().strip('\'"').Verification
The tests fail without the change
Three variants of
util/bump_version.py, same test module, same interpreter — a venv with pytest andpackagingbut nopcapkit, so nothing can reach the real checkout. Exit codes read from files, not from a pipe.origin/mainverbatimInvalidVersion: "1.5.0b4'\n"striprestoredVariant A proves the old script had no testable surface: merely importing it runs the bump, and it dies.
Variant B isolates the new behaviour from the refactor. The 22 failures are exactly the 21 citation tests plus the fallback-strip test; the 9 that pass are the behaviour-preservation set —
bump(), the__init__.pyrewrite, and the repository gate:And with the change:
The produced CFF is valid — through the real validator
cffconvertis not in any extra here, so it was installed into a throwaway venv rather than the repo's.--validateon the rewritten real file:The test fixture validates both before and after the rewrite, so the round trip is proven on a document the schema accepts rather than on a fragment. A PyYAML parse of the result confirms the field types, which is the point of the quoting:
Existing behaviour is unchanged
Three proofs, all run from outside the checkout with the real tree's sha verified unchanged before and after:
pcapkit/__init__.pyrewrite is byte-identical. The old rewrite block and the newrewrite_initwere each run on a copy of the real file: same SHA-256 (4f0782de…), same length (4479 → 4479 bytes), tail__version__ = '1.5.0b5'\n.pcapkitimportable, via a stub package): old and new produce byte-identicalpcapkit/__init__.pyandconda/build(0).conda/buildis still written relative to the working directory, exactly as before.Scoped test runs (never the whole tree — a
coverage run -m pytest tests/here reached 41 GB RSS):Measured against the worktree, not the stale main checkout —
pcapkit.__file__asserted to be inside this branch's tree before anything else was imported.Cross-review
Reviewed by an independent agent on a different model (Sonnet; the change was authored on Opus). Verdict: GOOD TO GO, no blocking changes. It independently re-derived the byte-identical
__init__.pyrewrite (same SHA-256), audited every write in the new script, attackedplan_citationwith its own adversarial fixtures and validated the outputs withcffconvert, confirmed thebash -estep ordering incron-vendor.yml, and spot-checked the never-published versions against PyPI.One disagreement, resolved against the reviewer. It measured the hand-authored share as 10 of the most recent 25 rather than 11, and called the "quarter" figure wrong. Re-measured directly —
git log -G'__version__ = ' -- pcapkit/__init__.pyoverorigin/main— gives 159 commits, 119 bot, 40 human: 25.2% lifetime and 11 of the most recent 25 (44%). Both original figures were right; the reviewer's window was "commits touchingpcapkit/__init__.py", which is not the same set. But the criticism landed anyway: the prose put a 25% claim next to an 11-of-25 figure without saying they measured different windows, which reads as self-contradictory. Reworded here, in the script docstring, in the test docstring and in the changelog entry.Two non-blocking notes it raised were also acted on: the trailing-inline-
#-comment limit and theversion :space-before-colon refusal now each have a dedicated test (the two added since the first push), andtest_the_release_date_defaults_to_today_in_utcno longer has a one-in-86400 flake at a UTC midnight boundary — it accepts either bound around the call, and additionally asserts the local date was not used whenever the runner's local date differs from UTC, which it does on the machine this was developed on (local 2026-09-21, UTC 2026-09-22).Found and deliberately not fixed
conda/buildis still resolved relative to the working directory, whereCITATION.cffis resolved relative to the repository root (the spellingutil/changelog_md.pydocuments and uses, so that the script answers the same from anywhere). The two disagree and the root-relative form is the better one, but changingconda/buildis a behaviour change for every existing caller and belongs in its own review.CITATION.cffis still not in the sdist.MANIFEST.inhas noinclude CITATION.cff— docs: add CITATION.cff and bring the copyright notice up to date #615 flagged this and docs(readme): trim to a landing page and convert to Markdown #619 did not add it. It does not affect the GitHub button, which reads the repository, but it does meanRepositoryCitationTestsskips when run from an unpacked sdist, which is why it is guarded.docs/source/conf.pystarts its copyright at 2017 whereLICENSEstarts at 2018. Inherited from docs: add CITATION.cff and bring the copyright notice up to date #615's own "not changed" list; still true, still the owner's call.info.versionfor this project reports1.4.1.post2, not1.5.0b4, because1.5.0b4is a prerelease and that field reports the latest stable. Anything that ever validatesCITATION.cffagainst "latest on PyPI" through that field will disagree with the checked-in value.1.3.1.post10–post17). Why is not established — I worked from commit, tag and PyPI metadata, not from Actions run logs — but it is the concrete reasondate-releasedat bump time is a prediction rather than a fact.packagingis an undeclared test dependency now.util/bump_version.pyimports it, and this is the first test module to load that script, so the suite now needspackagingwhere it did not before. It is present on every path that can run pytest at all —pip show pytestgivesRequires: iniconfig, packaging, pluggy, pygments— but it is nowhere inpyproject.toml. Adding it to thetestextra would make that explicit;pyproject.tomlwas outside this change's scope.