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

feat: refactor Version.bump() to accept bumping major/minor/patch/last #452

Merged
merged 19 commits into from
Jan 2, 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
3 changes: 2 additions & 1 deletion crates/rattler_conda_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ pub use repo_data::{
pub use repo_data_record::RepoDataRecord;
pub use run_export::RunExportKind;
pub use version::{
Component, ParseVersionError, ParseVersionErrorKind, StrictVersion, Version, VersionWithSource,
Component, ParseVersionError, ParseVersionErrorKind, StrictVersion, Version, VersionBumpError,
VersionBumpType, VersionWithSource,
};
pub use version_spec::VersionSpec;

Expand Down
39 changes: 39 additions & 0 deletions crates/rattler_conda_types/src/version/bump.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use thiserror::Error;

/// VersionBumpType is used to specify the type of bump to perform on a version.
#[derive(Clone)]
pub enum VersionBumpType {
/// Bump the major version number.
Major,
/// Bump the minor version number.
Minor,
/// Bump the patch version number.
Patch,
/// Bump the last version number.
Last,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a Segment(uint) type? So that we can explicitly select the segment to bump?

We might also consider that bumping non-existent segments should be possible. Usually, all non-existent segments are implicit .0, so bumping the patch of a 1.0 could also result in 1.0.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you add a Segment(uint) type? So that we can explicitly select the segment to bump?

Good idea. Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might also consider that bumping non-existent segments should be possible. Usually, all non-existent segments are implicit .0, so bumping the patch of a 1.0 could also result in 1.0.1.

Don't you prefer to instead fail, so it also builds incentive for the user to explicitly use x.x.x instead of x or x.x?

Unless you feel strongly here, I would prefer to let it for another PR (I can open a ticket if you want).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added support for VersionBumpType::Segment(i32) in 9283770 (rust side only).

/// Bump a given segment. If negative, count from the end.
Segment(i32),
}

/// VersionBumpError is used to specify the type of error that occurred when bumping a version.
#[derive(Error, Debug, PartialEq)]
pub enum VersionBumpError {
/// Cannot bump the major segment of a version with less than 1 segment.
#[error("cannot bump the major segment of a version with less than 1 segment")]
NoMajorSegment,
/// Cannot bump the minor segment of a version with less than 2 segments.
#[error("cannot bump the minor segment of a version with less than 2 segments")]
NoMinorSegment,
/// Cannot bump the patch segment of a version with less than 3 segments.
#[error("cannot bump the patch segment of a version with less than 3 segments")]
NoPatchSegment,
/// Cannot bump the last segment of a version with no segments.
#[error("cannot bump the last segment of a version with no segments")]
NoLastSegment,
/// Invalid segment index.
#[error("cannot bump the segment '{index:?}' of a version if it's not present")]
InvalidSegment {
/// The segment index that was attempted to be bumped.
index: i32,
},
}
249 changes: 235 additions & 14 deletions crates/rattler_conda_types/src/version/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ pub(crate) mod parse;
mod segment;
mod with_source;

pub(crate) mod bump;
pub use bump::{VersionBumpError, VersionBumpType};

use flags::Flags;
use segment::Segment;

Expand Down Expand Up @@ -236,8 +239,8 @@ impl Version {
})
}

