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

refactor: version parsing #240

Merged
merged 20 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from 9 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
7 changes: 6 additions & 1 deletion crates/rattler_conda_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,14 @@ glob = "0.3.1"

[dev-dependencies]
rand = "0.8.5"
insta = { version = "1.29.0", features = ["yaml", "redactions"] }
insta = { version = "1.29.0", features = ["yaml", "redactions", "toml"] }
rattler_package_streaming = { path = "../rattler_package_streaming", default-features = false, features=["rustls-tls"] }
tempfile = "3.6.0"
rstest = "0.17.0"
assert_matches = "1.5.0"
hex-literal = "0.4.1"
criterion = { version = "0.4", features = ["html_reports"] }

[[bench]]
name = "parse"
harness = false
14 changes: 14 additions & 0 deletions crates/rattler_conda_types/benches/parse.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use criterion::{black_box, criterion_group, criterion_main, Criterion};
use rattler_conda_types::Version;

fn criterion_benchmark(c: &mut Criterion) {
c.bench_function("parse simple version", |b| {
b.iter(|| black_box("3.11.4").parse::<Version>())
});
c.bench_function("parse complex version", |b| {
b.iter(|| black_box("1!1.0b2.post345.dev456+3.2.20.rc3").parse::<Version>())
});
}

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
Original file line number Diff line number Diff line change
Expand Up @@ -3106,7 +3106,7 @@ expression: "conda_lock.packages_for_platform(Platform::Linux64).collect::<Vec<_
manager: conda
platform: linux-64
dependencies:
certifi: ">=2020.06.20"
certifi: ">=2020.6.20"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened here? o.O

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's what I explained in:

As you review this PR you will notice that some version string representations have changed. This is because the string representation is now constructed from the version itself. However, some cases are very hard to reproduce, for instance, the number 06 is represented as 6 in our code. While reviewing, make sure you check these changes and decide if they make sense or not.

contourpy: ">=1.0.1"
cycler: ">=0.10"
fonttools: ">=4.22.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3129,7 +3129,7 @@ package:
manager: conda
platform: linux-64
dependencies:
certifi: ">=2020.06.20"
certifi: ">=2020.6.20"
contourpy: ">=1.0.1"
cycler: ">=0.10"
fonttools: ">=4.22.0"
Expand Down Expand Up @@ -5632,7 +5632,7 @@ package:
manager: conda
platform: linux-aarch64
dependencies:
certifi: ">=2020.06.20"
certifi: ">=2020.6.20"
contourpy: ">=1.0.1"
cycler: ">=0.10"
fonttools: ">=4.22.0"
Expand Down Expand Up @@ -8040,7 +8040,7 @@ package:
manager: conda
platform: linux-ppc64le
dependencies:
certifi: ">=2020.06.20"
certifi: ">=2020.6.20"
contourpy: ">=1.0.1"
cycler: ">=0.10"
fonttools: ">=4.22.0"
Expand Down Expand Up @@ -10512,7 +10512,7 @@ package:
platform: osx-64
dependencies:
__osx: ">=10.12"
certifi: ">=2020.06.20"
certifi: ">=2020.6.20"
contourpy: ">=1.0.1"
cycler: ">=0.10"
fonttools: ">=4.22.0"
Expand Down Expand Up @@ -12968,7 +12968,7 @@ package:
manager: conda
platform: osx-arm64
dependencies:
certifi: ">=2020.06.20"
certifi: ">=2020.6.20"
contourpy: ">=1.0.1"
cycler: ">=0.10"
fonttools: ">=4.22.0"
Expand Down
120 changes: 120 additions & 0 deletions crates/rattler_conda_types/src/version/flags.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
use std::fmt::{Debug, Formatter};

/// Bitmask indicates if the first component stored in a [`super::Version`] refers to an epoch.
const EPOCH_MASK: u8 = 0b1;
const EPOCH_OFFSET: u8 = 0;

/// Bitmask that indicates what the index is of the first segment that belongs to the local version
/// part. E.g. the part after the '+' sign in `1.2.3+4.5.6`.
const LOCAL_VERSION_MASK: u8 = (1 << 7) - 1;
const LOCAL_VERSION_OFFSET: u8 = 1;

/// Encodes several edge cases in a single byte.
///
/// The first bit is used to indicate whether or not there is an explicit epoch present in the
/// version. If the flag is set it means the first entry in the [`Version::components`] array refers
/// to the epoch instead of to the first component of the first segment.
///
/// The remaining bits are used to encode the index of the first segment that belongs to the local
/// version part instead of to the common part. A value of `0` indicates that there is not local
/// version part.
#[derive(Copy, Clone, Eq, PartialEq, Default)]
#[repr(transparent)]
pub struct Flags(u8);

impl Debug for Flags {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Flags")
.field("has_epoch", &self.has_epoch())
.field("local_segment_index", &self.local_segment_index())
.finish()
}
}

impl Flags {
/// Sets whether or not the version has an explicit epoch.
#[must_use]
pub fn with_has_epoch(self, has_epoch: bool) -> Self {
let flag = self.0 & !(EPOCH_MASK << EPOCH_OFFSET);
Self(
flag | if has_epoch {
EPOCH_MASK << EPOCH_OFFSET
} else {
0
},
)
}

/// Returns true if this instance indicates that an explicit epoch is present.
pub fn has_epoch(self) -> bool {
(self.0 >> EPOCH_OFFSET) & EPOCH_MASK != 0
}

/// Sets the index where the local segment starts. Returns `None` if the index is too large to
/// be stored.
#[must_use]
pub fn with_local_segment_index(self, index: u8) -> Option<Self> {
if index > LOCAL_VERSION_MASK {
None
} else {
Some(Self(
(self.0 & !(LOCAL_VERSION_MASK << LOCAL_VERSION_OFFSET))
| (index << LOCAL_VERSION_OFFSET),
))
}
}

/// Returns the index of the first segment that belongs to the local part of the version.
pub fn local_segment_index(self) -> u8 {
(self.0 >> LOCAL_VERSION_OFFSET) & LOCAL_VERSION_MASK
}
}

#[cfg(test)]
mod test {
use crate::version::flags::Flags;

#[test]
fn test_epoch() {
assert_eq!(Flags::default().has_epoch(), false);
assert_eq!(Flags::default().with_has_epoch(true).has_epoch(), true);
assert_eq!(
Flags::default()
.with_has_epoch(true)
.with_has_epoch(false)
.has_epoch(),
false
);
}

#[test]
fn test_local_segment_idx() {
assert_eq!(Flags::default().local_segment_index(), 0);
assert_eq!(
Flags::default()
.with_local_segment_index(42)
.unwrap()
.local_segment_index(),
42
);
assert_eq!(
Flags::default()
.with_local_segment_index(127)
.unwrap()
.local_segment_index(),
127
);
assert_eq!(Flags::default().with_local_segment_index(128), None);
}

#[test]
fn test_all_elements() {
let flags = Flags::default()
.with_has_epoch(true)
.with_local_segment_index(101)
.unwrap();

assert!(flags.has_epoch());
assert_eq!(flags.local_segment_index(), 101);
}
}
Loading