From b1435629f9b2e5d592971350d15162cc1e9b427d Mon Sep 17 00:00:00 2001
From: Fintan Halpenny <fintan.halpenny@gmail.com>
Date: Mon, 11 Mar 2024 11:38:48 +0100
Subject: [PATCH] fetch: re-work transport::done signal call

The protocol requires that the fetcher signals to the remote that it
is done with the fetching process, so that the remote can exit out of
its upload-pack process.

The first change is that the `done` signal can happen before the
validation, since nothing else needs to be fetched.

The second change is that the fetcher does not care if the remote
responds to this signal since it has the data required for validation
and applying the changes to its refdb. Thus, if an error is
encountered then it is logged, but the rest of the process proceeds as
normal.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
---
 radicle-fetch/src/lib.rs   | 15 ++++++++-------
 radicle-fetch/src/state.rs | 13 ++++++++++---
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/radicle-fetch/src/lib.rs b/radicle-fetch/src/lib.rs
index 071f6a7f..f49f62f1 100644
--- a/radicle-fetch/src/lib.rs
+++ b/radicle-fetch/src/lib.rs
@@ -9,18 +9,19 @@ mod refs;
 mod stage;
 mod state;
 
-use gix_protocol::handshake;
-pub use handle::Handle;
-pub use policy::{Allowed, BlockList, Scope};
-use radicle::storage::ReadRepository as _;
-pub use state::{FetchLimit, FetchResult};
-pub use transport::Transport;
-
 use std::io;
 use std::time::Instant;
 
+use gix_protocol::handshake;
+
+pub use handle::Handle;
+pub use policy::{Allowed, BlockList, Scope};
+pub use state::{FetchLimit, FetchResult};
+pub use transport::Transport;
+
 use radicle::crypto::PublicKey;
 use radicle::storage::refs::RefsAt;
+use radicle::storage::ReadRepository as _;
 use state::FetchState;
 use thiserror::Error;
 
diff --git a/radicle-fetch/src/state.rs b/radicle-fetch/src/state.rs
index 2cafa790..2f6a4c41 100644
--- a/radicle-fetch/src/state.rs
+++ b/radicle-fetch/src/state.rs
@@ -441,6 +441,16 @@ impl FetchState {
             start.elapsed().as_millis()
         );
 
+        // N.b. signal to exit the upload-pack sequence
+        // We're finished fetching on this side, and all that's left
+        // is validation.
+        match handle.transport.done() {
+            Ok(()) => log::debug!(target: "fetch", "Sent done signal to remote {remote}"),
+            Err(err) => {
+                log::warn!(target: "fetch", "Attempted to send done to remote {remote}: {err}")
+            }
+        }
+
         // Run validation of signed refs, pruning any offending
         // remotes from the tips, thus not updating the production Git
         // repository.
@@ -543,9 +553,6 @@ impl FetchState {
                 }
             }
         }
-        // N.b. signal to exit the upload-pack sequence
-        handle.transport.done()?;
-
         log::debug!(
             target: "fetch",
             "Validated {} remotes ({}ms)",