Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make < exclusive for non-prerelease markers #1878

Merged
merged 7 commits into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 142 additions & 24 deletions crates/pep440-rs/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,19 @@ impl Version {
}
}

/// Returns the min-release part of this version, if it exists.
///
/// The "min" component is internal-only, and does not exist in PEP 440.
/// The version `1.0min0` is smaller than all other `1.0` versions,
/// like `1.0a1`, `1.0dev0`, etc.
#[inline]
pub fn min(&self) -> Option<u64> {
match *self.inner {
VersionInner::Small { ref small } => small.min(),
VersionInner::Full { ref full } => full.min,
}
}

/// Set the release numbers and return the updated version.
///
/// Usually one can just use `Version::new` to create a new version with
Expand Down Expand Up @@ -512,13 +525,30 @@ impl Version {
self
}

/// Set the min-release component and return the updated version.
///
/// The "min" component is internal-only, and does not exist in PEP 440.
/// The version `1.0min0` is smaller than all other `1.0` versions,
/// like `1.0a1`, `1.0dev0`, etc.
#[inline]
pub fn with_min(mut self, value: Option<u64>) -> Version {
if let VersionInner::Small { ref mut small } = Arc::make_mut(&mut self.inner) {
if small.set_min(value) {
return self;
}
}
self.make_full().min = value;
self
}

