Skip to content

Commit

Permalink
Make extract_requirement_version return an Option (#294)
Browse files Browse the repository at this point in the history
Rust 1.83 added support for `Option::expect` in `const` contexts:
https://blog.rust-lang.org/2024/11/28/Rust-1.83.0.html#stabilized-apis

This means we can now return an `Option` from
`extract_requirement_version` instead of panicing within it, which makes
it easier to test the function, and avoids the need to explain that the
panic inside the function was actually safe.
  • Loading branch information
edmorley authored Nov 29, 2024
1 parent 148ccb9 commit e4b4114
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "python-buildpack"
edition = "2021"
rust-version = "1.80"
rust-version = "1.83"
# Disable automatic integration test discovery, since we import them in main.rs (see comment there).
autotests = false

Expand Down
27 changes: 16 additions & 11 deletions src/packaging_tool_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,28 @@ use std::str;
// Each file must contain a single package specifier in the format `package==1.2.3`,
// from which we extract/validate the version substring at compile time.
pub(crate) const PIP_VERSION: &str =
extract_requirement_version(include_str!("../requirements/pip.txt"));
extract_requirement_version(include_str!("../requirements/pip.txt"))
.expect("pip.txt must contain 'pip==VERSION'");
pub(crate) const POETRY_VERSION: &str =
extract_requirement_version(include_str!("../requirements/poetry.txt"));
extract_requirement_version(include_str!("../requirements/poetry.txt"))
.expect("poetry.txt must contain 'poetry==VERSION'");

// Extract the version substring from an exact-version package specifier (such as `foo==1.2.3`).
// This function should only be used to extract the version constants from the buildpack's own
// requirements files, which are controlled by us and don't require a full PEP 508 version parser.
// Since this is a `const fn` we cannot use iterators, most methods on `str`, `Result::expect` etc.
const fn extract_requirement_version(requirement: &'static str) -> &'static str {
// Note: Since this is a `const fn` we cannot use iterators and most methods on `str` / `Result`.
const fn extract_requirement_version(requirement: &'static str) -> Option<&'static str> {
let mut bytes = requirement.as_bytes();
while let [_, rest @ ..] = bytes {
if let [b'=', b'=', version @ ..] = rest {
if let Ok(version) = str::from_utf8(version.trim_ascii()) {
return version;
return Some(version);
}
break;
}
bytes = rest;
}
// This is safe, since this function is only used at compile time.
panic!("Requirement must be in the format: 'package==X.Y.Z'");
None
}

#[cfg(test)]
Expand All @@ -33,13 +34,17 @@ mod tests {

#[test]
fn extract_requirement_version_valid() {
assert_eq!(extract_requirement_version("package==1.2.3"), "1.2.3");
assert_eq!(extract_requirement_version("\npackage == 0.12\n"), "0.12");
assert_eq!(extract_requirement_version("package==1.2.3"), Some("1.2.3"));
assert_eq!(
extract_requirement_version("\npackage == 0.12\n"),
Some("0.12")
);
}

#[test]
#[should_panic(expected = "Requirement must be in the format")]
fn extract_requirement_version_invalid() {
extract_requirement_version("package=<1.2.3");
assert_eq!(extract_requirement_version(""), None);
assert_eq!(extract_requirement_version("package"), None);
assert_eq!(extract_requirement_version("package=<1.2.3"), None);
}
}

0 comments on commit e4b4114

Please sign in to comment.