/// Returns a new version where the last numerical segment of this version has been bumped.
pub fn bump(&self) -> Self {
/// Returns a new version after bumping it according to the specified bump type.
pub fn bump(&self, bump_type: VersionBumpType) -> Result<Self, VersionBumpError> {
let mut components = ComponentVec::new();
let mut segments = SegmentVec::new();
let mut flags = Flags::default();
Expand All @@ -248,6 +251,42 @@ impl Version {
flags = flags.with_has_epoch(true);
}

// Sanity check whether the version has enough segments for this bump type.
let segment_count = self.segment_count();
match bump_type {
hadim marked this conversation as resolved.
Show resolved Hide resolved
VersionBumpType::Major => {
if segment_count < 1 {
return Err(VersionBumpError::NoMajorSegment);
}
}
VersionBumpType::Minor => {
if segment_count < 2 {
return Err(VersionBumpError::NoMinorSegment);
}
}
VersionBumpType::Patch => {
if segment_count < 3 {
return Err(VersionBumpError::NoPatchSegment);
}
}
VersionBumpType::Last => {
if segment_count == 0 {
return Err(VersionBumpError::NoLastSegment);
}
}
VersionBumpType::Segment(index) => {
let uindex = if index < 0 {
segment_count as i32 + index
} else {
index
};

if uindex < 0 || uindex >= segment_count as i32 {
return Err(VersionBumpError::InvalidSegment { index });
}
}
}

// Copy over all the segments and bump the last segment.
let segment_count = self.segment_count();
for (idx, segment_iter) in self.segments().enumerate() {
Expand All @@ -256,14 +295,27 @@ impl Version {
let mut segment_components =
segment_iter.components().cloned().collect::<ComponentVec>();

// If this is the last segment of the version bump the last number. Each segment must at
// least start with a number so this should always work.
if idx == (segment_count - 1) {
// Determine whether this is the segment that needs to be bumped.
let is_segment_to_bump = match bump_type {
VersionBumpType::Major => idx == 0,
VersionBumpType::Minor => idx == 1,
VersionBumpType::Patch => idx == 2,
VersionBumpType::Last => idx == (segment_count - 1),
VersionBumpType::Segment(mut index_to_bump) => {
if index_to_bump < 0 {
index_to_bump += segment_count as i32;
}

idx == index_to_bump as usize
}
};

// Bump the segment if we need to. Each segment must at least start with a number so this should always work.
if is_segment_to_bump {
let last_numeral_component = segment_components
.iter_mut()
.filter_map(Component::as_number_mut)
.rev()
.next()
.next_back()
.expect("every segment must at least contain a single numeric component");
*last_numeral_component += 1;
}
Expand Down Expand Up @@ -299,11 +351,11 @@ impl Version {
.expect("this should never fail because no new segments are added");
}

Self {
Ok(Self {
components,
segments,
flags,
}
})
}

/// Returns the segments that belong the local part of the version.
Expand Down Expand Up @@ -1008,7 +1060,7 @@ mod test {

use rand::seq::SliceRandom;

use crate::version::StrictVersion;
use crate::version::{StrictVersion, VersionBumpError, VersionBumpType};

use super::Version;

Expand Down Expand Up @@ -1216,17 +1268,26 @@ mod test {
}

#[test]
fn bump() {
fn bump_last() {
assert_eq!(
Version::from_str("1.1").unwrap().bump(),
Version::from_str("1.1")
.unwrap()
.bump(VersionBumpType::Last)
.unwrap(),
Version::from_str("1.2").unwrap()
);
assert_eq!(
Version::from_str("1.1l").unwrap().bump(),
Version::from_str("1.1l")
.unwrap()
.bump(VersionBumpType::Last)
.unwrap(),
Version::from_str("1.2l").unwrap()
);
assert_eq!(
Version::from_str("5!1.alpha+3.4").unwrap().bump(),
Version::from_str("5!1.alpha+3.4")
.unwrap()
.bump(VersionBumpType::Last)
.unwrap(),
Version::from_str("5!1.1alpha+3.4").unwrap()
);
}
Expand Down Expand Up @@ -1361,4 +1422,164 @@ mod test {
Version::from_str("3!4.5a.6b").unwrap()
);
}

#[test]
fn bump_major() {
assert_eq!(
Version::from_str("1.1")
.unwrap()
.bump(VersionBumpType::Major)
.unwrap(),
Version::from_str("2.1").unwrap()
);
assert_eq!(
Version::from_str("2.1l")
.unwrap()
.bump(VersionBumpType::Major)
.unwrap(),
Version::from_str("3.1l").unwrap()
);
assert_eq!(
Version::from_str("5!1.alpha+3.4")
.unwrap()
.bump(VersionBumpType::Major)
.unwrap(),
Version::from_str("5!2.alpha+3.4").unwrap()
);
}

#[test]
fn bump_minor() {
assert_eq!(
Version::from_str("1.1")
.unwrap()
.bump(VersionBumpType::Minor)
.unwrap(),
Version::from_str("1.2").unwrap()
);
assert_eq!(
Version::from_str("2.1l")
.unwrap()
.bump(VersionBumpType::Minor)
.unwrap(),
Version::from_str("2.2l").unwrap()
);
assert_eq!(
Version::from_str("5!1.alpha+3.4")
.unwrap()
.bump(VersionBumpType::Minor)
.unwrap(),
Version::from_str("5!1.1alpha+3.4").unwrap()
);
}

#[test]
fn bump_minor_fail() {
let err = Version::from_str("1")
.unwrap()
.bump(VersionBumpType::Minor)
.unwrap_err();

assert_eq!(err, VersionBumpError::NoMinorSegment);
}

#[test]
fn bump_patch() {
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Patch)
.unwrap(),
Version::from_str("1.1.10").unwrap()
);
assert_eq!(
Version::from_str("2.1l.5alpha")
.unwrap()
.bump(VersionBumpType::Patch)
.unwrap(),
Version::from_str("2.1l.6alpha").unwrap()
);
assert_eq!(
Version::from_str("5!1.8.alpha+3.4")
.unwrap()
.bump(VersionBumpType::Patch)
.unwrap(),
Version::from_str("5!1.8.1alpha+3.4").unwrap()
);
}

#[test]
fn bump_patch_fail() {
let err = Version::from_str("1.3")
.unwrap()
.bump(VersionBumpType::Patch)
.unwrap_err();

assert_eq!(err, VersionBumpError::NoPatchSegment);
}

#[test]
fn bump_segment() {
// Positive index
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Segment(0))
.unwrap(),
Version::from_str("2.1.9").unwrap()
);
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Segment(1))
.unwrap(),
Version::from_str("1.2.9").unwrap()
);
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Segment(2))
.unwrap(),
Version::from_str("1.1.10").unwrap()
);
// Negative index
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Segment(-1))
.unwrap(),
Version::from_str("1.1.10").unwrap()
);
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Segment(-2))
.unwrap(),
Version::from_str("1.2.9").unwrap()
);
assert_eq!(
Version::from_str("1.1.9")
.unwrap()
.bump(VersionBumpType::Segment(-3))
.unwrap(),
Version::from_str("2.1.9").unwrap()
);
}

