Merge uv-pubgrub into uv-pep440 #8669
Conversation
Enabled by #8667 and pubgrub-rs/pubgrub#262, we can remove the uv-pubgrub crate and move the conversion between pep440 specifiers and version ranges directly into pep440. In a next step, we can remove the `VersionRangesSpecifier` intermediary and perform the conversion directly.
| Self::InvalidTildeEquals(specifier) => { | ||
| write!( | ||
| f, | ||
| "The `~=` operator requires at least two release segments: `{specifier}`" |
There was a problem hiding this comment.
@BurntSushi Can this error ever happen / could we catch that in the parser and make the conversion infallible?
There was a problem hiding this comment.
You mean the parser for marker expressions? I think there are tests covering it?
uv/crates/uv-pep508/src/marker/tree.rs
Line 2053 in 36102db
Whether ~= is actually used in the real world, I'm not sure. I don't recall seeing it.
(I'm not 100% certain I've understood everything correctly here.)
Oh, I think I did misunderstand. Perhaps it's the combination of ~= and a lack of two release segments. Indeed, PEP 440 says:
This operator MUST NOT be used with a single segment version number such as ~=1.
So I guess we could indeed catch this at parse time.
A secondary problem is how to represent this in the type system. Otherwise you'll need a panicking branch here instead of an error. Which I think seems fine as long as we document our guarantees.
There was a problem hiding this comment.
Thanks!
I will document the invariant and change to to an infallible conversion.
With #8669 as preparation, we can remove what used to be the `PubGrubSpecifier` and instead implement a direct conversion from `VersionSpecifiers` to `Range<Version>`. Thanks to #8669 (comment), this conversion is infallible, removing a lot of fallibility downstream. The conversion takes an owned value, to signal that we consume the value. Previously, the function had cloned the version internally.
With #8669 as preparation, we can remove what used to be the `PubGrubSpecifier` and instead implement a direct conversion from `VersionSpecifiers` to `Range<Version>`. Thanks to #8669 (comment), this conversion is infallible, removing a lot of fallibility downstream. The conversion takes an owned value, to signal that we consume the value. Previously, the function had cloned the version internally.
With #8669 as preparation, we can remove what used to be the `PubGrubSpecifier` and instead implement a direct conversion from `VersionSpecifiers` to `Range<Version>`. Thanks to #8669 (comment), this conversion is infallible, removing a lot of fallibility downstream. The conversion takes an owned value, to signal that we consume the value. Previously, the function had cloned the version internally.
Enabled by #8667 and pubgrub-rs/pubgrub#262, we can remove the uv-pubgrub crate and move the conversion between pep440 specifiers and version ranges directly into pep440.
In a next step, we can remove the
VersionRangesSpecifierintermediary and perform the conversion directly.