/// Convert this version to a "full" representation in-place and return a
/// mutable borrow to the full type.
fn make_full(&mut self) -> &mut VersionFull {
if let VersionInner::Small { ref small } = *self.inner {
let full = VersionFull {
epoch: small.epoch(),
release: small.release().to_vec(),
min: small.min(),
pre: small.pre(),
post: small.post(),
dev: small.dev(),
Expand Down Expand Up @@ -744,10 +774,13 @@ impl FromStr for Version {
/// * Bytes 5, 4 and 3 correspond to the second, third and fourth release
/// segments, respectively.
/// * Bytes 2, 1 and 0 represent *one* of the following:
/// `.devN, aN, bN, rcN, <no suffix>, .postN`. Its representation is thus:
/// `min, .devN, aN, bN, rcN, <no suffix>, .postN`.
/// Its representation is thus:
/// * The most significant 3 bits of Byte 2 corresponds to a value in
/// the range 0-5 inclusive, corresponding to dev, pre-a, pre-b, pre-rc,
/// no-suffix or post releases, respectively.
/// the range 0-6 inclusive, corresponding to min, dev, pre-a, pre-b, pre-rc,
/// no-suffix or post releases, respectively. `min` is a special version that
/// does not exist in PEP 440, but is used here to represent the smallest
/// possible version, preceding any `dev`, `pre`, `post` or releases.
/// * The low 5 bits combined with the bits in bytes 1 and 0 correspond
/// to the release number of the suffix, if one exists. If there is no
/// suffix, then this bits are always 0.
Expand Down Expand Up @@ -810,18 +843,19 @@ struct VersionSmall {
}

impl VersionSmall {
const SUFFIX_DEV: u64 = 0;
const SUFFIX_PRE_ALPHA: u64 = 1;
const SUFFIX_PRE_BETA: u64 = 2;
const SUFFIX_PRE_RC: u64 = 3;
const SUFFIX_NONE: u64 = 4;
const SUFFIX_POST: u64 = 5;
const SUFFIX_MIN: u64 = 0;
const SUFFIX_DEV: u64 = 1;
const SUFFIX_PRE_ALPHA: u64 = 2;
const SUFFIX_PRE_BETA: u64 = 3;
const SUFFIX_PRE_RC: u64 = 4;
const SUFFIX_NONE: u64 = 5;
const SUFFIX_POST: u64 = 6;
const SUFFIX_MAX_VERSION: u64 = 0x1FFFFF;

#[inline]
fn new() -> VersionSmall {
VersionSmall {
repr: 0x00000000_00800000,
repr: 0x00000000_00A00000,
release: [0, 0, 0, 0],
len: 0,
}
Expand Down Expand Up @@ -888,7 +922,7 @@ impl VersionSmall {

#[inline]
fn set_post(&mut self, value: Option<u64>) -> bool {
if self.pre().is_some() || self.dev().is_some() {
if self.min().is_some() || self.pre().is_some() || self.dev().is_some() {
return value.is_none();
}
match value {
Expand Down Expand Up @@ -931,7 +965,7 @@ impl VersionSmall {

#[inline]
fn set_pre(&mut self, value: Option<PreRelease>) -> bool {
if self.dev().is_some() || self.post().is_some() {
if self.min().is_some() || self.dev().is_some() || self.post().is_some() {
return value.is_none();
}
match value {
Expand Down Expand Up @@ -970,7 +1004,7 @@ impl VersionSmall {

#[inline]
fn set_dev(&mut self, value: Option<u64>) -> bool {
if self.pre().is_some() || self.post().is_some() {
if self.min().is_some() || self.pre().is_some() || self.post().is_some() {
return value.is_none();
}
match value {
Expand All @@ -988,6 +1022,35 @@ impl VersionSmall {
true
}

#[inline]
fn min(&self) -> Option<u64> {
if self.suffix_kind() == VersionSmall::SUFFIX_MIN {
Some(self.suffix_version())
} else {
None
}
}

#[inline]
fn set_min(&mut self, value: Option<u64>) -> bool {
if self.dev().is_some() || self.pre().is_some() || self.post().is_some() {
return value.is_none();
}
match value {
None => {
self.set_suffix_kind(VersionSmall::SUFFIX_NONE);
}
Some(number) => {
if number > VersionSmall::SUFFIX_MAX_VERSION {
return false;
}
self.set_suffix_kind(VersionSmall::SUFFIX_MIN);
self.set_suffix_version(number);
}
}
true
}

#[inline]
fn local(&self) -> &[LocalSegment] {
// A "small" version is never used if the version has a non-zero number
Expand Down Expand Up @@ -1079,6 +1142,10 @@ struct VersionFull {
/// > Local version labels have no specific semantics assigned, but
/// > some syntactic restrictions are imposed.
local: Vec<LocalSegment>,
/// An internal-only segment that does not exist in PEP 440, used to
/// represent the smallest possible version of a release, preceding any
/// `dev`, `pre`, `post` or releases.
min: Option<u64>,
}

/// A version number pattern.
Expand Down Expand Up @@ -1410,7 +1477,7 @@ impl<'a> Parser<'a> {
| (u64::from(release[1]) << 40)
| (u64::from(release[2]) << 32)
| (u64::from(release[3]) << 24)
| (0x80 << 16)
| (0xA0 << 16)
| (0x00 << 8)
| (0x00 << 0),
release: [
Expand Down Expand Up @@ -2243,9 +2310,9 @@ pub(crate) fn compare_release(this: &[u64], other: &[u64]) -> Ordering {
/// According to [a summary of permitted suffixes and relative
/// ordering][pep440-suffix-ordering] the order of pre/post-releases is: .devN,
/// aN, bN, rcN, <no suffix (final)>, .postN but also, you can have dev/post
/// releases on beta releases, so we make a three stage ordering: ({dev: 0, a:
/// 1, b: 2, rc: 3, (): 4, post: 5}, <preN>, <postN or None as smallest>, <devN
/// or Max as largest>, <local>)
/// releases on beta releases, so we make a three stage ordering: ({min: 0,
/// dev: 1, a: 2, b: 3, rc: 4, (): 5, post: 6}, <preN>, <postN or None as
/// smallest>, <devN or Max as largest>, <local>)
///
/// For post, any number is better than none (so None defaults to None<0),
/// but for dev, no number is better (so None default to the maximum). For
Expand All @@ -2254,9 +2321,11 @@ pub(crate) fn compare_release(this: &[u64], other: &[u64]) -> Ordering {
///
/// [pep440-suffix-ordering]: https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering
fn sortable_tuple(version: &Version) -> (u64, u64, Option<u64>, u64, &[LocalSegment]) {
match (version.pre(), version.post(), version.dev()) {
match (version.pre(), version.post(), version.dev(), version.min()) {
// min release
(_pre, post, _dev, Some(n)) => (0, 0, post, n, version.local()),
// dev release
(None, None, Some(n)) => (0, 0, None, n, version.local()),
(None, None, Some(n), None) => (1, 0, None, n, version.local()),
// alpha release
(
Some(PreRelease {
Expand All @@ -2265,7 +2334,8 @@ fn sortable_tuple(version: &Version) -> (u64, u64, Option<u64>, u64, &[LocalSegm
}),
post,
dev,
) => (1, n, post, dev.unwrap_or(u64::MAX), version.local()),
None,
) => (2, n, post, dev.unwrap_or(u64::MAX), version.local()),
// beta release
(
Some(PreRelease {
Expand All @@ -2274,7 +2344,8 @@ fn sortable_tuple(version: &Version) -> (u64, u64, Option<u64>, u64, &[LocalSegm
}),
post,
dev,
) => (2, n, post, dev.unwrap_or(u64::MAX), version.local()),
None,
) => (3, n, post, dev.unwrap_or(u64::MAX), version.local()),
// alpha release
(
Some(PreRelease {
Expand All @@ -2283,11 +2354,14 @@ fn sortable_tuple(version: &Version) -> (u64, u64, Option<u64>, u64, &[LocalSegm
}),
post,
dev,
) => (3, n, post, dev.unwrap_or(u64::MAX), version.local()),
None,
) => (4, n, post, dev.unwrap_or(u64::MAX), version.local()),
// final release
(None, None, None) => (4, 0, None, 0, version.local()),
(None, None, None, None) => (5, 0, None, 0, version.local()),
// post release
(None, Some(post), dev) => (5, 0, Some(post), dev.unwrap_or(u64::MAX), version.local()),
(None, Some(post), dev, None) => {
(6, 0, Some(post), dev.unwrap_or(u64::MAX), version.local())
}
}
}

Expand Down Expand Up @@ -3367,6 +3441,9 @@ mod tests {
])
);
assert_eq!(p(" \n5\n \t"), Version::new([5]));

// min tests
assert!(Parser::new("1.min0".as_bytes()).parse().is_err())
}

// Tests the error cases of our version parser.
Expand Down Expand Up @@ -3510,6 +3587,46 @@ mod tests {
}
}

#[test]
fn min_version() {
// Ensure that the `.min` suffix precedes all other suffixes.
let less = Version::new([1, 0]).with_min(Some(0));

let versions = &[
"1.dev0",
"1.0.dev456",
"1.0a1",
"1.0a2.dev456",
"1.0a12.dev456",
"1.0a12",
"1.0b1.dev456",
"1.0b2",
"1.0b2.post345.dev456",
"1.0b2.post345",
"1.0rc1.dev456",
"1.0rc1",
"1.0",
"1.0+abc.5",
"1.0+abc.7",
"1.0+5",
"1.0.post456.dev34",
"1.0.post456",
"1.0.15",
"1.1.dev1",
];

for greater in versions.iter() {
let greater = greater.parse::<Version>().unwrap();
assert_eq!(
less.cmp(&greater),
Ordering::Less,
"less: {:?}\ngreater: {:?}",
less.as_bloated_debug(),
greater.as_bloated_debug()
);
}
}

// Tests our bespoke u64 decimal integer parser.
#[test]
fn parse_number_u64() {
Expand Down Expand Up @@ -3577,6 +3694,7 @@ mod tests {
.field("post", &self.0.post())
.field("dev", &self.0.dev())
.field("local", &self.0.local())
.field("min", &self.0.min())
.finish()
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/uv-cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ impl CacheBucket {
CacheBucket::FlatIndex => "flat-index-v0",
CacheBucket::Git => "git-v0",
CacheBucket::Interpreter => "interpreter-v0",
CacheBucket::Simple => "simple-v2",
CacheBucket::Simple => "simple-v3",
CacheBucket::Wheels => "wheels-v0",
CacheBucket::Archive => "archive-v0",
}
Expand Down Expand Up @@ -677,13 +677,13 @@ impl ArchiveTimestamp {
}
}

impl std::cmp::PartialOrd for ArchiveTimestamp {
impl PartialOrd for ArchiveTimestamp {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.timestamp().cmp(&other.timestamp()))
}
}

impl std::cmp::Ord for ArchiveTimestamp {
impl Ord for ArchiveTimestamp {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.timestamp().cmp(&other.timestamp())
}
Expand Down
11 changes: 11 additions & 0 deletions crates/uv-resolver/src/prerelease_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,15 @@ impl PreReleaseStrategy {
),
}
}

/// Returns `true` if a [`PackageName`] is allowed to have pre-release versions.
pub(crate) fn allows(&self, package: &PackageName) -> bool {
match self {
Self::Disallow => false,
Self::Allow => true,
Self::IfNecessary => false,
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
Self::Explicit(packages) => packages.contains(package),
Self::IfNecessaryOrExplicit(packages) => packages.contains(package),
}
}
}
24 changes: 4 additions & 20 deletions crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use rustc_hash::FxHashMap;
use uv_normalize::PackageName;

use crate::candidate_selector::CandidateSelector;
use crate::prerelease_mode::PreReleaseStrategy;
use crate::python_requirement::PythonRequirement;
use crate::resolver::UnavailablePackage;

Expand Down Expand Up @@ -346,25 +345,10 @@ impl PubGrubReportFormatter<'_> {
) -> IndexSet<PubGrubHint> {
/// Returns `true` if pre-releases were allowed for a package.
fn allowed_prerelease(package: &PubGrubPackage, selector: &CandidateSelector) -> bool {
match selector.prerelease_strategy() {
PreReleaseStrategy::Disallow => false,
PreReleaseStrategy::Allow => true,
PreReleaseStrategy::IfNecessary => false,
PreReleaseStrategy::Explicit(packages) => {
if let PubGrubPackage::Package(package, ..) = package {
packages.contains(package)
} else {
false
}
}
PreReleaseStrategy::IfNecessaryOrExplicit(packages) => {
if let PubGrubPackage::Package(package, ..) = package {
packages.contains(package)
} else {
false
}
}
}
let PubGrubPackage::Package(package, ..) = package else {
return false;
};
selector.prerelease_strategy().allows(package)
}

let mut hints = IndexSet::default();
Expand Down
Loading