#[test]
fn bump_segment_fail() {
let err = Version::from_str("1.3")
.unwrap()
.bump(VersionBumpType::Segment(3))
.unwrap_err();

assert_eq!(err, VersionBumpError::InvalidSegment { index: 3 });

let err = Version::from_str("1.3")
.unwrap()
.bump(VersionBumpType::Segment(-3))
.unwrap_err();

assert_eq!(err, VersionBumpError::InvalidSegment { index: -3 });
}
}
4 changes: 3 additions & 1 deletion crates/rattler_conda_types/src/version/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ fn trailing_dash_underscore_parser(
dash_or_underscore: Option<char>,
) -> IResult<&str, (Option<Component>, Option<char>), ParseVersionErrorKind> {
// Parse a - or _. Return early if it cannot be found.
let (rest, Some(separator)) = opt(one_of::<_,_,(&str, ErrorKind)>("-_"))(input).map_err(|e| e.map(|(_, kind)| ParseVersionErrorKind::Nom(kind)))? else {
let (rest, Some(separator)) = opt(one_of::<_, _, (&str, ErrorKind)>("-_"))(input)
.map_err(|e| e.map(|(_, kind)| ParseVersionErrorKind::Nom(kind)))?
else {
return Ok((input, (None, dash_or_underscore)));
};

Expand Down
2 changes: 1 addition & 1 deletion crates/rattler_lock/src/conda.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl TryFrom<LockedDependency> for RepoDataRecord {
..
} = value;
let LockedDependencyKind::Conda(value) = specific else {
return Err(ConversionError::NotACondaRecord)
return Err(ConversionError::NotACondaRecord);
};

let version = version.parse()?;
Expand Down
Loading