node: Turn off `--atomic` when not supported

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,13 +67,17 @@ 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() {
+    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",
-            git::VERSION_REQUIRED,
+                radicle::git::VERSION_REQUIRED,
             ));
             term::blank();
         }
+    } else {
+        anyhow::bail!("Error retrieving git version; please check your installation");
+    }
 
     let home = profile::home()?;
     let passphrase = term::read_passphrase(options.stdin, true)?;

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)
 }
 

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<Self, Self::Err> {
-        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<Version, anyhow::Error> {
-    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<Repository, anyhow::Error> {
     match Repository::open(".") {
@@ -289,7 +219,7 @@ pub fn clone(repo: &str, destination: &Path) -> Result<String, io::Error> {
 
 /// Check that the system's git version is supported. Returns an error otherwise.
 pub fn check_version() -> Result<Version, anyhow::Error> {
-    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<Option<String>,
 
     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());
-    }
-}

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<G: Signer + EcSign + 'static> Runtime<G> {
         }
         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 {

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<G: Signer + EcSign> {
     pub fetch: Fetch,
@@ -38,6 +54,7 @@ struct Worker<G: Signer + EcSign> {
     daemon: net::SocketAddr,
     timeout: time::Duration,
     handle: Handle<G>,
+    atomic: bool,
     name: String,
 }
 
@@ -134,9 +151,13 @@ impl<G: Signer + EcSign + 'static> Worker<G> {
             .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("--verbose")
+
-            .arg(format!("git://{tunnel_addr}/{}", repo.id.canonical()))
+        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<G: Signer + EcSign + 'static>(
-        capacity: usize,
-        timeout: time::Duration,
-        storage: Storage,
-        daemon: net::SocketAddr,
         tasks: chan::Receiver<WorkerReq<G>>,
         handle: Handle<G>,
-        name: String,
+        config: Config,
     ) -> Self {
-        let mut pool = Vec::with_capacity(capacity);
+        let mut pool = Vec::with_capacity(config.capacity);
-        for _ in 0..capacity {
+        for _ in 0..config.capacity {
             let worker = Worker {
                 tasks: tasks.clone(),
-                storage: storage.clone(),
-                daemon,
                 handle: handle.clone(),
-                timeout,
+                storage: config.storage.clone(),
-                name: name.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();
 

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<Self, Self::Err> {
+        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<Version, VersionError> {
+    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());
+    }
+}