Now go all in and make it work with the new model

Features:

* make `fetch_only()` available in async mode

Author: Byron

crate-status.md

@@ -233,6 +233,7 @@ Check out the [performance discussion][gix-traverse-performance] as well.
   * [x] nested traversal
 * **commits**
   * [x] ancestor graph traversal similar to `git revlog`
+  * [ ] `commitgraph` support
 * [x] API documentation
     * [ ] Examples
 
@@ -671,7 +672,10 @@ See its [README.md](https://github.com/Byron/gitoxide/blob/main/gix-lock/README.
            * [x] shallow (remains shallow, options to adjust shallow boundary)
            * [ ] a way to auto-explode small packs to avoid them to pile up
            * [x] 'ref-in-want'
-           * [ ] standard negotiation algorithms (right now we only have a 'naive' one)
+           * [ ] 'wanted-ref'
+           * [ ] standard negotiation algorithms `consecutive`, `skipping` and `noop`.
+           * [ ] a more efficient way to deal [with common `have`](https://github.com/git/git/blob/9e49351c3060e1fa6e0d2de64505b7becf157f28/fetch-pack.c#L525)
+                 during negotiation - we would submit known non-common `haves` each round in stateless transports whereas git prunes the set to known common ones.
         * [ ] push
         * [x] ls-refs
         * [x] ls-refs with ref-spec filter

gix/src/clone/fetch/mod.rs

@@ -44,7 +44,12 @@ impl PrepareFetch {
     /// it was newly initialized.
     ///
     /// Note that all data we created will be removed once this instance drops if the operation wasn't successful.
-    pub fn fetch_only<P>(
+    ///
+    /// ### Note for users of `async`
+    ///
+    /// Even though
+    #[gix_protocol::maybe_async::maybe_async]
+    pub async fn fetch_only<P>(
         &mut self,
         mut progress: P,
         should_interrupt: &std::sync::atomic::AtomicBool,
@@ -101,17 +106,19 @@ impl PrepareFetch {
         .expect("valid")
         .to_owned();
         let pending_pack: remote::fetch::Prepare<'_, '_, _> = {
-            let mut connection = remote.connect(remote::Direction::Fetch)?;
+            let mut connection = remote.connect(remote::Direction::Fetch).await?;
             if let Some(f) = self.configure_connection.as_mut() {
                 f(&mut connection).map_err(|err| Error::RemoteConnection(err))?;
             }
-            connection.prepare_fetch(&mut progress, {
-                let mut opts = self.fetch_options.clone();
-                if !opts.extra_refspecs.contains(&head_refspec) {
-                    opts.extra_refspecs.push(head_refspec)
-                }
-                opts
-            })?
+            connection
+                .prepare_fetch(&mut progress, {
+                    let mut opts = self.fetch_options.clone();
+                    if !opts.extra_refspecs.contains(&head_refspec) {
+                        opts.extra_refspecs.push(head_refspec)
+                    }
+                    opts
+                })
+                .await?
         };
         if pending_pack.ref_map().object_hash != repo.object_hash() {
             unimplemented!("configure repository to expect a different object hash as advertised by the server")
@@ -127,7 +134,8 @@ impl PrepareFetch {
                 message: reflog_message.clone(),
             })
             .with_shallow(self.shallow.clone())
-            .receive(progress, should_interrupt)?;
+            .receive(progress, should_interrupt)
+            .await?;
 
         util::append_config_to_repo_config(repo, config);
         util::update_head(
@@ -141,6 +149,7 @@ impl PrepareFetch {
     }
 
     /// Similar to [`fetch_only()`][Self::fetch_only()`], but passes ownership to a utility type to configure a checkout operation.
+    #[cfg(feature = "blocking-network-client")]
     pub fn fetch_then_checkout<P>(
         &mut self,
         progress: P,
@@ -155,5 +164,4 @@ impl PrepareFetch {
     }
 }
 
-#[cfg(feature = "blocking-network-client")]
 mod util;

gix/src/clone/mod.rs

@@ -160,7 +160,7 @@ mod access_feat {
 }
 
 ///
-#[cfg(feature = "blocking-network-client")]
+#[cfg(any(feature = "async-network-client-async-std", feature = "blocking-network-client"))]
 pub mod fetch;
 
 ///

gix/src/config/tree/keys.rs

@@ -511,7 +511,8 @@ pub mod validate {
                 gix_config::Integer::try_from(value)?
                     .to_decimal()
                     .ok_or_else(|| format!("integer {value} cannot be represented as `usize`"))?,
-            )?;
+            )
+            .map_err(|_| "cannot use sign for unsigned integer")?;
             Ok(())
         }
     }

gix/src/config/tree/sections/protocol.rs

@@ -6,6 +6,8 @@ use crate::{
 impl Protocol {
     /// The `protocol.allow` key.
     pub const ALLOW: Allow = Allow::new_with_validate("allow", &config::Tree::PROTOCOL, validate::Allow);
+    /// The `protocol.version` key.
+    pub const VERSION: Version = Version::new_with_validate("version", &config::Tree::PROTOCOL, validate::Version);
 
     /// The `protocol.<name>` subsection
     pub const NAME_PARAMETER: NameParameter = NameParameter;
@@ -14,6 +16,9 @@ impl Protocol {
 /// The `protocol.allow` key type.
 pub type Allow = keys::Any<validate::Allow>;
 
+/// The `protocol.version` key.
+pub type Version = keys::Any<validate::Version>;
+
 #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
 mod allow {
     use std::borrow::Cow;
@@ -39,7 +44,7 @@ mod allow {
 pub struct NameParameter;
 
 impl NameParameter {
-    /// The `credential.<url>.helper` key.
+    /// The `protocol.<name>.allow` key.
     pub const ALLOW: Allow = Allow::new_with_validate("allow", &Protocol::NAME_PARAMETER, validate::Allow);
 }
 
@@ -63,14 +68,46 @@ impl Section for Protocol {
     }
 
     fn keys(&self) -> &[&dyn Key] {
-        &[&Self::ALLOW]
+        &[&Self::ALLOW, &Self::VERSION]
     }
 
     fn sub_sections(&self) -> &[&dyn Section] {
         &[&Self::NAME_PARAMETER]
     }
 }
 
+mod key_impls {
+    impl super::Version {
+        /// Convert `value` into the corresponding protocol version, possibly applying the correct default.
+        #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
+        pub fn try_into_protocol_version(
+            &'static self,
+            value: Option<Result<i64, gix_config::value::Error>>,
+        ) -> Result<gix_protocol::transport::Protocol, crate::config::key::GenericErrorWithValue> {
+            let value = match value {
+                None => return Ok(gix_protocol::transport::Protocol::V2),
+                Some(v) => v,
+            };
+            Ok(match value {
+                Ok(0) => gix_protocol::transport::Protocol::V0,
+                Ok(1) => gix_protocol::transport::Protocol::V1,
+                Ok(2) => gix_protocol::transport::Protocol::V2,
+                Ok(other) => {
+                    return Err(crate::config::key::GenericErrorWithValue::from_value(
+                        self,
+                        other.to_string().into(),
+                    ))
+                }
+                Err(err) => {
+                    return Err(
+                        crate::config::key::GenericErrorWithValue::from_value(self, "unknown".into()).with_source(err),
+                    )
+                }
+            })
+        }
+    }
+}
+
 mod validate {
     use crate::{bstr::BStr, config::tree::keys};
 
@@ -82,4 +119,17 @@ mod validate {
             Ok(())
         }
     }
+
+    pub struct Version;
+    impl keys::Validate for Version {
+        fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
+            let value = gix_config::Integer::try_from(value)?
+                .to_decimal()
+                .ok_or_else(|| format!("integer {value} cannot be represented as integer"))?;
+            match value {
+                0 | 1 | 2 => Ok(()),
+                _ => Err(format!("protocol version {value} is unknown").into()),
+            }
+        }
+    }
 }

gix/src/lib.rs

@@ -81,6 +81,7 @@ pub use gix_ignore as ignore;
 #[doc(inline)]
 pub use gix_index as index;
 pub use gix_lock as lock;
+pub use gix_negotiate as negotiate;
 pub use gix_object as objs;
 pub use gix_object::bstr;
 pub use gix_odb as odb;

gix/src/remote/connect.rs

@@ -24,8 +24,8 @@ mod error {
         Connect(#[from] gix_protocol::transport::client::connect::Error),
         #[error("The {} url was missing - don't know where to establish a connection to", direction.as_str())]
         MissingUrl { direction: remote::Direction },
-        #[error("Protocol named {given:?} is not a valid protocol. Choose between 1 and 2")]
-        UnknownProtocol { given: BString },
+        #[error("The given protocol version was invalid. Choose between 1 and 2")]
+        UnknownProtocol { source: config::key::GenericErrorWithValue },
         #[error("Could not verify that \"{}\" url is a valid git directory before attempting to use it", url.to_bstring())]
         FileUrl {
             source: Box<gix_discover::is_git::Error>,
@@ -128,25 +128,9 @@ impl<'repo> Remote<'repo> {
             Ok(url)
         }
 
-        use gix_protocol::transport::Protocol;
-        let version = self
-            .repo
-            .config
-            .resolved
-            .integer("protocol", None, "version")
-            .unwrap_or(Ok(2))
-            .map_err(|err| Error::UnknownProtocol { given: err.input })
-            .and_then(|num| {
-                Ok(match num {
-                    1 => Protocol::V1,
-                    2 => Protocol::V2,
-                    num => {
-                        return Err(Error::UnknownProtocol {
-                            given: num.to_string().into(),
-                        })
-                    }
-                })
-            })?;
+        let version = crate::config::tree::Protocol::VERSION
+            .try_into_protocol_version(self.repo.config.resolved.integer("protocol", None, "version"))
+            .map_err(|err| Error::UnknownProtocol { source: err })?;
 
         let url = self.url(direction).ok_or(Error::MissingUrl { direction })?.to_owned();
         if !self.repo.config.url_scheme()?.allow(&url.scheme) {

gix/src/remote/connection/fetch/mod.rs

@@ -43,13 +43,17 @@ impl RefLogMessage {
 pub enum Status {
     /// Nothing changed as the remote didn't have anything new compared to our tracking branches, thus no pack was received
     /// and no new object was added.
+    ///
+    /// As we could determine that nothing changed without remote interaction, there was no negotiation at all.
     NoPackReceived {
         /// However, depending on the refspecs, references might have been updated nonetheless to point to objects as
         /// reported by the remote.
         update_refs: refs::update::Outcome,
     },
     /// There was at least one tip with a new object which we received.
     Change {
+        /// The number of rounds it took to minimize the pack to contain only the objects we don't have.
+        negotiation_rounds: usize,
         /// Information collected while writing the pack and its index.
         write_pack_bundle: gix_pack::bundle::write::Outcome,
         /// Information collected while updating references.
@@ -58,6 +62,8 @@ pub enum Status {
     /// A dry run was performed which leaves the local repository without any change
     /// nor will a pack have been received.
     DryRun {
+        /// The number of rounds it took to minimize the *would-be-sent*-pack to contain only the objects we don't have.
+        negotiation_rounds: usize,
         /// Information about what updates to refs would have been done.
         update_refs: refs::update::Outcome,
     },