From e261cb3c27048001c6fb9acaf526464c529d1ef9 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 7 Nov 2024 22:56:17 +0000 Subject: [PATCH] chore: remove `git_version!` (#7270) Reading the Git version requires the entire Git repository to be present, including all tags. The tags are only created _after_ the artifact is being built, when we publish the release. Therefore, these tags are never included in the actual released binary. For Sentry, we use the `CARGO_PKG_VERSION` variable instead. This doesn't tell us whether somebody built a client from source and then used it so there could be some confusion in Sentry events. It is quite unlikely that this happens though so for the majority of Sentry alerts, this will give us the correct version. For the Android client, we also depend on the `GITHUB_SHA` env variable at compile-time. We do the same thing for the GUI client here. Resolves: #6925. --- rust/Cargo.lock | 21 ---------------- rust/bin-shared/Cargo.toml | 1 - rust/bin-shared/src/lib.rs | 23 ----------------- rust/gateway/src/main.rs | 2 +- rust/gui-client/src-common/src/controller.rs | 14 ++++++----- rust/gui-client/src-tauri/src/client.rs | 8 +++--- rust/gui-client/src-tauri/src/client/about.rs | 5 +--- rust/headless-client/src/ipc_service.rs | 25 ++++++++++++------- rust/headless-client/src/main.rs | 9 +++---- rust/telemetry/src/lib.rs | 4 +-- 10 files changed, 35 insertions(+), 77 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index ea7bae8ce..637095143 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1935,7 +1935,6 @@ dependencies = [ "clap", "firezone-logging", "futures", - "git-version", "hex-literal", "ip-packet", "ip_network", @@ -2641,26 +2640,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "git-version" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ad568aa3db0fcbc81f2f116137f263d7304f512a1209b35b85150d3ef88ad19" -dependencies = [ - "git-version-macro", -] - -[[package]] -name = "git-version-macro" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53010ccb100b96a67bc32c0175f0ed1426b31b655d562898e57325f81c023ac0" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.87", -] - [[package]] name = "glib" version = "0.18.5" diff --git a/rust/bin-shared/Cargo.toml b/rust/bin-shared/Cargo.toml index 6714827dc..f6fc89032 100644 --- a/rust/bin-shared/Cargo.toml +++ b/rust/bin-shared/Cargo.toml @@ -11,7 +11,6 @@ axum = { version = "0.7.7", default-features = false, features = ["http1", "toki clap = { version = "4.5.19", features = ["derive", "env"] } firezone-logging = { workspace = true } futures = "0.3" -git-version = "0.3.9" ip_network = { version = "0.4", default-features = false, features = ["serde"] } socket-factory = { workspace = true } thiserror = "1.0.68" diff --git a/rust/bin-shared/src/lib.rs b/rust/bin-shared/src/lib.rs index 089708fbb..508e933c0 100644 --- a/rust/bin-shared/src/lib.rs +++ b/rust/bin-shared/src/lib.rs @@ -43,26 +43,3 @@ pub use network_changes::{new_dns_notifier, new_network_notifier}; #[cfg(any(target_os = "linux", target_os = "windows"))] pub use tun_device_manager::TunDeviceManager; - -/// Output of `git describe` at compile time -/// e.g. `1.0.0-pre.4-20-ged5437c88-modified` where: -/// -/// * `1.0.0-pre.4` is the most recent ancestor tag -/// * `20` is the number of commits since then -/// * `g` doesn't mean anything -/// * `ed5437c88` is the Git commit hash -/// * `-modified` is present if the working dir has any changes from that commit number -#[macro_export] -macro_rules! git_version { - ($regex:literal) => { - $crate::__reexport::git_version!( - args = ["--always", "--dirty=-modified", "--tags", "--match", $regex], - fallback = env!("CARGO_PKG_VERSION") - ) - }; -} - -#[doc(hidden)] -pub mod __reexport { - pub use git_version::git_version; -} diff --git a/rust/gateway/src/main.rs b/rust/gateway/src/main.rs index df49d5b71..d5efba106 100644 --- a/rust/gateway/src/main.rs +++ b/rust/gateway/src/main.rs @@ -43,7 +43,7 @@ async fn main() { if cli.is_telemetry_allowed() { telemetry.start( cli.api_url.as_str(), - firezone_bin_shared::git_version!("gateway-*"), + env!("CARGO_PKG_VERSION"), firezone_telemetry::GATEWAY_DSN, ); } diff --git a/rust/gui-client/src-common/src/controller.rs b/rust/gui-client/src-common/src/controller.rs index 9de0b1e43..c6f687654 100644 --- a/rust/gui-client/src-common/src/controller.rs +++ b/rust/gui-client/src-common/src/controller.rs @@ -207,14 +207,16 @@ impl Controller { pub async fn main_loop(mut self) -> Result<(), Error> { // Start telemetry { + const VERSION: &str = env!("CARGO_PKG_VERSION"); + let environment = self.advanced_settings.api_url.to_string(); - self.telemetry.start( - &environment, - firezone_bin_shared::git_version!("gui-client-*"), - firezone_telemetry::GUI_DSN, - ); + self.telemetry + .start(&environment, VERSION, firezone_telemetry::GUI_DSN); self.ipc_client - .send_msg(&IpcClientMsg::StartTelemetry { environment }) + .send_msg(&IpcClientMsg::StartTelemetry { + environment, + version: VERSION.to_owned(), + }) .await?; } diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index e0ca3d25b..09db91962 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -64,7 +64,7 @@ pub(crate) fn run() -> Result<()> { let telemetry = telemetry::Telemetry::default(); telemetry.start( settings.api_url.as_ref(), - firezone_bin_shared::git_version!("gui-client-*"), + env!("CARGO_PKG_VERSION"), telemetry::GUI_DSN, ); // Don't fix the log filter for smoke tests @@ -96,7 +96,7 @@ fn run_gui(cli: Cli) -> Result<()> { // In the future telemetry will be opt-in per organization, that's why this isn't just at the top of `main` telemetry.start( settings.api_url.as_ref(), - firezone_bin_shared::git_version!("gui-client-*"), + env!("CARGO_PKG_VERSION"), telemetry::GUI_DSN, ); // Get the device ID before starting Tokio, so that all the worker threads will inherit the correct scope. @@ -156,13 +156,12 @@ fn fix_log_filter(settings: &mut AdvancedSettings) -> Result<()> { /// Don't drop the log handle or logging will stop. fn start_logging(directives: &str) -> Result { let logging_handles = common::logging::setup(directives)?; - let git_version = firezone_bin_shared::git_version!("gui-client-*"); let system_uptime_seconds = firezone_headless_client::uptime::get().map(|dur| dur.as_secs()); tracing::info!( arch = std::env::consts::ARCH, os = std::env::consts::OS, + version = env!("CARGO_PKG_VERSION"), ?directives, - ?git_version, ?system_uptime_seconds, "`gui-client` started logging" ); @@ -171,7 +170,6 @@ fn start_logging(directives: &str) -> Result { category: None, data: BTreeMap::from([ ("directives".into(), directives.into()), - ("git_version".into(), git_version.into()), ("system_uptime_seconds".into(), system_uptime_seconds.into()), ]), ..Default::default() diff --git a/rust/gui-client/src-tauri/src/client/about.rs b/rust/gui-client/src-tauri/src/client/about.rs index 232f73ef5..dc88641c5 100644 --- a/rust/gui-client/src-tauri/src/client/about.rs +++ b/rust/gui-client/src-tauri/src/client/about.rs @@ -7,7 +7,7 @@ pub(crate) fn get_cargo_version() -> String { #[tauri::command] pub(crate) fn get_git_version() -> String { - firezone_bin_shared::git_version!("gui-client-*").to_string() + option_env!("GITHUB_SHA").unwrap_or("unknown").to_owned() } #[cfg(test)] @@ -15,12 +15,9 @@ mod tests { #[test] fn version() { let cargo = super::get_cargo_version(); - let git = super::get_git_version(); assert!(cargo != "Unknown", "{}", cargo); assert!(cargo.starts_with("1.")); - assert!(git != "Unknown", "{}", git); assert!(cargo.len() >= 2, "{}", cargo); - assert!(git.len() >= 6, "{}", git); } } diff --git a/rust/headless-client/src/ipc_service.rs b/rust/headless-client/src/ipc_service.rs index 28468261f..4585818b8 100644 --- a/rust/headless-client/src/ipc_service.rs +++ b/rust/headless-client/src/ipc_service.rs @@ -84,13 +84,19 @@ impl Default for Cmd { #[derive(Debug, PartialEq, Deserialize, Serialize)] pub enum ClientMsg { ClearLogs, - Connect { api_url: String, token: String }, + Connect { + api_url: String, + token: String, + }, Disconnect, ReloadLogFilter, Reset, SetDns(Vec), SetDisabledResources(BTreeSet), - StartTelemetry { environment: String }, + StartTelemetry { + environment: String, + version: String, + }, StopTelemetry, } @@ -155,7 +161,7 @@ fn run_debug_ipc_service(cli: Cli) -> Result<()> { let log_filter_reloader = crate::setup_stdout_logging()?; tracing::info!( arch = std::env::consts::ARCH, - git_version = firezone_bin_shared::git_version!("gui-client-*"), + // version = env!("CARGO_PKG_VERSION"), TODO: Fix once `ipc_service` is moved to `gui-client`. system_uptime_seconds = crate::uptime::get().map(|dur| dur.as_secs()), ); if !platform::elevation_check()? { @@ -540,11 +546,12 @@ impl<'a> Handler<'a> { session.connlib.set_disabled_resources(disabled_resources); } - ClientMsg::StartTelemetry { environment } => self.telemetry.start( - &environment, - firezone_bin_shared::git_version!("gui-client-*"), - firezone_telemetry::IPC_SERVICE_DSN, - ), + ClientMsg::StartTelemetry { + environment, + version, + } => self + .telemetry + .start(&environment, &version, firezone_telemetry::IPC_SERVICE_DSN), ClientMsg::StopTelemetry => { self.telemetry.stop().await; } @@ -643,7 +650,7 @@ fn setup_logging( set_global_default(subscriber).context("`set_global_default` should always work)")?; tracing::info!( arch = std::env::consts::ARCH, - git_version = firezone_bin_shared::git_version!("gui-client-*"), + // version = env!("CARGO_PKG_VERSION"), TODO: Fix once `ipc_service` is moved to `gui-client`. system_uptime_seconds = crate::uptime::get().map(|dur| dur.as_secs()), ?directives ); diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index 68e2a238b..aa2042918 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -104,6 +104,8 @@ enum Cmd { Standalone, } +const VERSION: &str = env!("CARGO_PKG_VERSION"); + fn main() -> Result<()> { rustls::crypto::ring::default_provider() .install_default() @@ -130,7 +132,7 @@ fn main() -> Result<()> { let telemetry = Telemetry::default(); telemetry.start( cli.api_url.as_ref(), - firezone_bin_shared::git_version!("headless-client-*"), + VERSION, firezone_telemetry::HEADLESS_DSN, ); @@ -154,10 +156,7 @@ fn main() -> Result<()> { .unzip(); firezone_logging::setup_global_subscriber(layer); - tracing::info!( - arch = std::env::consts::ARCH, - git_version = firezone_bin_shared::git_version!("headless-client-*") - ); + tracing::info!(arch = std::env::consts::ARCH, version = VERSION); let rt = tokio::runtime::Builder::new_current_thread() .enable_all() diff --git a/rust/telemetry/src/lib.rs b/rust/telemetry/src/lib.rs index 7f9730110..6501d828b 100644 --- a/rust/telemetry/src/lib.rs +++ b/rust/telemetry/src/lib.rs @@ -48,7 +48,7 @@ impl Clone for Telemetry { } impl Telemetry { - pub fn start(&self, api_url: &str, release: &'static str, dsn: Dsn) { + pub fn start(&self, api_url: &str, release: &str, dsn: Dsn) { // Since it's `arc_swap` and not `Option`, there is a TOCTOU here, // but in practice it should never trigger if self.inner.load().is_some() { @@ -70,7 +70,7 @@ impl Telemetry { sentry::ClientOptions { environment: Some(environment.into()), // We can't get the release number ourselves because we don't know if we're embedded in a GUI Client or a Headless Client. - release: Some(release.into()), + release: Some(release.to_owned().into()), // We submit all spans but only send the ones with `target: telemetry`. // Those spans are created further down and are throttled at creation time to save CPU. traces_sample_rate: 1.0,