From b92dc59204cd87d10c9f019621d8e91934b98c6b Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Sun, 29 Jan 2023 18:22:41 +0100 Subject: [PATCH] node: Turn off `--atomic` when not supported --- radicle-cli/src/commands/auth.rs | 17 ++-- radicle-cli/src/commands/init.rs | 7 -- radicle-cli/src/git.rs | 140 +------------------------------ radicle-node/src/runtime.rs | 26 ++++-- radicle-node/src/worker.rs | 48 +++++++---- radicle/src/git.rs | 140 +++++++++++++++++++++++++++++++ 6 files changed, 207 insertions(+), 171 deletions(-) diff --git a/radicle-cli/src/commands/auth.rs b/radicle-cli/src/commands/auth.rs index e227d2d3..d9dc4d71 100644 --- a/radicle-cli/src/commands/auth.rs +++ b/radicle-cli/src/commands/auth.rs @@ -6,7 +6,6 @@ use anyhow::anyhow; use radicle::crypto::ssh; use radicle::{profile, Profile}; -use crate::git; use crate::terminal as term; use crate::terminal::args::{Args, Error, Help}; @@ -68,12 +67,16 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { pub fn init(options: Options) -> anyhow::Result<()> { term::headline("Initializing your 🌱 profile and identity"); - if git::check_version().is_err() { - term::warning(&format!( - "Your git version is unsupported, please upgrade to {} or later", - git::VERSION_REQUIRED, - )); - term::blank(); + if let Ok(version) = radicle::git::version() { + if version < radicle::git::VERSION_REQUIRED { + term::warning(&format!( + "Your git version is unsupported, please upgrade to {} or later", + radicle::git::VERSION_REQUIRED, + )); + term::blank(); + } + } else { + anyhow::bail!("Error retrieving git version; please check your installation"); } let home = profile::home()?; diff --git a/radicle-cli/src/commands/init.rs b/radicle-cli/src/commands/init.rs index 55e1e554..dca3c9b4 100644 --- a/radicle-cli/src/commands/init.rs +++ b/radicle-cli/src/commands/init.rs @@ -134,13 +134,6 @@ impl Args for Options { pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { let profile = ctx.profile()?; - if git::check_version().is_err() { - term::warning(&format!( - "Your git version is unsupported, please upgrade to {} or later", - git::VERSION_REQUIRED, - )); - term::blank(); - } init(options, &profile) } diff --git a/radicle-cli/src/git.rs b/radicle-cli/src/git.rs index a4ee3f6a..61c7c59a 100644 --- a/radicle-cli/src/git.rs +++ b/radicle-cli/src/git.rs @@ -11,7 +11,9 @@ use anyhow::anyhow; use anyhow::Context as _; use radicle::crypto::ssh; +use radicle::git; use radicle::git::raw as git2; +use radicle::git::{Version, VERSION_REQUIRED}; use radicle::prelude::{Id, NodeId}; pub use radicle::git::raw::{ @@ -25,78 +27,6 @@ pub const CONFIG_GPG_FORMAT: &str = "gpg.format"; pub const CONFIG_GPG_SSH_PROGRAM: &str = "gpg.ssh.program"; pub const CONFIG_GPG_SSH_ALLOWED_SIGNERS: &str = "gpg.ssh.allowedSignersFile"; -/// Minimum required git version. -pub const VERSION_REQUIRED: Version = Version { - major: 2, - minor: 34, - patch: 0, -}; - -/// A parsed git version. -#[derive(PartialEq, Eq, Debug, PartialOrd, Ord)] -pub struct Version { - pub major: u8, - pub minor: u8, - pub patch: u8, -} - -impl std::fmt::Display for Version { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}.{}.{}", self.major, self.minor, self.patch) - } -} - -impl std::str::FromStr for Version { - type Err = anyhow::Error; - - fn from_str(input: &str) -> Result { - let rest = input - .strip_prefix("git version ") - .ok_or_else(|| anyhow!("malformed git version string"))?; - let rest = rest - .split(' ') - .next() - .ok_or_else(|| anyhow!("malformed git version string"))?; - let rest = rest.trim_end(); - - let mut parts = rest.split('.'); - let major = parts - .next() - .ok_or_else(|| anyhow!("malformed git version string"))? - .parse()?; - let minor = parts - .next() - .ok_or_else(|| anyhow!("malformed git version string"))? - .parse()?; - - let patch = match parts.next() { - None => 0, - Some(patch) => patch.parse()?, - }; - - Ok(Self { - major, - minor, - patch, - }) - } -} - -/// Get the system's git version. -pub fn version() -> Result { - let output = Command::new("git").arg("version").output()?; - - if output.status.success() { - let output = String::from_utf8(output.stdout)?; - let version = output - .parse() - .with_context(|| format!("unable to parse git version string {:?}", output))?; - - return Ok(version); - } - Err(anyhow!("failed to run `git version`")) -} - /// Get the git repository in the current directory. pub fn repository() -> Result { match Repository::open(".") { @@ -289,7 +219,7 @@ pub fn clone(repo: &str, destination: &Path) -> Result { /// Check that the system's git version is supported. Returns an error otherwise. pub fn check_version() -> Result { - let git_version = self::version()?; + let git_version = git::version()?; if git_version < VERSION_REQUIRED { anyhow::bail!("a minimum git version of {} is required", VERSION_REQUIRED); @@ -382,67 +312,3 @@ pub fn commit_ssh_fingerprint(path: &Path, sha1: &str) -> Result, Ok(None) } - -#[cfg(test)] -mod test { - use super::*; - use std::str::FromStr; - - #[test] - fn test_version_ord() { - assert!( - Version { - major: 2, - minor: 34, - patch: 1 - } > Version { - major: 2, - minor: 34, - patch: 0 - } - ); - assert!( - Version { - major: 2, - minor: 24, - patch: 12 - } < Version { - major: 2, - minor: 34, - patch: 0 - } - ); - } - - #[test] - fn test_version_from_str() { - assert_eq!( - Version::from_str("git version 2.34.1\n").ok(), - Some(Version { - major: 2, - minor: 34, - patch: 1 - }) - ); - - assert_eq!( - Version::from_str("git version 2.34.1 (macOS)").ok(), - Some(Version { - major: 2, - minor: 34, - patch: 1 - }) - ); - - assert_eq!( - Version::from_str("git version 2.34").ok(), - Some(Version { - major: 2, - minor: 34, - patch: 0 - }) - ); - - assert!(Version::from_str("2.34").is_err()); - } -} diff --git a/radicle-node/src/runtime.rs b/radicle-node/src/runtime.rs index aad89eb2..c290f00c 100644 --- a/radicle-node/src/runtime.rs +++ b/radicle-node/src/runtime.rs @@ -8,6 +8,7 @@ use std::{fs, io, net, thread, time}; use crossbeam_channel as chan; use cyphernet::{Cert, EcSign}; use netservices::resource::NetAccept; +use radicle::git; use radicle::profile::Home; use radicle::Storage; use reactor::poller::popol; @@ -21,6 +22,7 @@ use crate::node::NodeId; use crate::service::{routing, tracking}; use crate::wire; use crate::wire::Wire; +use crate::worker; use crate::worker::{WorkerPool, WorkerReq}; use crate::{service, LocalTime}; @@ -61,6 +63,9 @@ pub enum Error { and restart the node" )] AlreadyRunning(PathBuf), + /// A git version error. + #[error("git version error: {0}")] + GitVersion(#[from] git::VersionError), } /// Holds join handles to the client threads, as well as a client handle. @@ -141,15 +146,26 @@ impl Runtime { } let reactor = Reactor::named(wire, popol::Poller::new(), id.to_human())?; let handle = Handle::new(home.clone(), reactor.controller()); + let atomic = git::version()? >= git::VERSION_REQUIRED; + + if !atomic { + log::warn!( + target: "node", + "Disabling atomic fetches; git version >= {} required", git::VERSION_REQUIRED + ); + } let pool = WorkerPool::with( - 8, - time::Duration::from_secs(9), - storage.clone(), - daemon, worker_recv, handle.clone(), - id.to_human(), + worker::Config { + capacity: 8, + name: id.to_human(), + timeout: time::Duration::from_secs(9), + storage: storage.clone(), + daemon, + atomic, + }, ); Ok(Runtime { diff --git a/radicle-node/src/worker.rs b/radicle-node/src/worker.rs index ec4c2132..f028e269 100644 --- a/radicle-node/src/worker.rs +++ b/radicle-node/src/worker.rs @@ -18,6 +18,22 @@ use crate::runtime::Handle; use crate::service::reactor::Fetch; use crate::wire::{WireReader, WireSession, WireWriter}; +/// Worker pool configuration. +pub struct Config { + /// Number of worker threads. + pub capacity: usize, + /// Whether to use atomic fetches. + pub atomic: bool, + /// Thread name. + pub name: String, + /// Timeout for all operations. + pub timeout: time::Duration, + /// Git daemon address. + pub daemon: net::SocketAddr, + /// Git storage. + pub storage: Storage, +} + /// Worker request. pub struct WorkerReq { pub fetch: Fetch, @@ -38,6 +54,7 @@ struct Worker { daemon: net::SocketAddr, timeout: time::Duration, handle: Handle, + atomic: bool, name: String, } @@ -134,9 +151,13 @@ impl Worker { .envs(env::vars().filter(|(k, _)| k == "PATH" || k.starts_with("GIT_TRACE"))) .args(["-c", "protocol.version=2"]) .arg("fetch") - .arg("--atomic") // FIXME: Not available on 2.30 (debian standard) - .arg("--verbose") - .arg(format!("git://{tunnel_addr}/{}", repo.id.canonical())) + .arg("--verbose"); + + if self.atomic { + // Enable atomic fetch. Only works with Git 2.31 and later. + cmd.arg("--atomic"); + } + cmd.arg(format!("git://{tunnel_addr}/{}", repo.id.canonical())) // FIXME: We need to omit our own namespace from this refspec in case we're fetching '*'. .arg(fetch.namespaces.as_fetchspec()) .stdout(process::Stdio::piped()) @@ -233,26 +254,23 @@ pub struct WorkerPool { impl WorkerPool { /// Create a new worker pool with the given parameters. pub fn with( - capacity: usize, - timeout: time::Duration, - storage: Storage, - daemon: net::SocketAddr, tasks: chan::Receiver>, handle: Handle, - name: String, + config: Config, ) -> Self { - let mut pool = Vec::with_capacity(capacity); - for _ in 0..capacity { + let mut pool = Vec::with_capacity(config.capacity); + for _ in 0..config.capacity { let worker = Worker { tasks: tasks.clone(), - storage: storage.clone(), - daemon, handle: handle.clone(), - timeout, - name: name.clone(), + storage: config.storage.clone(), + daemon: config.daemon, + timeout: config.timeout, + name: config.name.clone(), + atomic: config.atomic, }; let thread = thread::Builder::new() - .name(name.clone()) + .name(config.name.clone()) .spawn(|| worker.run()) .unwrap(); diff --git a/radicle/src/git.rs b/radicle/src/git.rs index 2ea87608..4025de98 100644 --- a/radicle/src/git.rs +++ b/radicle/src/git.rs @@ -27,6 +27,82 @@ pub use storage::BranchName; /// Default port of the `git` transport protocol. pub const PROTOCOL_PORT: u16 = 9418; +/// Minimum required git version. +pub const VERSION_REQUIRED: Version = Version { + major: 2, + minor: 31, + patch: 0, +}; + +/// A parsed git version. +#[derive(PartialEq, Eq, Debug, PartialOrd, Ord)] +pub struct Version { + pub major: u8, + pub minor: u8, + pub patch: u8, +} + +impl std::fmt::Display for Version { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}.{}.{}", self.major, self.minor, self.patch) + } +} + +#[derive(thiserror::Error, Debug)] +pub enum VersionError { + #[error("malformed git version string")] + Malformed, + #[error("malformed git version string: {0}")] + ParseInt(#[from] std::num::ParseIntError), + #[error("malformed git version string: {0}")] + Utf8(#[from] std::string::FromUtf8Error), + #[error("error retrieving git version: {0}")] + Io(#[from] io::Error), + #[error("error retrieving git version: {0}")] + Other(String), +} + +impl std::str::FromStr for Version { + type Err = VersionError; + + fn from_str(input: &str) -> Result { + let rest = input + .strip_prefix("git version ") + .ok_or(VersionError::Malformed)?; + let rest = rest.split(' ').next().ok_or(VersionError::Malformed)?; + let rest = rest.trim_end(); + + let mut parts = rest.split('.'); + let major = parts.next().ok_or(VersionError::Malformed)?.parse()?; + let minor = parts.next().ok_or(VersionError::Malformed)?.parse()?; + + let patch = match parts.next() { + None => 0, + Some(patch) => patch.parse()?, + }; + + Ok(Self { + major, + minor, + patch, + }) + } +} + +/// Get the system's git version. +pub fn version() -> Result { + let output = Command::new("git").arg("version").output()?; + + if output.status.success() { + let output = String::from_utf8(output.stdout)?; + let version = output.parse()?; + + return Ok(version); + } + Err(VersionError::Other( + String::from_utf8_lossy(&output.stderr).to_string(), + )) +} #[derive(thiserror::Error, Debug)] pub enum RefError { @@ -430,3 +506,67 @@ pub mod url { } } } + +#[cfg(test)] +mod test { + use super::*; + use std::str::FromStr; + + #[test] + fn test_version_ord() { + assert!( + Version { + major: 2, + minor: 34, + patch: 1 + } > Version { + major: 2, + minor: 34, + patch: 0 + } + ); + assert!( + Version { + major: 2, + minor: 24, + patch: 12 + } < Version { + major: 2, + minor: 34, + patch: 0 + } + ); + } + + #[test] + fn test_version_from_str() { + assert_eq!( + Version::from_str("git version 2.34.1\n").ok(), + Some(Version { + major: 2, + minor: 34, + patch: 1 + }) + ); + + assert_eq!( + Version::from_str("git version 2.34.1 (macOS)").ok(), + Some(Version { + major: 2, + minor: 34, + patch: 1 + }) + ); + + assert_eq!( + Version::from_str("git version 2.34").ok(), + Some(Version { + major: 2, + minor: 34, + patch: 0 + }) + ); + + assert!(Version::from_str("2.34").is_err()); + } +}