From a834b7787004b4fec5d53997f5d05b93f5b19cc2 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 7 Nov 2024 20:42:39 +0100 Subject: [PATCH] Support version strings with commit hashes (#8041) --- crates/build/re_build_info/src/build_info.rs | 18 +++- .../build/re_build_info/src/crate_version.rs | 101 +++++++++++++----- 2 files changed, 88 insertions(+), 31 deletions(-) diff --git a/crates/build/re_build_info/src/build_info.rs b/crates/build/re_build_info/src/build_info.rs index e81cc03dab01..de9167dd3c35 100644 --- a/crates/build/re_build_info/src/build_info.rs +++ b/crates/build/re_build_info/src/build_info.rs @@ -142,7 +142,10 @@ impl CrateVersion { /// [] /// ``` pub fn try_parse_from_build_info_string(s: impl AsRef) -> Result { - 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::>(); if parts.len() < 2 { return Err(format!("{s:?} is not a valid BuildInfo string")); @@ -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", @@ -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:?}" + ); } } diff --git a/crates/build/re_build_info/src/crate_version.rs b/crates/build/re_build_info/src/crate_version.rs index 0035a14404a0..a886c4c828b1 100644 --- a/crates/build/re_build_info/src/crate_version.rs +++ b/crates/build/re_build_info/src/crate_version.rs @@ -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, @@ -87,7 +86,24 @@ 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 { @@ -95,7 +111,9 @@ impl Meta { 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, } } @@ -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, } } @@ -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`). @@ -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) => { @@ -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 { + pub const fn try_parse(version_string: &'static str) -> Result { // 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]) { @@ -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() { @@ -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`"), @@ -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") + } + } } } } @@ -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") + }), } ); } @@ -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); } @@ -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(); @@ -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)) }