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

Extract/validate packaging tool versions at compile time #241

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Aug 7, 2024

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 the 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 consts. 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 in favour of referencing Rust constants.

The features we can use in const fns are limited - we can't use:

  • most methods on str, including split
  • 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.

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 (such as `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.
@edmorley edmorley added the skip changelog Skip the check-changelog check label Aug 7, 2024
@edmorley edmorley self-assigned this Aug 7, 2024
@edmorley edmorley marked this pull request as ready for review August 7, 2024 10:52
@edmorley edmorley requested a review from a team as a code owner August 7, 2024 10:52
@edmorley edmorley enabled auto-merge (squash) August 7, 2024 10:53
Copy link
Member

@joshwlewis joshwlewis left a comment

Choose a reason for hiding this comment

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

Neat!

@edmorley edmorley merged commit 1b30ed1 into main Aug 7, 2024
8 checks passed
@edmorley edmorley deleted the tool-versions-consts branch August 7, 2024 16:04
@edmorley edmorley added the internal Internal (non-user facing) buildpack changes label Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Internal (non-user facing) buildpack changes skip changelog Skip the check-changelog check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants