From 943e7881a8955c2ba1506a2a1d9a38057612c0d9 Mon Sep 17 00:00:00 2001
From: Ed Morley <501702+edmorley@users.noreply.github.com>
Date: Thu, 28 Nov 2024 15:51:23 +0000
Subject: [PATCH] Make `extract_requirement_version` return an `Option`

Rust 1.83 added support for `Option::expect` in `const` contexts:
https://blog.rust-lang.org/2024/11/28/Rust-1.83.0.html

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.
---
 Cargo.toml                     |  2 +-
 src/packaging_tool_versions.rs | 27 ++++++++++++++++-----------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 97599dc..8741cd6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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
 
diff --git a/src/packaging_tool_versions.rs b/src/packaging_tool_versions.rs
index 9f8fa16..6705218 100644
--- a/src/packaging_tool_versions.rs
+++ b/src/packaging_tool_versions.rs
@@ -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)]
@@ -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);
     }
 }