Skip to content

Commit 1b30ed1

Browse files
authored
Extract/validate packaging tool versions at compile time (#241)
We store the versions of the packaging tools used by the buildpack (such as pip, setuptools and wheel) in requirements files in the repo, so that Dependabot can update them. Whilst we can easily include the contents of those files at compile time using `include_str!`, the buildpack actually needs the version substring rather than the whole requirement specifier (the `1.2.3` from `foo==1.2.3`). Until now this extraction/validation of this version substring has been performed at runtime, which required using `.expect()` to ignore the "technically fallible but never going to fail for users in practice" result. Now, we use a `const fn` so this extraction/validation can be performed at compile time, allowing us to define these pinned versions as actual `const`s. In addition to compile time checks being preferable to those at runtime, this change will allow us to simplify/remove the `PackagingToolVersions` struct in future PRs. The features we can use in `const fn`s are limited - we can't use: - most methods on `str` - iterators (including for loops) - most methods on `Option` or `Result`, including `expect` or `unwrap*` As such, we convert to bytes and use a while/match pattern to iterate through the string - similar to some of the Rust stdlib `const fn` implementations: https://github.com/rust-lang/rust/blob/6a2cd0d50c9b7e1243d948641758c76d1f22e25e/library/core/src/slice/ascii.rs#L127-L139 The minimum Rust version has also been bumped, since `str::trim_ascii` only became a `const fn` in Rust 1.80. GUS-W-16436670.
1 parent 92c77a6 commit 1b30ed1

File tree

2 files changed

+32
-48
lines changed

2 files changed

+32
-48
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[package]
22
name = "python-buildpack"
33
edition = "2021"
4-
rust-version = "1.79"
4+
rust-version = "1.80"
55
# Disable automatic integration test discovery, since we import them in main.rs (see comment there).
66
autotests = false
77

src/packaging_tool_versions.rs

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
use serde::{Deserialize, Serialize};
2+
use std::str;
23

3-
const PIP_REQUIREMENT: &str = include_str!("../requirements/pip.txt");
4-
const SETUPTOOLS_REQUIREMENT: &str = include_str!("../requirements/setuptools.txt");
5-
const WHEEL_REQUIREMENT: &str = include_str!("../requirements/wheel.txt");
4+
// We store these versions in requirements files so that Dependabot can update them.
5+
// Each file must contain a single package specifier in the format `package==1.2.3`,
6+
// from which we extract/validate the version substring at compile time.
7+
const PIP_VERSION: &str = extract_requirement_version(include_str!("../requirements/pip.txt"));
8+
const SETUPTOOLS_VERSION: &str =
9+
extract_requirement_version(include_str!("../requirements/setuptools.txt"));
10+
const WHEEL_VERSION: &str = extract_requirement_version(include_str!("../requirements/wheel.txt"));
611

712
/// The versions of various packaging tools used during the build.
813
/// These are always installed, and are independent of the chosen package manager.
@@ -19,67 +24,46 @@ pub(crate) struct PackagingToolVersions {
1924

2025
impl Default for PackagingToolVersions {
2126
fn default() -> Self {
22-
// These versions are effectively buildpack constants, however, we want Dependabot to be able
23-
// to update them, which requires that they be in requirements files. The requirements files
24-
// contain contents like `package==1.2.3` (and not just the package version) so we have to
25-
// extract the version substring from it. Ideally this would be done at compile time, however,
26-
// using const functions would require use of unsafe and lots of boilerplate, and using proc
27-
// macros would require the overhead of adding a separate crate. As such, it ends up being
28-
// simpler to extract the version substring at runtime. Extracting the version is technically
29-
// fallible, however, we control the buildpack requirements files, so if they are invalid it
30-
// can only ever be a buildpack bug, and not something a user would ever see given the unit
31-
// and integration tests. As such, it's safe to use `.expect()` here, and doing so saves us
32-
// from having to add user-facing error messages that users will never see.
3327
Self {
34-
pip_version: extract_requirement_version(PIP_REQUIREMENT)
35-
.expect("pip requirement file must contain a valid version"),
36-
setuptools_version: extract_requirement_version(SETUPTOOLS_REQUIREMENT)
37-
.expect("setuptools requirement file must contain a valid version"),
38-
wheel_version: extract_requirement_version(WHEEL_REQUIREMENT)
39-
.expect("wheel requirement file must contain a valid version"),
28+
pip_version: PIP_VERSION.to_string(),
29+
setuptools_version: SETUPTOOLS_VERSION.to_string(),
30+
wheel_version: WHEEL_VERSION.to_string(),
4031
}
4132
}
4233
}
4334

44-
/// Extract the version substring from an exact-version requirement specifier (such as `foo==1.2.3`).
45-
/// This function should only be used to extract the version constants from the buildpack's own
46-
/// requirements files, which are controlled by us and don't require a full PEP 508 version parser.
47-
fn extract_requirement_version(requirement: &str) -> Option<String> {
48-
match requirement.split("==").collect::<Vec<_>>().as_slice() {
49-
&[_, version] => Some(version.trim().to_string()),
50-
_ => None,
35+
// Extract the version substring from an exact-version package specifier (such as `foo==1.2.3`).
36+
// This function should only be used to extract the version constants from the buildpack's own
37+
// requirements files, which are controlled by us and don't require a full PEP 508 version parser.
38+
// Since this is a `const fn` we cannot use iterators, most methods on `str`, `Result::expect` etc.
39+
const fn extract_requirement_version(requirement: &'static str) -> &'static str {
40+
let mut bytes = requirement.as_bytes();
41+
while let [_, rest @ ..] = bytes {
42+
if let [b'=', b'=', version @ ..] = rest {
43+
if let Ok(version) = str::from_utf8(version.trim_ascii()) {
44+
return version;
45+
}
46+
break;
47+
}
48+
bytes = rest;
5149
}
50+
// This is safe, since this function is only used at compile time.
51+
panic!("Requirement must be in the format: 'package==X.Y.Z'");
5252
}
5353

5454
#[cfg(test)]
5555
mod tests {
5656
use super::*;
5757

58-
#[test]
59-
fn default_packaging_tool_versions() {
60-
// If the versions in the buildpack's `requirements/*.txt` files are invalid, this will panic.
61-
PackagingToolVersions::default();
62-
}
63-
6458
#[test]
6559
fn extract_requirement_version_valid() {
66-
assert_eq!(
67-
extract_requirement_version("some_package==1.2.3"),
68-
Some("1.2.3".to_string())
69-
);
70-
assert_eq!(
71-
extract_requirement_version("\nsome_package == 1.2.3\n"),
72-
Some("1.2.3".to_string())
73-
);
60+
assert_eq!(extract_requirement_version("package==1.2.3"), "1.2.3");
61+
assert_eq!(extract_requirement_version("\npackage == 0.12\n"), "0.12");
7462
}
7563

7664
#[test]
65+
#[should_panic(expected = "Requirement must be in the format")]
7766
fn extract_requirement_version_invalid() {
78-
assert_eq!(extract_requirement_version("some_package"), None);
79-
assert_eq!(extract_requirement_version("some_package=<1.2.3"), None);
80-
assert_eq!(
81-
extract_requirement_version("some_package==1.2.3\nanother_package==4.5.6"),
82-
None
83-
);
67+
extract_requirement_version("package=<1.2.3");
8468
}
8569
}

0 commit comments

Comments
 (0)