Repository navigation
fix(version): require a comma between version constraints - #26
Merged
Merged
Conversation
Completes the invented-comma fix from 0.3.0, which closed the tokenizer route and left this one. rstudio/package-manager#19466's F5 residue. validConstraintRegexp ended each repetition with `\s*\,?`, making the separator OPTIONAL. Combined with the deliberately-admitted empty operator -- a bare version is a valid constraint -- adjacency did not merely parse, it RENDERED A COMMA THE INPUT NEVER CONTAINED: "==0.1dev10.3" -> "==0.1dev10" + bare "3" -> renders "==0.1dev10,3" A fabricated constraint boundary changes which versions match, and the publisher's intent is unrecoverable from the string.⚠️ THE R PATH WAS THE REASON THIS SAT OPEN, because validConstraintRegexp is shared with NewRSpecifiers, which PPM uses for R constraint parsing. Measured against real CRAN data rather than argued: 58,656 constraint occurrences, 2,939 distinct expressions (data/cache/23_PACKAGES_1-2026-05-01.gz, a real full CRAN PACKAGES file) -> IDENTICAL accept/reject before and after. Zero change. Independently corroborated over ~3.9M parenthesised constraint expressions extracted from production CRAN RSFs: zero with two operators, zero with an internal comma. Real CRAN ranges exist but are written as the same package repeated across a field ("R (>= 3.6.0), R (< 3.7.0)"), which is handled by src/description and never reaches NewRSpecifiers.⚠️ A NULL RESULT IS ONLY MEANINGFUL IF THE INSTRUMENT WORKS, so the change was verified to reject six adjacency shapes (">= 7 < 10", ">=1.0<2.0", ...) and to leave twelve load-bearing forms untouched -- R dash forms, the real CRAN "1.1-18-1" and "2016-12-12" shapes, bare versions, comma forms, trailing comma, "*", "===". Otherwise "no diff" could have meant an inert regex. PYTHON SIDE, over the production PyPI snapshot (2,804,136 distinct requirement strings): exactly 21 NEWLY REJECTED, 0 newly accepted. Every one is an operand whose version match is a proper prefix leaving a valid second token, so every one previously rendered a comma from nowhere -- "==0.1dev10.3", ">=9.0.0a1.0", "==1.1b02.1", ">=0.2.1dev4.dev0". Seven sampled forms confirmed REJECTED by pypa/packaging 26.2, so this closes a divergence rather than opening one. DELIBERATELY UNCHANGED: the empty operator stays, because a bare version is a valid constraint and PPM's /doc?version=<raw> passes one (narrowing it is rstudio/package-manager#18634); and a trailing comma is still tolerated, which upstream also accepts. One narrowing at a time is what keeps this measurable. ALSO FIXES FIVE TESTS THAT PINNED THE DEFECT. TestVersion_Check asserted ">= 1.0 != 1.3.4.* < 2.0" as valid. Measured: pypa/packaging 26.2 raises InvalidSpecifier on it and accepts the comma form. Rewritten to commas, which is what PEP 440 itself writes; each case's intent (the conjunction) is unchanged.⚠️ THE TWO ENTRY POINTS DIVERGE ON DASHES and a test that conflates them is wrong. newRSpecifiers rewrites "-" to "." BEFORE validating, so ">=0-12.1" is the single constraint ">=0.12.1" on the R path, while on the Python path PEP 440 reads "0-12" as "0.post12" and the trailing ".1" makes it an adjacent pair. Both halves are pinned. Also: Go permits a repeated capture-group name, so embedding the version pattern twice needs no name stripping. Verified rather than assumed -- it is the opposite of Python's rule, and assuming wrong would have meant a needless helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Completes the invented-comma fix from
0.3.0, which closed the tokenizer route and left this one. This is rstudio/package-manager#19466's F5 residue — the last open item from that audit.The defect
validConstraintRegexpended each repetition with\s*\,?, making the separator optional. Combined with the deliberately-admitted empty operator (a bare version is a valid constraint), adjacency did not merely parse — it rendered a comma the input never contained:A fabricated constraint boundary changes which versions match, and the publisher's intent is unrecoverable from the string.
validConstraintRegexpis shared withNewRSpecifiers, which PPM uses for R constraint parsing.Against a real full CRAN
PACKAGESfile:Independently corroborated over ~3.9M parenthesised constraint expressions extracted from production CRAN RSFs: zero with two operators, zero with an internal comma. Real CRAN ranges do exist, but are written as the same package repeated across a field (
R (>= 3.6.0), R (< 3.7.0)) — handled by PPM'ssrc/description, which never reachesNewRSpecifiers.A null result is only meaningful if the instrument works, so the change was verified to actually reject six adjacency shapes (
>= 7 < 10,>=1.0<2.0,>= 1.0 != 1.3.4.* < 2.0, …) and to leave twelve load-bearing forms untouched — R dash forms, the real CRAN1.1-18-1and2016-12-12shapes, bare versions, comma forms, trailing comma,*,===. Without that check, "no diff" could have meant an inert regex.Python side
Over the production PyPI snapshot, 2,804,136 distinct requirement strings: exactly 21 newly rejected, 0 newly accepted.
Every one is an operand whose version match is a proper prefix leaving a valid second token — so every one previously rendered a comma from nowhere:
Seven sampled forms confirmed rejected by
pypa/packaging26.2, so this closes a divergence rather than opening one. Controls (flask (>=3.0.0),urllib3 (>=1.26,<2.0),sgx-py (>=0.2.1.dev4)) all still accepted.Deliberately unchanged
GET …/doc?version=<raw>(optional auth) passes one through. Narrowing it is separately tracked in rstudio/package-manager#18634.>= 1.0,too.One narrowing at a time is what keeps the blast radius measurable.
Five tests pinned the defect
TestVersion_Checkasserted>= 1.0 != 1.3.4.* < 2.0as valid. Measured:pypa/packaging26.2 raisesInvalidSpecifieron it, and accepts the comma form. Rewritten to commas — which is what PEP 440 itself writes — with each case's intent (the conjunction) unchanged.newRSpecifiersrewrites-to.before validating, so>=0-12.1is the single constraint>=0.12.1on the R path — while on the Python path PEP 440 reads0-12as the post-release0.post12, and the trailing.1makes it an adjacent pair. A test asserting both paths reject it would have been wrong; both halves are now pinned.Sizing
Breaking. Narrows what parses, so per this module's policy it takes a minor bump (
0.4.0), not a patch. PPM pinsv0.1.1and imports onlygpp/license, so nothing reaches PPM until the cutover.Verification
go test ./...— all packages pass, includinginternal/proptest.gofmt -l .— empty.🤖 Generated with Claude Code