diff --git a/crates/fj-app/Cargo.toml b/crates/fj-app/Cargo.toml index dd3931a51..59c7bf3de 100644 --- a/crates/fj-app/Cargo.toml +++ b/crates/fj-app/Cargo.toml @@ -32,7 +32,7 @@ fj-window.workspace = true [dependencies.clap] version = "4.0.23" -features = ["derive"] +features = ["derive", "string"] [dependencies.figment] version = "0.10.8" diff --git a/crates/fj-app/src/args.rs b/crates/fj-app/src/args.rs index ecaaeff36..4ee2dc0e5 100644 --- a/crates/fj-app/src/args.rs +++ b/crates/fj-app/src/args.rs @@ -7,7 +7,7 @@ use fj_math::Scalar; /// Fornjot - Experimental CAD System #[derive(clap::Parser)] -#[command(version = fj::version::VERSION_FULL)] +#[command(version = fj::version::VERSION_FULL.to_string())] pub struct Args { /// The model to open pub model: Option, diff --git a/crates/fj-app/src/model_crate.rs b/crates/fj-app/src/model_crate.rs index 2c699eb2a..bbfbc9797 100644 --- a/crates/fj-app/src/model_crate.rs +++ b/crates/fj-app/src/model_crate.rs @@ -27,7 +27,8 @@ fn postprocess_model_files( ), ( r#"path = "../../crates/fj""#.to_owned(), - ["version = \"", fj::version::VERSION_PKG, "\""].concat(), + ["version = \"", &fj::version::VERSION_PKG.to_string(), "\""] + .concat(), ), ], )?; diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index 838e80b54..4308a9256 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -5,8 +5,8 @@ use std::{ str, }; -use fj::{abi, version::RawVersion}; -use tracing::warn; +use fj::{abi, version::Version}; +use tracing::{debug, warn}; use crate::{platform::HostPlatform, Parameters}; @@ -111,30 +111,40 @@ impl Model { https://github.com/hannobraun/Fornjot/issues/1307)" ); } else { - let version_pkg: libloading::Symbol RawVersion> = - lib.get(b"version_pkg").map_err(Error::LoadingVersion)?; + let version_pkg_host = fj::version::VERSION_PKG.to_string(); - let version_pkg = version_pkg().to_string(); - if fj::version::VERSION_PKG != version_pkg { - let host = String::from_utf8_lossy( - fj::version::VERSION_PKG.as_bytes(), - ) - .into_owned(); - let model = version_pkg; + let version_pkg_model: libloading::Symbol<*const Version> = + lib.get(b"VERSION_PKG").map_err(Error::LoadingVersion)?; + let version_pkg_mode = (**version_pkg_model).to_string(); + + debug!( + "Comparing package versions (host: {}, model: {})", + version_pkg_host, version_pkg_mode + ); + if version_pkg_host != version_pkg_mode { + let host = + String::from_utf8_lossy(version_pkg_host.as_bytes()) + .into_owned(); + let model = version_pkg_mode; return Err(Error::VersionMismatch { host, model }); } - let version_full: libloading::Symbol RawVersion> = - lib.get(b"version_full").map_err(Error::LoadingVersion)?; + let version_full_host = fj::version::VERSION_FULL.to_string(); - let version_full = version_full().to_string(); - if fj::version::VERSION_FULL != version_full { - let host = String::from_utf8_lossy( - fj::version::VERSION_FULL.as_bytes(), - ) - .into_owned(); - let model = version_full; + let version_full_model: libloading::Symbol<*const Version> = + lib.get(b"VERSION_FULL").map_err(Error::LoadingVersion)?; + let version_full_model = (**version_full_model).to_string(); + + debug!( + "Comparing full versions (host: {}, model: {})", + version_full_host, version_full_model + ); + if version_full_host != version_full_model { + let host = + String::from_utf8_lossy(version_full_host.as_bytes()) + .into_owned(); + let model = version_full_model; warn!("{}", Error::VersionMismatch { host, model }); } diff --git a/crates/fj/src/version.rs b/crates/fj/src/version.rs index 0d81ede60..23ebaf389 100644 --- a/crates/fj/src/version.rs +++ b/crates/fj/src/version.rs @@ -1,6 +1,6 @@ //! API for checking compatibility between the Fornjot app and a model -use core::slice; +use std::{fmt, slice}; /// The Fornjot package version /// @@ -11,53 +11,58 @@ use core::slice; /// constant between releases, even though changes are made throughout. A match /// of this version does not conclusively determine that the app and a model are /// compatible. -pub static VERSION_PKG: &str = env!("FJ_VERSION_PKG"); +#[no_mangle] +pub static VERSION_PKG: Version = + Version::from_static_str(env!("FJ_VERSION_PKG")); /// The full Fornjot version /// /// Can be used to check for compatibility between a model and the Fornjot app /// that runs it. -pub static VERSION_FULL: &str = env!("FJ_VERSION_FULL"); +#[no_mangle] +pub static VERSION_FULL: Version = + Version::from_static_str(env!("FJ_VERSION_FULL")); /// C-ABI-compatible representation of a version /// /// Used by the Fornjot application to check for compatibility between a model /// and the app. +#[derive(Clone, Copy, Debug)] #[repr(C)] -pub struct RawVersion { - /// The pointer to the `str` - pub ptr: *const u8, - - /// The length of the `str` - pub len: usize, +pub struct Version { + ptr: *const u8, + len: usize, } -impl RawVersion { - /// Convert the `RawVersion` into a string - /// - /// # Safety - /// - /// Must be a `RawVersion` returned from one of the hidden version functions - /// in this module. - #[allow(clippy::inherent_to_string)] - pub unsafe fn to_string(&self) -> String { - let slice = slice::from_raw_parts(self.ptr, self.len); - String::from_utf8_lossy(slice).into_owned() +impl Version { + const fn from_static_str(s: &'static str) -> Self { + Self { + ptr: s.as_ptr(), + len: s.len(), + } } } -#[no_mangle] -extern "C" fn version_pkg() -> RawVersion { - RawVersion { - ptr: VERSION_PKG.as_ptr(), - len: VERSION_PKG.len(), - } -} +impl fmt::Display for Version { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // This is sound. We only ever create `ptr` and `len` from static + // strings. + let slice = unsafe { slice::from_raw_parts(self.ptr, self.len) }; -#[no_mangle] -extern "C" fn version_full() -> RawVersion { - RawVersion { - ptr: VERSION_FULL.as_ptr(), - len: VERSION_FULL.len(), + write!(f, "{}", String::from_utf8_lossy(slice).into_owned()) } } + +// The only reason this is not derived automatically, is that `Version` contains +// a `*const u8`. `Version` can still safely be `Send`, for the following +// reasons: +// - The field is private, and no code in this module uses it for any write +// access, un-synchronized or not. +// - `Version` can only be constructed from strings with a static lifetime, so +// it's guaranteed that the pointer is valid over the whole lifetime of the +// program. +unsafe impl Send for Version {} + +// There is no reason why a `&Version` wouldn't be `Send`, so per definition, +// `Version` can be `Sync`. +unsafe impl Sync for Version {}