Skip to content

Commit

Permalink
Support version strings with commit hashes (#8041)
Browse files Browse the repository at this point in the history
  • Loading branch information
@emilk
emilk authored Nov 7, 2024
1 parent a23e6d9 commit a834b77
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 31 deletions.
18 changes: 14 additions & 4 deletions crates/build/re_build_info/src/build_info.rs
View file
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
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

0 comments on commit a834b77

Please sign in to comment.