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.

Sign up for GitHub

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

Support version strings with commit hashes #8041

Merged
merged 4 commits into from
Nov 7, 2024
Merged

Support version strings with commit hashes #8041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a151847
Support version strings with commit hashes
emilk Nov 7, 2024
791299c
Store commit hash
emilk Nov 7, 2024
d95fba6
&self -> self
emilk Nov 7, 2024
b01e0d5
Cleanup
emilk Nov 7, 2024
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
18 changes: 14 additions & 4 deletions crates/build/re_build_info/src/build_info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ impl CrateVersion {
/// <name> <semver> [<rust_info>] <target> <branch> <commit> <build_date>
/// ```
pub fn try_parse_from_build_info_string(s: impl AsRef<str>) -> Result<Self, String> {
let s = s.as_ref();
// `CrateVersion::try_parse` is `const` (for good reasons), and needs a `&'static str`.
// In order to accomplish this, we need to leak the string here.
let s = Box::leak(s.as_ref().to_owned().into_boxed_str());

let parts = s.split_whitespace().collect::<Vec<_>>();
if parts.len() < 2 {
return Err(format!("{s:?} is not a valid BuildInfo string"));
Expand All @@ -160,7 +163,10 @@ fn crate_version_from_build_info_string() {
major: 0,
minor: 10,
patch: 0,
meta: Some(crate::crate_version::Meta::DevAlpha(7)),
meta: Some(crate::crate_version::Meta::DevAlpha {
alpha: 7,
commit: None,
}),
},
rustc_version: "1.76.0 (d5c2e9c34 2023-09-13)",
llvm_version: "16.0.5",
Expand All @@ -175,8 +181,12 @@ fn crate_version_from_build_info_string() {

{
let expected_crate_version = build_info.version;
let crate_version = CrateVersion::try_parse_from_build_info_string(build_info_str);
let crate_version = CrateVersion::try_parse_from_build_info_string(&build_info_str);

assert_eq!(Ok(expected_crate_version), crate_version);
assert_eq!(
crate_version,
Ok(expected_crate_version),
"Failed to parse {build_info_str:?}"
);
}
}
101 changes: 74 additions & 27 deletions crates/build/re_build_info/src/crate_version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ mod meta {
/// - `01NNNNNN` -> `-rc.N`
/// - `00000000` -> none of the above
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub struct CrateVersion {
pub major: u8,
pub minor: u8,
Expand Down Expand Up @@ -87,15 +86,34 @@ impl CrateVersion {
pub enum Meta {
Rc(u8),
Alpha(u8),
DevAlpha(u8),

/// `0.19.1-alpha.2+dev` or `0.19.1-alpha.2+aab0b4e`
DevAlpha {
alpha: u8,

/// The commit hash, if known
///
/// `None` corresponds to `+dev`.
///
/// In order to support compile-time parsing of versions strings
/// this needs to be `&'static` and `[u8]` instead of `String`.
/// But in practice this is guaranteed to be valid UTF-8.
///
/// The commit hash is NOT sent over the wire,
/// so `0.19.1-alpha.2+aab0b4e` will end up as `0.19.1-alpha.2+dev`
/// on the other end.
commit: Option<&'static [u8]>,
},
}

impl Meta {
pub const fn to_byte(self) -> u8 {
match self {
Self::Rc(value) => value | meta::RC,
Self::Alpha(value) => value | meta::ALPHA,
Self::DevAlpha(value) => value | meta::DEV_ALPHA,

// We ignore the commit hash, if any
Self::DevAlpha { alpha, .. } => alpha | meta::DEV_ALPHA,
}
}

Expand All @@ -105,7 +123,10 @@ impl Meta {
match tag {
meta::RC => Some(Self::Rc(value)),
meta::ALPHA => Some(Self::Alpha(value)),
meta::DEV_ALPHA => Some(Self::DevAlpha(value)),
meta::DEV_ALPHA => Some(Self::DevAlpha {
alpha: value,
commit: None,
}),
_ => None,
}
}
Expand Down Expand Up @@ -203,12 +224,12 @@ impl CrateVersion {
/// This is used to identify builds which are not explicit releases,
/// such as local builds and CI builds for every commit.
pub fn is_dev(&self) -> bool {
matches!(self.meta, Some(Meta::DevAlpha(..)))
matches!(self.meta, Some(Meta::DevAlpha { .. }))
}

/// Whether or not this is an alpha version (`-alpha.N` or `-alpha.N+dev`).
pub fn is_alpha(&self) -> bool {
matches!(self.meta, Some(Meta::Alpha(..) | Meta::DevAlpha(..)))
matches!(self.meta, Some(Meta::Alpha(..) | Meta::DevAlpha { .. }))
}

/// Whether or not this is a release candidate (`-rc.N`).
Expand Down Expand Up @@ -261,11 +282,13 @@ impl CrateVersion {
self.major == other.major
}
}
}

impl CrateVersion {
/// Parse a version string according to our subset of semver.
///
/// See [`CrateVersion`] for more information.
pub const fn parse(version_string: &str) -> Self {
pub const fn parse(version_string: &'static str) -> Self {
match Self::try_parse(version_string) {
Ok(version) => version,
Err(_err) => {
Expand All @@ -279,7 +302,7 @@ impl CrateVersion {
/// Parse a version string according to our subset of semver.
///
/// See [`CrateVersion`] for more information.
pub const fn try_parse(version_string: &str) -> Result<Self, &'static str> {
pub const fn try_parse(version_string: &'static str) -> Result<Self, &'static str> {
// Note that this is a const function, which means we are extremely limited in what we can do!

const fn maybe(s: &[u8], c: u8) -> (bool, &[u8]) {
Expand Down Expand Up @@ -312,20 +335,6 @@ impl CrateVersion {
}};
}

macro_rules! eat_token {
($s:ident, $token:expr, $msg:literal) => {{
let token = $token;
if token.len() > $s.len() {
return Err($msg);
}
let (left, right) = split_at($s, token.len());
if !equals(left, token) {
return Err($msg);
}
right
}};
}

macro_rules! eat_u8 {
($s:ident, $msg:literal) => {{
if $s.is_empty() {
Expand Down Expand Up @@ -393,8 +402,22 @@ impl CrateVersion {
s = remainder;
match meta {
Some(Meta::Alpha(build)) => {
s = eat_token!(s, b"dev", "expected `dev` after `+`");
meta = Some(Meta::DevAlpha(build));
if let (true, remainder) = maybe_token(s, b"dev") {
s = remainder;
meta = Some(Meta::DevAlpha {
alpha: build,
commit: None,
});
} else if s.is_empty() {
return Err("expected `dev` after `+`");
} else {
let commit_hash = s;
s = &[];
meta = Some(Meta::DevAlpha {
alpha: build,
commit: Some(commit_hash),
});
}
}
Some(..) => return Err("unexpected `-rc` with `+dev`"),
None => return Err("unexpected `+dev` without `-alpha`"),
Expand All @@ -419,7 +442,13 @@ impl std::fmt::Display for Meta {
match self {
Self::Rc(build) => write!(f, "-rc.{build}"),
Self::Alpha(build) => write!(f, "-alpha.{build}"),
Self::DevAlpha(build) => write!(f, "-alpha.{build}+dev"),
Self::DevAlpha { alpha, commit } => {
if let Some(commit) = commit.and_then(|s| std::str::from_utf8(s).ok()) {
write!(f, "-alpha.{alpha}+{commit}")
} else {
write!(f, "-alpha.{alpha}+dev")
}
}
}
}
}
Expand Down Expand Up @@ -477,7 +506,23 @@ fn test_parse_version() {
major: 12,
minor: 23,
patch: 24,
meta: Some(Meta::DevAlpha(63)),
meta: Some(Meta::DevAlpha {
alpha: 63,
commit: None
}),
}
);
// We use commit hash suffixes in some cases:
assert_parse_ok!(
"12.23.24-alpha.63+aab0b4e",
CrateVersion {
major: 12,
minor: 23,
patch: 24,
meta: Some(Meta::DevAlpha {
alpha: 63,
commit: Some(b"aab0b4e")
}),
}
);
}
Expand All @@ -491,6 +536,7 @@ fn test_format_parse_roundtrip() {
"12.23.24-rc.63",
"12.23.24-alpha.63",
"12.23.24-alpha.63+dev",
"12.23.24-alpha.63+aab0b4e",
] {
assert_eq!(CrateVersion::parse(version).to_string(), version);
}
Expand All @@ -505,6 +551,7 @@ fn test_format_parse_roundtrip_bytes() {
"12.23.24-rc.63",
"12.23.24-alpha.63",
"12.23.24-alpha.63+dev",
// "12.23.24-alpha.63+aab0b4e", // we don't serialize commit hashes!
] {
let version = CrateVersion::parse(version);
let bytes = version.to_bytes();
Expand All @@ -514,7 +561,7 @@ fn test_format_parse_roundtrip_bytes() {

#[test]
fn test_compatibility() {
fn are_compatible(a: &str, b: &str) -> bool {
fn are_compatible(a: &'static str, b: &'static str) -> bool {
CrateVersion::parse(a).is_compatible_with(CrateVersion::parse(b))
}

Expand Down
Loading