From 5edd32b3640080b8b6d402d075ecbd82ca109333 Mon Sep 17 00:00:00 2001 From: Rain Date: Sat, 28 Sep 2024 21:28:01 -0700 Subject: [PATCH] [cargo-nextest] make version computation less expensive Realized that os_info would do a ton of calls that aren't necessary, slowing down nextest quite measurably. It's hard to integrate lazy version evaluation into clap (https://github.com/clap-rs/clap/issues/5543) so let's just not print OS info for now. If we do want OS info in the future it'll probably be as part of a support tarball or similar. --- Cargo.lock | 12 --------- Cargo.toml | 1 - cargo-nextest/Cargo.toml | 3 +-- cargo-nextest/src/dispatch.rs | 11 ++++----- cargo-nextest/src/version.rs | 46 ++++++++++++++++++++++++++++------- workspace-hack/Cargo.toml | 6 ++--- 6 files changed, 46 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f526f31d5b8..099bef0ee9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -382,7 +382,6 @@ dependencies = [ "nextest-runner", "nextest-workspace-hack", "once_cell", - "os_info", "owo-colors 4.1.0", "pathdiff", "quick-junit", @@ -1926,17 +1925,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "os_info" -version = "3.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae99c7fa6dd38c7cafe1ec085e804f8f555a2f8659b0dbe03f1f9963a9b51092" -dependencies = [ - "log", - "serde", - "windows-sys 0.52.0", -] - [[package]] name = "os_pipe" version = "1.2.1" diff --git a/Cargo.toml b/Cargo.toml index 44c1617ea56..e26c7c6d7d6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,7 +81,6 @@ nextest-metadata = { version = "0.12.1", path = "nextest-metadata" } nextest-workspace-hack = "0.1.0" nix = { version = "0.29.0", default-features = false, features = ["signal"] } once_cell = "1.19.0" -os_info = "3.8.2" owo-colors = "4.1.0" pathdiff = { version = "0.2.1", features = ["camino"] } pin-project-lite = "0.2.14" diff --git a/cargo-nextest/Cargo.toml b/cargo-nextest/Cargo.toml index 17124a9a94e..5d7bb362d81 100644 --- a/cargo-nextest/Cargo.toml +++ b/cargo-nextest/Cargo.toml @@ -14,7 +14,7 @@ rust-version.workspace = true [dependencies] camino.workspace = true cfg-if.workspace = true -clap = { workspace = true, features = ["derive", "env", "string", "unicode", "wrap_help"] } +clap = { workspace = true, features = ["derive", "env", "unicode", "wrap_help"] } color-eyre.workspace = true dialoguer.workspace = true duct.workspace = true @@ -31,7 +31,6 @@ nextest-metadata = { version = "=0.12.1", path = "../nextest-metadata" } nextest-runner = { version = "=0.61.0", path = "../nextest-runner" } nextest-workspace-hack.workspace = true once_cell.workspace = true -os_info.workspace = true owo-colors.workspace = true pathdiff.workspace = true quick-junit.workspace = true diff --git a/cargo-nextest/src/dispatch.rs b/cargo-nextest/src/dispatch.rs index ac0e77d6da6..ae146965f2f 100644 --- a/cargo-nextest/src/dispatch.rs +++ b/cargo-nextest/src/dispatch.rs @@ -5,8 +5,7 @@ use crate::{ cargo_cli::{CargoCli, CargoOptions}, output::{should_redact, OutputContext, OutputOpts, OutputWriter, StderrStyles}, reuse_build::{make_path_mapper, ArchiveFormatOpt, ReuseBuildOpts}, - version::VersionInfo, - ExpectedError, Result, ReuseBuildKind, + version, ExpectedError, Result, ReuseBuildKind, }; use camino::{Utf8Path, Utf8PathBuf}; use clap::{builder::BoolishValueParser, ArgAction, Args, Parser, Subcommand, ValueEnum}; @@ -66,8 +65,8 @@ use swrite::{swrite, SWrite}; /// this message will not be seen), not `cargo-nextest`. #[derive(Debug, Parser)] #[command( - version = VersionInfo::new().to_short_string(), - long_version = VersionInfo::new().to_long_string(), + version = version::short(), + long_version = version::long(), bin_name = "cargo", styles = crate::output::clap_styles::style(), max_term_width = 100, @@ -122,8 +121,8 @@ enum NextestSubcommand { #[derive(Debug, Args)] #[clap( - version = VersionInfo::new().to_short_string(), - long_version = VersionInfo::new().to_long_string(), + version = version::short(), + long_version = version::long(), display_name = "cargo-nextest", )] struct AppOpts { diff --git a/cargo-nextest/src/version.rs b/cargo-nextest/src/version.rs index c129d4cd5ff..c4135ff5b96 100644 --- a/cargo-nextest/src/version.rs +++ b/cargo-nextest/src/version.rs @@ -1,27 +1,49 @@ // Copyright (c) The nextest Contributors // SPDX-License-Identifier: MIT OR Apache-2.0 +use once_cell::sync::Lazy; use std::fmt::Write; -pub(crate) struct VersionInfo { +pub(crate) fn short() -> &'static str { + &VERSION_INFO.short +} + +pub(crate) fn long() -> &'static str { + &VERSION_INFO.long +} + +// All the data here is static so we can use a singleton. +static VERSION_INFO: Lazy = Lazy::new(|| { + let inner = VersionInfoInner::new(); + let short = inner.to_short(); + let long = inner.to_long(); + VersionInfo { short, long } +}); + +struct VersionInfo { + short: String, + long: String, +} + +struct VersionInfoInner { /// Nextest's version. version: &'static str, - /// Information about the Git repository that nextest was built from. + /// Information about the repository that nextest was built from. /// - /// `None` if the Git repository information is not available. + /// `None` if the repository information is not available. commit_info: Option, } -impl VersionInfo { - pub(crate) const fn new() -> Self { +impl VersionInfoInner { + const fn new() -> Self { Self { version: env!("CARGO_PKG_VERSION"), commit_info: CommitInfo::from_env(), } } - pub(crate) fn to_short_string(&self) -> String { + pub(crate) fn to_short(&self) -> String { let mut s = self.version.to_string(); if let Some(commit_info) = &self.commit_info { @@ -36,8 +58,8 @@ impl VersionInfo { s } - pub(crate) fn to_long_string(&self) -> String { - let mut s = self.to_short_string(); + pub(crate) fn to_long(&self) -> String { + let mut s = self.to_short(); write!(s, "\nrelease: {}", self.version).unwrap(); if let Some(commit_info) = &self.commit_info { @@ -45,7 +67,13 @@ impl VersionInfo { write!(s, "\ncommit-date: {}", commit_info.commit_date).unwrap(); } write!(s, "\nhost: {}", env!("NEXTEST_BUILD_HOST_TARGET")).unwrap(); - write!(s, "\nos: {}", os_info::get()).unwrap(); + + // rustc and cargo's version also prints host info here. Unfortunately, clap 4.0's version + // support only supports a static string rather than a callback, so version info computation + // can't be deferred. OS info is also quite expensive to compute. + // + // For now, just don't do this -- everything else can be statically computed. We'll probably + // add a dedicated command to collect support data if it's necessary. s } diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 06dc562e6c4..5c86807876c 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -17,8 +17,8 @@ publish = false [dependencies] backtrace = { version = "0.3.71", features = ["gimli-symbolize"] } camino = { version = "1.1.9", default-features = false, features = ["serde1"] } -clap = { version = "4.5.18", features = ["derive", "env", "string", "unicode", "wrap_help"] } -clap_builder = { version = "4.5.18", default-features = false, features = ["color", "env", "std", "string", "suggestions", "unicode", "usage", "wrap_help"] } +clap = { version = "4.5.18", features = ["derive", "env", "unicode", "wrap_help"] } +clap_builder = { version = "4.5.18", default-features = false, features = ["color", "env", "std", "suggestions", "unicode", "usage", "wrap_help"] } console = { version = "0.15.8" } either = { version = "1.13.0" } getrandom = { version = "0.2.15", default-features = false, features = ["std"] } @@ -74,6 +74,6 @@ futures-sink = { version = "0.3.30", default-features = false, features = ["std" smallvec = { version = "1.13.2", default-features = false, features = ["const_new"] } tokio = { version = "1.40.0", default-features = false, features = ["io-std", "net"] } windows-sys-73dcd821b1037cfd = { package = "windows-sys", version = "0.59.0", features = ["Win32_Globalization", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Console", "Win32_System_JobObjects", "Win32_System_Pipes", "Win32_System_Threading"] } -windows-sys-b21d60becc0929df = { package = "windows-sys", version = "0.52.0", features = ["Wdk_Foundation", "Wdk_Storage_FileSystem", "Wdk_System_IO", "Win32_Foundation", "Win32_Networking_WinSock", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Com", "Win32_System_Console", "Win32_System_Environment", "Win32_System_IO", "Win32_System_LibraryLoader", "Win32_System_Memory", "Win32_System_Pipes", "Win32_System_Registry", "Win32_System_SystemInformation", "Win32_System_SystemServices", "Win32_System_Threading", "Win32_System_WindowsProgramming", "Win32_UI_Input_KeyboardAndMouse", "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging"] } +windows-sys-b21d60becc0929df = { package = "windows-sys", version = "0.52.0", features = ["Wdk_Foundation", "Wdk_Storage_FileSystem", "Wdk_System_IO", "Win32_Foundation", "Win32_Networking_WinSock", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Com", "Win32_System_Console", "Win32_System_Environment", "Win32_System_IO", "Win32_System_LibraryLoader", "Win32_System_Memory", "Win32_System_Pipes", "Win32_System_SystemServices", "Win32_System_Threading", "Win32_System_WindowsProgramming", "Win32_UI_Input_KeyboardAndMouse", "Win32_UI_Shell"] } ### END HAKARI SECTION