feat: use gix-negotiate in fetch machinery.

Thanks to it we are finally able to do pack negotiations just like git can,
as many rounds as it takes and with all available algorithms.

Author: Byron

Cargo.lock

@@ -131,6 +131,7 @@ gix-object = { version = "^0.29.2", path = "../gix-object" }
 gix-actor = { version = "^0.20.0", path = "../gix-actor" }
 gix-pack = { version = "^0.35.0", path = "../gix-pack", features = ["object-cache-dynamic"] }
 gix-revision = { version = "^0.14.0", path = "../gix-revision" }
+gix-negotiate = { version = "0.1.0", path = "../gix-negotiate" }
 
 gix-path = { version = "^0.8.0", path = "../gix-path" }
 gix-url = { version = "^0.18.0", path = "../gix-url" }

gix/Cargo.toml

@@ -9,6 +9,24 @@ pub enum Error {
     NegotiationFailed { rounds: usize },
 }
 
+/// This function is modeled after the similarly named one in the git codebase and attempts to do the following:
+///
+/// ### Performance Notes
+///
+/// In `git`, the graph data structure is shared, whereas here we currently can't as the data we store is specific
+/// to the respective user. Git makes sure that the flags used there don't overlap, and we would have to do the same
+/// to get the same benefits. This explains why the negotiator code relies on commits being parsed already - it's a way
+/// to observe that the caller already traversed there.
+#[allow(dead_code)]
+pub(crate) fn mark_complete_and_common_ref(
+    _negotiator: &mut dyn gix_negotiate::Negotiator,
+    _ref_map: &fetch::RefMap,
+) -> Result<(), Error> {
+    // TODO: we probably want a priority list here, that's mostly how their linked list is used. Doing so might be faster,
+    //       maybe their code is old enough not to use a prio list? Or maybe it's the opposite.
+    todo!()
+}
+
 /// Negotiate one round with `algo` by looking at `ref_map` and adjust `arguments` to contain the haves and wants.
 /// If this is not the first round, the `previous_response` is set with the last recorded server response.
 /// Returns `true` if the negotiation is done from our side so the server won't keep asking.
@@ -34,9 +52,7 @@ pub(crate) fn one_round(
 
     match algo {
         Algorithm::Noop | Algorithm::Skipping | Algorithm::Consecutive => {
-            todo!()
-        }
-        Algorithm::Naive => {
+            // Use actual negotiation code here, this is the NAIVE implementation/hack
             assert_eq!(round, 1, "Naive always finishes after the first round, it claims.");
             let mut has_missing_tracking_branch = false;
             for mapping in &ref_map.mappings {

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

@@ -119,7 +119,7 @@ where
             .map(|n| Fetch::NEGOTIATION_ALGORITHM.try_into_negotiation_algorithm(n))
             .transpose()
             .with_leniency(repo.config.lenient_config)?
-            .unwrap_or(Algorithm::Naive); // TODO: use the default instead once consecutive is implemented
+            .unwrap_or(Algorithm::Consecutive);
 
         let reader = 'negotiation: loop {
             progress.step();

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

@@ -1,18 +1,6 @@
 ///
 pub mod negotiate {
-    /// The way the negotiation is performed.
-    #[derive(Default, Debug, Copy, Clone, Eq, PartialEq)]
-    pub enum Algorithm {
-        /// Do not send any information at all, likely at cost of larger-than-necessary packs.
-        Noop,
-        /// Walk over consecutive commits and check each one. This can be costly be assures packs are exactly the size they need to be.
-        #[default]
-        Consecutive,
-        /// Like `Consecutive`, but skips commits to converge faster, at the cost of receiving packs that are larger than they have to be.
-        Skipping,
-        /// Our very own implementation that probably should be replaced by one of the known algorithms soon.
-        Naive,
-    }
+    pub use gix_negotiate::Algorithm;
 
     #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
     pub use super::super::connection::fetch::negotiate::Error;

gix/src/remote/fetch.rs

@@ -308,3 +308,45 @@ git clone --shared base detached-head
 (cd detached-head
   git checkout @~1
 )
+
+function commit() {
+  local message=${1:?first argument is the commit message}
+  local file="$message.t"
+  echo "$1" > "$file"
+  git add -- "$file"
+  tick
+  git commit -m "$message"
+  git tag "$message"
+}
+
+function optimize_repo() {
+  git commit-graph write --no-progress --reachable
+  git repack -adq
+}
+
+(mkdir multi_round && cd multi_round
+  git init -q server && cd server
+    commit to_fetch
+  cd ..
+
+  git init -q client && cd client
+    for i in $(seq 8); do
+      git checkout --orphan b$i &&
+      commit b$i.c0
+    done
+
+    for j in $(seq 19); do
+      for i in $(seq 8); do
+        git checkout b$i &&
+        commit b$i.c$j
+      done
+    done
+    optimize_repo
+  cd ..
+  (cd server
+    git fetch --no-tags "$PWD/../client" b1:refs/heads/b1
+    git checkout b1
+    commit commit-on-b1
+    optimize_repo
+  )
+)

gix/tests/fixtures/make_remote_repos.sh

@@ -19,6 +19,7 @@ mod blocking_and_async_io {
     use gix_protocol::maybe_async;
 
     use crate::{
+        remote,
         remote::{into_daemon_remote_if_async, spawn_git_daemon_if_async},
         util::hex_to_id,
     };
@@ -106,6 +107,25 @@ mod blocking_and_async_io {
         Ok(())
     }
 
+    // TODO: try this as maybe-async
+    #[test]
+    #[cfg(feature = "blocking-network-client")]
+    #[ignore = "fails because of improper negotiation (it's hacked to work for our cases)"]
+    fn fetch_with_multi_round_negotiation() -> crate::Result {
+        let repo = remote::repo("multi_round/client");
+        let server_repo = remote::repo("multi_round/server");
+        let changes = repo
+            .remote_at(server_repo.work_dir().expect("non-bare"))?
+            .with_refspecs(Some("refs/heads/*:refs/remotes/origin/*"), Fetch)?
+            .connect(Fetch)?
+            .prepare_fetch(gix::progress::Discard, Default::default())?
+            .with_dry_run(true)
+            .receive(gix::progress::Discard, &AtomicBool::default())?;
+
+        dbg!(changes);
+        Ok(())
+    }
+
     #[maybe_async::test(
         feature = "blocking-network-client",
         async(feature = "async-network-client-async-std", async_std::test)